Merge topic 'custom-command-multiple-outputs'

6c67b816 Makefile: Workaround Borland Make bug with multiple outputs
65ea5eb7 Tests: Cover rebuild with multiple custom command outputs (#15116)
644b4688 Makefile: Fix rebuild with multiple custom command outputs (#15116)
8a4c6d2d Xcode: Fix rebuild with multiple custom command outputs (#15116)

Source/cmGlobalBorlandMakefileGenerator.cxx

@@ -49,6 +49,7 @@ cmLocalGenerator *cmGlobalBorlandMakefileGenerator::CreateLocalGenerator()
   lg->SetUnixCD(false);
   lg->SetMakeCommandEscapeTargetTwice(true);
   lg->SetBorlandMakeCurlyHack(true);
+  lg->SetNoMultiOutputMultiDepRules(true);
   return lg;
 }
 

Source/cmGlobalXCodeGenerator.cxx

@@ -1498,7 +1498,6 @@ cmGlobalXCodeGenerator::AddCommandsToBuildPhase(cmXCodeObject* buildphase,
                                                 const & commands,
                                                 const char* name)
 {
-  bool haveMultipleOutputPairs = false;
   std::string dir = this->CurrentMakefile->GetCurrentOutputDirectory();
   dir += "/CMakeScripts";
   cmSystemTools::MakeDirectory(dir.c_str());
@@ -1517,8 +1516,7 @@ cmGlobalXCodeGenerator::AddCommandsToBuildPhase(cmXCodeObject* buildphase,
     this->CreateCustomRulesMakefile(makefile.c_str(),
                                     target,
                                     commands,
-                                    currentConfig->c_str(),
+                                    currentConfig->c_str());
-                                    haveMultipleOutputPairs);
     }
 
   std::string cdir = this->CurrentMakefile->GetCurrentOutputDirectory();
@@ -1528,10 +1526,6 @@ cmGlobalXCodeGenerator::AddCommandsToBuildPhase(cmXCodeObject* buildphase,
   makecmd += " -f ";
   makecmd += this->ConvertToRelativeForMake(
                                           (makefile+"$CONFIGURATION").c_str());
-  if(haveMultipleOutputPairs)
-    {
-    makecmd += " cmake_check_multiple_outputs";
-    }
   makecmd += " all";
   cmSystemTools::ReplaceString(makecmd, "\\ ", "\\\\ ");
   buildphase->AddAttribute("shellScript",
@@ -1546,8 +1540,7 @@ void  cmGlobalXCodeGenerator
                             cmTarget& target,
                             std::vector<cmCustomCommand>
                             const & commands,
-                            const std::string& configName,
+                            const std::string& configName)
-                            bool& haveMultipleOutputPairs)
 {
   std::string makefileName=makefileBasename;
   if(this->XcodeVersion > 20)
@@ -1570,7 +1563,6 @@ void  cmGlobalXCodeGenerator
   makefileStream << "all: ";
   std::map<const cmCustomCommand*, std::string> tname;
   int count = 0;
-  std::map<std::string, std::string> multipleOutputPairs;
   for(std::vector<cmCustomCommand>::const_iterator i = commands.begin();
       i != commands.end(); ++i)
     {
@@ -1586,16 +1578,6 @@ void  cmGlobalXCodeGenerator
           makefileStream
               << "\\\n\t" << this->ConvertToRelativeForMake(o->c_str());
           }
-
-        // If there is more than one output treat the first as the
-        // primary output and make the rest depend on it.
-        std::vector<std::string>::const_iterator o = outputs.begin();
-        std::string primaryOutput = this->ConvertToRelativeForMake(o->c_str());
-        for(++o; o != outputs.end(); ++o)
-          {
-          std::string currentOutput=this->ConvertToRelativeForMake(o->c_str());
-          multipleOutputPairs[currentOutput] = primaryOutput;
-          }
         }
       else
         {
@@ -1618,9 +1600,15 @@ void  cmGlobalXCodeGenerator
       if(!outputs.empty())
         {
         // There is at least one output, start the rule for it
-        std::string primary_output =
+        const char* sep = "";
-            this->ConvertToRelativeForMake(outputs.begin()->c_str());
+        for(std::vector<std::string>::const_iterator oi = outputs.begin();
-        makefileStream << primary_output << ": ";
+            oi != outputs.end(); ++oi)
+          {
+          makefileStream << sep <<
+            this->ConvertToRelativeForMake(oi->c_str());
+          sep = " ";
+          }
+        makefileStream << ": ";
         }
       else
         {
@@ -1670,33 +1658,6 @@ void  cmGlobalXCodeGenerator
         }
       }
     }
-
-  // Add rules to deal with multiple outputs of custom commands.
-  if(!multipleOutputPairs.empty())
-    {
-    makefileStream <<
-        "\n# Dependencies of multiple outputs to their primary outputs \n";
-
-    for(std::map<std::string, std::string>::const_iterator o =
-        multipleOutputPairs.begin(); o != multipleOutputPairs.end(); ++o)
-      {
-      makefileStream << o->first << ": " << o->second << "\n";
-      }
-
-    makefileStream <<
-        "\n"
-        "cmake_check_multiple_outputs:\n";
-    for(std::map<std::string, std::string>::const_iterator o =
-        multipleOutputPairs.begin(); o != multipleOutputPairs.end(); ++o)
-      {
-      makefileStream << "\t@if [ ! -f "
-          << o->first << " ]; then rm -f "
-          << o->second << "; fi\n";
-      }
-    }
-
-  haveMultipleOutputPairs =
-    haveMultipleOutputPairs || !multipleOutputPairs.empty();
 }
 
 //----------------------------------------------------------------------------

