Makefile: Fix rebuild with multiple custom command outputs (#15116)

Fix the generated makefiles for custom commands with multiple outputs to
list all the outputs on the left hand side of the build rule.  This is
much simpler and more reliable than the old multiple-output-pair
infrastructure.

5 files changed, 68 insertions, 111 deletions

diff --git a/Source/cmLocalUnixMakefileGenerator3.cxx b/Source/cmLocalUnixMakefileGenerator3.cxx
index ff8ba8b..56b2b97 100644
--- a/Source/cmLocalUnixMakefileGenerator3.cxx
+++ b/Source/cmLocalUnixMakefileGenerator3.cxx
@@ -618,6 +618,30 @@ cmLocalUnixMakefileGenerator3
                          comment);
     return;
     }
+  std::vector<std::string> outputs(1, target);
+  this->WriteMakeRule(os, comment,
+                      outputs, depends, commands,
+                      symbolic, in_help);
+}
+
+//----------------------------------------------------------------------------
+void
+cmLocalUnixMakefileGenerator3
+::WriteMakeRule(std::ostream& os,
+                const char* comment,
+                const std::vector<std::string>& outputs,
+                const std::vector<std::string>& depends,
+                const std::vector<std::string>& commands,
+                bool symbolic,
+                bool in_help)
+{
+  // Make sure there is an output.
+  if(outputs.empty())
+    {
+    cmSystemTools::Error("No outputs for WriteMakeRule! called with comment: ",
+                         comment);
+    return;
+    }
 
   std::string replace;
 
@@ -636,8 +660,18 @@ cmLocalUnixMakefileGenerator3
     }
 
   // Construct the left hand side of the rule.
-  replace = target;
-  std::string tgt = this->Convert(replace,HOME_OUTPUT,MAKERULE);
+  std::string tgt;
+  {
+  const char* sep = "";
+  for (std::vector<std::string>::const_iterator i = outputs.begin();
+       i != outputs.end(); ++i)
+    {
+    tgt += sep;
+    tgt += this->Convert(*i,HOME_OUTPUT,MAKERULE);
+    sep = " ";
+    }
+  }
+
   const char* space = "";
   if(tgt.size() == 1)
     {
@@ -690,7 +724,11 @@ cmLocalUnixMakefileGenerator3
   // Add the output to the local help if requested.
   if(in_help)
     {
-    this->LocalHelp.push_back(target);
+    for (std::vector<std::string>::const_iterator i = outputs.begin();
+         i != outputs.end(); ++i)
+      {
+      this->LocalHelp.push_back(*i);
+      }
     }
 }
 
@@ -1709,6 +1747,8 @@ cmLocalUnixMakefileGenerator3
 //----------------------------------------------------------------------------
 void cmLocalUnixMakefileGenerator3::CheckMultipleOutputs(bool verbose)
 {
+  // Nothing populates multiple output pairs anymore, but we need to
+  // honor it when working in a build tree generated by an older CMake.
   cmMakefile* mf = this->Makefile;
 
   // Get the string listing the multiple output pairs.
diff --git a/Source/cmLocalUnixMakefileGenerator3.h b/Source/cmLocalUnixMakefileGenerator3.h
index 4f2e4a0..65265ce 100644
--- a/Source/cmLocalUnixMakefileGenerator3.h
+++ b/Source/cmLocalUnixMakefileGenerator3.h
@@ -61,6 +61,13 @@ public:
                      const std::vector<std::string>& commands,
                      bool symbolic,
                      bool in_help = false);
+  void WriteMakeRule(std::ostream& os,
+                     const char* comment,
+                     const std::vector<std::string>& outputs,
+                     const std::vector<std::string>& depends,
+                     const std::vector<std::string>& commands,
+                     bool symbolic,
+                     bool in_help = false);
 
   // write the main variables used by the makefiles
   void WriteMakeVariables(std::ostream& makefileStream);
diff --git a/Source/cmMakefileLibraryTargetGenerator.cxx b/Source/cmMakefileLibraryTargetGenerator.cxx
index 80473f6..305d81d 100644
--- a/Source/cmMakefileLibraryTargetGenerator.cxx
+++ b/Source/cmMakefileLibraryTargetGenerator.cxx
@@ -752,26 +752,23 @@ void cmMakefileLibraryTargetGenerator::WriteLibraryRules
                            this->Target);
     }
 
