From 4926ab24542d2c9e644feee88efd4a96f8bcab97 Mon Sep 17 00:00:00 2001 From: Daniel Eiband Date: Fri, 6 Sep 2019 19:17:37 +0200 Subject: cmMakefile: Create all generated byproducts as known sources --- Source/cmMakefile.cxx | 11 ++++++----- Source/cmMakefile.h | 4 +--- 2 files changed, 7 insertions(+), 8 deletions(-) diff --git a/Source/cmMakefile.cxx b/Source/cmMakefile.cxx index f35b999..564ac56 100644 --- a/Source/cmMakefile.cxx +++ b/Source/cmMakefile.cxx @@ -970,8 +970,8 @@ cmSourceFile* cmMakefile::AddCustomCommandToOutput( } // Always create the output sources and mark them generated. - this->CreateGeneratedSources(outputs, cmSourceFileLocationKind::Known); - this->CreateGeneratedSources(byproducts, cmSourceFileLocationKind::Known); + this->CreateGeneratedSources(outputs); + this->CreateGeneratedSources(byproducts); // Choose a source file on which to store the custom command. cmSourceFile* file = nullptr; @@ -1239,7 +1239,7 @@ cmTarget* cmMakefile::AddUtilityCommand( // Store the custom command in the target. if (!commandLines.empty() || !depends.empty()) { // Always create the byproduct sources and mark them generated. - this->CreateGeneratedSources(byproducts, cmSourceFileLocationKind::Known); + this->CreateGeneratedSources(byproducts); std::string force = cmStrCat(this->GetCurrentBinaryDirectory(), "/CMakeFiles/", utilityName); @@ -3444,10 +3444,11 @@ cmSourceFile* cmMakefile::GetOrCreateSource(const std::string& sourceName, } void cmMakefile::CreateGeneratedSources( - const std::vector& outputs, cmSourceFileLocationKind kind) + const std::vector& outputs) { for (std::string const& output : outputs) { - if (cmSourceFile* out = this->GetOrCreateSource(output, true, kind)) { + if (cmSourceFile* out = this->GetOrCreateSource( + output, true, cmSourceFileLocationKind::Known)) { out->SetProperty("GENERATED", "1"); } } diff --git a/Source/cmMakefile.h b/Source/cmMakefile.h index 1944879..28f5452 100644 --- a/Source/cmMakefile.h +++ b/Source/cmMakefile.h @@ -1067,9 +1067,7 @@ private: bool atOnly, const char* filename, long line, bool replaceAt) const; - void CreateGeneratedSources( - const std::vector& outputs, - cmSourceFileLocationKind kind = cmSourceFileLocationKind::Ambiguous); + void CreateGeneratedSources(const std::vector& outputs); /** * See LinearGetSourceFileWithOutput for background information -- cgit v0.12 From f1e846fddece96d76d4d6b53ff1ca8ed197af550 Mon Sep 17 00:00:00 2001 From: Daniel Eiband Date: Sat, 14 Sep 2019 14:16:14 +0200 Subject: cmMakefile: Extract custom command validation method --- Source/cmMakefile.cxx | 27 +++++++++++++++++++-------- Source/cmMakefile.h | 2 ++ 2 files changed, 21 insertions(+), 8 deletions(-) diff --git a/Source/cmMakefile.cxx b/Source/cmMakefile.cxx index 564ac56..6462c26 100644 --- a/Source/cmMakefile.cxx +++ b/Source/cmMakefile.cxx @@ -821,6 +821,22 @@ void cmMakefile::ConfigureFinalPass() } } +bool cmMakefile::ValidateCustomCommand( + const cmCustomCommandLines& commandLines) const +{ + // TODO: More strict? + for (cmCustomCommandLine const& cl : commandLines) { + if (!cl.empty() && !cl[0].empty() && cl[0][0] == '"') { + std::ostringstream e; + e << "COMMAND may not contain literal quotes:\n " << cl[0] << "\n"; + this->IssueMessage(MessageType::FATAL_ERROR, e.str()); + return false; + } + } + + return true; +} + void cmMakefile::AddCustomCommandToTarget( const std::string& target, const std::vector& byproducts, const std::vector& depends, @@ -959,14 +975,9 @@ cmSourceFile* cmMakefile::AddCustomCommandToOutput( return nullptr; } - // Validate custom commands. TODO: More strict? - for (cmCustomCommandLine const& cl : commandLines) { - if (!cl.empty() && !cl[0].empty() && cl[0][0] == '"') { - std::ostringstream e; - e << "COMMAND may not contain literal quotes:\n " << cl[0] << "\n"; - this->IssueMessage(MessageType::FATAL_ERROR, e.str()); - return nullptr; - } + // Validate custom commands. + if (!this->ValidateCustomCommand(commandLines)) { + return nullptr; } // Always create the output sources and mark them generated. diff --git a/Source/cmMakefile.h b/Source/cmMakefile.h index 28f5452..4a1af7d 100644 --- a/Source/cmMakefile.h +++ b/Source/cmMakefile.h @@ -1067,6 +1067,8 @@ private: bool atOnly, const char* filename, long line, bool replaceAt) const; + bool ValidateCustomCommand(const cmCustomCommandLines& commandLines) const; + void CreateGeneratedSources(const std::vector& outputs); /** -- cgit v0.12 From e893ab94baa29b4a21d3edd86174cd19d8cd5a1e Mon Sep 17 00:00:00 2001 From: Daniel Eiband Date: Sat, 14 Sep 2019 15:29:59 +0200 Subject: cmMakefile: Validate command line for all custom commands --- Source/cmMakefile.cxx | 92 +++++++++++++++++++++++++++++---------------------- Source/cmMakefile.h | 2 +- 2 files changed, 54 insertions(+), 40 deletions(-) diff --git a/Source/cmMakefile.cxx b/Source/cmMakefile.cxx index 6462c26..6707b1c 100644 --- a/Source/cmMakefile.cxx +++ b/Source/cmMakefile.cxx @@ -837,7 +837,7 @@ bool cmMakefile::ValidateCustomCommand( return true; } -void cmMakefile::AddCustomCommandToTarget( +cmTarget* cmMakefile::AddCustomCommandToTarget( const std::string& target, const std::vector& byproducts, const std::vector& depends, const cmCustomCommandLines& commandLines, cmTarget::CustomCommandType type, @@ -880,26 +880,31 @@ void cmMakefile::AddCustomCommandToTarget( this->IssueMessage(messageType, e.str()); } - return; + return nullptr; } - cmTarget& t = ti->second; + cmTarget* t = &ti->second; if (objLibraryCommands == RejectObjectLibraryCommands && - t.GetType() == cmStateEnums::OBJECT_LIBRARY) { + t->GetType() == cmStateEnums::OBJECT_LIBRARY) { std::ostringstream e; e << "Target \"" << target << "\" is an OBJECT library " "that may not have PRE_BUILD, PRE_LINK, or POST_BUILD commands."; this->IssueMessage(MessageType::FATAL_ERROR, e.str()); - return; + return nullptr; } - if (t.GetType() == cmStateEnums::INTERFACE_LIBRARY) { + if (t->GetType() == cmStateEnums::INTERFACE_LIBRARY) { std::ostringstream e; e << "Target \"" << target << "\" is an INTERFACE library " "that may not have PRE_BUILD, PRE_LINK, or POST_BUILD commands."; this->IssueMessage(MessageType::FATAL_ERROR, e.str()); - return; + return nullptr; + } + + // Validate custom commands. + if (!this->ValidateCustomCommand(commandLines)) { + return t; } // Always create the byproduct sources and mark them generated. @@ -917,16 +922,18 @@ void cmMakefile::AddCustomCommandToTarget( cc.SetJobPool(job_pool); switch (type) { case cmTarget::PRE_BUILD: - t.AddPreBuildCommand(cc); + t->AddPreBuildCommand(cc); break; case cmTarget::PRE_LINK: - t.AddPreLinkCommand(cc); + t->AddPreLinkCommand(cc); break; case cmTarget::POST_BUILD: - t.AddPostBuildCommand(cc); + t->AddPostBuildCommand(cc); break; } - this->UpdateOutputToSourceMap(byproducts, &t); + this->UpdateOutputToSourceMap(byproducts, t); + + return t; } void cmMakefile::UpdateOutputToSourceMap( @@ -1175,9 +1182,12 @@ bool cmMakefile::AppendCustomCommandToOutput( // Lookup an existing command. if (cmSourceFile* sf = this->GetSourceFileWithOutput(output)) { if (cmCustomCommand* cc = sf->GetCustomCommand()) { - cc->AppendCommands(commandLines); - cc->AppendDepends(depends); - cc->AppendImplicitDepends(implicit_depends); + // Validate custom commands. + if (this->ValidateCustomCommand(commandLines)) { + cc->AppendCommands(commandLines); + cc->AppendDepends(depends); + cc->AppendImplicitDepends(implicit_depends); + } return true; } } @@ -1247,33 +1257,37 @@ cmTarget* cmMakefile::AddUtilityCommand( comment = ""; } - // Store the custom command in the target. - if (!commandLines.empty() || !depends.empty()) { - // Always create the byproduct sources and mark them generated. - this->CreateGeneratedSources(byproducts); - - std::string force = - cmStrCat(this->GetCurrentBinaryDirectory(), "/CMakeFiles/", utilityName); - std::vector forced; - forced.push_back(force); - std::string no_main_dependency; - cmImplicitDependsList no_implicit_depends; - bool no_replace = false; - 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); - - // 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); - } + // Validate custom commands. + if (!this->ValidateCustomCommand(commandLines) || + (commandLines.empty() && depends.empty())) { + return target; + } + + // Always create the byproduct sources and mark them generated. + this->CreateGeneratedSources(byproducts); - this->UpdateOutputToSourceMap(byproducts, target); + std::string force = + cmStrCat(this->GetCurrentBinaryDirectory(), "/CMakeFiles/", utilityName); + std::vector forced; + forced.push_back(force); + std::string no_main_dependency; + cmImplicitDependsList no_implicit_depends; + bool no_replace = false; + 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); + + // 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); } + + this->UpdateOutputToSourceMap(byproducts, target); + return target; } diff --git a/Source/cmMakefile.h b/Source/cmMakefile.h index 4a1af7d..ed118db 100644 --- a/Source/cmMakefile.h +++ b/Source/cmMakefile.h @@ -172,7 +172,7 @@ public: }; /** Add a custom command to the build. */ - void AddCustomCommandToTarget( + cmTarget* AddCustomCommandToTarget( const std::string& target, const std::vector& byproducts, const std::vector& depends, const cmCustomCommandLines& commandLines, cmTarget::CustomCommandType type, -- cgit v0.12 From 3061dc6ac967e859424f81fb70bbc70a74246055 Mon Sep 17 00:00:00 2001 From: Daniel Eiband Date: Sat, 14 Sep 2019 21:44:42 +0200 Subject: add_custom_command: Add tests for rejecting literal quotes in commands --- Tests/RunCMake/add_custom_command/AppendLiteralQuotes-result.txt | 1 + Tests/RunCMake/add_custom_command/AppendLiteralQuotes-stderr.txt | 7 +++++++ Tests/RunCMake/add_custom_command/AppendLiteralQuotes.cmake | 2 ++ Tests/RunCMake/add_custom_command/LiteralQuotes-result.txt | 1 + Tests/RunCMake/add_custom_command/LiteralQuotes-stderr.txt | 7 +++++++ Tests/RunCMake/add_custom_command/LiteralQuotes.cmake | 1 + Tests/RunCMake/add_custom_command/RunCMakeTest.cmake | 3 +++ Tests/RunCMake/add_custom_command/TargetLiteralQuotes-result.txt | 1 + Tests/RunCMake/add_custom_command/TargetLiteralQuotes-stderr.txt | 7 +++++++ Tests/RunCMake/add_custom_command/TargetLiteralQuotes.cmake | 2 ++ Tests/RunCMake/add_custom_target/LiteralQuotes-result.txt | 1 + Tests/RunCMake/add_custom_target/LiteralQuotes-stderr.txt | 7 +++++++ Tests/RunCMake/add_custom_target/LiteralQuotes.cmake | 1 + Tests/RunCMake/add_custom_target/RunCMakeTest.cmake | 1 + 14 files changed, 42 insertions(+) create mode 100644 Tests/RunCMake/add_custom_command/AppendLiteralQuotes-result.txt create mode 100644 Tests/RunCMake/add_custom_command/AppendLiteralQuotes-stderr.txt create mode 100644 Tests/RunCMake/add_custom_command/AppendLiteralQuotes.cmake create mode 100644 Tests/RunCMake/add_custom_command/LiteralQuotes-result.txt create mode 100644 Tests/RunCMake/add_custom_command/LiteralQuotes-stderr.txt create mode 100644 Tests/RunCMake/add_custom_command/LiteralQuotes.cmake create mode 100644 Tests/RunCMake/add_custom_command/TargetLiteralQuotes-result.txt create mode 100644 Tests/RunCMake/add_custom_command/TargetLiteralQuotes-stderr.txt create mode 100644 Tests/RunCMake/add_custom_command/TargetLiteralQuotes.cmake create mode 100644 Tests/RunCMake/add_custom_target/LiteralQuotes-result.txt create mode 100644 Tests/RunCMake/add_custom_target/LiteralQuotes-stderr.txt create mode 100644 Tests/RunCMake/add_custom_target/LiteralQuotes.cmake diff --git a/Tests/RunCMake/add_custom_command/AppendLiteralQuotes-result.txt b/Tests/RunCMake/add_custom_command/AppendLiteralQuotes-result.txt new file mode 100644 index 0000000..d00491f --- /dev/null +++ b/Tests/RunCMake/add_custom_command/AppendLiteralQuotes-result.txt @@ -0,0 +1 @@ +1 diff --git a/Tests/RunCMake/add_custom_command/AppendLiteralQuotes-stderr.txt b/Tests/RunCMake/add_custom_command/AppendLiteralQuotes-stderr.txt new file mode 100644 index 0000000..503385f --- /dev/null +++ b/Tests/RunCMake/add_custom_command/AppendLiteralQuotes-stderr.txt @@ -0,0 +1,7 @@ +CMake Error at AppendLiteralQuotes.cmake:2 \(add_custom_command\): + COMMAND may not contain literal quotes: + + "c" + +Call Stack \(most recent call first\): + CMakeLists.txt:3 \(include\) diff --git a/Tests/RunCMake/add_custom_command/AppendLiteralQuotes.cmake b/Tests/RunCMake/add_custom_command/AppendLiteralQuotes.cmake new file mode 100644 index 0000000..038fb3f --- /dev/null +++ b/Tests/RunCMake/add_custom_command/AppendLiteralQuotes.cmake @@ -0,0 +1,2 @@ +add_custom_command(OUTPUT a COMMAND b) +add_custom_command(OUTPUT a COMMAND "\"c\"" APPEND) diff --git a/Tests/RunCMake/add_custom_command/LiteralQuotes-result.txt b/Tests/RunCMake/add_custom_command/LiteralQuotes-result.txt new file mode 100644 index 0000000..d00491f --- /dev/null +++ b/Tests/RunCMake/add_custom_command/LiteralQuotes-result.txt @@ -0,0 +1 @@ +1 diff --git a/Tests/RunCMake/add_custom_command/LiteralQuotes-stderr.txt b/Tests/RunCMake/add_custom_command/LiteralQuotes-stderr.txt new file mode 100644 index 0000000..e7d6155 --- /dev/null +++ b/Tests/RunCMake/add_custom_command/LiteralQuotes-stderr.txt @@ -0,0 +1,7 @@ +CMake Error at LiteralQuotes.cmake:1 \(add_custom_command\): + COMMAND may not contain literal quotes: + + "b" + +Call Stack \(most recent call first\): + CMakeLists.txt:3 \(include\) diff --git a/Tests/RunCMake/add_custom_command/LiteralQuotes.cmake b/Tests/RunCMake/add_custom_command/LiteralQuotes.cmake new file mode 100644 index 0000000..046ddd9 --- /dev/null +++ b/Tests/RunCMake/add_custom_command/LiteralQuotes.cmake @@ -0,0 +1 @@ +add_custom_command(OUTPUT a COMMAND "\"b\"") diff --git a/Tests/RunCMake/add_custom_command/RunCMakeTest.cmake b/Tests/RunCMake/add_custom_command/RunCMakeTest.cmake index 20097b7..270df2f 100644 --- a/Tests/RunCMake/add_custom_command/RunCMakeTest.cmake +++ b/Tests/RunCMake/add_custom_command/RunCMakeTest.cmake @@ -1,15 +1,18 @@ include(RunCMake) +run_cmake(AppendLiteralQuotes) run_cmake(AppendNoOutput) run_cmake(AppendNotOutput) run_cmake(BadArgument) run_cmake(GeneratedProperty) +run_cmake(LiteralQuotes) run_cmake(NoArguments) run_cmake(NoOutputOrTarget) run_cmake(OutputAndTarget) run_cmake(SourceByproducts) run_cmake(SourceUsesTerminal) run_cmake(TargetImported) +run_cmake(TargetLiteralQuotes) run_cmake(TargetNotInDir) if(${RunCMake_GENERATOR} MATCHES "Visual Studio ([^89]|[89][0-9])") diff --git a/Tests/RunCMake/add_custom_command/TargetLiteralQuotes-result.txt b/Tests/RunCMake/add_custom_command/TargetLiteralQuotes-result.txt new file mode 100644 index 0000000..d00491f --- /dev/null +++ b/Tests/RunCMake/add_custom_command/TargetLiteralQuotes-result.txt @@ -0,0 +1 @@ +1 diff --git a/Tests/RunCMake/add_custom_command/TargetLiteralQuotes-stderr.txt b/Tests/RunCMake/add_custom_command/TargetLiteralQuotes-stderr.txt new file mode 100644 index 0000000..c4589b4 --- /dev/null +++ b/Tests/RunCMake/add_custom_command/TargetLiteralQuotes-stderr.txt @@ -0,0 +1,7 @@ +CMake Error at TargetLiteralQuotes.cmake:2 \(add_custom_command\): + COMMAND may not contain literal quotes: + + "b" + +Call Stack \(most recent call first\): + CMakeLists.txt:3 \(include\) diff --git a/Tests/RunCMake/add_custom_command/TargetLiteralQuotes.cmake b/Tests/RunCMake/add_custom_command/TargetLiteralQuotes.cmake new file mode 100644 index 0000000..5f11ed1 --- /dev/null +++ b/Tests/RunCMake/add_custom_command/TargetLiteralQuotes.cmake @@ -0,0 +1,2 @@ +add_library(a) +add_custom_command(TARGET a POST_BUILD COMMAND "\"b\"") diff --git a/Tests/RunCMake/add_custom_target/LiteralQuotes-result.txt b/Tests/RunCMake/add_custom_target/LiteralQuotes-result.txt new file mode 100644 index 0000000..d00491f --- /dev/null +++ b/Tests/RunCMake/add_custom_target/LiteralQuotes-result.txt @@ -0,0 +1 @@ +1 diff --git a/Tests/RunCMake/add_custom_target/LiteralQuotes-stderr.txt b/Tests/RunCMake/add_custom_target/LiteralQuotes-stderr.txt new file mode 100644 index 0000000..bc5187a --- /dev/null +++ b/Tests/RunCMake/add_custom_target/LiteralQuotes-stderr.txt @@ -0,0 +1,7 @@ +CMake Error at LiteralQuotes.cmake:1 \(add_custom_target\): + COMMAND may not contain literal quotes: + + "b" + +Call Stack \(most recent call first\): + CMakeLists.txt:3 \(include\) diff --git a/Tests/RunCMake/add_custom_target/LiteralQuotes.cmake b/Tests/RunCMake/add_custom_target/LiteralQuotes.cmake new file mode 100644 index 0000000..9870608 --- /dev/null +++ b/Tests/RunCMake/add_custom_target/LiteralQuotes.cmake @@ -0,0 +1 @@ +add_custom_target(a COMMAND "\"b\"") diff --git a/Tests/RunCMake/add_custom_target/RunCMakeTest.cmake b/Tests/RunCMake/add_custom_target/RunCMakeTest.cmake index d80ca19..49c7d3e 100644 --- a/Tests/RunCMake/add_custom_target/RunCMakeTest.cmake +++ b/Tests/RunCMake/add_custom_target/RunCMakeTest.cmake @@ -5,6 +5,7 @@ run_cmake(GeneratedProperty) run_cmake(NoArguments) run_cmake(BadTargetName) run_cmake(ByproductsNoCommand) +run_cmake(LiteralQuotes) run_cmake(UsesTerminalNoCommand) function(run_TargetOrder) -- cgit v0.12 From 56c204e8eb8914b2ca273a56119cf1c40e13d75d Mon Sep 17 00:00:00 2001 From: Daniel Eiband Date: Sat, 14 Sep 2019 22:01:20 +0200 Subject: cmMakefile: Refactor AddCustomCommandOldStyle to be delay friendly Custom command functions consist of two parts: setup and commit. Only the commit part will be delayed to generate time. This change puts the commit part of AddCustomCommandOldStyle into a lambda. When delayed, it will not be possible to break the loop over the outputs if an error occurs which seems reasonable. --- Source/cmMakefile.cxx | 63 +++++++++++++++++++++++++++++---------------------- 1 file changed, 36 insertions(+), 27 deletions(-) diff --git a/Source/cmMakefile.cxx b/Source/cmMakefile.cxx index 6707b1c..19cd068 100644 --- a/Source/cmMakefile.cxx +++ b/Source/cmMakefile.cxx @@ -1134,41 +1134,50 @@ void cmMakefile::AddCustomCommandOldStyle( return; } - // Each output must get its own copy of this rule. - cmsys::RegularExpression sourceFiles("\\.(C|M|c|c\\+\\+|cc|cpp|cxx|cu|m|mm|" - "rc|def|r|odl|idl|hpj|bat|h|h\\+\\+|" - "hm|hpp|hxx|in|txx|inl)$"); - for (std::string const& oi : outputs) { - // Get the name of this output. - const char* output = oi.c_str(); - cmSourceFile* sf; - - // Choose whether to use a main dependency. - if (sourceFiles.find(source)) { - // The source looks like a real file. Use it as the main dependency. - sf = this->AddCustomCommandToOutput(output, depends, source, - commandLines, comment, nullptr); - } else { - // The source may not be a real file. Do not use a main dependency. - std::string no_main_dependency; - std::vector depends2 = depends; - depends2.push_back(source); - sf = this->AddCustomCommandToOutput(output, depends2, no_main_dependency, - commandLines, comment, nullptr); - } + auto ti = this->Targets.find(target); + cmTarget* t = ti != this->Targets.end() ? &ti->second : nullptr; + auto addRuleFileToTarget = [=](cmSourceFile* sf) { // If the rule was added to the source (and not a .rule file), // then add the source to the target to make sure the rule is // included. - if (sf && !sf->GetPropertyAsBool("__CMAKE_RULE")) { - auto ti = this->Targets.find(target); - if (ti != this->Targets.end()) { - ti->second.AddSource(sf->ResolveFullPath()); + if (!sf->GetPropertyAsBool("__CMAKE_RULE")) { + if (t) { + t->AddSource(sf->ResolveFullPath()); } else { cmSystemTools::Error("Attempt to add a custom rule to a target " "that does not exist yet for target " + target); - return; + } + } + }; + + // Each output must get its own copy of this rule. + cmsys::RegularExpression sourceFiles("\\.(C|M|c|c\\+\\+|cc|cpp|cxx|cu|m|mm|" + "rc|def|r|odl|idl|hpj|bat|h|h\\+\\+|" + "hm|hpp|hxx|in|txx|inl)$"); + + // Choose whether to use a main dependency. + if (sourceFiles.find(source)) { + // The source looks like a real file. Use it as the main dependency. + for (std::string const& output : outputs) { + cmSourceFile* sf = this->AddCustomCommandToOutput( + output, depends, source, commandLines, comment, nullptr); + if (sf) { + addRuleFileToTarget(sf); + } + } + } else { + std::string no_main_dependency; + std::vector depends2 = depends; + depends2.push_back(source); + + // The source may not be a real file. Do not use a main dependency. + for (std::string const& output : outputs) { + cmSourceFile* sf = this->AddCustomCommandToOutput( + output, depends2, no_main_dependency, commandLines, comment, nullptr); + if (sf) { + addRuleFileToTarget(sf); } } } -- cgit v0.12 From 0e1faa28cbd12d400b876f7a21d91aad5a837196 Mon Sep 17 00:00:00 2001 From: Daniel Eiband Date: Sat, 14 Sep 2019 22:59:05 +0200 Subject: 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 | 189 ++++++++++++++++++++++++++++++++++++-------------- Source/cmMakefile.h | 48 +++++++++++-- Source/cmTarget.cxx | 8 +-- Source/cmTarget.h | 2 +- 4 files changed, 184 insertions(+), 63 deletions(-) diff --git a/Source/cmMakefile.cxx b/Source/cmMakefile.cxx index 19cd068..b3a0678 100644 --- a/Source/cmMakefile.cxx +++ b/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& byproducts, + const std::vector& 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 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); - - return t; + this->UpdateOutputToSourceMap(byproducts, target); } void cmMakefile::UpdateOutputToSourceMap( @@ -967,6 +980,23 @@ void cmMakefile::UpdateOutputToSourceMap(std::string const& byproduct, } cmSourceFile* cmMakefile::AddCustomCommandToOutput( + const std::string& output, const std::vector& 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 outputs; + outputs.push_back(output); + std::vector 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& outputs, const std::vector& byproducts, const std::vector& depends, const std::string& main_dependency, @@ -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& outputs, + const std::vector& byproducts, + const std::vector& 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& 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 outputs; - outputs.push_back(output); - std::vector 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& outputs, const std::vector& depends, const std::string& source, @@ -1188,19 +1217,34 @@ bool cmMakefile::AppendCustomCommandToOutput( 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& 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. - if (this->ValidateCustomCommand(commandLines)) { - cc->AppendCommands(commandLines); - cc->AppendDepends(depends); - cc->AppendImplicitDepends(implicit_depends); - } - return true; + cc->AppendCommands(commandLines); + cc->AppendDepends(depends); + cc->AppendImplicitDepends(implicit_depends); } } - 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& byproducts, + const std::vector& depends, + const cmCustomCommandLines& commandLines, bool escapeOldStyle, + const char* comment, bool uses_terminal, bool command_expand_lists, + const std::string& job_pool) +{ std::vector 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); - - // The output is not actually created so mark it symbolic. + if (!forceCMP0049.empty()) { + target->AddSource(forceCMP0049); + } if (sf) { - sf->SetProperty("SYMBOLIC", "1"); - } else { - cmSystemTools::Error("Could not get source file entry for " + force); + this->UpdateOutputToSourceMap(byproducts, target); } - - 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& 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& outputs) { for (std::string const& output : outputs) { - if (cmSourceFile* out = this->GetOrCreateSource( - output, true, cmSourceFileLocationKind::Known)) { - out->SetProperty("GENERATED", "1"); - } + this->CreateGeneratedSource(output); } } diff --git a/Source/cmMakefile.h b/Source/cmMakefile.h index ed118db..19a71ad 100644 --- a/Source/cmMakefile.h +++ b/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& outputs, - const std::vector& byproducts, - const std::vector& depends, + const std::string& output, const std::vector& 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& depends, + const std::vector& outputs, + const std::vector& byproducts, + const std::vector& 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& byproducts, + const std::vector& 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& outputs, + const std::vector& byproducts, + const std::vector& 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& 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& byproducts, + const std::vector& 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& 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; diff --git a/Source/cmTarget.cxx b/Source/cmTarget.cxx index 6637e32..83d69a4 100644 --- a/Source/cmTarget.cxx +++ b/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); - if (!s.empty() && src.empty()) { - return nullptr; - } - return this->AddSource(src); + return impl->ProcessSourceItemCMP0049(s); } struct CreateLocation diff --git a/Source/cmTarget.h b/Source/cmTarget.h index e9bcffe..f4726d3 100644 --- a/Source/cmTarget.h +++ b/Source/cmTarget.h @@ -103,7 +103,7 @@ public: //! Add sources to the target. void AddSources(std::vector const& srcs); void AddTracedSources(std::vector 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 -- cgit v0.12