[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()); +}