From 5e0b06fe849ee035bedbeac1bd2bfe05f7f8d860 Mon Sep 17 00:00:00 2001 From: Brad King Date: Wed, 3 Aug 2022 13:38:21 -0400 Subject: Makefiles: Restore path consistency in the global dispatch makefile Directory-level rules in `CMakeFiles/Makefile2` were previously previously written by each directory's local generator using its own decision for using relative or absolute paths. Since commit d33b12d84b (Add support for build tree symlink inside source tree, 2022-02-25, v3.24.0-rc1~583^2), each local generator explicitly models the relationship between its source and build paths, and uses this to determine when it is safe to use relative paths. Because `add_subdirectory` supports arbitrary placement of the source and build directories, different local generators may have different relationships between their source and build paths. This can cause disagreement among rules written to `CMakeFiles/Makefile2`. Restore consistency by always using the root local generator to write rules to `CMakeFiles/Makefile2`. Relative paths should always be expressed w.r.t. the top-level build directory since that is the working directory in which the `make` tool processing the file will run. Fixes: #23814 --- Source/cmGlobalUnixMakefileGenerator3.cxx | 73 +++++++++++++++------------- Source/cmGlobalUnixMakefileGenerator3.h | 5 +- Tests/OutOfSource/CMakeLists.txt | 3 ++ Tests/OutOfSource/SubInBuildCMakeLists.cmake | 1 + 4 files changed, 48 insertions(+), 34 deletions(-) create mode 100644 Tests/OutOfSource/SubInBuildCMakeLists.cmake diff --git a/Source/cmGlobalUnixMakefileGenerator3.cxx b/Source/cmGlobalUnixMakefileGenerator3.cxx index ab9ca50..21aa89c 100644 --- a/Source/cmGlobalUnixMakefileGenerator3.cxx +++ b/Source/cmGlobalUnixMakefileGenerator3.cxx @@ -181,12 +181,12 @@ void cmGlobalUnixMakefileGenerator3::WriteMainMakefile2() return; } - // get a local generator for some useful methods - auto& lg = cm::static_reference_cast( + // The global dependency graph is expressed via the root local generator. + auto& rootLG = cm::static_reference_cast( this->LocalGenerators[0]); // Write the do not edit header. - lg.WriteDisclaimer(makefileStream); + rootLG.WriteDisclaimer(makefileStream); // Write the main entry point target. This must be the VERY first // target so that make with no arguments will run it. @@ -196,10 +196,10 @@ void cmGlobalUnixMakefileGenerator3::WriteMainMakefile2() depends.emplace_back("all"); // Write the rule. - lg.WriteMakeRule(makefileStream, - "Default target executed when no arguments are " - "given to make.", - "default_target", depends, no_commands, true); + rootLG.WriteMakeRule(makefileStream, + "Default target executed when no arguments are " + "given to make.", + "default_target", depends, no_commands, true); depends.clear(); @@ -210,22 +210,22 @@ void cmGlobalUnixMakefileGenerator3::WriteMainMakefile2() } // Write out the "special" stuff - lg.WriteSpecialTargetsTop(makefileStream); + rootLG.WriteSpecialTargetsTop(makefileStream); // Write the directory level rules. for (auto const& it : this->ComputeDirectoryTargets()) { - this->WriteDirectoryRules2(makefileStream, it.second); + this->WriteDirectoryRules2(makefileStream, rootLG, it.second); } // Write the target convenience rules for (const auto& localGen : this->LocalGenerators) { this->WriteConvenienceRules2( - makefileStream, + makefileStream, rootLG, cm::static_reference_cast(localGen)); } // Write special bottom targets - lg.WriteSpecialTargetsBottom(makefileStream); + rootLG.WriteSpecialTargetsBottom(makefileStream); } void cmGlobalUnixMakefileGenerator3::WriteMainCMakefile() @@ -359,8 +359,9 @@ void cmGlobalUnixMakefileGenerator3::WriteMainCMakefileLanguageRules( } void cmGlobalUnixMakefileGenerator3::WriteDirectoryRule2( - std::ostream& ruleFileStream, DirectoryTarget const& dt, const char* pass, - bool check_all, bool check_relink, std::vector const& commands) + std::ostream& ruleFileStream, cmLocalUnixMakefileGenerator3& rootLG, + DirectoryTarget const& dt, const char* pass, bool check_all, + bool check_relink, std::vector const& commands) { auto* lg = static_cast(dt.LG); std::string makeTarget = @@ -406,19 +407,21 @@ void cmGlobalUnixMakefileGenerator3::WriteDirectoryRule2( } else { doc = cmStrCat("Recursive \"", pass, "\" directory target."); } - lg->WriteMakeRule(ruleFileStream, doc.c_str(), makeTarget, depends, commands, - true); + + rootLG.WriteMakeRule(ruleFileStream, doc.c_str(), makeTarget, depends, + commands, true); } void cmGlobalUnixMakefileGenerator3::WriteDirectoryRules2( - std::ostream& ruleFileStream, DirectoryTarget const& dt) + std::ostream& ruleFileStream, cmLocalUnixMakefileGenerator3& rootLG, + DirectoryTarget const& dt) { auto* lg = static_cast(dt.LG); // Begin the directory-level rules section. { std::string dir = cmSystemTools::ConvertToOutputPath( - lg->MaybeRelativeToTopBinDir(lg->GetCurrentBinaryDirectory())); - lg->WriteDivider(ruleFileStream); + rootLG.MaybeRelativeToTopBinDir(lg->GetCurrentBinaryDirectory())); + rootLG.WriteDivider(ruleFileStream); if (lg->IsRootMakefile()) { ruleFileStream << "# Directory level rules for the build root directory"; } else { @@ -428,16 +431,18 @@ void cmGlobalUnixMakefileGenerator3::WriteDirectoryRules2( } // Write directory-level rules for "all". - this->WriteDirectoryRule2(ruleFileStream, dt, "all", true, false); + this->WriteDirectoryRule2(ruleFileStream, rootLG, dt, "all", true, false); // Write directory-level rules for "preinstall". - this->WriteDirectoryRule2(ruleFileStream, dt, "preinstall", true, true); + this->WriteDirectoryRule2(ruleFileStream, rootLG, dt, "preinstall", true, + true); // Write directory-level rules for "clean". { std::vector cmds; lg->AppendDirectoryCleanCommand(cmds); - this->WriteDirectoryRule2(ruleFileStream, dt, "clean", false, false, cmds); + this->WriteDirectoryRule2(ruleFileStream, rootLG, dt, "clean", false, + false, cmds); } } @@ -632,7 +637,8 @@ void cmGlobalUnixMakefileGenerator3::WriteConvenienceRules( } void cmGlobalUnixMakefileGenerator3::WriteConvenienceRules2( - std::ostream& ruleFileStream, cmLocalUnixMakefileGenerator3& lg) + std::ostream& ruleFileStream, cmLocalUnixMakefileGenerator3& rootLG, + cmLocalUnixMakefileGenerator3& lg) { std::vector depends; std::vector commands; @@ -696,8 +702,8 @@ void cmGlobalUnixMakefileGenerator3::WriteConvenienceRules2( } this->AppendGlobalTargetDepends(depends, gtarget.get()); - lg.WriteMakeRule(ruleFileStream, "All Build rule for target.", localName, - depends, commands, true); + rootLG.WriteMakeRule(ruleFileStream, "All Build rule for target.", + localName, depends, commands, true); // Write the rule. commands.clear(); @@ -731,16 +737,16 @@ void cmGlobalUnixMakefileGenerator3::WriteConvenienceRules2( } localName = cmStrCat(lg.GetRelativeTargetDirectory(gtarget.get()), "/rule"); - lg.WriteMakeRule(ruleFileStream, - "Build rule for subdir invocation for target.", - localName, depends, commands, true); + rootLG.WriteMakeRule(ruleFileStream, + "Build rule for subdir invocation for target.", + localName, depends, commands, true); // Add a target with the canonical name (no prefix, suffix or path). commands.clear(); depends.clear(); depends.push_back(localName); - lg.WriteMakeRule(ruleFileStream, "Convenience name for target.", name, - depends, commands, true); + rootLG.WriteMakeRule(ruleFileStream, "Convenience name for target.", + name, depends, commands, true); // Add rules to prepare the target for installation. if (gtarget->NeedRelinkBeforeInstall(lg.GetConfigName())) { @@ -749,8 +755,9 @@ void cmGlobalUnixMakefileGenerator3::WriteConvenienceRules2( depends.clear(); commands.clear(); commands.push_back(lg.GetRecursiveMakeCall(makefileName, localName)); - lg.WriteMakeRule(ruleFileStream, "Pre-install relink rule for target.", - localName, depends, commands, true); + rootLG.WriteMakeRule(ruleFileStream, + "Pre-install relink rule for target.", localName, + depends, commands, true); } // add the clean rule @@ -760,8 +767,8 @@ void cmGlobalUnixMakefileGenerator3::WriteConvenienceRules2( commands.clear(); commands.push_back( lg.GetRecursiveMakeCall(makefileName, makeTargetName)); - lg.WriteMakeRule(ruleFileStream, "clean rule for target.", - makeTargetName, depends, commands, true); + rootLG.WriteMakeRule(ruleFileStream, "clean rule for target.", + makeTargetName, depends, commands, true); commands.clear(); } } diff --git a/Source/cmGlobalUnixMakefileGenerator3.h b/Source/cmGlobalUnixMakefileGenerator3.h index 5157826..b9d333e 100644 --- a/Source/cmGlobalUnixMakefileGenerator3.h +++ b/Source/cmGlobalUnixMakefileGenerator3.h @@ -200,13 +200,16 @@ protected: void WriteMainCMakefile(); void WriteConvenienceRules2(std::ostream& ruleFileStream, - cmLocalUnixMakefileGenerator3&); + cmLocalUnixMakefileGenerator3& rootLG, + cmLocalUnixMakefileGenerator3& lg); void WriteDirectoryRule2(std::ostream& ruleFileStream, + cmLocalUnixMakefileGenerator3& rootLG, DirectoryTarget const& dt, const char* pass, bool check_all, bool check_relink, std::vector const& commands = {}); void WriteDirectoryRules2(std::ostream& ruleFileStream, + cmLocalUnixMakefileGenerator3& rootLG, DirectoryTarget const& dt); void AppendGlobalTargetDepends(std::vector& depends, diff --git a/Tests/OutOfSource/CMakeLists.txt b/Tests/OutOfSource/CMakeLists.txt index 4687882..c82d077 100644 --- a/Tests/OutOfSource/CMakeLists.txt +++ b/Tests/OutOfSource/CMakeLists.txt @@ -16,3 +16,6 @@ configure_file( ) set(KEN 1) + +configure_file(SubInBuildCMakeLists.cmake ${CMAKE_CURRENT_BINARY_DIR}/SubInBuild/CMakeLists.txt COPYONLY) +add_subdirectory(${CMAKE_CURRENT_BINARY_DIR}/SubInBuild ${CMAKE_CURRENT_BINARY_DIR}/SubInBuild/Build) diff --git a/Tests/OutOfSource/SubInBuildCMakeLists.cmake b/Tests/OutOfSource/SubInBuildCMakeLists.cmake new file mode 100644 index 0000000..c2e2942 --- /dev/null +++ b/Tests/OutOfSource/SubInBuildCMakeLists.cmake @@ -0,0 +1 @@ +add_custom_target(SubInBuildCustom ALL) -- cgit v0.12