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-Commit-Position: refs/heads/master@{#408676}

tools/gn/builder.cc

@@ -404,9 +404,15 @@ bool Builder::AddToolchainDep(BuilderRecord* record,
 
 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 @@ bool Builder::ResolveItem(BuilderRecord* record, Err* err) {
 
   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()) {

tools/gn/builder.h

@@ -22,15 +22,16 @@ class ParseNode;
 // 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 @@ class Builder {
   typedef base::hash_map<Label, BuilderRecord*> RecordMap;
   RecordMap records_;
 
-  ResolvedCallback resolved_callback_;
+  ResolvedGeneratedCallback resolved_and_generated_callback_;
 
   DISALLOW_COPY_AND_ASSIGN(Builder);
 };

tools/gn/command_gen.cc

@@ -67,8 +67,8 @@ void BackgroundDoWrite(TargetWriteInfo* write_info, const Target* target) {
 }
 
 // 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 @@ int RunGen(const std::vector<std::string>& args) {
 
   // 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())