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

4 files changed, 48 insertions, 34 deletions

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<cmLocalUnixMakefileGenerator3>(
+  // The global dependency graph is expressed via the root local generator.
+  auto& rootLG = cm::static_reference_cast<cmLocalUnixMakefileGenerator3>(
     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<cmLocalUnixMakefileGenerator3>(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<std::string> const& commands)
+  std::ostream& ruleFileStream, cmLocalUnixMakefileGenerator3& rootLG,
+  DirectoryTarget const& dt, const char* pass, bool check_all,
+  bool check_relink, std::vector<std::string> const& commands)
 {
   auto* lg = static_cast<cmLocalUnixMakefileGenerator3*>(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<cmLocalUnixMakefileGenerator3*>(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<std::string> 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<std::string> depends;
   std::vector<std::string> 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<std::string> const& commands = {});
   void WriteDirectoryRules2(std::ostream& ruleFileStream,
+                            cmLocalUnixMakefileGenerator3& rootLG,
                             DirectoryTarget const& dt);
 
   void AppendGlobalTargetDepends(std::vector<std::string>& 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)