Skip to content

Commit d46bac5

Browse files
committed
Makefile: Fix regression in dependencies on relative includes
Since commit a13a5c9 (Replace use of CollapseCombinedPath with CollapseFullPath, 2019-03-19, v3.15.0-rc1~361^2~1), one code path now calls `CollapseFullPath` with a base path that may be relative. Backport KWSys commit c6f8e24a3 (SystemTools: Fix CollapseFullPath with relative base path, 2019-07-24) to handle such base paths. This case occurs when a build tree is placed in a directory inside a source tree such that CMake is willing to generate a relative path from the build tree to the source tree. Add a test covering this case. Fixes: #19507
1 parent 79bcf4e commit d46bac5

7 files changed

+47
-4
lines changed

Source/kwsys/SystemTools.cxx

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3394,8 +3394,13 @@ static void SystemToolsAppendComponents(
33943394
static const std::string cur = ".";
33953395
for (std::vector<std::string>::const_iterator i = first; i != last; ++i) {
33963396
if (*i == up) {
3397-
if (out_components.size() > 1) {
3397+
// Remove the previous component if possible. Ignore ../ components
3398+
// that try to go above the root. Keep ../ components if they are
3399+
// at the beginning of a relative path (base path is relative).
3400+
if (out_components.size() > 1 && out_components.back() != up) {
33983401
out_components.resize(out_components.size() - 1);
3402+
} else if (!out_components.empty() && out_components[0].empty()) {
3403+
out_components.emplace_back(std::move(*i));
33993404
}
34003405
} else if (!i->empty() && *i != cur) {
34013406
#if __cplusplus >= 201103L || (defined(_MSVC_LANG) && _MSVC_LANG >= 201103L)

Source/kwsys/testSystemTools.cxx

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -684,9 +684,10 @@ static bool CheckRelativePaths()
684684
}
685685

686686
static bool CheckCollapsePath(const std::string& path,
687-
const std::string& expected)
687+
const std::string& expected,
688+
const char* base = nullptr)
688689
{
689-
std::string result = kwsys::SystemTools::CollapseFullPath(path);
690+
std::string result = kwsys::SystemTools::CollapseFullPath(path, base);
690691
if (!kwsys::SystemTools::ComparePath(expected, result)) {
691692
std::cerr << "CollapseFullPath(" << path << ") yielded " << result
692693
<< " instead of " << expected << std::endl;
@@ -710,6 +711,9 @@ static bool CheckCollapsePath()
710711
res &= CheckCollapsePath("C:/", "C:/");
711712
res &= CheckCollapsePath("C:/../", "C:/");
712713
res &= CheckCollapsePath("C:/../../", "C:/");
714+
res &= CheckCollapsePath("../b", "../../b", "../");
715+
res &= CheckCollapsePath("../a/../b", "../b", "../rel");
716+
res &= CheckCollapsePath("a/../b", "../rel/b", "../rel");
713717
return res;
714718
}
715719

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
#include "BuildUnderSource.h"
2+
int main(void)
3+
{
4+
return BUILD_UNDER_SOURCE;
5+
}
Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
enable_language(C)
2+
include_directories(include)
3+
add_executable(BuildUnderSource BuildUnderSource.c)
4+
5+
file(GENERATE OUTPUT ${CMAKE_CURRENT_BINARY_DIR}/check-$<LOWER_CASE:$<CONFIG>>.cmake CONTENT "
6+
set(check_pairs
7+
\"$<TARGET_FILE:BuildUnderSource>|${CMAKE_CURRENT_SOURCE_DIR}/include/BuildUnderSource.h\"
8+
)
9+
")
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
file(WRITE "${RunCMake_TEST_SOURCE_DIR}/include/BuildUnderSource.h" [[
2+
#define BUILD_UNDER_SOURCE 1
3+
]])
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
file(WRITE "${RunCMake_TEST_SOURCE_DIR}/include/BuildUnderSource.h" [[
2+
#define BUILD_UNDER_SOURCE 2
3+
]])

Tests/RunCMake/BuildDepends/RunCMakeTest.cmake

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,9 @@ endif()
99

1010
function(run_BuildDepends CASE)
1111
# Use a single build tree for a few tests without cleaning.
12-
set(RunCMake_TEST_BINARY_DIR ${RunCMake_BINARY_DIR}/${CASE}-build)
12+
if(NOT RunCMake_TEST_BINARY_DIR)
13+
set(RunCMake_TEST_BINARY_DIR ${RunCMake_BINARY_DIR}/${CASE}-build)
14+
endif()
1315
set(RunCMake_TEST_NO_CLEAN 1)
1416
if(NOT RunCMake_GENERATOR_IS_MULTI_CONFIG)
1517
set(RunCMake_TEST_OPTIONS -DCMAKE_BUILD_TYPE=Debug)
@@ -44,6 +46,18 @@ endif()
4446
run_BuildDepends(Custom-Symbolic-and-Byproduct)
4547
run_BuildDepends(Custom-Always)
4648

49+
# Test header dependencies with a build tree underneath a source tree.
50+
set(RunCMake_TEST_SOURCE_DIR "${RunCMake_BINARY_DIR}/BuildUnderSource")
51+
set(RunCMake_TEST_BINARY_DIR "${RunCMake_BINARY_DIR}/BuildUnderSource/build")
52+
file(REMOVE_RECURSE "${RunCMake_TEST_SOURCE_DIR}")
53+
file(MAKE_DIRECTORY "${RunCMake_TEST_SOURCE_DIR}/include")
54+
foreach(f CMakeLists.txt BuildUnderSource.cmake BuildUnderSource.c)
55+
configure_file("${RunCMake_SOURCE_DIR}/${f}" "${RunCMake_TEST_SOURCE_DIR}/${f}" COPYONLY)
56+
endforeach()
57+
run_BuildDepends(BuildUnderSource)
58+
unset(RunCMake_TEST_BINARY_DIR)
59+
unset(RunCMake_TEST_SOURCE_DIR)
60+
4761
if(RunCMake_GENERATOR MATCHES "Make")
4862
run_BuildDepends(MakeCustomIncludes)
4963
if(NOT "${RunCMake_BINARY_DIR}" STREQUAL "${RunCMake_SOURCE_DIR}")

0 commit comments

Comments
 (0)