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.

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;
-  std::string tgt = this->Convert(replace,HOME_OUTPUT,MAKERULE);
+  {
+  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.

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);

Source/cmMakefileLibraryTargetGenerator.cxx

@@ -752,26 +752,23 @@ void cmMakefileLibraryTargetGenerator::WriteLibraryRules
                            this->Target);
     }
 
-  // Write the build rule.
+  // Compute the list of outputs.
-  this->LocalGenerator->WriteMakeRule(*this->BuildFileStream, 0,
+  std::vector<std::string> outputs(1, targetFullPathReal);
-                                      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;
   if(targetNameSO != targetNameReal)
     {
-    this->GenerateExtraOutput(targetFullPathSO.c_str(),
+    outputs.push_back(targetFullPathSO);
-                              targetFullPathReal.c_str());
     }
   if(targetName != targetNameSO &&
      targetName != targetNameReal)
     {
-    this->GenerateExtraOutput(targetFullPath.c_str(),
+    outputs.push_back(targetFullPath);
-                              targetFullPathReal.c_str());
     }
 
+  // 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);
 

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, outputs);
-    cmSystemTools::ExpandListArgument(extra_outputs_str, extra_outputs);
+    for(std::vector<std::string>::const_iterator eoi = outputs.begin()+1;
-    for(std::vector<std::string>::const_iterator eoi = extra_outputs.begin();
+        eoi != outputs.end(); ++eoi)
-        eoi != extra_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();
@@ -1314,32 +1274,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)
@@ -1767,15 +1701,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

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;