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_;