-  // Write the build rule.
-  this->LocalGenerator->WriteMakeRule(*this->BuildFileStream, 0,
-                                      targetFullPathReal,
-                                      depends, commands, false);
-
-  // Some targets have more than one output file.  Create rules to
-  // drive the build if any extra outputs are missing.
-  std::vector<std::string> extraOutputs;
+  // Compute the list of outputs.
+  std::vector<std::string> outputs(1, targetFullPathReal);
   if(targetNameSO != targetNameReal)
     {
-    this->GenerateExtraOutput(targetFullPathSO.c_str(),
-                              targetFullPathReal.c_str());
+    outputs.push_back(targetFullPathSO);
     }
   if(targetName != targetNameSO &&
      targetName != targetNameReal)
     {
-    this->GenerateExtraOutput(targetFullPath.c_str(),
-                              targetFullPathReal.c_str());
+    outputs.push_back(targetFullPath);
     }
 
+  // Write the build rule.
+  this->LocalGenerator->WriteMakeRule(*this->BuildFileStream, 0,
+                                      outputs, depends, commands, false);
+
+
   // Write the main driver rule to build everything in this target.
   this->WriteTargetDriverRule(targetFullPath, relink);
 
diff --git a/Source/cmMakefileTargetGenerator.cxx b/Source/cmMakefileTargetGenerator.cxx
index 8444dfb..067714e 100644
--- a/Source/cmMakefileTargetGenerator.cxx
+++ b/Source/cmMakefileTargetGenerator.cxx
@@ -754,30 +754,24 @@ cmMakefileTargetGenerator
                   compileCommands.begin(), compileCommands.end());
   }
 
-  // Write the rule.
-  this->LocalGenerator->WriteMakeRule(*this->BuildFileStream, 0,
-                                      relativeObj,
-                                      depends, commands, false);
-
   // Check for extra outputs created by the compilation.
+  std::vector<std::string> outputs(1, relativeObj);
   if(const char* extra_outputs_str =
      source.GetProperty("OBJECT_OUTPUTS"))
     {
-    std::vector<std::string> extra_outputs;
-    cmSystemTools::ExpandListArgument(extra_outputs_str, extra_outputs);
-    for(std::vector<std::string>::const_iterator eoi = extra_outputs.begin();
-        eoi != extra_outputs.end(); ++eoi)
+    cmSystemTools::ExpandListArgument(extra_outputs_str, outputs);
+    for(std::vector<std::string>::const_iterator eoi = outputs.begin()+1;
+        eoi != outputs.end(); ++eoi)
       {
-      // Register this as an extra output for the object file rule.
-      // This will cause the object file to be rebuilt if the extra
-      // output is missing.
-      this->GenerateExtraOutput(eoi->c_str(), relativeObj.c_str(), false);
-
       // Register this as an extra file to clean.
       this->CleanFiles.push_back(*eoi);
       }
     }
 
+  // Write the rule.
+  this->LocalGenerator->WriteMakeRule(*this->BuildFileStream, 0,
+                                      outputs, depends, commands, false);
+
   bool do_preprocess_rules = lang_has_preprocessor &&
     this->LocalGenerator->GetCreatePreprocessedSourceRules();
   bool do_assembly_rules = lang_has_assembly &&
@@ -1017,25 +1011,6 @@ void cmMakefileTargetGenerator::WriteTargetDependRules()
   this->LocalGenerator->
     WriteDependLanguageInfo(*this->InfoFileStream,*this->Target);
 
-  // Store multiple output pairs in the depend info file.
-  if(!this->MultipleOutputPairs.empty())
-    {
-    *this->InfoFileStream
-      << "\n"
-      << "# Pairs of files generated by the same build rule.\n"
-      << "set(CMAKE_MULTIPLE_OUTPUT_PAIRS\n";
-    for(MultipleOutputPairsType::const_iterator pi =
-          this->MultipleOutputPairs.begin();
-        pi != this->MultipleOutputPairs.end(); ++pi)
-      {
-      *this->InfoFileStream
-        << "  " << this->LocalGenerator->EscapeForCMake(pi->first)
-        << " "  << this->LocalGenerator->EscapeForCMake(pi->second)
-        << "\n";
-      }
-    *this->InfoFileStream << "  )\n\n";
-    }
-
   // Store list of targets linked directly or transitively.
   {
   *this->InfoFileStream
@@ -1273,7 +1248,7 @@ void cmMakefileTargetGenerator
       }
     }
   this->LocalGenerator->WriteMakeRule(*this->BuildFileStream, 0,
-                                      *o, depends, commands,
+                                      outputs, depends, commands,
                                       symbolic);
 
   // If the rule has changed make sure the output is rebuilt.
