GN only writes files if they've changed

The write_file function should only actually update the file contents if the contents have changed to avoid unnecessary rebuilds of targets that depend on the written file.

R=thakis@chromium.org

Review URL: https://codereview.chromium.org/301973005

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@273855 0039d316-1c4b-4281-b951-d872f2087c98

tools/gn/BUILD.gn

@@ -184,6 +184,7 @@ test("gn_unittests") {
     "function_get_label_info_unittest.cc",
     "function_get_target_outputs_unittest.cc",
     "function_rebase_path_unittest.cc",
+    "function_write_file_unittest.cc",
     "functions_unittest.cc",
     "header_checker_unittest.cc",
     "input_conversion_unittest.cc",

tools/gn/function_write_file.cc

@@ -28,6 +28,10 @@ const char kWriteFile_Help[] =
     "  If data is a list, the list will be written one-item-per-line with no\n"
     "  quoting or brackets.\n"
     "\n"
+    "  If the file exists and the contents are identical to that being\n"
+    "  written, the file will not be updated. This will prevent unnecessary\n"
+    "  rebuilds of targets that depend on this file.\n"
+    "\n"
     "  TODO(brettw) we probably need an optional third argument to control\n"
     "  list formatting.\n"
     "\n"
@@ -68,18 +72,25 @@ Value RunWriteFile(Scope* scope,
   } else {
     contents << args[1].ToString(false);
   }
-
-  // Write file, creating the directory if necessary.
+  const std::string& new_contents = contents.str();
   base::FilePath file_path =
       scope->settings()->build_settings()->GetFullPath(source_file);
-  const std::string& contents_string = contents.str();
+
+  // Make sure we're not replacing the same contents.
+  std::string existing_contents;
+  if (base::ReadFileToString(file_path, &existing_contents) &&
+      existing_contents == new_contents)
+    return Value();  // Nothing to do.
+
+  // Write file, creating the directory if necessary.
   if (!base::CreateDirectory(file_path.DirName())) {
     *err = Err(function->function(), "Unable to create directory.",
                "I was using \"" + FilePathToUTF8(file_path.DirName()) + "\".");
     return Value();
   }
-  int int_size = static_cast<int>(contents_string.size());
-  if (base::WriteFile(file_path, contents_string.c_str(), int_size)
+
+  int int_size = static_cast<int>(new_contents.size());
+  if (base::WriteFile(file_path, new_contents.c_str(), int_size)
       != int_size) {
     *err = Err(function->function(), "Unable to write file.",
                "I was writing \"" + FilePathToUTF8(file_path) + "\".");

tools/gn/function_write_file_unittest.cc

+// Copyright 2014 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "base/file_util.h"
+#include "base/files/file.h"
+#include "base/files/scoped_temp_dir.h"
+#include "testing/gtest/include/gtest/gtest.h"
+#include "tools/gn/functions.h"
+#include "tools/gn/test_with_scope.h"
+
+namespace {
+
+// Returns true on success, false if write_file signaled an error.
+bool CallWriteFile(Scope* scope,
+                   const std::string& filename,
+                   const Value& data) {
+  Err err;
+
+  std::vector<Value> args;
+  args.push_back(Value(NULL, filename));
+  args.push_back(data);
+
+  FunctionCallNode function_call;
+  Value result = functions::RunWriteFile(scope, &function_call, args, &err);
+  EXPECT_EQ(Value::NONE, result.type());  // Should always return none.
+
+  return !err.has_error();
+}
+
+}  // namespace
+
+TEST(WriteFile, WithData) {
+  TestWithScope setup;
+
+  // Make a real directory for writing the files.
+  base::ScopedTempDir temp_dir;
+  ASSERT_TRUE(temp_dir.CreateUniqueTempDir());
+  setup.build_settings()->SetRootPath(temp_dir.path());
+  setup.build_settings()->SetBuildDir(SourceDir("//out/"));
+
+  Value some_string(NULL, "some string contents");
+
+  // Should refuse to write files outside of the output dir.
+  EXPECT_FALSE(CallWriteFile(setup.scope(), "//in_root.txt", some_string));
+  EXPECT_FALSE(CallWriteFile(setup.scope(), "//other_dir/foo.txt",
+                             some_string));
+
+  // Should be able to write to a new dir inside the out dir.
+  EXPECT_TRUE(CallWriteFile(setup.scope(), "//out/foo.txt", some_string));
+  base::FilePath foo_name = temp_dir.path().Append(FILE_PATH_LITERAL("out"))
+      .Append(FILE_PATH_LITERAL("foo.txt"));
+  std::string result_contents;
+  EXPECT_TRUE(base::ReadFileToString(foo_name, &result_contents));
+  EXPECT_EQ(some_string.string_value(), result_contents);
+
+  // Update the contents with a list of a string and a number.
+  Value some_list(NULL, Value::LIST);
+  some_list.list_value().push_back(Value(NULL, "line 1"));
+  some_list.list_value().push_back(Value(NULL, static_cast<int64>(2)));
+  EXPECT_TRUE(CallWriteFile(setup.scope(), "//out/foo.txt", some_list));
+  EXPECT_TRUE(base::ReadFileToString(foo_name, &result_contents));
+  EXPECT_EQ("line 1\n2\n", result_contents);
+
+  // Test that the file is not rewritten if the contents are not changed.
+  // Start by setting the modified time to something old to avoid clock
+  // resolution issues.
+  base::Time old_time = base::Time::Now() - base::TimeDelta::FromDays(1);
+  base::File foo_file(foo_name,
+                      base::File::FLAG_OPEN |
+                      base::File::FLAG_READ | base::File::FLAG_WRITE);
+  ASSERT_TRUE(foo_file.IsValid());
+  foo_file.SetTimes(old_time, old_time);
+
+  // Read the current time to avoid timer resolution issues when comparing
+  // below.
+  base::File::Info original_info;
+  foo_file.GetInfo(&original_info);
+
+  EXPECT_TRUE(CallWriteFile(setup.scope(), "//out/foo.txt", some_list));
+
+  // Verify that the last modified time is the same as before.
+  base::File::Info new_info;
+  foo_file.GetInfo(&new_info);
+  EXPECT_EQ(original_info.last_modified, new_info.last_modified);
+}

tools/gn/gn.gyp

@@ -185,6 +185,7 @@
         'function_get_label_info_unittest.cc',
         'function_get_target_outputs_unittest.cc',
         'function_rebase_path_unittest.cc',
+        'function_write_file_unittest.cc',
         'functions_unittest.cc',
         'header_checker_unittest.cc',
         'input_conversion_unittest.cc',