From 644b4688d71cc52f8499d6103495de0909319557 Mon Sep 17 00:00:00 2001 From: Brad King Date: Fri, 5 Dec 2014 09:55:49 -0500 Subject: [PATCH] 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 | 46 +++++++++- Source/cmLocalUnixMakefileGenerator3.h | 7 ++ Source/cmMakefileLibraryTargetGenerator.cxx | 21 ++--- Source/cmMakefileTargetGenerator.cxx | 93 ++------------------- Source/cmMakefileTargetGenerator.h | 12 --- 5 files changed, 68 insertions(+), 111 deletions(-) diff --git a/Source/cmLocalUnixMakefileGenerator3.cxx b/Source/cmLocalUnixMakefileGenerator3.cxx index ff8ba8be89..56b2b97099 100644 --- a/Source/cmLocalUnixMakefileGenerator3.cxx +++ b/Source/cmLocalUnixMakefileGenerator3.cxx @@ -618,6 +618,30 @@ cmLocalUnixMakefileGenerator3 comment); return; } + std::vector 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& outputs, + const std::vector& depends, + const std::vector& 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::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::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 4f2e4a0355..65265ce08f 100644 --- a/Source/cmLocalUnixMakefileGenerator3.h +++ b/Source/cmLocalUnixMakefileGenerator3.h @@ -61,6 +61,13 @@ public: const std::vector& commands, bool symbolic, bool in_help = false); + void WriteMakeRule(std::ostream& os, + const char* comment, + const std::vector& outputs, + const std::vector& depends, + const std::vector& 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 80473f6131..305d81d7b7 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 extraOutputs; + // Compute the list of outputs. + std::vector 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 8444dfb7a1..067714ef9e 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 outputs(1, relativeObj); if(const char* extra_outputs_str = source.GetProperty("OBJECT_OUTPUTS")) { - std::vector extra_outputs; - cmSystemTools::ExpandListArgument(extra_outputs_str, extra_outputs); - for(std::vector::const_iterator eoi = extra_outputs.begin(); - eoi != extra_outputs.end(); ++eoi) + cmSystemTools::ExpandListArgument(extra_outputs_str, outputs); + for(std::vector::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(); @@ -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 commands; - std::vector 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& 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 diff --git a/Source/cmMakefileTargetGenerator.h b/Source/cmMakefileTargetGenerator.h index 9fac5746f4..e31e0863ac 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 ExtraFiles; - typedef std::map MultipleOutputPairsType; - MultipleOutputPairsType MultipleOutputPairs; - // Target name info. std::string TargetNameOut; std::string TargetNameSO;