Merge topic 'dev/refactor-source-depends-in-ninja'

2583eff6 ninja: Factor out custom command order-only depends
18e478a8 ninja: Factor out target-level order-only dependencies
6fa6bedf LocalGenerator: Add a string overload for AppendFlags
01b79c63 ninja: Don't use a stringstream to build an argument list
3c640891 ninja: Use string parameters

Source/cmExtraSublimeTextGenerator.cxx

@@ -399,7 +399,7 @@ cmExtraSublimeTextGenerator::ComputeFlagsForObject(cmSourceFile* source,
   lg->GetIncludeDirectories(includes, gtgt, language, config);
   std::string includeFlags =
     lg->GetIncludeFlags(includes, gtgt, language, true); // full include paths
-  lg->AppendFlags(flags, includeFlags.c_str());
+  lg->AppendFlags(flags, includeFlags);
   }
 
   // Append old-style preprocessor definition flags.

Source/cmGlobalNinjaGenerator.cxx

@@ -141,7 +141,7 @@ void cmGlobalNinjaGenerator::WriteBuild(std::ostream& os,
 
   cmGlobalNinjaGenerator::WriteComment(os, comment);
 
-  cmOStringStream arguments;
+  std::string arguments;
 
   // TODO: Better formatting for when there are multiple input/output files.
 
@@ -150,7 +150,7 @@ void cmGlobalNinjaGenerator::WriteBuild(std::ostream& os,
       i != explicitDeps.end();
       ++i)
     {
-    arguments  << " " << EncodeIdent(EncodePath(*i), os);
+    arguments += " " + EncodeIdent(EncodePath(*i), os);
 
     //we need to track every dependency that comes in, since we are trying
     //to find dependencies that are side effects of build commands
@@ -161,39 +161,39 @@ void cmGlobalNinjaGenerator::WriteBuild(std::ostream& os,
   // Write implicit dependencies.
   if(!implicitDeps.empty())
     {
-    arguments << " |";
+    arguments += " |";
     for(cmNinjaDeps::const_iterator i = implicitDeps.begin();
         i != implicitDeps.end();
         ++i)
-      arguments  << " " << EncodeIdent(EncodePath(*i), os);
+      arguments += " " + EncodeIdent(EncodePath(*i), os);
     }
 
   // Write order-only dependencies.
   if(!orderOnlyDeps.empty())
     {
-    arguments << " ||";
+    arguments += " ||";
     for(cmNinjaDeps::const_iterator i = orderOnlyDeps.begin();
         i != orderOnlyDeps.end();
         ++i)
-      arguments  << " " << EncodeIdent(EncodePath(*i), os);
+      arguments += " " + EncodeIdent(EncodePath(*i), os);
     }
 
-  arguments << "\n";
+  arguments += "\n";
 
-  cmOStringStream build;
+  std::string build;
 
   // Write outputs files.
-  build << "build";
+  build += "build";
   for(cmNinjaDeps::const_iterator i = outputs.begin();
       i != outputs.end(); ++i)
     {
-    build << " " << EncodeIdent(EncodePath(*i), os);
+    build += " " + EncodeIdent(EncodePath(*i), os);
     this->CombinedBuildOutputs.insert( EncodePath(*i) );
     }
-  build << ":";
+  build += ":";
 
   // Write the rule.
-  build << " " << rule;
+  build += " " + rule;
 
   // Write the variables bound to this build statement.
   cmOStringStream variable_assignments;
@@ -203,9 +203,9 @@ void cmGlobalNinjaGenerator::WriteBuild(std::ostream& os,
                                           i->first, i->second, "", 1);
 
   // check if a response file rule should be used
-  std::string buildstr = build.str();
+  std::string buildstr = build;
   std::string assignments = variable_assignments.str();
-  const std::string args = arguments.str();
+  const std::string& args = arguments;
   if (cmdLineLimit > 0
       && args.size() + buildstr.size() + assignments.size()
                                                     > (size_t) cmdLineLimit) {

Source/cmLocalGenerator.cxx

@@ -2214,7 +2214,7 @@ static void AddVisibilityCompileOption(std::string &flags, cmTarget* target,
     return;
     }
   std::string option = std::string(opt) + prop;
-  lg->AppendFlags(flags, option.c_str());
+  lg->AppendFlags(flags, option);
 }
 
 static void AddInlineVisibilityCompileOption(std::string &flags,
@@ -2398,11 +2398,10 @@ void cmLocalGenerator::AddConfigVariableFlags(std::string& flags,
 
 //----------------------------------------------------------------------------
 void cmLocalGenerator::AppendFlags(std::string& flags,
-                                                const char* newFlags)
+                                   const std::string& newFlags)
 {
-  if(newFlags && *newFlags)
+  if(!newFlags.empty())
     {
-    std::string newf = newFlags;
     if(flags.size())
       {
       flags += " ";
@@ -2411,11 +2410,21 @@ void cmLocalGenerator::AppendFlags(std::string& flags,
     }
 }
 
+//----------------------------------------------------------------------------
+void cmLocalGenerator::AppendFlags(std::string& flags,
+                                   const char* newFlags)
+{
+  if(newFlags && *newFlags)
+    {
+    this->AppendFlags(flags, std::string(newFlags));
+    }
+}
+
 //----------------------------------------------------------------------------
 void cmLocalGenerator::AppendFlagEscape(std::string& flags,
                                         const std::string& rawFlag)
 {
-  this->AppendFlags(flags, this->EscapeForShell(rawFlag).c_str());
+  this->AppendFlags(flags, this->EscapeForShell(rawFlag));
 }
 
 //----------------------------------------------------------------------------

Source/cmLocalGenerator.h

@@ -152,6 +152,7 @@ public:
   void AddCompilerRequirementFlag(std::string &flags, cmTarget* target,
                                   const std::string& lang);
   ///! Append flags to a string.
+  virtual void AppendFlags(std::string& flags, const std::string& newFlags);
   virtual void AppendFlags(std::string& flags, const char* newFlags);
   virtual void AppendFlagEscape(std::string& flags,
                                 const std::string& rawFlag);

Source/cmLocalNinjaGenerator.cxx

@@ -294,7 +294,7 @@ void cmLocalNinjaGenerator::WriteProcessedMakefile(std::ostream& os)
   os << std::endl;
 }
 
-std::string cmLocalNinjaGenerator::ConvertToNinjaPath(const char *path)
+std::string cmLocalNinjaGenerator::ConvertToNinjaPath(const std::string& path)
 {
   std::string convPath = this->Convert(path, cmLocalGenerator::HOME_OUTPUT);
 #ifdef _WIN32

Source/cmLocalNinjaGenerator.h

@@ -67,7 +67,7 @@ public:
   std::string GetHomeRelativeOutputPath() const
   { return this->HomeRelativeOutputPath; }
 
-  std::string ConvertToNinjaPath(const char *path);
+  std::string ConvertToNinjaPath(const std::string& path);
 
   struct map_to_ninja_path {
     cmLocalNinjaGenerator *LocalGenerator;

Source/cmLocalUnixMakefileGenerator3.cxx

@@ -956,21 +956,28 @@ cmLocalUnixMakefileGenerator3
 
 //----------------------------------------------------------------------------
 void cmLocalUnixMakefileGenerator3::AppendFlags(std::string& flags,
-                                                const char* newFlags)
+                                                const std::string& newFlags)
 {
-  if(this->WatcomWMake && newFlags && *newFlags)
+  if(this->WatcomWMake && !newFlags.empty())
     {
     std::string newf = newFlags;
     if(newf.find("\\\"") != newf.npos)
       {
       cmSystemTools::ReplaceString(newf, "\\\"", "\"");
-      this->cmLocalGenerator::AppendFlags(flags, newf.c_str());
+      this->cmLocalGenerator::AppendFlags(flags, newf);
       return;
       }
     }
   this->cmLocalGenerator::AppendFlags(flags, newFlags);
 }
 
+//----------------------------------------------------------------------------
+void cmLocalUnixMakefileGenerator3::AppendFlags(std::string& flags,
+                                                const char* newFlags)
+{
+  this->cmLocalGenerator::AppendFlags(flags, newFlags);
+}
+
 //----------------------------------------------------------------------------
 void
 cmLocalUnixMakefileGenerator3

Source/cmLocalUnixMakefileGenerator3.h

@@ -168,6 +168,7 @@ public:
                                    const std::string& tgt);
 
   // append flags to a string
+  virtual void AppendFlags(std::string& flags, const std::string& newFlags);
   virtual void AppendFlags(std::string& flags, const char* newFlags);
 
   // append an echo command

Source/cmMakefileLibraryTargetGenerator.cxx

@@ -249,7 +249,7 @@ void cmMakefileLibraryTargetGenerator::WriteLibraryRules
 
   // Create set of linking flags.
   std::string linkFlags;
-  this->LocalGenerator->AppendFlags(linkFlags, extraFlags.c_str());
+  this->LocalGenerator->AppendFlags(linkFlags, extraFlags);
 
   // Add OSX version flags, if any.
   if(this->Target->GetType() == cmTarget::SHARED_LIBRARY ||
@@ -810,6 +810,6 @@ cmMakefileLibraryTargetGenerator
     // Append the flag since a non-zero version is specified.
     cmOStringStream vflag;
     vflag << flag << major << "." << minor << "." << patch;
-    this->LocalGenerator->AppendFlags(flags, vflag.str().c_str());
+    this->LocalGenerator->AppendFlags(flags, vflag.str());
     }
 }

Source/cmMakefileTargetGenerator.cxx

@@ -303,7 +303,7 @@ std::string cmMakefileTargetGenerator::GetFlags(const std::string &l)
 
     // Add include directory flags.
     this->LocalGenerator->
-      AppendFlags(flags,this->GetFrameworkFlags(l).c_str());
+      AppendFlags(flags,this->GetFrameworkFlags(l));
 
     // Add target-specific flags.
     this->LocalGenerator->AddCompileOptions(flags, this->Target,
@@ -551,7 +551,7 @@ cmMakefileTargetGenerator
   std::string langFlags = "$(";
   langFlags += lang;
   langFlags += "_FLAGS)";
-  this->LocalGenerator->AppendFlags(flags, langFlags.c_str());
+  this->LocalGenerator->AppendFlags(flags, langFlags);
 
   std::string configUpper =
     cmSystemTools::UpperCase(this->LocalGenerator->ConfigurationName);
@@ -1968,11 +1968,11 @@ void cmMakefileTargetGenerator::AddIncludeFlags(std::string& flags,
     std::string arg = "@" +
       this->CreateResponseFile(name.c_str(), includeFlags,
                                this->FlagFileDepends[lang]);
-    this->LocalGenerator->AppendFlags(flags, arg.c_str());
+    this->LocalGenerator->AppendFlags(flags, arg);
     }
   else
     {
-    this->LocalGenerator->AppendFlags(flags, includeFlags.c_str());
+    this->LocalGenerator->AppendFlags(flags, includeFlags);
     }
 }
 
@@ -2044,7 +2044,7 @@ void cmMakefileTargetGenerator::AddFortranFlags(std::string& flags)
     modflag += this->Convert(mod_dir,
                              cmLocalGenerator::START_OUTPUT,
                              cmLocalGenerator::SHELL);
-    this->LocalGenerator->AppendFlags(flags, modflag.c_str());
+    this->LocalGenerator->AppendFlags(flags, modflag);
     }
 
   // If there is a separate module path flag then duplicate the
@@ -2066,7 +2066,7 @@ void cmMakefileTargetGenerator::AddFortranFlags(std::string& flags)
       flg += this->Convert(*idi,
                            cmLocalGenerator::NONE,
                            cmLocalGenerator::SHELL);
-      this->LocalGenerator->AppendFlags(flags, flg.c_str());
+      this->LocalGenerator->AppendFlags(flags, flg);
       }
     }
 }
