GN: Make write_runtime_deps take effect only a target is resolved BUG=604104 Review URL: https://codereview.chromium.org/1892393002 Cr-Original-Commit-Position: refs/heads/master@{#388496} Cr-Mirrored-From: https://chromium.googlesource.com/chromium/src Cr-Mirrored-Commit: fb8986d4df3e8b84db16cd5d719a2e38bb8a7b02
diff --git a/tools/gn/BUILD.gn b/tools/gn/BUILD.gn index 2728c0d..bb36b1f 100644 --- a/tools/gn/BUILD.gn +++ b/tools/gn/BUILD.gn
@@ -277,7 +277,6 @@ "function_write_file_unittest.cc", "functions_target_unittest.cc", "functions_unittest.cc", - "group_target_generator_unittest.cc", "header_checker_unittest.cc", "inherited_libraries_unittest.cc", "input_conversion_unittest.cc",
diff --git a/tools/gn/group_target_generator_unittest.cc b/tools/gn/group_target_generator_unittest.cc deleted file mode 100644 index 3b0c824..0000000 --- a/tools/gn/group_target_generator_unittest.cc +++ /dev/null
@@ -1,46 +0,0 @@ -// Copyright 2016 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 "testing/gtest/include/gtest/gtest.h" -#include "tools/gn/group_target_generator.h" -#include "tools/gn/scheduler.h" -#include "tools/gn/test_with_scope.h" - -namespace { - -// Returns true on success, false if write_file signaled an error. -bool ParseWriteRuntimeDeps(Scope* scope, const std::string& value) { - TestParseInput input( - "group(\"foo\") { write_runtime_deps = " + value + "}"); - if (input.has_error()) - return false; - - Err err; - input.parsed()->Execute(scope, &err); - return !err.has_error(); -} - -} // namespace - - -// Tests that actions can't have output substitutions. -TEST(GroupTargetGenerator, WriteRuntimeDeps) { - Scheduler scheduler; - TestWithScope setup; - Scope::ItemVector items_; - setup.scope()->set_item_collector(&items_); - - // Should refuse to write files outside of the output dir. - EXPECT_FALSE(ParseWriteRuntimeDeps(setup.scope(), "\"//foo.txt\"")); - EXPECT_EQ(0U, scheduler.GetWriteRuntimeDepsTargets().size()); - - // Should fail for garbage inputs. - EXPECT_FALSE(ParseWriteRuntimeDeps(setup.scope(), "0")); - EXPECT_EQ(0U, scheduler.GetWriteRuntimeDepsTargets().size()); - - // Should be able to write inside the out dir. - EXPECT_TRUE(ParseWriteRuntimeDeps(setup.scope(), "\"//out/Debug/foo.txt\"")); - EXPECT_EQ(1U, scheduler.GetWriteRuntimeDepsTargets().size()); -} -
diff --git a/tools/gn/runtime_deps_unittest.cc b/tools/gn/runtime_deps_unittest.cc index 915fbc2..6c2013d 100644 --- a/tools/gn/runtime_deps_unittest.cc +++ b/tools/gn/runtime_deps_unittest.cc
@@ -8,6 +8,7 @@ #include "testing/gtest/include/gtest/gtest.h" #include "tools/gn/runtime_deps.h" +#include "tools/gn/scheduler.h" #include "tools/gn/target.h" #include "tools/gn/test_with_scope.h" @@ -282,3 +283,31 @@ MakePair("../../action.output", &action)) != result.end()) << GetVectorDescription(result); } + +// Tests that actions can't have output substitutions. +TEST(RuntimeDeps, WriteRuntimeDepsVariable) { + Scheduler scheduler; + TestWithScope setup; + Err err; + + // Should refuse to write files outside of the output dir. + EXPECT_FALSE(setup.ExecuteSnippet( + "group(\"foo\") { write_runtime_deps = \"//foo.txt\" }", &err)); + + // Should fail for garbage inputs. + err = Err(); + EXPECT_FALSE(setup.ExecuteSnippet( + "group(\"foo\") { write_runtime_deps = 0 }", &err)); + + // Should be able to write inside the out dir, and shouldn't write the one + // in the else clause. + err = Err(); + EXPECT_TRUE(setup.ExecuteSnippet( + "if (true) {\n" + " group(\"foo\") { write_runtime_deps = \"//out/Debug/foo.txt\" }\n" + "} else {\n" + " group(\"bar\") { write_runtime_deps = \"//out/Debug/bar.txt\" }\n" + "}", &err)); + EXPECT_EQ(1U, setup.items().size()); + EXPECT_EQ(1U, scheduler.GetWriteRuntimeDepsTargets().size()); +}
diff --git a/tools/gn/target.cc b/tools/gn/target.cc index 18c353e..e88c82f 100644 --- a/tools/gn/target.cc +++ b/tools/gn/target.cc
@@ -324,6 +324,9 @@ CheckSourcesGenerated(); } + if (!write_runtime_deps_output_.value().empty()) + g_scheduler->AddWriteRuntimeDepsTarget(this); + return true; }
diff --git a/tools/gn/target_generator.cc b/tools/gn/target_generator.cc index 2132a5b..90642d4 100644 --- a/tools/gn/target_generator.cc +++ b/tools/gn/target_generator.cc
@@ -403,6 +403,5 @@ OutputFile output_file(GetBuildSettings(), source_file); target_->set_write_runtime_deps_output(output_file); - g_scheduler->AddWriteRuntimeDepsTarget(target_); return true; }
diff --git a/tools/gn/test_with_scope.cc b/tools/gn/test_with_scope.cc index 32d5dd6..0ddd463 100644 --- a/tools/gn/test_with_scope.cc +++ b/tools/gn/test_with_scope.cc
@@ -24,6 +24,7 @@ settings_.set_default_toolchain_label(toolchain_.label()); SetupToolchain(&toolchain_); + scope_.set_item_collector(&items_); } TestWithScope::~TestWithScope() { @@ -37,6 +38,28 @@ return result; } +bool TestWithScope::ExecuteSnippet(const std::string& str, Err* err) { + TestParseInput input(str); + if (input.has_error()) { + *err = input.parse_err(); + return false; + } + + size_t first_item = items_.size(); + input.parsed()->Execute(&scope_, err); + if (err->has_error()) + return false; + + for (size_t i = first_item; i < items_.size(); ++i) { + CHECK(items_[i]->AsTarget() != nullptr) + << "Only targets are supported in ExecuteSnippet()"; + items_[i]->AsTarget()->SetToolchain(&toolchain_); + if (!items_[i]->OnResolved(err)) + return false; + } + return true; +} + // static void TestWithScope::SetupToolchain(Toolchain* toolchain) { Err err;
diff --git a/tools/gn/test_with_scope.h b/tools/gn/test_with_scope.h index fa6fb4b..8853243 100644 --- a/tools/gn/test_with_scope.h +++ b/tools/gn/test_with_scope.h
@@ -34,6 +34,7 @@ Toolchain* toolchain() { return &toolchain_; } const Toolchain* toolchain() const { return &toolchain_; } Scope* scope() { return &scope_; } + const Scope::ItemVector& items() { return items_; } // This buffer accumulates output from any print() commands executed in the // context of this test. Note that the implementation of this is not @@ -44,6 +45,11 @@ // assert if the label isn't valid (this is intended for hardcoded labels). Label ParseLabel(const std::string& str) const; + // Parses, evaluates, and resolves targets from the given snippet of code. + // All targets must be defined in dependency order (does not use a Builder, + // just blindly resolves all targets in order). + bool ExecuteSnippet(const std::string& str, Err* err); + // Fills in the tools for the given toolchain with reasonable default values. // The toolchain in this object will be automatically set up with this // function, it is exposed to allow tests to get the same functionality for @@ -62,6 +68,7 @@ Settings settings_; Toolchain toolchain_; Scope scope_; + Scope::ItemVector items_; // Supplies the scope with built-in variables like root_out_dir. ScopePerFileProvider scope_progammatic_provider_;