GN: only write targets that should be generated.
Apparently forever GN has been writing .ninja files for targets that were resolved but didn't have the "should generate" bit set. The "should generate" bit will be unset for things in non-default toolchains that aren't required to build anything in the main toolchain. This set of things is small since most things will have incomplete deps if they're not required. But it comes up for a few NaCl copy rules.
The extra files were never a problem because only "should generate" targets were written to the main ninja file, so the extra files were just unreferenced. The recent change that moves more stuff into the main file rather than subninjas exposed the bug, which now appears as multiply-defined targets.
This fix only issues the callback when an item is both resolved and "should generate" is set.
Review-Url: https://codereview.chromium.org/2195753002
Cr-Original-Commit-Position: refs/heads/master@{#408676}
Cr-Mirrored-From: https://chromium.googlesource.com/chromium/src
Cr-Mirrored-Commit: da21696cd246f78f66bdb5d6f6aa9deadf2e6abc
diff --git a/tools/gn/builder.cc b/tools/gn/builder.cc
index 93aec15..bb3f834 100644
--- a/tools/gn/builder.cc
+++ b/tools/gn/builder.cc
@@ -404,9 +404,15 @@
void Builder::RecursiveSetShouldGenerate(BuilderRecord* record,
bool force) {
- if (!force && record->should_generate())
- return; // Already set.
- record->set_should_generate(true);
+ if (!record->should_generate()) {
+ record->set_should_generate(true);
+
+ // This may have caused the item to go into "resolved and generated" state.
+ if (record->resolved() && !resolved_and_generated_callback_.is_null())
+ resolved_and_generated_callback_.Run(record);
+ } else if (!force) {
+ return; // Already set and we're not required to iterate dependencies.
+ }
for (auto* cur : record->all_deps()) {
if (!cur->should_generate()) {
@@ -451,8 +457,8 @@
if (!record->item()->OnResolved(err))
return false;
- if (!resolved_callback_.is_null())
- resolved_callback_.Run(record);
+ if (record->should_generate() && !resolved_and_generated_callback_.is_null())
+ resolved_and_generated_callback_.Run(record);
// Recursively update everybody waiting on this item to be resolved.
for (BuilderRecord* waiting : record->waiting_on_resolution()) {
diff --git a/tools/gn/builder.h b/tools/gn/builder.h
index d8209bc..c8b54c9 100644
--- a/tools/gn/builder.h
+++ b/tools/gn/builder.h
@@ -22,15 +22,16 @@
// the main thread only. See also BuilderRecord.
class Builder {
public:
- typedef base::Callback<void(const BuilderRecord*)> ResolvedCallback;
+ typedef base::Callback<void(const BuilderRecord*)> ResolvedGeneratedCallback;
explicit Builder(Loader* loader);
~Builder();
- // The resolved callback is called whenever a target has been resolved. This
- // will be executed only on the main thread.
- void set_resolved_callback(const ResolvedCallback& cb) {
- resolved_callback_ = cb;
+ // The resolved callback is called when a target has been both resolved and
+ // marked generated. This will be executed only on the main thread.
+ void set_resolved_and_generated_callback(
+ const ResolvedGeneratedCallback& cb) {
+ resolved_and_generated_callback_ = cb;
}
Loader* loader() const { return loader_; }
@@ -132,7 +133,7 @@
typedef base::hash_map<Label, BuilderRecord*> RecordMap;
RecordMap records_;
- ResolvedCallback resolved_callback_;
+ ResolvedGeneratedCallback resolved_and_generated_callback_;
DISALLOW_COPY_AND_ASSIGN(Builder);
};
diff --git a/tools/gn/command_gen.cc b/tools/gn/command_gen.cc
index 7de9aed..60735f8 100644
--- a/tools/gn/command_gen.cc
+++ b/tools/gn/command_gen.cc
@@ -67,8 +67,8 @@
}
// Called on the main thread.
-void ItemResolvedCallback(TargetWriteInfo* write_info,
- const BuilderRecord* record) {
+void ItemResolvedAndGeneratedCallback(TargetWriteInfo* write_info,
+ const BuilderRecord* record) {
const Item* item = record->item();
const Target* target = item->AsTarget();
if (target) {
@@ -387,8 +387,8 @@
// Cause the load to also generate the ninja files for each target.
TargetWriteInfo write_info;
- setup->builder().set_resolved_callback(
- base::Bind(&ItemResolvedCallback, &write_info));
+ setup->builder().set_resolved_and_generated_callback(
+ base::Bind(&ItemResolvedAndGeneratedCallback, &write_info));
// Do the actual load. This will also write out the target ninja files.
if (!setup->Run())