Source/cmGlobalXCodeGenerator.h

@@ -118,8 +118,7 @@ private:
   void CreateCustomRulesMakefile(const char* makefileBasename,
                                  cmTarget& target,
                                  std::vector<cmCustomCommand> const & commands,
-                                 const std::string& configName,
+                                 const std::string& configName);
-                                 bool& haveMultipleOutputPairs);
 
   cmXCodeObject* FindXCodeTarget(cmTarget const*);
   std::string GetOrCreateId(const std::string& name, const std::string& id);

Source/cmLocalUnixMakefileGenerator3.cxx

@@ -92,6 +92,7 @@ cmLocalUnixMakefileGenerator3::cmLocalUnixMakefileGenerator3()
   this->SkipAssemblySourceRules = false;
   this->MakeCommandEscapeTargetTwice = false;
   this->BorlandMakeCurlyHack = false;
+  this->NoMultiOutputMultiDepRules = false;
 }
 
 //----------------------------------------------------------------------------
@@ -618,6 +619,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 +661,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)
     {
@@ -662,6 +697,19 @@ cmLocalUnixMakefileGenerator3
     // No dependencies.  The commands will always run.
     os << cmMakeSafe(tgt) << space << ":\n";
     }
+  else if(this->NoMultiOutputMultiDepRules && outputs.size() >= 2)
+    {
+    // Borland make does not understand multiple dependency rules when
+    // there are multiple outputs, so write them all on one line.
+    os << cmMakeSafe(tgt) << space << ":";
+    for(std::vector<std::string>::const_iterator dep = depends.begin();
+        dep != depends.end(); ++dep)
+      {
+      replace = this->Convert(*dep, HOME_OUTPUT, MAKERULE);
+      os << " " << cmMakeSafe(replace);
+      }
+    os << "\n";
+    }
   else
     {
     // Split dependencies into multiple rule lines.  This allows for
@@ -690,7 +738,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 +1761,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);
@@ -154,6 +161,9 @@ public:
   void SetBorlandMakeCurlyHack(bool b)
     { this->BorlandMakeCurlyHack = b; }
 
+  void SetNoMultiOutputMultiDepRules(bool b)
+    { this->NoMultiOutputMultiDepRules = b; }
+
   // used in writing out Cmake files such as WriteDirectoryInformation
   static void WriteCMakeArgument(std::ostream& os, const char* s);
 
