From 55db2cf1e59a2138992ae673db96c0a40f87324a Mon Sep 17 00:00:00 2001 From: Brad King Date: Mon, 28 Feb 2022 09:41:53 -0500 Subject: Makefiles: Fix "make depend" with add_custom_command DEPFILE Since commit cfd8a5ac1f (Makefiles: Add support of DEPFILE for add_custom_command, 2020-12-04, v3.20.0-rc1~237^2~1) we store in `CMAKE_DEPENDS_DEPENDENCY_FILES` an empty string as the source file with which the dependencies of a custom command depfile are associated. When this list is later expanded by `make depend`, the empty element is removed and breaks list indexing. Fix the list expansion to preserve empty elements. --- Source/cmLocalUnixMakefileGenerator3.cxx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Source/cmLocalUnixMakefileGenerator3.cxx b/Source/cmLocalUnixMakefileGenerator3.cxx index 0f8cdca..e123cf9 100644 --- a/Source/cmLocalUnixMakefileGenerator3.cxx +++ b/Source/cmLocalUnixMakefileGenerator3.cxx @@ -1849,7 +1849,7 @@ void cmLocalUnixMakefileGenerator3::ClearDependencies(cmMakefile* mf, cmSystemTools::Touch(DepTimestamp.GenericString(), true); // clear the dependencies files generated by the compiler - std::vector dependencies = cmExpandedList(depsFiles); + std::vector dependencies = cmExpandedList(depsFiles, true); cmDependsCompiler depsManager; depsManager.SetVerbose(verbose); depsManager.ClearDependencies(dependencies); -- cgit v0.12 From de766bc7e0f4a856abc712e64e4248e966ced37f Mon Sep 17 00:00:00 2001 From: Brad King Date: Thu, 24 Feb 2022 18:12:22 -0500 Subject: Xcode: Fix support for source tree symlink inside build tree Since commit 61495cdaae (Fix Xcode project references to the source tree, 2009-09-22, v2.8.0~43) we force source file references to use relative paths from the source tree. If the source tree path is a symbolic link inside the build tree, the relative `../` sequence goes to the wrong place. The problem with debug breakpoints motivating that change does not seem to occur in modern Xcode versions, so update the logic to use a relative path only when it does not need to start in any `../` sequence. --- Source/cmGlobalXCodeGenerator.cxx | 10 ++++++---- Tests/RunCMake/SymlinkTrees/RunCMakeTest.cmake | 6 ------ 2 files changed, 6 insertions(+), 10 deletions(-) diff --git a/Source/cmGlobalXCodeGenerator.cxx b/Source/cmGlobalXCodeGenerator.cxx index b752c41..1e6624e 100644 --- a/Source/cmGlobalXCodeGenerator.cxx +++ b/Source/cmGlobalXCodeGenerator.cxx @@ -4695,10 +4695,12 @@ std::string cmGlobalXCodeGenerator::ConvertToRelativeForMake( std::string cmGlobalXCodeGenerator::RelativeToSource(const std::string& p) { - // We force conversion because Xcode breakpoints do not work unless - // they are in a file named relative to the source tree. - return cmSystemTools::ForceToRelativePath( - this->CurrentRootGenerator->GetCurrentSourceDirectory(), p); + std::string const& rootSrc = + this->CurrentRootGenerator->GetCurrentSourceDirectory(); + if (cmSystemTools::IsSubDirectory(p, rootSrc)) { + return cmSystemTools::ForceToRelativePath(rootSrc, p); + } + return p; } std::string cmGlobalXCodeGenerator::RelativeToBinary(const std::string& p) diff --git a/Tests/RunCMake/SymlinkTrees/RunCMakeTest.cmake b/Tests/RunCMake/SymlinkTrees/RunCMakeTest.cmake index 304e9de..3f3aafe 100644 --- a/Tests/RunCMake/SymlinkTrees/RunCMakeTest.cmake +++ b/Tests/RunCMake/SymlinkTrees/RunCMakeTest.cmake @@ -82,12 +82,6 @@ function (run_symlink_test case src bin src_from_bin bin_from_src) message(STATUS "${case}-exe-build - SKIPPED") return() endif() - if(case MATCHES "^(different|asymmetric)-src_in_bin$" AND RunCMake_GENERATOR STREQUAL "Xcode") - # FIXME: The Xcode generator computes an incorrect relative path. - message(STATUS "${case}-exe - SKIPPED") - message(STATUS "${case}-exe-build - SKIPPED") - return() - endif() unset(RunCMake_TEST_VARIANT_DESCRIPTION) run_symlink_test_case("${case}-exe" -S "${src}" -B "${bin}") if (RunCMake_GENERATOR MATCHES "Xcode") -- cgit v0.12 From 43416c48ed6bb7d3b8063ac4ea1ba02a4aab828e Mon Sep 17 00:00:00 2001 From: Brad King Date: Mon, 28 Feb 2022 10:16:51 -0500 Subject: cmOutputConverter: Always set relative path top source and binary together Refactor to set both at once so we have a single place in the code that knows both have been set. --- Source/cmGlobalNinjaGenerator.cxx | 3 +-- Source/cmLocalUnixMakefileGenerator3.cxx | 13 ++++++------- Source/cmOutputConverter.cxx | 11 ++++------- Source/cmOutputConverter.h | 4 ++-- Source/cmcmd.cxx | 6 ++---- 5 files changed, 15 insertions(+), 22 deletions(-) diff --git a/Source/cmGlobalNinjaGenerator.cxx b/Source/cmGlobalNinjaGenerator.cxx index 4245037..bbc9c54 100644 --- a/Source/cmGlobalNinjaGenerator.cxx +++ b/Source/cmGlobalNinjaGenerator.cxx @@ -2495,8 +2495,7 @@ bool cmGlobalNinjaGenerator::WriteDyndepFile( snapshot.GetDirectory().SetCurrentBinary(dir_cur_bld); auto mfd = cm::make_unique(this, snapshot); auto lgd = this->CreateLocalGenerator(mfd.get()); - lgd->SetRelativePathTopSource(dir_top_src); - lgd->SetRelativePathTopBinary(dir_top_bld); + lgd->SetRelativePathTop(dir_top_src, dir_top_bld); this->Makefiles.push_back(std::move(mfd)); this->LocalGenerators.push_back(std::move(lgd)); } diff --git a/Source/cmLocalUnixMakefileGenerator3.cxx b/Source/cmLocalUnixMakefileGenerator3.cxx index e123cf9..7c4af7d 100644 --- a/Source/cmLocalUnixMakefileGenerator3.cxx +++ b/Source/cmLocalUnixMakefileGenerator3.cxx @@ -1508,13 +1508,12 @@ bool cmLocalUnixMakefileGenerator3::ScanDependencies( } // Setup relative path top directories. - if (cmValue relativePathTopSource = - mf->GetDefinition("CMAKE_RELATIVE_PATH_TOP_SOURCE")) { - this->SetRelativePathTopSource(*relativePathTopSource); - } - if (cmValue relativePathTopBinary = - mf->GetDefinition("CMAKE_RELATIVE_PATH_TOP_BINARY")) { - this->SetRelativePathTopBinary(*relativePathTopBinary); + cmValue relativePathTopSource = + mf->GetDefinition("CMAKE_RELATIVE_PATH_TOP_SOURCE"); + cmValue relativePathTopBinary = + mf->GetDefinition("CMAKE_RELATIVE_PATH_TOP_BINARY"); + if (relativePathTopSource && relativePathTopBinary) { + this->SetRelativePathTop(*relativePathTopSource, *relativePathTopBinary); } } else { cmSystemTools::Error("Directory Information file not found"); diff --git a/Source/cmOutputConverter.cxx b/Source/cmOutputConverter.cxx index b143170..3d61743 100644 --- a/Source/cmOutputConverter.cxx +++ b/Source/cmOutputConverter.cxx @@ -79,14 +79,11 @@ std::string const& cmOutputConverter::GetRelativePathTopBinary() const return this->RelativePathTopBinary; } -void cmOutputConverter::SetRelativePathTopSource(std::string const& top) +void cmOutputConverter::SetRelativePathTop(std::string const& topSource, + std::string const& topBinary) { - this->RelativePathTopSource = top; -} - -void cmOutputConverter::SetRelativePathTopBinary(std::string const& top) -{ - this->RelativePathTopBinary = top; + this->RelativePathTopSource = topSource; + this->RelativePathTopBinary = topBinary; } std::string cmOutputConverter::MaybeRelativeTo( diff --git a/Source/cmOutputConverter.h b/Source/cmOutputConverter.h index 335442d..290809f 100644 --- a/Source/cmOutputConverter.h +++ b/Source/cmOutputConverter.h @@ -29,8 +29,8 @@ public: std::string const& GetRelativePathTopSource() const; std::string const& GetRelativePathTopBinary() const; - void SetRelativePathTopSource(std::string const& top); - void SetRelativePathTopBinary(std::string const& top); + void SetRelativePathTop(std::string const& topSource, + std::string const& topBinary); enum OutputFormat { diff --git a/Source/cmcmd.cxx b/Source/cmcmd.cxx index ba61a83..f1c1bdc 100644 --- a/Source/cmcmd.cxx +++ b/Source/cmcmd.cxx @@ -1286,8 +1286,7 @@ int cmcmd::ExecuteCMakeCommand(std::vector const& args, // FIXME: With advanced add_subdirectory usage, these are // not necessarily the same as the generator originally used. // We should pass all these directories through an info file. - lgd->SetRelativePathTopSource(homeDir); - lgd->SetRelativePathTopBinary(homeOutDir); + lgd->SetRelativePathTop(homeDir, homeOutDir); // Actually scan dependencies. return lgd->UpdateDependencies(depInfo, verbose, color) ? 0 : 2; @@ -1569,8 +1568,7 @@ int cmcmd::ExecuteCMakeCommand(std::vector const& args, // FIXME: With advanced add_subdirectory usage, these are // not necessarily the same as the generator originally used. // We should pass all these directories through an info file. - lgd->SetRelativePathTopSource(homeDir); - lgd->SetRelativePathTopBinary(homeOutDir); + lgd->SetRelativePathTop(homeDir, homeOutDir); return cmTransformDepfile(format, *lgd, args[8], args[9]) ? 0 : 2; } -- cgit v0.12 From d33b12d84b59fd3310c31de7d9c3f83b28b681e2 Mon Sep 17 00:00:00 2001 From: Brad King Date: Fri, 25 Feb 2022 08:35:46 -0500 Subject: Add support for build tree symlink inside source tree Since commit c564a3e3ff (Ninja: Always compile sources using absolute paths, 2021-05-19, v3.21.0-rc1~129^2), both the Ninja and Makefile generators pass source files and include directories to the compiler as absolute paths. However, in some other contexts within generated build systems, we generate paths that may be relative or absolute. In these contexts, we prefer relative paths, but avoid them when they contain a `../` sequence that leaves both the build tree and the source tree: * When the build tree is outside of the source tree, all paths to the source tree are absolute. * When the build tree is inside the source tree, we previously assumed that it is a real directory such that exiting the build tree with `../` enters the source tree. This allowed paths to the source tree to be relative to the build tree. In the latter case, we previously did not support using a symbolic link inside the source tree to point at the build tree. This is because relative paths to the source tree would be generated with `../` sequences leaving the build tree, but they would jump to the parent of the real build tree, which is not the source tree. Fix this by requiring that `../` sequences stay inside the build tree even if its path appears to be inside the source tree. When the build tree is inside the source tree, all paths to the source tree are now absolute. For consistency, this applies regardless of whether the path to the build tree contains a symbolic link. Fixes: #21819 --- Source/cmOutputConverter.cxx | 46 +++++++++++++++++++++++--- Source/cmOutputConverter.h | 9 +++++ Tests/RunCMake/SymlinkTrees/RunCMakeTest.cmake | 6 ---- 3 files changed, 51 insertions(+), 10 deletions(-) diff --git a/Source/cmOutputConverter.cxx b/Source/cmOutputConverter.cxx index 3d61743..e62e0cd 100644 --- a/Source/cmOutputConverter.cxx +++ b/Source/cmOutputConverter.cxx @@ -34,6 +34,7 @@ cmOutputConverter::cmOutputConverter(cmStateSnapshot const& snapshot) assert(this->StateSnapshot.IsValid()); this->ComputeRelativePathTopSource(); this->ComputeRelativePathTopBinary(); + this->ComputeRelativePathTopRelation(); } void cmOutputConverter::ComputeRelativePathTopSource() @@ -69,6 +70,22 @@ void cmOutputConverter::ComputeRelativePathTopBinary() this->RelativePathTopBinary = snapshot.GetDirectory().GetCurrentBinary(); } +void cmOutputConverter::ComputeRelativePathTopRelation() +{ + if (cmSystemTools::ComparePath(this->RelativePathTopSource, + this->RelativePathTopBinary)) { + this->RelativePathTopRelation = TopRelation::InSource; + } else if (cmSystemTools::IsSubDirectory(this->RelativePathTopBinary, + this->RelativePathTopSource)) { + this->RelativePathTopRelation = TopRelation::BinInSrc; + } else if (cmSystemTools::IsSubDirectory(this->RelativePathTopSource, + this->RelativePathTopBinary)) { + this->RelativePathTopRelation = TopRelation::SrcInBin; + } else { + this->RelativePathTopRelation = TopRelation::Separate; + } +} + std::string const& cmOutputConverter::GetRelativePathTopSource() const { return this->RelativePathTopSource; @@ -84,19 +101,40 @@ void cmOutputConverter::SetRelativePathTop(std::string const& topSource, { this->RelativePathTopSource = topSource; this->RelativePathTopBinary = topBinary; + this->ComputeRelativePathTopRelation(); } std::string cmOutputConverter::MaybeRelativeTo( std::string const& local_path, std::string const& remote_path) const { - bool bothInBinary = - PathEqOrSubDir(local_path, this->RelativePathTopBinary) && + bool localInBinary = PathEqOrSubDir(local_path, this->RelativePathTopBinary); + bool remoteInBinary = PathEqOrSubDir(remote_path, this->RelativePathTopBinary); - bool bothInSource = - PathEqOrSubDir(local_path, this->RelativePathTopSource) && + bool localInSource = PathEqOrSubDir(local_path, this->RelativePathTopSource); + bool remoteInSource = PathEqOrSubDir(remote_path, this->RelativePathTopSource); + switch (this->RelativePathTopRelation) { + case TopRelation::Separate: + // Checks are independent. + break; + case TopRelation::BinInSrc: + localInSource = localInSource && !localInBinary; + remoteInSource = remoteInSource && !remoteInBinary; + break; + case TopRelation::SrcInBin: + localInBinary = localInBinary && !localInSource; + remoteInBinary = remoteInBinary && !remoteInSource; + break; + case TopRelation::InSource: + // Checks are identical. + break; + }; + + bool const bothInBinary = localInBinary && remoteInBinary; + bool const bothInSource = localInSource && remoteInSource; + if (bothInBinary || bothInSource) { return cmSystemTools::ForceToRelativePath(local_path, remote_path); } diff --git a/Source/cmOutputConverter.h b/Source/cmOutputConverter.h index 290809f..d19bccc 100644 --- a/Source/cmOutputConverter.h +++ b/Source/cmOutputConverter.h @@ -147,8 +147,17 @@ private: // safely by the build tools. std::string RelativePathTopSource; std::string RelativePathTopBinary; + enum class TopRelation + { + Separate, + BinInSrc, + SrcInBin, + InSource, + }; + TopRelation RelativePathTopRelation = TopRelation::Separate; void ComputeRelativePathTopSource(); void ComputeRelativePathTopBinary(); + void ComputeRelativePathTopRelation(); std::string MaybeRelativeTo(std::string const& local_path, std::string const& remote_path) const; }; diff --git a/Tests/RunCMake/SymlinkTrees/RunCMakeTest.cmake b/Tests/RunCMake/SymlinkTrees/RunCMakeTest.cmake index 3f3aafe..e5bfac4 100644 --- a/Tests/RunCMake/SymlinkTrees/RunCMakeTest.cmake +++ b/Tests/RunCMake/SymlinkTrees/RunCMakeTest.cmake @@ -76,12 +76,6 @@ function (run_symlink_test case src bin src_from_bin bin_from_src) run_symlink_test_case("${case}" -S "../${name}/${src}" -B "../${name}/${bin}") # Verify paths passed to compiler. - if(case MATCHES "^(different|asymmetric)-bin_in_src$") - # FIXME: Some generators compute incorrect relative paths. - message(STATUS "${case}-exe - SKIPPED") - message(STATUS "${case}-exe-build - SKIPPED") - return() - endif() unset(RunCMake_TEST_VARIANT_DESCRIPTION) run_symlink_test_case("${case}-exe" -S "${src}" -B "${bin}") if (RunCMake_GENERATOR MATCHES "Xcode") -- cgit v0.12