@@ -2093,7 +2093,7 @@ void cmMakefileTargetGenerator::AddModuleDefinitionFlag(std::string& flags)
   // vs6's "cl -link" pass it to the linker.
   std::string flag = defFileFlag;
   flag += (this->LocalGenerator->ConvertToLinkReference(def));
-  this->LocalGenerator->AppendFlags(flags, flag.c_str());
+  this->LocalGenerator->AppendFlags(flags, flag);
 }
 
 //----------------------------------------------------------------------------

Source/cmNinjaTargetGenerator.cxx

@@ -121,6 +121,12 @@ void cmNinjaTargetGenerator::AddFeatureFlags(std::string& flags,
     }
 }
 
+std::string
+cmNinjaTargetGenerator::OrderDependsTargetForTarget()
+{
+  return "cmake_order_depends_target_" + this->GetTargetName();
+}
+
 // TODO: Most of the code is picked up from
 // void cmMakefileExecutableTargetGenerator::WriteExecutableRule(bool relink),
 // void cmMakefileTargetGenerator::WriteTargetLanguageFlags()
@@ -169,7 +175,7 @@ cmNinjaTargetGenerator::ComputeFlagsForObject(cmSourceFile const* source,
     if(cmGlobalNinjaGenerator::IsMinGW())
       cmSystemTools::ReplaceString(includeFlags, "\\", "/");
 
-    this->LocalGenerator->AppendFlags(languageFlags, includeFlags.c_str());
+    this->LocalGenerator->AppendFlags(languageFlags, includeFlags);
 
     // Append old-style preprocessor definition flags.
     this->LocalGenerator->AppendFlags(languageFlags,
@@ -496,7 +502,7 @@ cmNinjaTargetGenerator
      this->GetLocalGenerator()->AddCustomCommandTarget(cc, this->GetTarget());
      // Record the custom commands for this target. The container is used
      // in WriteObjectBuildStatement when called in a loop below.
-     this->CustomCommands.push_back((*si)->GetCustomCommand());
+     this->CustomCommands.push_back(cc);
      }
   std::vector<cmSourceFile const*> headerSources;
   this->GeneratorTarget->GetHeaderSources(headerSources, config);
@@ -516,6 +522,33 @@ cmNinjaTargetGenerator
     {
     this->Objects.push_back(this->GetSourceFilePath(*si));
     }
+
+  cmNinjaDeps orderOnlyDeps;
+  this->GetLocalGenerator()->AppendTargetDepends(this->Target, orderOnlyDeps);
+
+  // Add order-only dependencies on custom command outputs.
+  for(std::vector<cmCustomCommand const*>::const_iterator
+        cci = this->CustomCommands.begin();
+      cci != this->CustomCommands.end(); ++cci)
+    {
+    cmCustomCommand const* cc = *cci;
+    cmCustomCommandGenerator ccg(*cc, this->GetConfigName(),
+                                 this->GetMakefile());
+    const std::vector<std::string>& ccoutputs = ccg.GetOutputs();
+    std::transform(ccoutputs.begin(), ccoutputs.end(),
+                   std::back_inserter(orderOnlyDeps), MapToNinjaPath());
+    }
+
+  cmNinjaDeps orderOnlyTarget;
+  orderOnlyTarget.push_back(this->OrderDependsTargetForTarget());
+  this->GetGlobalGenerator()->WritePhonyBuild(this->GetBuildFileStream(),
+                                          "Order-only phony target for "
+                                            + this->GetTargetName(),
+                                          orderOnlyTarget,
+                                          cmNinjaDeps(),
+                                          cmNinjaDeps(),
+                                          orderOnlyDeps);
+
   std::vector<cmSourceFile const*> objectSources;
   this->GeneratorTarget->GetObjectSources(objectSources, config);
   for(std::vector<cmSourceFile const*>::const_iterator
@@ -554,11 +587,6 @@ cmNinjaTargetGenerator
     sourceFileName = this->GetSourceFilePath(source);
   explicitDeps.push_back(sourceFileName);
 
-  // Ensure that the target dependencies are built before any source file in
-  // the target, using order-only dependencies.
-  cmNinjaDeps orderOnlyDeps;
-  this->GetLocalGenerator()->AppendTargetDepends(this->Target, orderOnlyDeps);
-
   cmNinjaDeps implicitDeps;
   if(const char* objectDeps = source->GetProperty("OBJECT_DEPENDS")) {
     std::vector<std::string> depList;
@@ -567,18 +595,8 @@ cmNinjaTargetGenerator
                    std::back_inserter(implicitDeps), MapToNinjaPath());
   }
 
-  // Add order-only dependencies on custom command outputs.
+  cmNinjaDeps orderOnlyDeps;
-  for(std::vector<cmCustomCommand const*>::const_iterator
+  orderOnlyDeps.push_back(this->OrderDependsTargetForTarget());
-        cci = this->CustomCommands.begin();
-      cci != this->CustomCommands.end(); ++cci)
-    {
-    cmCustomCommand const* cc = *cci;
-    cmCustomCommandGenerator ccg(*cc, this->GetConfigName(),
-                                 this->GetMakefile());
-    const std::vector<std::string>& ccoutputs = ccg.GetOutputs();
-    std::transform(ccoutputs.begin(), ccoutputs.end(),
-                   std::back_inserter(orderOnlyDeps), MapToNinjaPath());
-    }
 
   // If the source file is GENERATED and does not have a custom command
   // (either attached to this source file or another one), assume that one of
@@ -698,7 +716,7 @@ cmNinjaTargetGenerator
   std::string flag = defFileFlag;
   flag += (this->LocalGenerator->ConvertToLinkReference(
              this->ModuleDefinitionFile));
-  this->LocalGenerator->AppendFlags(flags, flag.c_str());
+  this->LocalGenerator->AppendFlags(flags, flag);
 }
 
 void
