From c08543d71196783c754b8b868584dd46ddbeb15c Mon Sep 17 00:00:00 2001 From: Brad King Date: Tue, 10 Dec 2024 10:17:55 -0500 Subject: [PATCH 1/6] cmGlobalNinjaGenerator: Remove unused local variable --- Source/cmGlobalNinjaGenerator.cxx | 3 --- 1 file changed, 3 deletions(-) diff --git a/Source/cmGlobalNinjaGenerator.cxx b/Source/cmGlobalNinjaGenerator.cxx index 17e9c440a8..c33da9cbb5 100644 --- a/Source/cmGlobalNinjaGenerator.cxx +++ b/Source/cmGlobalNinjaGenerator.cxx @@ -1635,7 +1635,6 @@ void cmGlobalNinjaGenerator::WriteFolderTargets(std::ostream& os) cmMakefile::IncludeEmptyConfig); // Setup target - cmNinjaDeps configDeps; build.Comment = cmStrCat("Folder: ", currentBinaryDir); build.Outputs.emplace_back(); std::string const buildDirAllTarget = @@ -1716,7 +1715,6 @@ void cmGlobalNinjaGenerator::WriteFolderTargets(std::ostream& os) static_cast(dt.LG)->GetConfigNames(); // Setup target - cmNinjaDeps configDeps; build.Comment = cmStrCat("Folder: ", currentBinaryDir); build.Outputs.emplace_back(); std::string const buildDirAllTarget = @@ -1724,7 +1722,6 @@ void cmGlobalNinjaGenerator::WriteFolderTargets(std::ostream& os) for (auto const& config : configs) { build.ExplicitDeps.clear(); build.Outputs.front() = this->BuildAlias(buildDirAllTarget, config); - configDeps.emplace_back(build.Outputs.front()); for (DirectoryTarget::Target const& t : dt.Targets) { if (!this->IsExcludedFromAllInConfig(t, config)) { this->AppendTargetOutputs(t.GT, build.ExplicitDeps, config, From e308d1bb880e9761ec2e2c623869bc221d6159ee Mon Sep 17 00:00:00 2001 From: Brad King Date: Tue, 10 Dec 2024 10:28:39 -0500 Subject: [PATCH 2/6] cmGlobalNinjaGenerator: Remove unnecessary local variable --- Source/cmGlobalNinjaGenerator.cxx | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/Source/cmGlobalNinjaGenerator.cxx b/Source/cmGlobalNinjaGenerator.cxx index c33da9cbb5..b89ea5ca4a 100644 --- a/Source/cmGlobalNinjaGenerator.cxx +++ b/Source/cmGlobalNinjaGenerator.cxx @@ -1639,12 +1639,8 @@ void cmGlobalNinjaGenerator::WriteFolderTargets(std::ostream& os) build.Outputs.emplace_back(); std::string const buildDirAllTarget = this->ConvertToNinjaPath(cmStrCat(currentBinaryDir, "/codegen")); - - cmNinjaDeps& explicitDeps = build.ExplicitDeps; - for (auto const& config : configs) { - explicitDeps.clear(); - + build.ExplicitDeps.clear(); for (DirectoryTarget::Target const& t : dt.Targets) { if (this->IsExcludedFromAllInConfig(t, config)) { continue; @@ -1658,7 +1654,7 @@ void cmGlobalNinjaGenerator::WriteFolderTargets(std::ostream& os) auto const& outputs = cc->GetOutputs(); std::transform(outputs.begin(), outputs.end(), - std::back_inserter(explicitDeps), + std::back_inserter(build.ExplicitDeps), this->MapToNinjaPath()); } } From 5f33736c030d5b48e8df8ccf7c61c3bf233a22a9 Mon Sep 17 00:00:00 2001 From: Brad King Date: Tue, 10 Dec 2024 10:25:21 -0500 Subject: [PATCH 3/6] cmGlobalNinjaGenerator: Fix local variable name for codegen target --- Source/cmGlobalNinjaGenerator.cxx | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/Source/cmGlobalNinjaGenerator.cxx b/Source/cmGlobalNinjaGenerator.cxx index b89ea5ca4a..9542237d23 100644 --- a/Source/cmGlobalNinjaGenerator.cxx +++ b/Source/cmGlobalNinjaGenerator.cxx @@ -1637,7 +1637,7 @@ void cmGlobalNinjaGenerator::WriteFolderTargets(std::ostream& os) // Setup target build.Comment = cmStrCat("Folder: ", currentBinaryDir); build.Outputs.emplace_back(); - std::string const buildDirAllTarget = + std::string const buildDirCodegenTarget = this->ConvertToNinjaPath(cmStrCat(currentBinaryDir, "/codegen")); for (auto const& config : configs) { build.ExplicitDeps.clear(); @@ -1660,7 +1660,8 @@ void cmGlobalNinjaGenerator::WriteFolderTargets(std::ostream& os) } } - build.Outputs.front() = this->BuildAlias(buildDirAllTarget, config); + build.Outputs.front() = + this->BuildAlias(buildDirCodegenTarget, config); // Write target this->WriteBuild(this->EnableCrossConfigBuild() && this->CrossConfigs.count(config) @@ -1672,8 +1673,9 @@ void cmGlobalNinjaGenerator::WriteFolderTargets(std::ostream& os) // Add shortcut target if (this->IsMultiConfig()) { for (auto const& config : configs) { - build.ExplicitDeps = { this->BuildAlias(buildDirAllTarget, config) }; - build.Outputs.front() = buildDirAllTarget; + build.ExplicitDeps = { this->BuildAlias(buildDirCodegenTarget, + config) }; + build.Outputs.front() = buildDirCodegenTarget; this->WriteBuild(*this->GetConfigFileStream(config), build); } @@ -1681,9 +1683,9 @@ void cmGlobalNinjaGenerator::WriteFolderTargets(std::ostream& os) build.ExplicitDeps.clear(); for (auto const& config : this->DefaultConfigs) { build.ExplicitDeps.push_back( - this->BuildAlias(buildDirAllTarget, config)); + this->BuildAlias(buildDirCodegenTarget, config)); } - build.Outputs.front() = buildDirAllTarget; + build.Outputs.front() = buildDirCodegenTarget; this->WriteBuild(*this->GetDefaultFileStream(), build); } } @@ -1693,9 +1695,10 @@ void cmGlobalNinjaGenerator::WriteFolderTargets(std::ostream& os) build.ExplicitDeps.clear(); for (auto const& config : this->CrossConfigs) { build.ExplicitDeps.push_back( - this->BuildAlias(buildDirAllTarget, config)); + this->BuildAlias(buildDirCodegenTarget, config)); } - build.Outputs.front() = this->BuildAlias(buildDirAllTarget, "codegen"); + build.Outputs.front() = + this->BuildAlias(buildDirCodegenTarget, "codegen"); this->WriteBuild(os, build); } } From 505ffdcbde4bf19b61e82dbb6a4e17d8ba831716 Mon Sep 17 00:00:00 2001 From: Brad King Date: Tue, 10 Dec 2024 10:27:06 -0500 Subject: [PATCH 4/6] cmGlobalNinjaGenerator: Clarify order of codegen build statement logic Make it more consistent with the equivalent logic for "all". --- Source/cmGlobalNinjaGenerator.cxx | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Source/cmGlobalNinjaGenerator.cxx b/Source/cmGlobalNinjaGenerator.cxx index 9542237d23..ee02237b0b 100644 --- a/Source/cmGlobalNinjaGenerator.cxx +++ b/Source/cmGlobalNinjaGenerator.cxx @@ -1641,11 +1641,13 @@ void cmGlobalNinjaGenerator::WriteFolderTargets(std::ostream& os) this->ConvertToNinjaPath(cmStrCat(currentBinaryDir, "/codegen")); for (auto const& config : configs) { build.ExplicitDeps.clear(); + build.Outputs.front() = + this->BuildAlias(buildDirCodegenTarget, config); + for (DirectoryTarget::Target const& t : dt.Targets) { if (this->IsExcludedFromAllInConfig(t, config)) { continue; } - std::vector customCommandSources; t.GT->GetCustomCommands(customCommandSources, config); for (cmSourceFile const* sf : customCommandSources) { @@ -1660,8 +1662,6 @@ void cmGlobalNinjaGenerator::WriteFolderTargets(std::ostream& os) } } - build.Outputs.front() = - this->BuildAlias(buildDirCodegenTarget, config); // Write target this->WriteBuild(this->EnableCrossConfigBuild() && this->CrossConfigs.count(config) From 5d0f2aba7e231efefa28f54f9c1a5e36b4f374ea Mon Sep 17 00:00:00 2001 From: Brad King Date: Tue, 10 Dec 2024 10:34:06 -0500 Subject: [PATCH 5/6] cmGlobalNinjaGenerator: Simplify per-directory configuration list lookup --- Source/cmGlobalNinjaGenerator.cxx | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/Source/cmGlobalNinjaGenerator.cxx b/Source/cmGlobalNinjaGenerator.cxx index ee02237b0b..04c8c023b3 100644 --- a/Source/cmGlobalNinjaGenerator.cxx +++ b/Source/cmGlobalNinjaGenerator.cxx @@ -1631,8 +1631,7 @@ void cmGlobalNinjaGenerator::WriteFolderTargets(std::ostream& os) std::string const& currentBinaryDir = it.first; DirectoryTarget const& dt = it.second; std::vector configs = - dt.LG->GetMakefile()->GetGeneratorConfigs( - cmMakefile::IncludeEmptyConfig); + static_cast(dt.LG)->GetConfigNames(); // Setup target build.Comment = cmStrCat("Folder: ", currentBinaryDir); From 5ce1ca607f8c99a81b7155940ff3ce7afa13140b Mon Sep 17 00:00:00 2001 From: Brad King Date: Tue, 10 Dec 2024 10:09:14 -0500 Subject: [PATCH 6/6] Ninja: Add missing top-level codegen dependencies on per-directory codegen In commit 197cb419d1 (add_custom_command: Add CODEGEN support, 2024-05-27, v3.31.0-rc1~394^2) we accidentally left out the global `codegen` target's dependencies on the per-directory `codegen` targets. Add them for parity with the `all` target. Fixes: #26517 --- Source/cmGlobalNinjaGenerator.cxx | 7 +++++++ Tests/RunCMake/Codegen/RunCMakeTest.cmake | 4 ++++ Tests/RunCMake/Codegen/SubDir-build-check.cmake | 9 +++++++++ Tests/RunCMake/Codegen/SubDir-common.cmake | 15 +++++++++++++++ Tests/RunCMake/Codegen/SubDir.cmake | 2 ++ Tests/RunCMake/Codegen/SubDir/CMakeLists.txt | 15 +++++++++++++++ .../SubDirExcludeFromAll-build-check.cmake | 9 +++++++++ Tests/RunCMake/Codegen/SubDirExcludeFromAll.cmake | 2 ++ 8 files changed, 63 insertions(+) create mode 100644 Tests/RunCMake/Codegen/SubDir-build-check.cmake create mode 100644 Tests/RunCMake/Codegen/SubDir-common.cmake create mode 100644 Tests/RunCMake/Codegen/SubDir.cmake create mode 100644 Tests/RunCMake/Codegen/SubDir/CMakeLists.txt create mode 100644 Tests/RunCMake/Codegen/SubDirExcludeFromAll-build-check.cmake create mode 100644 Tests/RunCMake/Codegen/SubDirExcludeFromAll.cmake diff --git a/Source/cmGlobalNinjaGenerator.cxx b/Source/cmGlobalNinjaGenerator.cxx index 04c8c023b3..2933a4f189 100644 --- a/Source/cmGlobalNinjaGenerator.cxx +++ b/Source/cmGlobalNinjaGenerator.cxx @@ -1661,6 +1661,13 @@ void cmGlobalNinjaGenerator::WriteFolderTargets(std::ostream& os) } } + for (DirectoryTarget::Dir const& d : dt.Children) { + if (!d.ExcludeFromAll) { + build.ExplicitDeps.emplace_back(this->BuildAlias( + this->ConvertToNinjaPath(cmStrCat(d.Path, "/codegen")), config)); + } + } + // Write target this->WriteBuild(this->EnableCrossConfigBuild() && this->CrossConfigs.count(config) diff --git a/Tests/RunCMake/Codegen/RunCMakeTest.cmake b/Tests/RunCMake/Codegen/RunCMakeTest.cmake index bbd70b0748..7b072bac31 100644 --- a/Tests/RunCMake/Codegen/RunCMakeTest.cmake +++ b/Tests/RunCMake/Codegen/RunCMakeTest.cmake @@ -31,3 +31,7 @@ run_cmake("implicit-depends") run_cmake("implicit-depends-append-codegen") run_cmake("append-implicit-depends") run_cmake("no-output") + +# Top-level codegen depends on that of subdirectories. +run_codegen(SubDir) +run_codegen(SubDirExcludeFromAll) diff --git a/Tests/RunCMake/Codegen/SubDir-build-check.cmake b/Tests/RunCMake/Codegen/SubDir-build-check.cmake new file mode 100644 index 0000000000..d81aa40a58 --- /dev/null +++ b/Tests/RunCMake/Codegen/SubDir-build-check.cmake @@ -0,0 +1,9 @@ +set(filename "${RunCMake_TEST_BINARY_DIR}/generated.h") +if(NOT EXISTS "${filename}") + string(APPEND RunCMake_TEST_FAILED "expected file NOT created:\n ${filename}\n") +endif() + +set(filename "${RunCMake_TEST_BINARY_DIR}/SubDir/generated.h") +if(NOT EXISTS "${filename}") + string(APPEND RunCMake_TEST_FAILED "expected file NOT created:\n ${filename}\n") +endif() diff --git a/Tests/RunCMake/Codegen/SubDir-common.cmake b/Tests/RunCMake/Codegen/SubDir-common.cmake new file mode 100644 index 0000000000..b4013d9cab --- /dev/null +++ b/Tests/RunCMake/Codegen/SubDir-common.cmake @@ -0,0 +1,15 @@ +add_custom_command( + OUTPUT + ${CMAKE_CURRENT_BINARY_DIR}/generated.h + COMMAND + ${CMAKE_COMMAND} -E + copy ${CMAKE_CURRENT_SOURCE_DIR}/generated.h.in + ${CMAKE_CURRENT_BINARY_DIR}/generated.h + CODEGEN +) + +add_library(errorlib_top + # If this library is built error.c will cause the build to fail + error.c + ${CMAKE_CURRENT_BINARY_DIR}/generated.h +) diff --git a/Tests/RunCMake/Codegen/SubDir.cmake b/Tests/RunCMake/Codegen/SubDir.cmake new file mode 100644 index 0000000000..5e5ff7b8b7 --- /dev/null +++ b/Tests/RunCMake/Codegen/SubDir.cmake @@ -0,0 +1,2 @@ +add_subdirectory(SubDir) +include(SubDir-common.cmake) diff --git a/Tests/RunCMake/Codegen/SubDir/CMakeLists.txt b/Tests/RunCMake/Codegen/SubDir/CMakeLists.txt new file mode 100644 index 0000000000..58f2f6e898 --- /dev/null +++ b/Tests/RunCMake/Codegen/SubDir/CMakeLists.txt @@ -0,0 +1,15 @@ +add_custom_command( + OUTPUT + ${CMAKE_CURRENT_BINARY_DIR}/generated.h + COMMAND + ${CMAKE_COMMAND} -E + copy ${CMAKE_CURRENT_SOURCE_DIR}/../generated.h.in + ${CMAKE_CURRENT_BINARY_DIR}/generated.h + CODEGEN +) + +add_library(errorlib_subdir + # If this library is built error.c will cause the build to fail + ../error.c + ${CMAKE_CURRENT_BINARY_DIR}/generated.h +) diff --git a/Tests/RunCMake/Codegen/SubDirExcludeFromAll-build-check.cmake b/Tests/RunCMake/Codegen/SubDirExcludeFromAll-build-check.cmake new file mode 100644 index 0000000000..9357747349 --- /dev/null +++ b/Tests/RunCMake/Codegen/SubDirExcludeFromAll-build-check.cmake @@ -0,0 +1,9 @@ +set(filename "${RunCMake_TEST_BINARY_DIR}/generated.h") +if(NOT EXISTS "${filename}") + string(APPEND RunCMake_TEST_FAILED "expected file NOT created:\n ${filename}\n") +endif() + +set(filename "${RunCMake_TEST_BINARY_DIR}/SubDir/generated.h") +if(EXISTS "${filename}") + string(APPEND RunCMake_TEST_FAILED "unexpected file created:\n ${filename}\n") +endif() diff --git a/Tests/RunCMake/Codegen/SubDirExcludeFromAll.cmake b/Tests/RunCMake/Codegen/SubDirExcludeFromAll.cmake new file mode 100644 index 0000000000..707da29525 --- /dev/null +++ b/Tests/RunCMake/Codegen/SubDirExcludeFromAll.cmake @@ -0,0 +1,2 @@ +add_subdirectory(SubDir EXCLUDE_FROM_ALL) +include(SubDir-common.cmake)