From ceb82752efbd7a25d1b54e0b038712eecb70163b Mon Sep 17 00:00:00 2001 From: Brad King Date: Tue, 18 May 2021 13:10:51 -0400 Subject: cmLocalNinjaGenerator: Use variable for main custom command output path --- Source/cmLocalNinjaGenerator.cxx | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/Source/cmLocalNinjaGenerator.cxx b/Source/cmLocalNinjaGenerator.cxx index a8570e2..82aa3c4 100644 --- a/Source/cmLocalNinjaGenerator.cxx +++ b/Source/cmLocalNinjaGenerator.cxx @@ -649,6 +649,8 @@ void cmLocalNinjaGenerator::WriteCustomCommandBuildStatement( gg->SeenCustomCommandOutput(ninjaOutput); } + std::string mainOutput = ninjaOutputs[0]; + cmNinjaDeps ninjaDeps; this->AppendCustomCommandDeps(ccg, ninjaDeps, fileConfig); @@ -657,13 +659,13 @@ void cmLocalNinjaGenerator::WriteCustomCommandBuildStatement( if (cmdLines.empty()) { cmNinjaBuild build("phony"); - build.Comment = "Phony custom command for " + ninjaOutputs[0]; + build.Comment = cmStrCat("Phony custom command for ", mainOutput); build.Outputs = std::move(ninjaOutputs); build.ExplicitDeps = std::move(ninjaDeps); build.OrderOnlyDeps = orderOnlyDeps; gg->WriteBuild(this->GetImplFileStream(fileConfig), build); } else { - std::string customStep = cmSystemTools::GetFilenameName(ninjaOutputs[0]); + std::string customStep = cmSystemTools::GetFilenameName(mainOutput); if (this->GlobalGenerator->IsMultiConfig()) { customStep += '-'; customStep += fileConfig; @@ -673,7 +675,7 @@ void cmLocalNinjaGenerator::WriteCustomCommandBuildStatement( // Hash full path to make unique. customStep += '-'; cmCryptoHash hash(cmCryptoHash::AlgoSHA256); - customStep += hash.HashString(ninjaOutputs[0]).substr(0, 7); + customStep += hash.HashString(mainOutput).substr(0, 7); std::string depfile = ccg.GetDepfile(); if (!depfile.empty()) { @@ -701,11 +703,12 @@ void cmLocalNinjaGenerator::WriteCustomCommandBuildStatement( } } + std::string comment = cmStrCat("Custom command for ", mainOutput); gg->WriteCustomCommandBuild( this->BuildCommandLine(cmdLines, ccg.GetOutputConfig(), fileConfig, customStep), - this->ConstructComment(ccg), "Custom command for " + ninjaOutputs[0], - depfile, cc->GetJobPool(), cc->GetUsesTerminal(), + this->ConstructComment(ccg), comment, depfile, cc->GetJobPool(), + cc->GetUsesTerminal(), /*restat*/ !symbolic || !byproducts.empty(), ninjaOutputs, fileConfig, ninjaDeps, orderOnlyDeps); } -- cgit v0.12 From ddc030f5ca20d4519559e052b7c5538cf40f34f6 Mon Sep 17 00:00:00 2001 From: Brad King Date: Tue, 18 May 2021 11:08:55 -0400 Subject: cmGlobalNinjaGenerator: Record implicit outputs as known too --- Source/cmGlobalNinjaGenerator.cxx | 3 +++ 1 file changed, 3 insertions(+) diff --git a/Source/cmGlobalNinjaGenerator.cxx b/Source/cmGlobalNinjaGenerator.cxx index 565b951..a1a17c3 100644 --- a/Source/cmGlobalNinjaGenerator.cxx +++ b/Source/cmGlobalNinjaGenerator.cxx @@ -226,6 +226,9 @@ void cmGlobalNinjaGenerator::WriteBuild(std::ostream& os, buildStr += " |"; for (std::string const& implicitOut : build.ImplicitOuts) { buildStr += cmStrCat(' ', this->EncodePath(implicitOut)); + if (this->ComputingUnknownDependencies) { + this->CombinedBuildOutputs.insert(implicitOut); + } } } buildStr += ':'; -- cgit v0.12 From 8bac527b0c8ada5c40171cf01f673f9597ffdb90 Mon Sep 17 00:00:00 2001 From: Brad King Date: Tue, 18 May 2021 14:34:48 -0400 Subject: cmGlobalNinjaGenerator: Re-order logic in WriteCustomCommandBuild Save explicit dependencies earlier. --- Source/cmGlobalNinjaGenerator.cxx | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/Source/cmGlobalNinjaGenerator.cxx b/Source/cmGlobalNinjaGenerator.cxx index a1a17c3..f7b338f 100644 --- a/Source/cmGlobalNinjaGenerator.cxx +++ b/Source/cmGlobalNinjaGenerator.cxx @@ -319,6 +319,14 @@ void cmGlobalNinjaGenerator::WriteCustomCommandBuild( { this->AddCustomCommandRule(); + if (this->ComputingUnknownDependencies) { + // we need to track every dependency that comes in, since we are trying + // to find dependencies that are side effects of build commands + for (std::string const& dep : explicitDeps) { + this->CombinedCustomCommandExplicitDependencies.insert(dep); + } + } + { cmNinjaBuild build("CUSTOM_COMMAND"); build.Comment = comment; @@ -354,14 +362,6 @@ void cmGlobalNinjaGenerator::WriteCustomCommandBuild( this->WriteBuild(*this->GetImplFileStream(config), build); } } - - if (this->ComputingUnknownDependencies) { - // we need to track every dependency that comes in, since we are trying - // to find dependencies that are side effects of build commands - for (std::string const& dep : explicitDeps) { - this->CombinedCustomCommandExplicitDependencies.insert(dep); - } - } } void cmGlobalNinjaGenerator::AddMacOSXContentRule() -- cgit v0.12 From c5195193d3525dc2b661757e0039486e39b94f27 Mon Sep 17 00:00:00 2001 From: Brad King Date: Tue, 18 May 2021 11:09:22 -0400 Subject: cmGlobalNinjaGenerator: Reduce string copies in WriteCustomCommandBuild Re-order arguments to group those with similar roles. Use move semantics to avoid copying vectors of strings. --- Source/cmGlobalNinjaGenerator.cxx | 20 ++++++++++---------- Source/cmGlobalNinjaGenerator.h | 16 +++++++++------- Source/cmLocalNinjaGenerator.cxx | 5 +++-- Source/cmNinjaUtilityTargetGenerator.cxx | 2 +- 4 files changed, 23 insertions(+), 20 deletions(-) diff --git a/Source/cmGlobalNinjaGenerator.cxx b/Source/cmGlobalNinjaGenerator.cxx index f7b338f..06f7e45 100644 --- a/Source/cmGlobalNinjaGenerator.cxx +++ b/Source/cmGlobalNinjaGenerator.cxx @@ -311,11 +311,11 @@ void cmGlobalNinjaGenerator::AddCustomCommandRule() } void cmGlobalNinjaGenerator::WriteCustomCommandBuild( - const std::string& command, const std::string& description, - const std::string& comment, const std::string& depfile, - const std::string& job_pool, bool uses_terminal, bool restat, - const cmNinjaDeps& outputs, const std::string& config, - const cmNinjaDeps& explicitDeps, const cmNinjaDeps& orderOnlyDeps) + std::string const& command, std::string const& description, + std::string const& comment, std::string const& depfile, + std::string const& job_pool, bool uses_terminal, bool restat, + std::string const& config, cmNinjaDeps outputs, cmNinjaDeps explicitDeps, + cmNinjaDeps orderOnlyDeps) { this->AddCustomCommandRule(); @@ -330,9 +330,9 @@ void cmGlobalNinjaGenerator::WriteCustomCommandBuild( { cmNinjaBuild build("CUSTOM_COMMAND"); build.Comment = comment; - build.Outputs = outputs; - build.ExplicitDeps = explicitDeps; - build.OrderOnlyDeps = orderOnlyDeps; + build.Outputs = std::move(outputs); + build.ExplicitDeps = std::move(explicitDeps); + build.OrderOnlyDeps = std::move(orderOnlyDeps); cmNinjaVars& vars = build.Variables; { @@ -1209,8 +1209,8 @@ void cmGlobalNinjaGenerator::WriteAssumedSourceDependencies() "Assume dependencies for generated source file.", /*depfile*/ "", /*job_pool*/ "", /*uses_terminal*/ false, - /*restat*/ true, cmNinjaDeps(1, asd.first), "", cmNinjaDeps(), - orderOnlyDeps); + /*restat*/ true, std::string(), cmNinjaDeps(1, asd.first), cmNinjaDeps(), + std::move(orderOnlyDeps)); } } diff --git a/Source/cmGlobalNinjaGenerator.h b/Source/cmGlobalNinjaGenerator.h index 9f31708..6fbdb82 100644 --- a/Source/cmGlobalNinjaGenerator.h +++ b/Source/cmGlobalNinjaGenerator.h @@ -110,13 +110,15 @@ public: void WriteBuild(std::ostream& os, cmNinjaBuild const& build, int cmdLineLimit = 0, bool* usedResponseFile = nullptr); - void WriteCustomCommandBuild( - const std::string& command, const std::string& description, - const std::string& comment, const std::string& depfile, - const std::string& pool, bool uses_terminal, bool restat, - const cmNinjaDeps& outputs, const std::string& config, - const cmNinjaDeps& explicitDeps = cmNinjaDeps(), - const cmNinjaDeps& orderOnlyDeps = cmNinjaDeps()); + void WriteCustomCommandBuild(std::string const& command, + std::string const& description, + std::string const& comment, + std::string const& depfile, + std::string const& pool, bool uses_terminal, + bool restat, std::string const& config, + cmNinjaDeps outputs, + cmNinjaDeps explicitDeps = cmNinjaDeps(), + cmNinjaDeps orderOnlyDeps = cmNinjaDeps()); void WriteMacOSXContentBuild(std::string input, std::string output, const std::string& config); diff --git a/Source/cmLocalNinjaGenerator.cxx b/Source/cmLocalNinjaGenerator.cxx index 82aa3c4..a2090cf 100644 --- a/Source/cmLocalNinjaGenerator.cxx +++ b/Source/cmLocalNinjaGenerator.cxx @@ -709,8 +709,9 @@ void cmLocalNinjaGenerator::WriteCustomCommandBuildStatement( customStep), this->ConstructComment(ccg), comment, depfile, cc->GetJobPool(), cc->GetUsesTerminal(), - /*restat*/ !symbolic || !byproducts.empty(), ninjaOutputs, fileConfig, - ninjaDeps, orderOnlyDeps); + /*restat*/ !symbolic || !byproducts.empty(), fileConfig, + std::move(ninjaOutputs), std::move(ninjaDeps), + std::move(orderOnlyDeps)); } } } diff --git a/Source/cmNinjaUtilityTargetGenerator.cxx b/Source/cmNinjaUtilityTargetGenerator.cxx index 7a04c47..6297252 100644 --- a/Source/cmNinjaUtilityTargetGenerator.cxx +++ b/Source/cmNinjaUtilityTargetGenerator.cxx @@ -180,7 +180,7 @@ void cmNinjaUtilityTargetGenerator::WriteUtilBuildStatements( gg->WriteCustomCommandBuild( command, desc, "Utility command for " + this->GetTargetName(), /*depfile*/ "", /*job_pool*/ "", uses_terminal, - /*restat*/ true, util_outputs, ccConfig, deps); + /*restat*/ true, ccConfig, std::move(util_outputs), std::move(deps)); } phonyBuild.ExplicitDeps.push_back(utilCommandName); -- cgit v0.12 From 68e5f92cada35068f71a8c46388aeb4eb1383bca Mon Sep 17 00:00:00 2001 From: Brad King Date: Tue, 18 May 2021 11:43:11 -0400 Subject: cmGlobalNinjaGenerator: Factor out custom command output collection De-duplicate code paths calling ConvertToNinjaPath and SeenCustomCommandOutput on custom command outputs and custom target byproducts. --- Source/cmGlobalNinjaGenerator.cxx | 18 +++++++++++++++--- Source/cmGlobalNinjaGenerator.h | 15 ++++++++++++++- Source/cmLocalNinjaGenerator.cxx | 20 ++++++-------------- Source/cmNinjaUtilityTargetGenerator.cxx | 15 +++++---------- 4 files changed, 40 insertions(+), 28 deletions(-) diff --git a/Source/cmGlobalNinjaGenerator.cxx b/Source/cmGlobalNinjaGenerator.cxx index 06f7e45..64aaf48 100644 --- a/Source/cmGlobalNinjaGenerator.cxx +++ b/Source/cmGlobalNinjaGenerator.cxx @@ -310,11 +310,21 @@ void cmGlobalNinjaGenerator::AddCustomCommandRule() this->AddRule(rule); } +void cmGlobalNinjaGenerator::CCOutputs::Add( + std::vector const& paths) +{ + for (std::string const& path : paths) { + std::string out = this->GG->ConvertToNinjaPath(path); + this->GG->SeenCustomCommandOutput(out); + this->ExplicitOuts.emplace_back(std::move(out)); + } +} + void cmGlobalNinjaGenerator::WriteCustomCommandBuild( std::string const& command, std::string const& description, std::string const& comment, std::string const& depfile, std::string const& job_pool, bool uses_terminal, bool restat, - std::string const& config, cmNinjaDeps outputs, cmNinjaDeps explicitDeps, + std::string const& config, CCOutputs outputs, cmNinjaDeps explicitDeps, cmNinjaDeps orderOnlyDeps) { this->AddCustomCommandRule(); @@ -330,7 +340,7 @@ void cmGlobalNinjaGenerator::WriteCustomCommandBuild( { cmNinjaBuild build("CUSTOM_COMMAND"); build.Comment = comment; - build.Outputs = std::move(outputs); + build.Outputs = std::move(outputs.ExplicitOuts); build.ExplicitDeps = std::move(explicitDeps); build.OrderOnlyDeps = std::move(orderOnlyDeps); @@ -1201,6 +1211,8 @@ void cmGlobalNinjaGenerator::WriteDisclaimer(std::ostream& os) const void cmGlobalNinjaGenerator::WriteAssumedSourceDependencies() { for (auto const& asd : this->AssumedSourceDependencies) { + CCOutputs outputs(this); + outputs.ExplicitOuts.emplace_back(asd.first); cmNinjaDeps orderOnlyDeps; std::copy(asd.second.begin(), asd.second.end(), std::back_inserter(orderOnlyDeps)); @@ -1209,7 +1221,7 @@ void cmGlobalNinjaGenerator::WriteAssumedSourceDependencies() "Assume dependencies for generated source file.", /*depfile*/ "", /*job_pool*/ "", /*uses_terminal*/ false, - /*restat*/ true, std::string(), cmNinjaDeps(1, asd.first), cmNinjaDeps(), + /*restat*/ true, std::string(), outputs, cmNinjaDeps(), std::move(orderOnlyDeps)); } } diff --git a/Source/cmGlobalNinjaGenerator.h b/Source/cmGlobalNinjaGenerator.h index 6fbdb82..2833367 100644 --- a/Source/cmGlobalNinjaGenerator.h +++ b/Source/cmGlobalNinjaGenerator.h @@ -110,13 +110,26 @@ public: void WriteBuild(std::ostream& os, cmNinjaBuild const& build, int cmdLineLimit = 0, bool* usedResponseFile = nullptr); + class CCOutputs + { + cmGlobalNinjaGenerator* GG; + + public: + CCOutputs(cmGlobalNinjaGenerator* gg) + : GG(gg) + { + } + void Add(std::vector const& outputs); + cmNinjaDeps ExplicitOuts; + }; + void WriteCustomCommandBuild(std::string const& command, std::string const& description, std::string const& comment, std::string const& depfile, std::string const& pool, bool uses_terminal, bool restat, std::string const& config, - cmNinjaDeps outputs, + CCOutputs outputs, cmNinjaDeps explicitDeps = cmNinjaDeps(), cmNinjaDeps orderOnlyDeps = cmNinjaDeps()); diff --git a/Source/cmLocalNinjaGenerator.cxx b/Source/cmLocalNinjaGenerator.cxx index a2090cf..9eb3e46 100644 --- a/Source/cmLocalNinjaGenerator.cxx +++ b/Source/cmLocalNinjaGenerator.cxx @@ -638,18 +638,11 @@ void cmLocalNinjaGenerator::WriteCustomCommandBuildStatement( } } - cmNinjaDeps ninjaOutputs(outputs.size() + byproducts.size()); - std::transform(outputs.begin(), outputs.end(), ninjaOutputs.begin(), - gg->MapToNinjaPath()); - std::transform(byproducts.begin(), byproducts.end(), - ninjaOutputs.begin() + outputs.size(), - gg->MapToNinjaPath()); - - for (std::string const& ninjaOutput : ninjaOutputs) { - gg->SeenCustomCommandOutput(ninjaOutput); - } + cmGlobalNinjaGenerator::CCOutputs ccOutputs(gg); + ccOutputs.Add(outputs); + ccOutputs.Add(byproducts); - std::string mainOutput = ninjaOutputs[0]; + std::string mainOutput = ccOutputs.ExplicitOuts[0]; cmNinjaDeps ninjaDeps; this->AppendCustomCommandDeps(ccg, ninjaDeps, fileConfig); @@ -660,7 +653,7 @@ void cmLocalNinjaGenerator::WriteCustomCommandBuildStatement( if (cmdLines.empty()) { cmNinjaBuild build("phony"); build.Comment = cmStrCat("Phony custom command for ", mainOutput); - build.Outputs = std::move(ninjaOutputs); + build.Outputs = std::move(ccOutputs.ExplicitOuts); build.ExplicitDeps = std::move(ninjaDeps); build.OrderOnlyDeps = orderOnlyDeps; gg->WriteBuild(this->GetImplFileStream(fileConfig), build); @@ -710,8 +703,7 @@ void cmLocalNinjaGenerator::WriteCustomCommandBuildStatement( this->ConstructComment(ccg), comment, depfile, cc->GetJobPool(), cc->GetUsesTerminal(), /*restat*/ !symbolic || !byproducts.empty(), fileConfig, - std::move(ninjaOutputs), std::move(ninjaDeps), - std::move(orderOnlyDeps)); + std::move(ccOutputs), std::move(ninjaDeps), std::move(orderOnlyDeps)); } } } diff --git a/Source/cmNinjaUtilityTargetGenerator.cxx b/Source/cmNinjaUtilityTargetGenerator.cxx index 6297252..1f5a7ff 100644 --- a/Source/cmNinjaUtilityTargetGenerator.cxx +++ b/Source/cmNinjaUtilityTargetGenerator.cxx @@ -73,7 +73,8 @@ void cmNinjaUtilityTargetGenerator::WriteUtilBuildStatements( cmNinjaBuild phonyBuild("phony"); std::vector commands; cmNinjaDeps deps; - cmNinjaDeps util_outputs(1, utilCommandName); + cmGlobalNinjaGenerator::CCOutputs util_outputs(gg); + util_outputs.ExplicitOuts.emplace_back(utilCommandName); bool uses_terminal = false; { @@ -86,10 +87,7 @@ void cmNinjaUtilityTargetGenerator::WriteUtilBuildStatements( cmCustomCommandGenerator ccg(ci, fileConfig, lg); lg->AppendCustomCommandDeps(ccg, deps, fileConfig); lg->AppendCustomCommandLines(ccg, commands); - std::vector const& ccByproducts = ccg.GetByproducts(); - std::transform(ccByproducts.begin(), ccByproducts.end(), - std::back_inserter(util_outputs), - this->MapToNinjaPath()); + util_outputs.Add(ccg.GetByproducts()); if (ci.GetUsesTerminal()) { uses_terminal = true; } @@ -124,7 +122,8 @@ void cmNinjaUtilityTargetGenerator::WriteUtilBuildStatements( if (genTarget->Target->GetType() != cmStateEnums::GLOBAL_TARGET) { lg->AppendTargetOutputs(genTarget, gg->GetByproductsForCleanTarget(), config); - std::copy(util_outputs.begin(), util_outputs.end(), + std::copy(util_outputs.ExplicitOuts.begin(), + util_outputs.ExplicitOuts.end(), std::back_inserter(gg->GetByproductsForCleanTarget())); } lg->AppendTargetDepends(genTarget, deps, config, fileConfig, @@ -166,10 +165,6 @@ void cmNinjaUtilityTargetGenerator::WriteUtilBuildStatements( return; } - for (std::string const& util_output : util_outputs) { - gg->SeenCustomCommandOutput(util_output); - } - std::string ccConfig; if (genTarget->Target->IsPerConfig() && genTarget->GetType() != cmStateEnums::GLOBAL_TARGET) { -- cgit v0.12 From ae927f936d6bfc6c5004fc6c47994f481b9d6796 Mon Sep 17 00:00:00 2001 From: Brad King Date: Wed, 19 May 2021 11:32:02 -0400 Subject: cmGlobalNinjaGenerator: Improve allocation pattern in WriteBuild --- Source/cmGlobalNinjaGenerator.cxx | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/Source/cmGlobalNinjaGenerator.cxx b/Source/cmGlobalNinjaGenerator.cxx index 64aaf48..a0baf3f 100644 --- a/Source/cmGlobalNinjaGenerator.cxx +++ b/Source/cmGlobalNinjaGenerator.cxx @@ -216,25 +216,24 @@ void cmGlobalNinjaGenerator::WriteBuild(std::ostream& os, { // Write explicit outputs for (std::string const& output : build.Outputs) { - buildStr += cmStrCat(' ', this->EncodePath(output)); + buildStr = cmStrCat(buildStr, ' ', this->EncodePath(output)); if (this->ComputingUnknownDependencies) { this->CombinedBuildOutputs.insert(output); } } // Write implicit outputs if (!build.ImplicitOuts.empty()) { - buildStr += " |"; + buildStr = cmStrCat(buildStr, " |"); for (std::string const& implicitOut : build.ImplicitOuts) { - buildStr += cmStrCat(' ', this->EncodePath(implicitOut)); + buildStr = cmStrCat(buildStr, ' ', this->EncodePath(implicitOut)); if (this->ComputingUnknownDependencies) { this->CombinedBuildOutputs.insert(implicitOut); } } } - buildStr += ':'; // Write the rule. - buildStr += cmStrCat(' ', build.Rule); + buildStr = cmStrCat(buildStr, ": ", build.Rule); } std::string arguments; -- cgit v0.12 From bc40cd7a4e107be68debb01760700850d4c1106c Mon Sep 17 00:00:00 2001 From: Brad King Date: Tue, 18 May 2021 13:24:43 -0400 Subject: Tests: Add case covering a unity build with a generated source Exclude the case on generators where it does not yet work. Issue: #21865 --- .../BuildDepends/CustomCommandUnityBuild.cmake | 19 ++++++++++++++++ .../CustomCommandUnityBuild.step1.cmake | 3 +++ .../CustomCommandUnityBuild.step2.cmake | 3 +++ Tests/RunCMake/BuildDepends/RunCMakeTest.cmake | 25 ++++++++++++++++++++++ 4 files changed, 50 insertions(+) create mode 100644 Tests/RunCMake/BuildDepends/CustomCommandUnityBuild.cmake create mode 100644 Tests/RunCMake/BuildDepends/CustomCommandUnityBuild.step1.cmake create mode 100644 Tests/RunCMake/BuildDepends/CustomCommandUnityBuild.step2.cmake diff --git a/Tests/RunCMake/BuildDepends/CustomCommandUnityBuild.cmake b/Tests/RunCMake/BuildDepends/CustomCommandUnityBuild.cmake new file mode 100644 index 0000000..22e7f27 --- /dev/null +++ b/Tests/RunCMake/BuildDepends/CustomCommandUnityBuild.cmake @@ -0,0 +1,19 @@ +enable_language(C) + +add_custom_command( + OUTPUT main.c + COMMAND ${CMAKE_COMMAND} -E copy main.c.in main.c + DEPENDS ${CMAKE_CURRENT_BINARY_DIR}/main.c.in + ) +add_executable(main main.c) +set_property(TARGET main PROPERTY UNITY_BUILD ON) + +file(GENERATE OUTPUT ${CMAKE_CURRENT_BINARY_DIR}/check-$>.cmake CONTENT " +set(check_pairs + \"$|${CMAKE_CURRENT_BINARY_DIR}/main.c.in\" + \"$|${CMAKE_CURRENT_BINARY_DIR}/main.c\" + ) +set(check_exes + \"$\" + ) +") diff --git a/Tests/RunCMake/BuildDepends/CustomCommandUnityBuild.step1.cmake b/Tests/RunCMake/BuildDepends/CustomCommandUnityBuild.step1.cmake new file mode 100644 index 0000000..87576eb --- /dev/null +++ b/Tests/RunCMake/BuildDepends/CustomCommandUnityBuild.step1.cmake @@ -0,0 +1,3 @@ +file(WRITE "${RunCMake_TEST_BINARY_DIR}/main.c.in" [[ +int main(void) { return 1; } +]]) diff --git a/Tests/RunCMake/BuildDepends/CustomCommandUnityBuild.step2.cmake b/Tests/RunCMake/BuildDepends/CustomCommandUnityBuild.step2.cmake new file mode 100644 index 0000000..69b21b8 --- /dev/null +++ b/Tests/RunCMake/BuildDepends/CustomCommandUnityBuild.step2.cmake @@ -0,0 +1,3 @@ +file(WRITE "${RunCMake_TEST_BINARY_DIR}/main.c.in" [[ +int main(void) { return 2; } +]]) diff --git a/Tests/RunCMake/BuildDepends/RunCMakeTest.cmake b/Tests/RunCMake/BuildDepends/RunCMakeTest.cmake index 72faddb..947a2fe 100644 --- a/Tests/RunCMake/BuildDepends/RunCMakeTest.cmake +++ b/Tests/RunCMake/BuildDepends/RunCMakeTest.cmake @@ -1,5 +1,22 @@ include(RunCMake) +if(RunCMake_GENERATOR MATCHES "Ninja") + # Detect ninja version so we know what tests can be supported. + execute_process( + COMMAND "${RunCMake_MAKE_PROGRAM}" --version + OUTPUT_VARIABLE ninja_out + ERROR_VARIABLE ninja_out + RESULT_VARIABLE ninja_res + OUTPUT_STRIP_TRAILING_WHITESPACE + ) + if(ninja_res EQUAL 0 AND "x${ninja_out}" MATCHES "^x[0-9]+\\.[0-9]+") + set(ninja_version "${ninja_out}") + message(STATUS "ninja version: ${ninja_version}") + else() + message(FATAL_ERROR "'ninja --version' reported:\n${ninja_out}") + endif() +endif() + if(RunCMake_GENERATOR STREQUAL "Borland Makefiles" OR RunCMake_GENERATOR STREQUAL "Watcom WMake") set(fs_delay 3) @@ -164,3 +181,11 @@ endif() if(RunCMake_GENERATOR MATCHES "Make") run_BuildDepends(MakeDependencies) endif() + +if(RunCMake_GENERATOR MATCHES "^Visual Studio 9 " OR + RunCMake_GENERATOR MATCHES "Ninja") + # This build tool misses the dependency. + set(run_BuildDepends_skip_step_2 1) +endif() +run_BuildDepends(CustomCommandUnityBuild) +unset(run_BuildDepends_skip_step_2) -- cgit v0.12 From 2725ecff38ccd25905a9bc968bfb1b2f0d09b34a Mon Sep 17 00:00:00 2001 From: Brad King Date: Wed, 19 May 2021 10:21:11 -0400 Subject: Ninja: Handle depfiles with absolute paths to generated files Ninja treats every (normalized) path as its own node. It does not recognize `/abs/path/to/file` in a depfile as matching `path/to/file` even when `build.ninja` and the working directory are in `/abs/`. See Ninja Issue 1251. In cases where we pass absolute paths to the compiler, it will write a depfile containing absolute paths. If those files are generated in the build tree by custom commands, `build.ninja` references them by relative path in build statement outputs, so Ninja does not hook up the dependency and rebuild the project correctly. Add infrastructure to work around this problem by adding implicit outputs to custom command build statements that reference the main outputs by absolute path. Use a `${cmake_ninja_workdir}` placeholder to avoid repeating the base path. For example: build out.txt | ${cmake_ninja_workdir}out.txt: CUSTOM_COMMAND ... Ninja will create two nodes for the output file, one with a relative path and one with an absolute path. A depfile may then mention either form of the path and Ninja will hook up the dependency. Unfortunately Ninja will also stat the file twice. Issue: #13894 Fixes: #21865 --- Source/cmGlobalNinjaGenerator.cxx | 17 ++++++++++++++++- Source/cmGlobalNinjaGenerator.h | 1 + Source/cmLocalNinjaGenerator.cxx | 13 +++++++++++++ Source/cmLocalNinjaGenerator.h | 1 + Source/cmNinjaTypes.h | 1 + Tests/RunCMake/BuildDepends/RunCMakeTest.cmake | 2 +- 6 files changed, 33 insertions(+), 2 deletions(-) diff --git a/Source/cmGlobalNinjaGenerator.cxx b/Source/cmGlobalNinjaGenerator.cxx index a0baf3f..6034434 100644 --- a/Source/cmGlobalNinjaGenerator.cxx +++ b/Source/cmGlobalNinjaGenerator.cxx @@ -222,7 +222,7 @@ void cmGlobalNinjaGenerator::WriteBuild(std::ostream& os, } } // Write implicit outputs - if (!build.ImplicitOuts.empty()) { + if (!build.ImplicitOuts.empty() || !build.WorkDirOuts.empty()) { buildStr = cmStrCat(buildStr, " |"); for (std::string const& implicitOut : build.ImplicitOuts) { buildStr = cmStrCat(buildStr, ' ', this->EncodePath(implicitOut)); @@ -230,6 +230,14 @@ void cmGlobalNinjaGenerator::WriteBuild(std::ostream& os, this->CombinedBuildOutputs.insert(implicitOut); } } + for (std::string const& workdirOut : build.WorkDirOuts) { + // Repeat some outputs, but expressed as absolute paths. + // This helps Ninja handle absolute paths found in a depfile. + // FIXME: Unfortunately this causes Ninja to stat the file twice. + // We could avoid this if Ninja Issue 1251 were fixed. + buildStr = cmStrCat(buildStr, " ${cmake_ninja_workdir}", + this->EncodePath(workdirOut)); + } } // Write the rule. @@ -314,6 +322,12 @@ void cmGlobalNinjaGenerator::CCOutputs::Add( { for (std::string const& path : paths) { std::string out = this->GG->ConvertToNinjaPath(path); + if (this->GG->SupportsImplicitOuts() && + !cmSystemTools::FileIsFullPath(out)) { + // This output is expressed as a relative path. Repeat it, + // but expressed as an absolute path for Ninja Issue 1251. + this->WorkDirOuts.emplace_back(out); + } this->GG->SeenCustomCommandOutput(out); this->ExplicitOuts.emplace_back(std::move(out)); } @@ -340,6 +354,7 @@ void cmGlobalNinjaGenerator::WriteCustomCommandBuild( cmNinjaBuild build("CUSTOM_COMMAND"); build.Comment = comment; build.Outputs = std::move(outputs.ExplicitOuts); + build.WorkDirOuts = std::move(outputs.WorkDirOuts); build.ExplicitDeps = std::move(explicitDeps); build.OrderOnlyDeps = std::move(orderOnlyDeps); diff --git a/Source/cmGlobalNinjaGenerator.h b/Source/cmGlobalNinjaGenerator.h index 2833367..7a3674e 100644 --- a/Source/cmGlobalNinjaGenerator.h +++ b/Source/cmGlobalNinjaGenerator.h @@ -121,6 +121,7 @@ public: } void Add(std::vector const& outputs); cmNinjaDeps ExplicitOuts; + cmNinjaDeps WorkDirOuts; }; void WriteCustomCommandBuild(std::string const& command, diff --git a/Source/cmLocalNinjaGenerator.cxx b/Source/cmLocalNinjaGenerator.cxx index 9eb3e46..fb6c730 100644 --- a/Source/cmLocalNinjaGenerator.cxx +++ b/Source/cmLocalNinjaGenerator.cxx @@ -263,6 +263,7 @@ void cmLocalNinjaGenerator::WriteBuildFileTop() this->GetConfigNames().front()); } this->WriteNinjaFilesInclusionCommon(this->GetCommonFileStream()); + this->WriteNinjaWorkDir(this->GetCommonFileStream()); // For the rule file. this->WriteProjectHeader(this->GetRulesFileStream()); @@ -364,6 +365,17 @@ void cmLocalNinjaGenerator::WriteNinjaFilesInclusionCommon(std::ostream& os) os << "\n"; } +void cmLocalNinjaGenerator::WriteNinjaWorkDir(std::ostream& os) +{ + cmGlobalNinjaGenerator::WriteDivider(os); + cmGlobalNinjaGenerator::WriteComment( + os, "Logical path to working directory; prefix for absolute paths."); + cmGlobalNinjaGenerator* ng = this->GetGlobalNinjaGenerator(); + std::string ninja_workdir = this->GetBinaryDirectory(); + ng->StripNinjaOutputPathPrefixAsSuffix(ninja_workdir); // Also appends '/'. + os << "cmake_ninja_workdir = " << ng->EncodePath(ninja_workdir) << "\n"; +} + void cmLocalNinjaGenerator::WriteProcessedMakefile(std::ostream& os) { cmGlobalNinjaGenerator::WriteDivider(os); @@ -654,6 +666,7 @@ void cmLocalNinjaGenerator::WriteCustomCommandBuildStatement( cmNinjaBuild build("phony"); build.Comment = cmStrCat("Phony custom command for ", mainOutput); build.Outputs = std::move(ccOutputs.ExplicitOuts); + build.WorkDirOuts = std::move(ccOutputs.WorkDirOuts); build.ExplicitDeps = std::move(ninjaDeps); build.OrderOnlyDeps = orderOnlyDeps; gg->WriteBuild(this->GetImplFileStream(fileConfig), build); diff --git a/Source/cmLocalNinjaGenerator.h b/Source/cmLocalNinjaGenerator.h index a73fa27..6404037 100644 --- a/Source/cmLocalNinjaGenerator.h +++ b/Source/cmLocalNinjaGenerator.h @@ -108,6 +108,7 @@ private: const std::string& config); void WriteNinjaFilesInclusionConfig(std::ostream& os); void WriteNinjaFilesInclusionCommon(std::ostream& os); + void WriteNinjaWorkDir(std::ostream& os); void WriteProcessedMakefile(std::ostream& os); void WritePools(std::ostream& os); diff --git a/Source/cmNinjaTypes.h b/Source/cmNinjaTypes.h index 320f41b..c8a411e 100644 --- a/Source/cmNinjaTypes.h +++ b/Source/cmNinjaTypes.h @@ -53,6 +53,7 @@ public: std::string Rule; cmNinjaDeps Outputs; cmNinjaDeps ImplicitOuts; + cmNinjaDeps WorkDirOuts; // For cmake_ninja_workdir. cmNinjaDeps ExplicitDeps; cmNinjaDeps ImplicitDeps; cmNinjaDeps OrderOnlyDeps; diff --git a/Tests/RunCMake/BuildDepends/RunCMakeTest.cmake b/Tests/RunCMake/BuildDepends/RunCMakeTest.cmake index 947a2fe..0a80580 100644 --- a/Tests/RunCMake/BuildDepends/RunCMakeTest.cmake +++ b/Tests/RunCMake/BuildDepends/RunCMakeTest.cmake @@ -183,7 +183,7 @@ if(RunCMake_GENERATOR MATCHES "Make") endif() if(RunCMake_GENERATOR MATCHES "^Visual Studio 9 " OR - RunCMake_GENERATOR MATCHES "Ninja") + (RunCMake_GENERATOR MATCHES "Ninja" AND ninja_version VERSION_LESS 1.7)) # This build tool misses the dependency. set(run_BuildDepends_skip_step_2 1) endif() -- cgit v0.12