Record generated_file()s as outputs of `gn` This is an accurate relationship and allows for tooling to ensure a given generated_file() is up-to-date without an unconditional `gn gen`. build.ninja was manually verified in a particular GN-based build, wherein `ninja $SOME_GENERATED_FILE` now works. Fixed: 301 Change-Id: I06c7ec142072d2e73860cdde4898af6f9e65065a Reviewed-on: https://gn-review.googlesource.com/c/gn/+/14580 Commit-Queue: Joshua Seaton <joshuaseaton@google.com> Reviewed-by: Brett Wilson <brettw@chromium.org>
diff --git a/src/gn/ninja_build_writer.cc b/src/gn/ninja_build_writer.cc index 6a5b91d..5e83bc8 100644 --- a/src/gn/ninja_build_writer.cc +++ b/src/gn/ninja_build_writer.cc
@@ -23,6 +23,7 @@ #include "gn/input_file_manager.h" #include "gn/loader.h" #include "gn/ninja_utils.h" +#include "gn/pointer_set.h" #include "gn/pool.h" #include "gn/scheduler.h" #include "gn/string_atom.h" @@ -140,10 +141,10 @@ // Given an output that appears more than once, generates an error message // that describes the problem and which targets generate it. -Err GetDuplicateOutputError(const std::vector<const Target*>& all_targets, +Err GetDuplicateOutputError(const std::vector<const Target*>& targets, const OutputFile& bad_output) { std::vector<const Target*> matches; - for (const Target* target : all_targets) { + for (const Target* target : targets) { for (const auto& output : target->computed_outputs()) { if (output == bad_output) { matches.push_back(target); @@ -196,7 +197,7 @@ const BuildSettings* build_settings, const std::unordered_map<const Settings*, const Toolchain*>& used_toolchains, - const std::vector<const Target*>& all_targets, + const PointerSet<const Target>& all_targets, const Toolchain* default_toolchain, const std::vector<const Target*>& default_toolchain_targets, std::ostream& out, @@ -240,9 +241,11 @@ // skip adding it to the list every time in the loop. used_toolchains[default_toolchain_settings] = default_toolchain; + PointerSet<const Target> target_set; std::vector<const Target*> default_toolchain_targets; default_toolchain_targets.reserve(all_targets.size()); for (const Target* target : all_targets) { + target_set.insert(target); if (target->settings() == default_toolchain_settings) { default_toolchain_targets.push_back(target); // The default toolchain will already have been added to the used @@ -256,7 +259,7 @@ std::stringstream file; std::stringstream depfile; - NinjaBuildWriter gen(build_settings, used_toolchains, all_targets, + NinjaBuildWriter gen(build_settings, used_toolchains, target_set, default_toolchain, default_toolchain_targets, file, depfile); if (!gen.Run(err)) @@ -332,7 +335,27 @@ << "# build.ninja edge is separate to prevent ninja from deleting it\n" << "# (due to depfile usage) if interrupted. gn uses atomic writes to\n" << "# ensure that build.ninja is always valid even if interrupted.\n" - << "build build.ninja.stamp: gn\n" + << "build build.ninja.stamp"; + + // Record all generated files as outputs of `gn` as well. + // + // TODO(fxbug.dev/303): We currently rely on the sort below to dedupe + // entries. + std::multimap<OutputFile, const Target*> generated_files = g_scheduler->GetGeneratedFiles(); + VectorSetSorter<OutputFile> generated_file_sorter(generated_files.size()); + for (const auto& entry : generated_files) { + // Only includes those actually in the build. + if (all_targets_.contains(entry.second)) { + generated_file_sorter.Add(entry.first); + } + } + generated_file_sorter.IterateOver( + [this](const OutputFile& output_file) { + out_ << " "; + path_output_.WriteFile(out_, output_file); + }); + + out_ << ": gn\n" << " generator = 1\n" << " depfile = build.ninja.d\n" << "\n" @@ -359,7 +382,6 @@ const base::FilePath build_path = build_settings_->build_dir().Resolve(build_settings_->root_path()); - EscapeOptions depfile_escape; depfile_escape.mode = ESCAPE_DEPFILE; auto item_callback = [this, &depfile_escape,
diff --git a/src/gn/ninja_build_writer.h b/src/gn/ninja_build_writer.h index c21769a..dbfe891 100644 --- a/src/gn/ninja_build_writer.h +++ b/src/gn/ninja_build_writer.h
@@ -12,6 +12,7 @@ #include <vector> #include "gn/path_output.h" +#include "gn/pointer_set.h" class Builder; class BuildSettings; @@ -32,7 +33,7 @@ NinjaBuildWriter(const BuildSettings* settings, const std::unordered_map<const Settings*, const Toolchain*>& used_toolchains, - const std::vector<const Target*>& all_targets, + const PointerSet<const Target>& all_targets, const Toolchain* default_toolchain, const std::vector<const Target*>& default_toolchain_targets, std::ostream& out, @@ -88,7 +89,7 @@ const BuildSettings* build_settings_; const std::unordered_map<const Settings*, const Toolchain*>& used_toolchains_; - const std::vector<const Target*>& all_targets_; + const PointerSet<const Target>& all_targets_; const Toolchain* default_toolchain_; const std::vector<const Target*>& default_toolchain_targets_;
diff --git a/src/gn/ninja_build_writer_unittest.cc b/src/gn/ninja_build_writer_unittest.cc index 8ab5e56..8966f59 100644 --- a/src/gn/ninja_build_writer_unittest.cc +++ b/src/gn/ninja_build_writer_unittest.cc
@@ -134,13 +134,14 @@ used_toolchains[setup.settings()] = setup.toolchain(); used_toolchains[&other_settings] = &other_toolchain; - std::vector<const Target*> targets = {&target_foo, &target_bar, &target_baz}; + std::vector<const Target*> target_list = {&target_foo, &target_bar, &target_baz}; + PointerSet<const Target> target_set(target_list.begin(), target_list.end()); std::ostringstream ninja_out; std::ostringstream depfile_out; - NinjaBuildWriter writer(setup.build_settings(), used_toolchains, targets, - setup.toolchain(), targets, ninja_out, depfile_out); + NinjaBuildWriter writer(setup.build_settings(), used_toolchains, target_set, + setup.toolchain(), target_list, ninja_out, depfile_out); ASSERT_TRUE(writer.Run(&err)); const char expected_rule_gn[] = "rule gn\n"; @@ -189,6 +190,65 @@ EXPECT_EQ(std::string::npos, out_str.find("pool console")); } +TEST_F(NinjaBuildWriterTest, GeneratedFiles) { + TestWithScope setup; + Err err; + + Target target_foo(setup.settings(), Label(SourceDir("//foo/"), "foo")); + target_foo.set_output_type(Target::GENERATED_FILE); + target_foo.action_values().outputs() = SubstitutionList::MakeForTest( + "//out/Debug/foo.json"); + target_foo.SetToolchain(setup.toolchain()); + ASSERT_TRUE(target_foo.OnResolved(&err)); + + Target target_bar(setup.settings(), Label(SourceDir("//bar/"), "bar")); + target_bar.set_output_type(Target::GENERATED_FILE); + target_bar.action_values().outputs() = SubstitutionList::MakeForTest( + "//out/Debug/bar.txt"); + target_bar.SetToolchain(setup.toolchain()); + ASSERT_TRUE(target_bar.OnResolved(&err)); + + // Make a secondary toolchain that references two pools. + Label other_toolchain_label(SourceDir("//other/"), "toolchain"); + Toolchain other_toolchain(setup.settings(), other_toolchain_label); + TestWithScope::SetupToolchain(&other_toolchain); + + std::unordered_map<const Settings*, const Toolchain*> used_toolchains; + used_toolchains[setup.settings()] = setup.toolchain(); + + std::vector<const Target*> target_list = {&target_foo, &target_bar}; + PointerSet<const Target> target_set(target_list.begin(), target_list.end()); + + std::ostringstream ninja_out; + std::ostringstream depfile_out; + + NinjaBuildWriter writer(setup.build_settings(), used_toolchains, target_set, + setup.toolchain(), target_list, ninja_out, depfile_out); + ASSERT_TRUE(writer.Run(&err)); + + const char expected_rule_gn[] = "rule gn\n"; + const char expected_build_ninja_stamp[] = + "build build.ninja.stamp bar.txt foo.json: gn\n" + " generator = 1\n" + " depfile = build.ninja.d\n"; + const char expected_build_ninja[] = + "build build.ninja: phony build.ninja.stamp\n" + " generator = 1\n"; + const char expected_toolchain[] = "subninja toolchain.ninja\n"; + const char expected_default[] = "default all\n"; + std::string out_str = ninja_out.str(); +#define EXPECT_SNIPPET(expected) \ + EXPECT_NE(std::string::npos, out_str.find(expected)) \ + << "Expected to find: " << expected << "\n" \ + << "Within: " << out_str + EXPECT_SNIPPET(expected_rule_gn); + EXPECT_SNIPPET(expected_build_ninja_stamp); + EXPECT_SNIPPET(expected_build_ninja); + EXPECT_SNIPPET(expected_toolchain); + EXPECT_SNIPPET(expected_default); +#undef EXPECT_SNIPPET +} + TEST_F(NinjaBuildWriterTest, ExtractRegenerationCommands) { TestWithScope setup; Err err; @@ -210,13 +270,14 @@ std::unordered_map<const Settings*, const Toolchain*> used_toolchains; used_toolchains[setup.settings()] = setup.toolchain(); - std::vector<const Target*> targets = {&target_foo}; + std::vector<const Target*> target_list = {&target_foo}; + PointerSet<const Target> target_set(target_list.begin(), target_list.end()); std::stringstream ninja_out; std::ostringstream depfile_out; - NinjaBuildWriter writer(setup.build_settings(), used_toolchains, targets, - setup.toolchain(), targets, ninja_out, depfile_out); + NinjaBuildWriter writer(setup.build_settings(), used_toolchains, target_set, + setup.toolchain(), target_list, ninja_out, depfile_out); ASSERT_TRUE(writer.Run(&err)); const char expected_rule_gn[] = "rule gn\n"; @@ -287,11 +348,10 @@ std::unordered_map<const Settings*, const Toolchain*> used_toolchains; used_toolchains[setup.settings()] = setup.toolchain(); - std::vector<const Target*> targets; std::ostringstream ninja_out; std::ostringstream depfile_out; - NinjaBuildWriter writer(setup.build_settings(), used_toolchains, targets, - setup.toolchain(), targets, ninja_out, depfile_out); + NinjaBuildWriter writer(setup.build_settings(), used_toolchains, {}, + setup.toolchain(), {}, ninja_out, depfile_out); ASSERT_TRUE(writer.Run(&err)); EXPECT_EQ(depfile_out.str(), @@ -320,11 +380,13 @@ std::unordered_map<const Settings*, const Toolchain*> used_toolchains; used_toolchains[setup.settings()] = setup.toolchain(); - std::vector<const Target*> targets = {&target_foo, &target_bar}; + + std::vector<const Target*> target_list = {&target_foo, &target_bar}; + PointerSet<const Target> target_set(target_list.begin(), target_list.end()); std::ostringstream ninja_out; std::ostringstream depfile_out; - NinjaBuildWriter writer(setup.build_settings(), used_toolchains, targets, - setup.toolchain(), targets, ninja_out, depfile_out); + NinjaBuildWriter writer(setup.build_settings(), used_toolchains, target_set, + setup.toolchain(), target_list, ninja_out, depfile_out); ASSERT_FALSE(writer.Run(&err)); const char expected_help_test[] =
diff --git a/src/gn/scheduler.cc b/src/gn/scheduler.cc index 6525f33..35ae097 100644 --- a/src/gn/scheduler.cc +++ b/src/gn/scheduler.cc
@@ -111,16 +111,21 @@ return false; } -void Scheduler::AddGeneratedFile(const SourceFile& entry) { +void Scheduler::AddGeneratedFile(const Target* target, const OutputFile& file) { std::lock_guard<std::mutex> lock(lock_); - generated_files_.insert(std::make_pair(entry, true)); + generated_files_.insert(std::make_pair(file, target)); } -bool Scheduler::IsFileGeneratedByTarget(const SourceFile& file) const { +bool Scheduler::IsFileGeneratedByTarget(const OutputFile& file) const { std::lock_guard<std::mutex> lock(lock_); return generated_files_.find(file) != generated_files_.end(); } +std::multimap<OutputFile, const Target*> Scheduler::GetGeneratedFiles() const { + std::lock_guard<std::mutex> lock(lock_); + return generated_files_; +} + std::multimap<SourceFile, const Target*> Scheduler::GetUnknownGeneratedInputs() const { std::lock_guard<std::mutex> lock(lock_);
diff --git a/src/gn/scheduler.h b/src/gn/scheduler.h index cf29fe8..52e0b47 100644 --- a/src/gn/scheduler.h +++ b/src/gn/scheduler.h
@@ -70,8 +70,17 @@ bool IsFileGeneratedByWriteRuntimeDeps(const OutputFile& file) const; // Tracks generated_file calls. - void AddGeneratedFile(const SourceFile& entry); - bool IsFileGeneratedByTarget(const SourceFile& file) const; + void AddGeneratedFile(const Target* target, const OutputFile& file); + bool IsFileGeneratedByTarget(const OutputFile& file) const; + + // Returns the collection of generated files. The associated target is + // tracked as well in order to determine whether the generated file actually + // was created (i.e., whether the target was included in the build). This + // target pointer must only be dereferenced after the graph is complete. + // + // TODO(crbug.com/gn/303): This should be a map but currently GN allows + // multiple overlapping generated files. + std::multimap<OutputFile, const Target*> GetGeneratedFiles() 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. @@ -145,7 +154,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_; + std::multimap<OutputFile, const Target*> generated_files_; Scheduler(const Scheduler&) = delete; Scheduler& operator=(const Scheduler&) = delete;
diff --git a/src/gn/target.cc b/src/gn/target.cc index 7b545b5..ea6277a 100644 --- a/src/gn/target.cc +++ b/src/gn/target.cc
@@ -526,8 +526,7 @@ if (output_type_ == GENERATED_FILE) { DCHECK(!computed_outputs_.empty()); - g_scheduler->AddGeneratedFile( - computed_outputs_[0].AsSourceFile(settings()->build_settings())); + g_scheduler->AddGeneratedFile(this, computed_outputs_[0]); } return true; @@ -1221,7 +1220,7 @@ // Allow dependency to be through data_deps for files generated by gn. check_data_deps = g_scheduler->IsFileGeneratedByWriteRuntimeDeps(out_file) || - g_scheduler->IsFileGeneratedByTarget(source); + g_scheduler->IsFileGeneratedByTarget(out_file); // Check object files (much slower and very rare) only if the "normal" // output check failed. consider_object_files = !check_data_deps;