@@ -760,9 +778,10 @@ cmNinjaTargetGenerator::MacOSXContentGeneratorType::operator()(
   this->Generator->GetGlobalGenerator()->AddDependencyToAll(output);
 }
 
-void cmNinjaTargetGenerator::addPoolNinjaVariable(const char* pool_property,
+void cmNinjaTargetGenerator::addPoolNinjaVariable(
-                                                  cmTarget* target,
+                                              const std::string& pool_property,
-                                                  cmNinjaVars& vars)
+                                              cmTarget* target,
+                                              cmNinjaVars& vars)
 {
     const char* pool = target->GetProperty(pool_property);
     if (pool)

Source/cmNinjaTargetGenerator.h

@@ -74,6 +74,10 @@ protected:
   bool GetFeatureAsBool(const std::string& feature);
   void AddFeatureFlags(std::string& flags, const std::string& lang);
 
+  std::string OrderDependsTargetForTarget();
+
+  std::string ComputeOrderDependsForTarget();
+
   /**
    * Compute the flags for compilation of object files for a given @a language.
    * @note Generally it is the value of the variable whose name is computed
@@ -85,7 +89,7 @@ protected:
   std::string ComputeDefines(cmSourceFile const* source,
                              const std::string& language);
 
-  std::string ConvertToNinjaPath(const char *path) const {
+  std::string ConvertToNinjaPath(const std::string& path) const {
     return this->GetLocalGenerator()->ConvertToNinjaPath(path);
   }
   cmLocalNinjaGenerator::map_to_ninja_path MapToNinjaPath() const {
@@ -142,7 +146,7 @@ protected:
   cmOSXBundleGenerator* OSXBundleGenerator;
   std::set<std::string> MacContentFolders;
 
-  void addPoolNinjaVariable(const char* pool_property,
+  void addPoolNinjaVariable(const std::string& pool_property,
                             cmTarget* target,
                             cmNinjaVars& vars);