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;