@@ -338,6 +348,7 @@ private:
   bool PassMakeflags;
   bool MakeCommandEscapeTargetTwice;
   bool BorlandMakeCurlyHack;
+  bool NoMultiOutputMultiDepRules;
   //==========================================================================
 
   std::string HomeRelativeOutputPath;

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;

Tests/BuildDepends/CMakeLists.txt

@@ -65,6 +65,7 @@ file(WRITE ${BuildDepends_BINARY_DIR}/Project/link_depends_no_shared_exe.h
 set(link_depends_no_shared_check_txt ${BuildDepends_BINARY_DIR}/Project/link_depends_no_shared_check.txt)
 
 file(WRITE ${BuildDepends_BINARY_DIR}/Project/external.in "external original\n")
+file(WRITE ${BuildDepends_BINARY_DIR}/Project/multi1-in.txt "multi1-in original\n")
 
 help_xcode_depends()
 
@@ -177,6 +178,19 @@ else()
     "external.out is missing")
 endif()
 
+if(EXISTS ${BuildDepends_BINARY_DIR}/Project/multi1-out2-copy.txt)
+  file(STRINGS ${BuildDepends_BINARY_DIR}/Project/multi1-out2-copy.txt multi1_out)
+  if("${multi1_out}" STREQUAL "multi1-in original")
+    message(STATUS "multi1-out2-copy.txt contains '${multi1_out}'")
+  else()
+    message(SEND_ERROR "Project did not initially build properly: "
+      "multi1-out2-copy.txt contains '${multi1_out}'")
+  endif()
+else()
+  message(SEND_ERROR "Project did not initially build properly: "
+    "multi1-out2-copy.txt is missing")
+endif()
+
 message("Waiting 3 seconds...")
 execute_process(COMMAND ${CMAKE_COMMAND} -E sleep 3)
 
@@ -202,6 +216,7 @@ if(TEST_LINK_DEPENDS)
 endif()
 
 file(WRITE ${BuildDepends_BINARY_DIR}/Project/external.in "external changed\n")
+file(WRITE ${BuildDepends_BINARY_DIR}/Project/multi1-in.txt "multi1-in changed\n")
 
 help_xcode_depends()
 
@@ -319,3 +334,16 @@ else()
   message(SEND_ERROR "Project did not rebuild properly: "
     "external.out is missing")
 endif()
+
+if(EXISTS ${BuildDepends_BINARY_DIR}/Project/multi1-out2-copy.txt)
+  file(STRINGS ${BuildDepends_BINARY_DIR}/Project/multi1-out2-copy.txt multi1_out)
+  if("${multi1_out}" STREQUAL "multi1-in changed")
+    message(STATUS "multi1-out2-copy.txt contains '${multi1_out}'")
+  else()
+    message(SEND_ERROR "Project did not rebuild properly: "
+      "multi1-out2-copy.txt contains '${multi1_out}'")
+  endif()
+else()
+  message(SEND_ERROR "Project did not rebuild properly: "
+    "multi1-out2-copy.txt is missing")
+endif()

Tests/BuildDepends/Project/CMakeLists.txt

@@ -151,3 +151,16 @@ ExternalProject_Add(ExternalBuild
     -Dexternal_out=${CMAKE_CURRENT_BINARY_DIR}/external.out
   INSTALL_COMMAND ""
   )
+
+add_custom_command(
+  OUTPUT multi1-out1.txt multi1-out2.txt
+  COMMAND ${CMAKE_COMMAND} -E copy multi1-in.txt multi1-out1.txt
+  COMMAND ${CMAKE_COMMAND} -E copy multi1-in.txt multi1-out2.txt
+  DEPENDS ${CMAKE_CURRENT_BINARY_DIR}/multi1-in.txt
+  )
+add_custom_command(
+  OUTPUT multi1-out2-copy.txt
+  COMMAND ${CMAKE_COMMAND} -E copy multi1-out2.txt multi1-out2-copy.txt
+  DEPENDS multi1-out2.txt
+  )
+add_custom_target(multi1 ALL DEPENDS multi1-out2-copy.txt)