From 0826c20128fad20aafac2006ac0371e381e3cf60 Mon Sep 17 00:00:00 2001 From: Brad King Date: Wed, 25 Apr 2018 10:53:15 -0400 Subject: Ninja: Do not add empty custom command for file(GENERATE) outputs Internally we mark `file(GENERATE)` outputs as `GENERATED` in order to tell custom command dependency tracing logic not to expect the files to exist on disk yet. This is because we do not generate the files until after that tracing is done. The Ninja generator also interprets the `GENERATED` property to mean that it is expected that some build rule will generate the file if another build rule depends on it. If the generator does not know of a custom command that generates the file then it adds an empty one so that the `ninja` build tool does not complain about a dependency on a file that does not exist and has no rule to generate it. However, this step is not necessary for `file(GENERATE)` outputs because there is no build rule to generate them and they will exist before `ninja` runs. Add an additional `__CMAKE_GENERATED_BY_CMAKE` property internally to tell the Ninja generator that a `GENERATED` file will exist before the build starts and is not expected to have a build rule producing it. Fixes: #17942 --- Source/cmGeneratorExpressionEvaluationFile.cxx | 6 ++++++ Source/cmNinjaTargetGenerator.cxx | 4 +++- 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/Source/cmGeneratorExpressionEvaluationFile.cxx b/Source/cmGeneratorExpressionEvaluationFile.cxx index c544141..99b9261 100644 --- a/Source/cmGeneratorExpressionEvaluationFile.cxx +++ b/Source/cmGeneratorExpressionEvaluationFile.cxx @@ -105,8 +105,14 @@ void cmGeneratorExpressionEvaluationFile::CreateOutputFile( lg, config, false, nullptr, nullptr, nullptr, le); cmSourceFile* sf = lg->GetMakefile()->GetOrCreateSource( name, false, cmSourceFileLocationKind::Known); + // Tell TraceDependencies that the file is not expected to exist + // on disk yet. We generate it after that runs. sf->SetProperty("GENERATED", "1"); + // Tell the build system generators that there is no build rule + // to generate the file. + sf->SetProperty("__CMAKE_GENERATED_BY_CMAKE", "1"); + gg->SetFilenameTargetDepends( sf, this->OutputFileExpr->GetSourceSensitiveTargets()); } diff --git a/Source/cmNinjaTargetGenerator.cxx b/Source/cmNinjaTargetGenerator.cxx index f4faf47..a6a3efb 100644 --- a/Source/cmNinjaTargetGenerator.cxx +++ b/Source/cmNinjaTargetGenerator.cxx @@ -896,7 +896,9 @@ void cmNinjaTargetGenerator::WriteObjectBuildStatement( // (either attached to this source file or another one), assume that one of // the target dependencies, OBJECT_DEPENDS or header file custom commands // will rebuild the file. - if (source->GetPropertyAsBool("GENERATED") && !source->GetCustomCommand() && + if (source->GetPropertyAsBool("GENERATED") && + !source->GetPropertyAsBool("__CMAKE_GENERATED_BY_CMAKE") && + !source->GetCustomCommand() && !this->GetGlobalGenerator()->HasCustomCommandOutput(sourceFileName)) { this->GetGlobalGenerator()->AddAssumedSourceDependencies(sourceFileName, orderOnlyDeps); -- cgit v0.12 From 625b8f9076080b3831ef7821812461b694ef5586 Mon Sep 17 00:00:00 2001 From: Brad King Date: Wed, 25 Apr 2018 11:33:53 -0400 Subject: Ninja: Avoid empty phony edges for target ordering Since commit v3.9.0-rc1~230^2~2 (ninja: break unnecessary target dependencies, 2017-04-17) we unconditionally generate a phony edge for target ordering. It is needed in case a later target depends on it. However, if the phony edge has no inputs then `ninja -d explain` prints: ninja explain: output ... of phony edge with no inputs doesn't exist Furthermore the phony edge's output is considered dirty and can cause dependents to be incorrectly considered dirty. Avoid this by always generating at least one input to the target ordering phony edges. If we have no real dependencies just use a path that always exists. Fixes: #17942 --- Source/cmNinjaTargetGenerator.cxx | 13 +++++++++++++ Tests/RunCMake/Ninja/NoWorkToDo-nowork-stdout.txt | 1 + Tests/RunCMake/Ninja/NoWorkToDo.cmake | 2 ++ Tests/RunCMake/Ninja/RunCMakeTest.cmake | 9 +++++++++ 4 files changed, 25 insertions(+) create mode 100644 Tests/RunCMake/Ninja/NoWorkToDo-nowork-stdout.txt create mode 100644 Tests/RunCMake/Ninja/NoWorkToDo.cmake diff --git a/Source/cmNinjaTargetGenerator.cxx b/Source/cmNinjaTargetGenerator.cxx index 4ce3cef..7dfce57 100644 --- a/Source/cmNinjaTargetGenerator.cxx +++ b/Source/cmNinjaTargetGenerator.cxx @@ -821,6 +821,19 @@ void cmNinjaTargetGenerator::WriteObjectBuildStatements() orderOnlyDeps.erase(std::unique(orderOnlyDeps.begin(), orderOnlyDeps.end()), orderOnlyDeps.end()); + // The phony target must depend on at least one input or ninja will explain + // that "output ... of phony edge with no inputs doesn't exist" and consider + // the phony output "dirty". + if (orderOnlyDeps.empty()) { + // Any path that always exists will work here. It would be nice to + // use just "." but that is not supported by Ninja < 1.7. + std::string tgtDir; + tgtDir += this->LocalGenerator->GetCurrentBinaryDirectory(); + tgtDir += "/"; + tgtDir += this->LocalGenerator->GetTargetDirectory(this->GeneratorTarget); + orderOnlyDeps.push_back(this->ConvertToNinjaPath(tgtDir)); + } + { cmNinjaDeps orderOnlyTarget; orderOnlyTarget.push_back(this->OrderDependsTargetForTarget()); diff --git a/Tests/RunCMake/Ninja/NoWorkToDo-nowork-stdout.txt b/Tests/RunCMake/Ninja/NoWorkToDo-nowork-stdout.txt new file mode 100644 index 0000000..60a9228 --- /dev/null +++ b/Tests/RunCMake/Ninja/NoWorkToDo-nowork-stdout.txt @@ -0,0 +1 @@ +^ninja: no work to do diff --git a/Tests/RunCMake/Ninja/NoWorkToDo.cmake b/Tests/RunCMake/Ninja/NoWorkToDo.cmake new file mode 100644 index 0000000..a2fa0cb --- /dev/null +++ b/Tests/RunCMake/Ninja/NoWorkToDo.cmake @@ -0,0 +1,2 @@ +enable_language(C) +add_executable(hello hello.c) diff --git a/Tests/RunCMake/Ninja/RunCMakeTest.cmake b/Tests/RunCMake/Ninja/RunCMakeTest.cmake index 3bb2b6b..b6e6cd4 100644 --- a/Tests/RunCMake/Ninja/RunCMakeTest.cmake +++ b/Tests/RunCMake/Ninja/RunCMakeTest.cmake @@ -21,6 +21,15 @@ function(run_NinjaToolMissing) endfunction() run_NinjaToolMissing() +function(run_NoWorkToDo) + run_cmake(NoWorkToDo) + set(RunCMake_TEST_NO_CLEAN 1) + set(RunCMake_TEST_BINARY_DIR ${RunCMake_BINARY_DIR}/NoWorkToDo-build) + run_cmake_command(NoWorkToDo-build ${CMAKE_COMMAND} --build .) + run_cmake_command(NoWorkToDo-nowork ${CMAKE_COMMAND} --build . -- -d explain) +endfunction() +run_NoWorkToDo() + function(run_CMP0058 case) # Use a single build tree for a few tests without cleaning. set(RunCMake_TEST_BINARY_DIR ${RunCMake_BINARY_DIR}/CMP0058-${case}-build) -- cgit v0.12 From ee44f390ceaadee1517043dfb5f21fce147e740e Mon Sep 17 00:00:00 2001 From: Brad King Date: Wed, 25 Apr 2018 12:57:03 -0400 Subject: Ninja: Make assumed source dependencies order-only Since its beginning the Ninja generator has handled `GENERATED` source files that have no custom command producing them by writing a dummy custom command for them that depends on the target ordering phony edge. Make the custom command's dependency order-only since the phony edge also has only order-only dependencies. The dummy custom command should never be considered "dirty" by `ninja`. Fixes: #17942 --- Source/cmGlobalNinjaGenerator.cxx | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/Source/cmGlobalNinjaGenerator.cxx b/Source/cmGlobalNinjaGenerator.cxx index 2a8576f..c19a61c 100644 --- a/Source/cmGlobalNinjaGenerator.cxx +++ b/Source/cmGlobalNinjaGenerator.cxx @@ -931,12 +931,14 @@ void cmGlobalNinjaGenerator::AddDependencyToAll(const std::string& input) void cmGlobalNinjaGenerator::WriteAssumedSourceDependencies() { for (auto const& asd : this->AssumedSourceDependencies) { - cmNinjaDeps deps; - std::copy(asd.second.begin(), asd.second.end(), std::back_inserter(deps)); + cmNinjaDeps orderOnlyDeps; + std::copy(asd.second.begin(), asd.second.end(), + std::back_inserter(orderOnlyDeps)); WriteCustomCommandBuild(/*command=*/"", /*description=*/"", "Assume dependencies for generated source file.", /*depfile*/ "", /*uses_terminal*/ false, - /*restat*/ true, cmNinjaDeps(1, asd.first), deps); + /*restat*/ true, cmNinjaDeps(1, asd.first), + cmNinjaDeps(), orderOnlyDeps); } } -- cgit v0.12