cmMakefile: Separate custom command setup from actual creation

Refactor custom command manipulation functions to consist of a setup and a
commit stage.  The commit stage will be delayed to generate time.

Source/cmMakefile.cxx

@@ -910,6 +910,21 @@ cmTarget* cmMakefile::AddCustomCommandToTarget(
   // Always create the byproduct sources and mark them generated.
   this->CreateGeneratedSources(byproducts);
 
+  this->CommitCustomCommandToTarget(
+    t, byproducts, depends, commandLines, type, comment, workingDir,
+    escapeOldStyle, uses_terminal, depfile, job_pool, command_expand_lists);
+
+  return t;
+}
+
+void cmMakefile::CommitCustomCommandToTarget(
+  cmTarget* target, const std::vector<std::string>& byproducts,
+  const std::vector<std::string>& depends,
+  const cmCustomCommandLines& commandLines, cmTarget::CustomCommandType type,
+  const char* comment, const char* workingDir, bool escapeOldStyle,
+  bool uses_terminal, const std::string& depfile, const std::string& job_pool,
+  bool command_expand_lists)
+{
   // Add the command to the appropriate build step for the target.
   std::vector<std::string> no_output;
   cmCustomCommand cc(this, no_output, byproducts, depends, commandLines,
@@ -922,18 +937,16 @@ cmTarget* cmMakefile::AddCustomCommandToTarget(
   cc.SetJobPool(job_pool);
   switch (type) {
     case cmTarget::PRE_BUILD:
-      t->AddPreBuildCommand(cc);
+      target->AddPreBuildCommand(cc);
       break;
     case cmTarget::PRE_LINK:
-      t->AddPreLinkCommand(cc);
+      target->AddPreLinkCommand(cc);
       break;
     case cmTarget::POST_BUILD:
-      t->AddPostBuildCommand(cc);
+      target->AddPostBuildCommand(cc);
       break;
   }
-  this->UpdateOutputToSourceMap(byproducts, t);
+  this->UpdateOutputToSourceMap(byproducts, target);
-
-  return t;
 }
 
 void cmMakefile::UpdateOutputToSourceMap(
@@ -966,6 +979,23 @@ void cmMakefile::UpdateOutputToSourceMap(std::string const& byproduct,
   }
 }
 
+cmSourceFile* cmMakefile::AddCustomCommandToOutput(
+  const std::string& output, const std::vector<std::string>& depends,
+  const std::string& main_dependency, const cmCustomCommandLines& commandLines,
+  const char* comment, const char* workingDir, bool replace,
+  bool escapeOldStyle, bool uses_terminal, bool command_expand_lists,
+  const std::string& depfile, const std::string& job_pool)
+{
+  std::vector<std::string> outputs;
+  outputs.push_back(output);
+  std::vector<std::string> no_byproducts;
+  cmImplicitDependsList no_implicit_depends;
+  return this->AddCustomCommandToOutput(
+    outputs, no_byproducts, depends, main_dependency, no_implicit_depends,
+    commandLines, comment, workingDir, replace, escapeOldStyle, uses_terminal,
+    command_expand_lists, depfile, job_pool);
+}
+
 cmSourceFile* cmMakefile::AddCustomCommandToOutput(
   const std::vector<std::string>& outputs,
   const std::vector<std::string>& byproducts,
@@ -991,6 +1021,22 @@ cmSourceFile* cmMakefile::AddCustomCommandToOutput(
   this->CreateGeneratedSources(outputs);
   this->CreateGeneratedSources(byproducts);
 
+  return this->CommitCustomCommandToOutput(
+    outputs, byproducts, depends, main_dependency, implicit_depends,
+    commandLines, comment, workingDir, replace, escapeOldStyle, uses_terminal,
+    command_expand_lists, depfile, job_pool);
+}
+
+cmSourceFile* cmMakefile::CommitCustomCommandToOutput(
+  const std::vector<std::string>& outputs,
+  const std::vector<std::string>& byproducts,
+  const std::vector<std::string>& depends, const std::string& main_dependency,
+  const cmImplicitDependsList& implicit_depends,
+  const cmCustomCommandLines& commandLines, const char* comment,
+  const char* workingDir, bool replace, bool escapeOldStyle,
+  bool uses_terminal, bool command_expand_lists, const std::string& depfile,
+  const std::string& job_pool)
+{
   // Choose a source file on which to store the custom command.
   cmSourceFile* file = nullptr;
   if (!commandLines.empty() && !main_dependency.empty()) {
@@ -1099,23 +1145,6 @@ void cmMakefile::UpdateOutputToSourceMap(std::string const& output,
   }
 }
 
-cmSourceFile* cmMakefile::AddCustomCommandToOutput(
-  const std::string& output, const std::vector<std::string>& depends,
-  const std::string& main_dependency, const cmCustomCommandLines& commandLines,
-  const char* comment, const char* workingDir, bool replace,
-  bool escapeOldStyle, bool uses_terminal, bool command_expand_lists,
-  const std::string& depfile, const std::string& job_pool)
-{
-  std::vector<std::string> outputs;
-  outputs.push_back(output);
-  std::vector<std::string> no_byproducts;
-  cmImplicitDependsList no_implicit_depends;
-  return this->AddCustomCommandToOutput(
-    outputs, no_byproducts, depends, main_dependency, no_implicit_depends,
-    commandLines, comment, workingDir, replace, escapeOldStyle, uses_terminal,
-    command_expand_lists, depfile, job_pool);
-}
-
 void cmMakefile::AddCustomCommandOldStyle(
   const std::string& target, const std::vector<std::string>& outputs,
   const std::vector<std::string>& depends, const std::string& source,
@@ -1187,20 +1216,35 @@ bool cmMakefile::AppendCustomCommandToOutput(
   const std::string& output, const std::vector<std::string>& depends,
   const cmImplicitDependsList& implicit_depends,
   const cmCustomCommandLines& commandLines)
+{
+  // Check as good as we can if there will be a command for this output.
+  if (!this->MightHaveCustomCommand(output)) {
+    return false;
+  }
+
+  // Validate custom commands.
+  if (this->ValidateCustomCommand(commandLines)) {
+    // Add command factory to allow generator expressions in output.
+    this->CommitAppendCustomCommandToOutput(output, depends, implicit_depends,
+                                            commandLines);
+  }
+
+  return true;
+}
+
+void cmMakefile::CommitAppendCustomCommandToOutput(
+  const std::string& output, const std::vector<std::string>& depends,
+  const cmImplicitDependsList& implicit_depends,
+  const cmCustomCommandLines& commandLines)
 {
   // Lookup an existing command.
   if (cmSourceFile* sf = this->GetSourceFileWithOutput(output)) {
     if (cmCustomCommand* cc = sf->GetCustomCommand()) {
-      // Validate custom commands.
+      cc->AppendCommands(commandLines);
-      if (this->ValidateCustomCommand(commandLines)) {
+      cc->AppendDepends(depends);
-        cc->AppendCommands(commandLines);
+      cc->AppendImplicitDepends(implicit_depends);
-        cc->AppendDepends(depends);
-        cc->AppendImplicitDepends(implicit_depends);
-      }
-      return true;
     }
   }
-  return false;
 }
 
 cmTarget* cmMakefile::AddUtilityCommand(
@@ -1261,11 +1305,6 @@ cmTarget* cmMakefile::AddUtilityCommand(
     target->SetProperty("EXCLUDE_FROM_ALL", "TRUE");
   }
 
-  if (!comment) {
-    // Use an empty comment to avoid generation of default comment.
-    comment = "";
-  }
-
   // Validate custom commands.
   if (!this->ValidateCustomCommand(commandLines) ||
       (commandLines.empty() && depends.empty())) {
@@ -1277,27 +1316,58 @@ cmTarget* cmMakefile::AddUtilityCommand(
 
   std::string force =
     cmStrCat(this->GetCurrentBinaryDirectory(), "/CMakeFiles/", utilityName);
+  this->CreateGeneratedSource(force);
+  std::string forceCMP0049 = target->GetSourceCMP0049(force);
+  {
+    cmSourceFile* sf = nullptr;
+    if (!forceCMP0049.empty()) {
+      sf = this->GetOrCreateSource(forceCMP0049, false,
+                                   cmSourceFileLocationKind::Known);
+    }
+    // The output is not actually created so mark it symbolic.
+    if (sf) {
+      sf->SetProperty("SYMBOLIC", "1");
+    } else {
+      cmSystemTools::Error("Could not get source file entry for " + force);
+    }
+  }
+
+  if (!comment) {
+    // Use an empty comment to avoid generation of default comment.
+    comment = "";
+  }
+
+  this->CommitUtilityCommand(target, force, forceCMP0049, workingDirectory,
+                             byproducts, depends, commandLines, escapeOldStyle,
+                             comment, uses_terminal, command_expand_lists,
+                             job_pool);
+
+  return target;
+}
+
+void cmMakefile::CommitUtilityCommand(
+  cmTarget* target, const std::string& force, const std::string& forceCMP0049,
+  const char* workingDirectory, const std::vector<std::string>& byproducts,
+  const std::vector<std::string>& depends,
+  const cmCustomCommandLines& commandLines, bool escapeOldStyle,
+  const char* comment, bool uses_terminal, bool command_expand_lists,
+  const std::string& job_pool)
+{
   std::vector<std::string> forced;
   forced.push_back(force);
   std::string no_main_dependency;
   cmImplicitDependsList no_implicit_depends;
   bool no_replace = false;
-  this->AddCustomCommandToOutput(
+  cmSourceFile* sf = this->AddCustomCommandToOutput(
     forced, byproducts, depends, no_main_dependency, no_implicit_depends,
     commandLines, comment, workingDirectory, no_replace, escapeOldStyle,
     uses_terminal, command_expand_lists, /*depfile=*/"", job_pool);
-  cmSourceFile* sf = target->AddSourceCMP0049(force);
+  if (!forceCMP0049.empty()) {
-
+    target->AddSource(forceCMP0049);
-  // The output is not actually created so mark it symbolic.
+  }
-  if (sf) {
+  if (sf) {
-    sf->SetProperty("SYMBOLIC", "1");
+    this->UpdateOutputToSourceMap(byproducts, target);
-  } else {
-    cmSystemTools::Error("Could not get source file entry for " + force);
   }
-
-  this->UpdateOutputToSourceMap(byproducts, target);
-
-  return target;
 }
 
 static void s_AddDefineFlag(std::string const& flag, std::string& dflags)
@@ -2233,6 +2303,18 @@ cmSourceFile* cmMakefile::GetSourceFileWithOutput(
   return nullptr;
 }
 
+bool cmMakefile::MightHaveCustomCommand(const std::string& name) const
+{
+  // This will have to be changed for delaying custom command creation, because
+  // GetSourceFileWithOutput requires the command to be already created.
+  if (cmSourceFile* sf = this->GetSourceFileWithOutput(name)) {
+    if (sf->GetCustomCommand()) {
+      return true;
+    }
+  }
+  return false;
+}
+
 #if !defined(CMAKE_BOOTSTRAP)
 cmSourceGroup* cmMakefile::GetSourceGroup(
   const std::vector<std::string>& name) const
@@ -3477,14 +3559,19 @@ cmSourceFile* cmMakefile::GetOrCreateSource(const std::string& sourceName,
   return this->CreateSource(sourceName, generated, kind);
 }
 
+void cmMakefile::CreateGeneratedSource(const std::string& output)
+{
+  if (cmSourceFile* out = this->GetOrCreateSource(
+        output, true, cmSourceFileLocationKind::Known)) {
+    out->SetProperty("GENERATED", "1");
+  }
+}
+
 void cmMakefile::CreateGeneratedSources(
   const std::vector<std::string>& outputs)
 {
   for (std::string const& output : outputs) {
-    if (cmSourceFile* out = this->GetOrCreateSource(
+    this->CreateGeneratedSource(output);
-          output, true, cmSourceFileLocationKind::Known)) {
-      out->SetProperty("GENERATED", "1");
-    }
   }
 }
 

Source/cmMakefile.h

@@ -181,18 +181,18 @@ public:
     const std::string& job_pool = "", bool command_expand_lists = false,
     ObjectLibraryCommands objLibraryCommands = RejectObjectLibraryCommands);
   cmSourceFile* AddCustomCommandToOutput(
-    const std::vector<std::string>& outputs,
+    const std::string& output, const std::vector<std::string>& depends,
-    const std::vector<std::string>& byproducts,
-    const std::vector<std::string>& depends,
     const std::string& main_dependency,
-    const cmImplicitDependsList& implicit_depends,
     const cmCustomCommandLines& commandLines, const char* comment,
     const char* workingDir, bool replace = false, bool escapeOldStyle = true,
     bool uses_terminal = false, bool command_expand_lists = false,
     const std::string& depfile = "", const std::string& job_pool = "");
   cmSourceFile* AddCustomCommandToOutput(
-    const std::string& output, const std::vector<std::string>& depends,
+    const std::vector<std::string>& outputs,
+    const std::vector<std::string>& byproducts,
+    const std::vector<std::string>& depends,
     const std::string& main_dependency,
+    const cmImplicitDependsList& implicit_depends,
     const cmCustomCommandLines& commandLines, const char* comment,
     const char* workingDir, bool replace = false, bool escapeOldStyle = true,
     bool uses_terminal = false, bool command_expand_lists = false,
@@ -1069,6 +1069,39 @@ private:
 
   bool ValidateCustomCommand(const cmCustomCommandLines& commandLines) const;
 
+  void CommitCustomCommandToTarget(
+    cmTarget* target, const std::vector<std::string>& byproducts,
+    const std::vector<std::string>& depends,
+    const cmCustomCommandLines& commandLines, cmTarget::CustomCommandType type,
+    const char* comment, const char* workingDir, bool escapeOldStyle,
+    bool uses_terminal, const std::string& depfile,
+    const std::string& job_pool, bool command_expand_lists);
+  cmSourceFile* CommitCustomCommandToOutput(
+    const std::vector<std::string>& outputs,
+    const std::vector<std::string>& byproducts,
+    const std::vector<std::string>& depends,
+    const std::string& main_dependency,
+    const cmImplicitDependsList& implicit_depends,
+    const cmCustomCommandLines& commandLines, const char* comment,
+    const char* workingDir, bool replace, bool escapeOldStyle,
+    bool uses_terminal, bool command_expand_lists, const std::string& depfile,
+    const std::string& job_pool);
+  void CommitAppendCustomCommandToOutput(
+    const std::string& output, const std::vector<std::string>& depends,
+    const cmImplicitDependsList& implicit_depends,
+    const cmCustomCommandLines& commandLines);
+
+  void CommitUtilityCommand(cmTarget* target, const std::string& force,
+                            const std::string& forceCMP0049,
+                            const char* workingDirectory,
+                            const std::vector<std::string>& byproducts,
+                            const std::vector<std::string>& depends,
+                            const cmCustomCommandLines& commandLines,
+                            bool escapeOldStyle, const char* comment,
+                            bool uses_terminal, bool command_expand_lists,
+                            const std::string& job_pool);
+
+  void CreateGeneratedSource(const std::string& output);
   void CreateGeneratedSources(const std::vector<std::string>& outputs);
 
   /**
@@ -1104,6 +1137,11 @@ private:
   void UpdateOutputToSourceMap(std::string const& output, cmSourceFile* source,
                                bool byproduct);
 
+  /**
+   * Return if the provided source file might have a custom command.
+   */
+  bool MightHaveCustomCommand(const std::string& name) const;
+
   bool AddRequiredTargetCFeature(cmTarget* target, const std::string& feature,
                                  std::string* error = nullptr) const;
 

Source/cmTarget.cxx

@@ -694,13 +694,9 @@ std::string cmTargetInternals::ProcessSourceItemCMP0049(const std::string& s)
   return src;
 }
 
-cmSourceFile* cmTarget::AddSourceCMP0049(const std::string& s)
+std::string cmTarget::GetSourceCMP0049(const std::string& s)
 {
-  std::string src = impl->ProcessSourceItemCMP0049(s);
+  return impl->ProcessSourceItemCMP0049(s);
-  if (!s.empty() && src.empty()) {
-    return nullptr;
-  }
-  return this->AddSource(src);
 }
 
 struct CreateLocation

Source/cmTarget.h

@@ -103,7 +103,7 @@ public:
   //! Add sources to the target.
   void AddSources(std::vector<std::string> const& srcs);
   void AddTracedSources(std::vector<std::string> const& srcs);
-  cmSourceFile* AddSourceCMP0049(const std::string& src);
+  std::string GetSourceCMP0049(const std::string& src);
   cmSourceFile* AddSource(const std::string& src, bool before = false);
 
   //! how we identify a library, by name and type