GN: Generate error if multiple rules generate same file

This will pretty much always result in a bad build but is only a warning
in ninja.

Review URL: https://codereview.chromium.org/455193003

Cr-Commit-Position: refs/heads/master@{#297077}

tools/gn/command_gen.cc

@@ -94,10 +94,14 @@ int RunGen(const std::vector<std::string>& args) {
   if (!setup->Run())
     return 1;
 
+  Err err;
   // Write the root ninja files.
   if (!NinjaWriter::RunAndWriteFiles(&setup->build_settings(),
-                                     setup->builder()))
+                                     setup->builder(),
+                                     &err)) {
+    err.PrintToStdout();
     return 1;
+  }
 
   base::TimeDelta elapsed_time = timer.Elapsed();
 

tools/gn/ninja_build_writer.cc

@@ -15,6 +15,7 @@
 #include "base/strings/utf_string_conversions.h"
 #include "build/build_config.h"
 #include "tools/gn/build_settings.h"
+#include "tools/gn/err.h"
 #include "tools/gn/escape.h"
 #include "tools/gn/filesystem_utils.h"
 #include "tools/gn/input_file_manager.h"
@@ -91,11 +92,11 @@ NinjaBuildWriter::NinjaBuildWriter(
 NinjaBuildWriter::~NinjaBuildWriter() {
 }
 
-void NinjaBuildWriter::Run() {
+bool NinjaBuildWriter::Run(Err* err) {
   WriteNinjaRules();
   WriteLinkPool();
   WriteSubninjas();
-  WritePhonyAndAllRules();
+  return WritePhonyAndAllRules(err);
 }
 
 // static
@@ -103,7 +104,8 @@ bool NinjaBuildWriter::RunAndWriteFile(
     const BuildSettings* build_settings,
     const std::vector<const Settings*>& all_settings,
     const Toolchain* default_toolchain,
-    const std::vector<const Target*>& default_toolchain_targets) {
+    const std::vector<const Target*>& default_toolchain_targets,
+    Err* err) {
   ScopedTrace trace(TraceItem::TRACE_FILE_WRITE, "build.ninja");
 
   base::FilePath ninja_file(build_settings->GetFullPath(
@@ -113,19 +115,22 @@ bool NinjaBuildWriter::RunAndWriteFile(
   std::ofstream file;
   file.open(FilePathToUTF8(ninja_file).c_str(),
             std::ios_base::out | std::ios_base::binary);
-  if (file.fail())
+  if (file.fail()) {
+    *err = Err(Location(), "Couldn't open build.ninja for writing");
     return false;
+  }
 
   std::ofstream depfile;
   depfile.open((FilePathToUTF8(ninja_file) + ".d").c_str(),
                std::ios_base::out | std::ios_base::binary);
-  if (depfile.fail())
+  if (depfile.fail()) {
+    *err = Err(Location(), "Couldn't open depfile for writing");
     return false;
+  }
 
   NinjaBuildWriter gen(build_settings, all_settings, default_toolchain,
                        default_toolchain_targets, file, depfile);
-  gen.Run();
-  return true;
+  return gen.Run(err);
 }
 
 void NinjaBuildWriter::WriteNinjaRules() {
@@ -171,7 +176,7 @@ void NinjaBuildWriter::WriteSubninjas() {
   out_ << std::endl;
 }
 
-void NinjaBuildWriter::WritePhonyAndAllRules() {
+bool NinjaBuildWriter::WritePhonyAndAllRules(Err* err) {
   std::string all_rules;
 
   // Write phony rules for all uniquely-named targets in the default toolchain.
@@ -180,6 +185,7 @@ void NinjaBuildWriter::WritePhonyAndAllRules() {
   // which we also find.
   std::map<std::string, int> small_name_count;
   std::vector<const Target*> toplevel_targets;
+  base::hash_set<std::string> target_files;
   for (size_t i = 0; i < default_toolchain_targets_.size(); i++) {
     const Target* target = default_toolchain_targets_[i];
     const Label& label = target->label();
@@ -204,6 +210,10 @@ void NinjaBuildWriter::WritePhonyAndAllRules() {
     OutputFile target_file(target->dependency_output_file());
     // The output files may have leading "./" so normalize those away.
     NormalizePath(&target_file.value());
+    if (!target_files.insert(target_file.value()).second) {
+      *err = Err(Location(), "Duplicate rules for " + target_file.value());
+      return false;
+    }
 
     // Write the long name "foo/bar:baz" for the target "//foo/bar:baz".
     std::string long_name = label.GetUserVisibleName(false);
@@ -244,6 +254,7 @@ void NinjaBuildWriter::WritePhonyAndAllRules() {
     out_ << "\nbuild all: phony " << all_rules << std::endl;
     out_ << "default all" << std::endl;
   }
+  return true;
 }
 
 void NinjaBuildWriter::WritePhonyRule(const Target* target,

tools/gn/ninja_build_writer.h

@@ -11,6 +11,7 @@
 #include "tools/gn/path_output.h"
 
 class BuildSettings;
+class Err;
 class Settings;
 class Target;
 class Toolchain;
@@ -24,7 +25,8 @@ class NinjaBuildWriter {
       const BuildSettings* settings,
       const std::vector<const Settings*>& all_settings,
       const Toolchain* default_toolchain,
-      const std::vector<const Target*>& default_toolchain_targets);
+      const std::vector<const Target*>& default_toolchain_targets,
+      Err* err);
 
  private:
   NinjaBuildWriter(const BuildSettings* settings,
@@ -35,12 +37,12 @@ class NinjaBuildWriter {
                    std::ostream& dep_out);
   ~NinjaBuildWriter();
 
-  void Run();
+  bool Run(Err* err);
 
   void WriteNinjaRules();
   void WriteLinkPool();
   void WriteSubninjas();
-  void WritePhonyAndAllRules();
+  bool WritePhonyAndAllRules(Err* err);
 
   void WritePhonyRule(const Target* target, const OutputFile& target_file,
                       const std::string& phony_name);

tools/gn/ninja_writer.cc

@@ -22,28 +22,31 @@ NinjaWriter::~NinjaWriter() {
 
 // static
 bool NinjaWriter::RunAndWriteFiles(const BuildSettings* build_settings,
-                                   Builder* builder) {
+                                   Builder* builder,
+                                   Err* err) {
   NinjaWriter writer(build_settings, builder);
 
   std::vector<const Settings*> all_settings;
   std::vector<const Target*> default_targets;
-  if (!writer.WriteToolchains(&all_settings, &default_targets))
+  if (!writer.WriteToolchains(&all_settings, &default_targets, err))
     return false;
-  return writer.WriteRootBuildfiles(all_settings, default_targets);
+  return writer.WriteRootBuildfiles(all_settings, default_targets, err);
 }
 
 // static
 bool NinjaWriter::RunAndWriteToolchainFiles(
     const BuildSettings* build_settings,
     Builder* builder,
-    std::vector<const Settings*>* all_settings) {
+    std::vector<const Settings*>* all_settings,
+    Err* err) {
   NinjaWriter writer(build_settings, builder);
   std::vector<const Target*> default_targets;
-  return writer.WriteToolchains(all_settings, &default_targets);
+  return writer.WriteToolchains(all_settings, &default_targets, err);
 }
 
 bool NinjaWriter::WriteToolchains(std::vector<const Settings*>* all_settings,
-                                  std::vector<const Target*>* default_targets) {
+                                  std::vector<const Target*>* default_targets,
+                                  Err* err) {
   // Categorize all targets by toolchain.
   typedef std::map<Label, std::vector<const Target*> > CategorizedMap;
   CategorizedMap categorized;
@@ -88,7 +91,8 @@ bool NinjaWriter::WriteToolchains(std::vector<const Settings*>* all_settings,
 
 bool NinjaWriter::WriteRootBuildfiles(
     const std::vector<const Settings*>& all_settings,
-    const std::vector<const Target*>& default_targets) {
+    const std::vector<const Target*>& default_targets,
+    Err* err) {
   // All Settings objects should have the same default toolchain, and there
   // should always be at least one settings object in the build.
   CHECK(!all_settings.empty());
@@ -96,11 +100,7 @@ bool NinjaWriter::WriteRootBuildfiles(
       builder_->GetToolchain(all_settings[0]->default_toolchain_label());
 
   // Write the root buildfile.
-  if (!NinjaBuildWriter::RunAndWriteFile(build_settings_, all_settings,
-                                         default_toolchain, default_targets)) {
-    Err(Location(),
-        "Couldn't open toolchain buildfile(s) for writing").PrintToStdout();
-    return false;
-  }
-  return true;
+  return NinjaBuildWriter::RunAndWriteFile(build_settings_, all_settings,
+                                           default_toolchain, default_targets,
+                                           err);
 }

tools/gn/ninja_writer.h

@@ -13,21 +13,24 @@
 
 class Builder;
 class BuildSettings;
+class Err;
 class Settings;
 class Target;
 
 class NinjaWriter {
  public:
-  // On failure will print an error and will return false.
+  // On failure will populate |err| and will return false.
   static bool RunAndWriteFiles(const BuildSettings* build_settings,
-                               Builder* builder);
+                               Builder* builder,
+                               Err* err);
 
   // Writes only the toolchain.ninja files, skipping the root buildfile. The
   // settings for the files written will be added to the vector.
   static bool RunAndWriteToolchainFiles(
       const BuildSettings* build_settings,
       Builder* builder,
-      std::vector<const Settings*>* all_settings);
+      std::vector<const Settings*>* all_settings,
+      Err* err);
 
  private:
   NinjaWriter(const BuildSettings* build_settings, Builder* builder);
@@ -35,9 +38,11 @@ class NinjaWriter {
 
   bool WriteToolchains(
       std::vector<const Settings*>* all_settings,
-      std::vector<const Target*>* default_targets);
+      std::vector<const Target*>* default_targets,
+      Err* err);
   bool WriteRootBuildfiles(const std::vector<const Settings*>& all_settings,
-                           const std::vector<const Target*>& default_targets);
+                           const std::vector<const Target*>& default_targets,
+                           Err* err);
 
   const BuildSettings* build_settings_;
   Builder* builder_;