@@ -1283,21 +1258,6 @@ void cmMakefileTargetGenerator
     }
   }
 
-  // Write rules to drive building any outputs beyond the first.
-  const char* in = o->c_str();
-  for(++o; o != outputs.end(); ++o)
-    {
-    bool symbolic = false;
-    if(need_symbolic)
-      {
-      if(cmSourceFile* sf = this->Makefile->GetSource(*o))
-        {
-        symbolic = sf->GetPropertyAsBool("SYMBOLIC");
-        }
-      }
-    this->GenerateExtraOutput(o->c_str(), in, symbolic);
-    }
-
   // Setup implicit dependency scanning.
   for(cmCustomCommand::ImplicitDependsList::const_iterator
         idi = ccg.GetCC().GetImplicitDepends().begin();
@@ -1316,32 +1276,6 @@ void cmMakefileTargetGenerator
 
 //----------------------------------------------------------------------------
 void
-cmMakefileTargetGenerator
-::GenerateExtraOutput(const char* out, const char* in, bool symbolic)
-{
-  // Add a rule to build the primary output if the extra output needs
-  // to be created.
-  std::vector<std::string> commands;
-  std::vector<std::string> depends;
-  std::string emptyCommand = this->GlobalGenerator->GetEmptyRuleHackCommand();
-  if(!emptyCommand.empty())
-    {
-    commands.push_back(emptyCommand);
-    }
-  depends.push_back(in);
-  this->LocalGenerator->WriteMakeRule(*this->BuildFileStream, 0,
-                                      out, depends, commands,
-                                      symbolic);
-
-  // Register the extra output as paired with the first output so that
-  // the check-build-system step will remove the primary output if any
-  // extra outputs are missing.  This forces the rule to regenerate
-  // all outputs.
-  this->AddMultipleOutputPair(out, in);
-}
-
-//----------------------------------------------------------------------------
-void
 cmMakefileTargetGenerator::AppendProgress(std::vector<std::string>& commands)
 {
   this->NumberOfProgressActions++;
@@ -1770,15 +1704,6 @@ void cmMakefileTargetGenerator::RemoveForbiddenFlags(const char* flagVar,
 //----------------------------------------------------------------------------
 void
 cmMakefileTargetGenerator
-::AddMultipleOutputPair(const char* depender, const char* dependee)
-{
-  MultipleOutputPairsType::value_type p(depender, dependee);
-  this->MultipleOutputPairs.insert(p);
-}
-
-//----------------------------------------------------------------------------
-void
-cmMakefileTargetGenerator
 ::CreateLinkScript(const char* name,
                    std::vector<std::string> const& link_commands,
                    std::vector<std::string>& makefile_commands,
diff --git a/Source/cmMakefileTargetGenerator.h b/Source/cmMakefileTargetGenerator.h
index 9fac574..e31e086 100644
--- a/Source/cmMakefileTargetGenerator.h
+++ b/Source/cmMakefileTargetGenerator.h
@@ -142,15 +142,6 @@ protected:
   // Lookup the link rule for this target.
   std::string GetLinkRule(const std::string& linkRuleVar);
 
-  /** In order to support parallel builds for custom commands with
-      multiple outputs the outputs are given a serial order, and only
-      the first output actually has the build rule.  Other outputs
-      just depend on the first one.  The check-build-system step must
-      remove a dependee if the depender is missing to make sure both
-      are regenerated properly.  This method is used by the local
-      makefile generators to register such pairs.  */
-  void AddMultipleOutputPair(const char* depender, const char* dependee);
-
   /** Create a script to hold link rules and a command to invoke the
       script at build time.  */
   void CreateLinkScript(const char* name,
@@ -231,9 +222,6 @@ protected:
   // Set of extra output files to be driven by the build.
   std::set<std::string> ExtraFiles;
 
-  typedef std::map<std::string, std::string> MultipleOutputPairsType;
-  MultipleOutputPairsType MultipleOutputPairs;
-
   // Target name info.
   std::string TargetNameOut;
   std::string TargetNameSO;