[metadata] Actually collect the metadata
Change-Id: I30aa7b457da815f2adb30b207e1c68c421304b6d
Reviewed-on: https://gn-review.googlesource.com/c/3422
Commit-Queue: Julie Hockett <juliehockett@google.com>
Reviewed-by: Brett Wilson <brettw@chromium.org>
diff --git a/tools/gn/functions_target.cc b/tools/gn/functions_target.cc
index 282b793..93553b9 100644
--- a/tools/gn/functions_target.cc
+++ b/tools/gn/functions_target.cc
@@ -789,12 +789,108 @@
The `output_conversion` variable specified the format to write the
value. See `gn help output_conversion`.
- One of `data` or `data_keys` must be specified; use of `data` will write the
- contents of that value to file, while use of `data_keys` will trigger a
+ One of `contents` or `data_keys` must be specified; use of `data` will write
+ the contents of that value to file, while use of `data_keys` will trigger a
metadata collection walk based on the dependencies of the target and the
optional values of the `rebase` and `walk_keys` variables. See
`gn help metadata`.
+Example (metadata collection)
+
+ Given the following targets defined in //base/BUILD.gn, where A -> B and B -> C and D:
+
+ group("a") {
+ metadata = {
+ doom_melon = [ "enable" ]
+ my_files = [ "foo.cpp" ]
+
+ // Note: this is functionally equivalent to not defining `my_barrier`
+ // at all in this target's metadata.
+ my_barrier = [ "" ]
+ }
+
+ deps = [ ":b" ]
+ }
+
+ group("c") {
+ metadata = {
+ my_files = [ "bar.cpp" ]
+ my_barrier = [ ":c" ]
+ }
+
+ deps = [ ":c", ":d" ]
+ }
+
+ group("c") {
+ metadata = {
+ doom_melon = [ "disable" ]
+ my_files = [ "baz.cpp" ]
+ }
+ }
+
+ group("d") {
+ metadata = {
+ my_files = [ "missing.cpp" ]
+ }
+ }
+
+ If the following generated_file target is defined:
+
+ generated_file("my_files_metadata") {
+ outputs = [ "$root_build_dir/my_files.json" ]
+ data_keys = [ "my_files" ]
+
+ deps = [ "//base:a" ]
+ }
+
+ The following will be written to "$root_build_dir/my_files.json" (less the
+ comments):
+ [
+ "foo.cpp", // from //base:a
+ "bar.cpp", // from //base:b via //base:a
+ "baz.cpp", // from //base:c via //base:b
+ "missing.cpp" // from //base:d via //base:b
+ ]
+
+ Alternatively, as an example of using walk_keys, if the following
+ generated_file target is defined:
+
+ generated_file("my_files_metadata") {
+ outputs = [ "$root_build_dir/my_files.json" ]
+ data_keys = [ "my_files" ]
+ walk_keys = [ "my_barrier" ]
+
+ deps = [ "//base:a" ]
+ }
+
+ The following will be written to "$root_build_dir/my_files.json" (again less
+ the comments):
+ [
+ "foo.cpp", // from //base:a
+ "bar.cpp", // from //base:b via //base:a
+ "baz.cpp", // from //base:c via //base:b
+ ]
+
+ If `rebase` is used in the following generated_file target:
+
+ generated_file("my_files_metadata") {
+ outputs = [ "$root_build_dir/my_files.json" ]
+ data_keys = [ "my_files" ]
+ walk_keys = [ "my_barrier" ]
+ rebase = root_build_dir
+
+ deps = [ "//base:a" ]
+ }
+
+ The following will be written to "$root_build_dir/my_files.json" (again less
+ the comments) (assuming root_build_dir = "//out"):
+ [
+ "../base/foo.cpp", // from //base:a
+ "../base/bar.cpp", // from //base:b via //base:a
+ "../base/baz.cpp", // from //base:c via //base:b
+ ]
+
+
Variables
data_keys
diff --git a/tools/gn/metadata_walk.cc b/tools/gn/metadata_walk.cc
index dabe285..3022e5e 100644
--- a/tools/gn/metadata_walk.cc
+++ b/tools/gn/metadata_walk.cc
@@ -15,7 +15,7 @@
for (const auto* target : targets_to_walk) {
auto pair = targets_walked->insert(target);
if (pair.second) {
- if (!target->GetMetadata(keys_to_extract, keys_to_walk, rebase_dir,
+ if (!target->GetMetadata(keys_to_extract, keys_to_walk, rebase_dir, false,
&result, targets_walked, err))
return std::vector<Value>();
}
diff --git a/tools/gn/ninja_generated_file_target_writer.cc b/tools/gn/ninja_generated_file_target_writer.cc
index bf57f0c..7dc60b0 100644
--- a/tools/gn/ninja_generated_file_target_writer.cc
+++ b/tools/gn/ninja_generated_file_target_writer.cc
@@ -42,6 +42,28 @@
}
void NinjaGeneratedFileTargetWriter::GenerateFile() {
+ Err err;
+
+ // If this is a metadata target, populate the write value with the appropriate
+ // data.
+ Value contents;
+ if (target_->contents().type() == Value::NONE) {
+ // Origin is set to the outputs location, so that errors with this value
+ // get flagged on the right target.
+ CHECK(target_->action_values().outputs().list().size() == 1U);
+ contents = Value(target_->action_values().outputs().list()[0].origin(),
+ Value::LIST);
+ std::set<const Target*> targets_walked;
+ if (!target_->GetMetadata(target_->data_keys(), target_->walk_keys(),
+ target_->rebase(), /*deps_only = */ true,
+ &contents.list_value(), &targets_walked, &err)) {
+ g_scheduler->FailWithError(err);
+ return;
+ }
+ } else {
+ contents = target_->contents();
+ }
+
std::vector<SourceFile> outputs_as_sources;
target_->action_values().GetOutputsAsSourceFiles(target_,
&outputs_as_sources);
@@ -52,17 +74,16 @@
ScopedTrace trace(TraceItem::TRACE_FILE_WRITE, outputs_as_sources[0].value());
// Compute output.
- Err err;
- std::ostringstream contents;
- ConvertValueToOutput(settings_, target_->contents(),
- target_->output_conversion(), contents, &err);
+ std::ostringstream out;
+ ConvertValueToOutput(settings_, contents, target_->output_conversion(), out,
+ &err);
if (err.has_error()) {
g_scheduler->FailWithError(err);
return;
}
- WriteFileIfChanged(output, contents.str(), &err);
+ WriteFileIfChanged(output, out.str(), &err);
if (err.has_error()) {
g_scheduler->FailWithError(err);
diff --git a/tools/gn/scheduler.cc b/tools/gn/scheduler.cc
index bca1472..3e4eee9 100644
--- a/tools/gn/scheduler.cc
+++ b/tools/gn/scheduler.cc
@@ -123,6 +123,16 @@
return false;
}
+void Scheduler::AddGeneratedFile(const SourceFile& entry) {
+ std::lock_guard<std::mutex> lock(lock_);
+ generated_files_.insert(std::make_pair(entry, true));
+}
+
+bool Scheduler::IsFileGeneratedByTarget(const SourceFile& file) const {
+ std::lock_guard<std::mutex> lock(lock_);
+ return generated_files_.find(file) != generated_files_.end();
+}
+
std::multimap<SourceFile, const Target*> Scheduler::GetUnknownGeneratedInputs()
const {
std::lock_guard<std::mutex> lock(lock_);
diff --git a/tools/gn/scheduler.h b/tools/gn/scheduler.h
index b6f5532..160921e 100644
--- a/tools/gn/scheduler.h
+++ b/tools/gn/scheduler.h
@@ -68,6 +68,10 @@
std::vector<const Target*> GetWriteRuntimeDepsTargets() const;
bool IsFileGeneratedByWriteRuntimeDeps(const OutputFile& file) const;
+ // Tracks generated_file calls.
+ void AddGeneratedFile(const SourceFile& entry);
+ bool IsFileGeneratedByTarget(const SourceFile& file) const;
+
// Unknown generated inputs are files that a target declares as an input
// in the output directory, but which aren't generated by any dependency.
//
@@ -140,6 +144,7 @@
std::vector<SourceFile> written_files_;
std::vector<const Target*> write_runtime_deps_targets_;
std::multimap<SourceFile, const Target*> unknown_generated_inputs_;
+ std::map<SourceFile, bool> generated_files_;
DISALLOW_COPY_AND_ASSIGN(Scheduler);
};
diff --git a/tools/gn/substitution_pattern.h b/tools/gn/substitution_pattern.h
index 2f01e6b..850d736 100644
--- a/tools/gn/substitution_pattern.h
+++ b/tools/gn/substitution_pattern.h
@@ -67,6 +67,8 @@
const std::vector<Subrange>& ranges() const { return ranges_; }
bool empty() const { return ranges_.empty(); }
+ const ParseNode* origin() const { return origin_; }
+
private:
std::vector<Subrange> ranges_;
const ParseNode* origin_;
diff --git a/tools/gn/target.cc b/tools/gn/target.cc
index c5eebd3..0e9b245 100644
--- a/tools/gn/target.cc
+++ b/tools/gn/target.cc
@@ -394,6 +394,12 @@
if (!write_runtime_deps_output_.value().empty())
g_scheduler->AddWriteRuntimeDepsTarget(this);
+ if (output_type_ == GENERATED_FILE) {
+ DCHECK(!computed_outputs_.empty());
+ g_scheduler->AddGeneratedFile(
+ computed_outputs_[0].AsSourceFile(settings()->build_settings()));
+ }
+
return true;
}
@@ -866,7 +872,9 @@
&seen_targets)) {
seen_targets.clear();
// Allow dependency to be through data_deps for files generated by gn.
- check_data_deps = g_scheduler->IsFileGeneratedByWriteRuntimeDeps(out_file);
+ check_data_deps =
+ g_scheduler->IsFileGeneratedByWriteRuntimeDeps(out_file) ||
+ g_scheduler->IsFileGeneratedByTarget(source);
// Check object files (much slower and very rare) only if the "normal"
// output check failed.
consider_object_files = !check_data_deps;
@@ -880,14 +888,25 @@
bool Target::GetMetadata(const std::vector<std::string>& keys_to_extract,
const std::vector<std::string>& keys_to_walk,
const SourceDir& rebase_dir,
+ bool deps_only,
std::vector<Value>* result,
std::set<const Target*>* targets_walked,
Err* err) const {
std::vector<Value> next_walk_keys;
- if (!metadata_.WalkStep(settings()->build_settings(), keys_to_extract,
- keys_to_walk, rebase_dir, &next_walk_keys, result,
- err))
- return false;
+ // If deps_only, this is the top-level target and thus we don't want to
+ // collect its metadata, only that of its deps and data_deps.
+ if (deps_only) {
+ // Empty string will be converted below to mean all deps and data_deps.
+ // Origin is null because this isn't declared anywhere, and should never
+ // trigger any errors.
+ next_walk_keys.push_back(Value(nullptr, ""));
+ } else {
+ // Otherwise, we walk this target and collect the appropriate data.
+ if (!metadata_.WalkStep(settings()->build_settings(), keys_to_extract,
+ keys_to_walk, rebase_dir, &next_walk_keys, result,
+ err))
+ return false;
+ }
// Gather walk keys and find the appropriate target. Targets identified in
// the walk key set must be deps or data_deps of the declaring target.
@@ -905,7 +924,7 @@
auto pair = targets_walked->insert(dep.ptr);
if (pair.second) {
if (!dep.ptr->GetMetadata(keys_to_extract, keys_to_walk, rebase_dir,
- result, targets_walked, err))
+ false, result, targets_walked, err))
return false;
}
}
@@ -924,7 +943,7 @@
auto pair = targets_walked->insert(dep.ptr);
if (pair.second) {
if (!dep.ptr->GetMetadata(keys_to_extract, keys_to_walk, rebase_dir,
- result, targets_walked, err))
+ false, result, targets_walked, err))
return false;
}
// We found it, so we can exit this search now.
diff --git a/tools/gn/target.h b/tools/gn/target.h
index 0087e9e..abc357e 100644
--- a/tools/gn/target.h
+++ b/tools/gn/target.h
@@ -155,6 +155,7 @@
bool GetMetadata(const std::vector<std::string>& keys_to_extract,
const std::vector<std::string>& keys_to_walk,
const SourceDir& rebase_dir,
+ bool deps_only,
std::vector<Value>* result,
std::set<const Target*>* targets_walked,
Err* err) const;
@@ -167,7 +168,7 @@
const Value& output_conversion() const { return output_conversion_; }
void set_output_conversion(const Value& value) { output_conversion_ = value; }
- // Metadata collection methods for WriteData targets.
+ // Metadata collection methods for GeneratedFile targets.
const SourceDir& rebase() const { return rebase_; }
void set_rebase(const SourceDir& value) { rebase_ = value; }
const std::vector<std::string>& data_keys() const { return data_keys_; }
diff --git a/tools/gn/target_unittest.cc b/tools/gn/target_unittest.cc
index 55605c6..1cc4d96 100644
--- a/tools/gn/target_unittest.cc
+++ b/tools/gn/target_unittest.cc
@@ -1118,7 +1118,8 @@
Err err;
std::vector<Value> result;
std::set<const Target*> targets;
- one.GetMetadata(data_keys, walk_keys, SourceDir(), &result, &targets, &err);
+ one.GetMetadata(data_keys, walk_keys, SourceDir(), false, &result, &targets,
+ &err);
EXPECT_FALSE(err.has_error());
std::vector<Value> expected;
@@ -1158,7 +1159,8 @@
Err err;
std::vector<Value> result;
std::set<const Target*> targets;
- one.GetMetadata(data_keys, walk_keys, SourceDir(), &result, &targets, &err);
+ one.GetMetadata(data_keys, walk_keys, SourceDir(), false, &result, &targets,
+ &err);
EXPECT_FALSE(err.has_error());
std::vector<Value> expected;
@@ -1207,7 +1209,8 @@
Err err;
std::vector<Value> result;
std::set<const Target*> targets;
- one.GetMetadata(data_keys, walk_keys, SourceDir(), &result, &targets, &err);
+ one.GetMetadata(data_keys, walk_keys, SourceDir(), false, &result, &targets,
+ &err);
EXPECT_FALSE(err.has_error()) << err.message();
std::vector<Value> expected;
@@ -1239,7 +1242,8 @@
Err err;
std::vector<Value> result;
std::set<const Target*> targets;
- one.GetMetadata(data_keys, walk_keys, SourceDir(), &result, &targets, &err);
+ one.GetMetadata(data_keys, walk_keys, SourceDir(), false, &result, &targets,
+ &err);
EXPECT_TRUE(err.has_error());
EXPECT_EQ(err.message(),
"I was expecting //foo:missing to be a dependency of "
@@ -1248,3 +1252,51 @@
"specified the appropriate toolchain.")
<< err.message();
}
+
+TEST_F(TargetTest, WriteMetadataCollection) {
+ TestWithScope setup;
+ Err err;
+
+ SourceFile source_file("//out/Debug/metadata.json");
+ OutputFile output_file(setup.build_settings(), source_file);
+
+ TestTarget generator(setup, "//foo:write", Target::GENERATED_FILE);
+ generator.action_values().outputs() =
+ SubstitutionList::MakeForTest("//out/Debug/metadata.json");
+ EXPECT_TRUE(generator.OnResolved(&err));
+
+ TestTarget middle_data_dep(setup, "//foo:middle", Target::EXECUTABLE);
+ middle_data_dep.data_deps().push_back(LabelTargetPair(&generator));
+ EXPECT_TRUE(middle_data_dep.OnResolved(&err));
+
+ // This target has a generated metadata input and no dependency makes it.
+ TestTarget dep_missing(setup, "//foo:no_dep", Target::EXECUTABLE);
+ dep_missing.sources().push_back(source_file);
+ EXPECT_TRUE(dep_missing.OnResolved(&err));
+ AssertSchedulerHasOneUnknownFileMatching(&dep_missing, source_file);
+ scheduler().ClearUnknownGeneratedInputsAndWrittenFiles();
+
+ // This target has a generated file and we've directly dependended on it.
+ TestTarget dep_present(setup, "//foo:with_dep", Target::EXECUTABLE);
+ dep_present.sources().push_back(source_file);
+ dep_present.private_deps().push_back(LabelTargetPair(&generator));
+ EXPECT_TRUE(dep_present.OnResolved(&err));
+ EXPECT_TRUE(scheduler().GetUnknownGeneratedInputs().empty());
+
+ // This target has a generated file and we've indirectly dependended on it
+ // via data_deps.
+ TestTarget dep_indirect(setup, "//foo:indirect_dep", Target::EXECUTABLE);
+ dep_indirect.sources().push_back(source_file);
+ dep_indirect.data_deps().push_back(LabelTargetPair(&middle_data_dep));
+ EXPECT_TRUE(dep_indirect.OnResolved(&err));
+ AssertSchedulerHasOneUnknownFileMatching(&dep_indirect, source_file);
+ scheduler().ClearUnknownGeneratedInputsAndWrittenFiles();
+
+ // This target has a generated file and we've directly dependended on it
+ // via data_deps.
+ TestTarget data_dep_present(setup, "//foo:with_data_dep", Target::EXECUTABLE);
+ data_dep_present.sources().push_back(source_file);
+ data_dep_present.data_deps().push_back(LabelTargetPair(&generator));
+ EXPECT_TRUE(data_dep_present.OnResolved(&err));
+ EXPECT_TRUE(scheduler().GetUnknownGeneratedInputs().empty());
+}
