From 86ad7cc886cf477514689c44e02739401cd52402 Mon Sep 17 00:00:00 2001 From: Craig Scott Date: Sun, 15 Sep 2024 17:46:37 +1000 Subject: project: Only define non-cache vars if already defined In c1ece78d11 (project: non cache prefix variables are also created, 2024-08-27), we started explicitly setting the non-cache variable for _SOURCE_DIR, _BINARY_DIR, and _IS_TOP_LEVEL in addition to setting them as cache variables. This changed the behavior when a project name was used more than once, and the second project call happens in the same scope or a child scope of the first. Previously, the first project call would set cache variables, and the second project call would not overwrite those cache variables. With the change in c1ece78d11, after the second project call the non-cache variables would mask the cache variables and the project code would see a different value to what it did before. Setting the non-cache variable was added to handle the case where a call to FetchContent_MakeAvailable() would set some non-cache variables, and it just so happened those matched the same cache variables that the project() command would set in the project being fetched. The fetched project would then see a different set of project-specific variables compared to when it was built standalone. This commit here narrows the change from c1ece78d11 such that the non-cache variable is only set by project() if there was already a non-cache variable set. This still fixes the motivating problem c1ece78d11 was intended to solve, but it avoids changing the variable values seen by a project that re-uses the same project name in related scopes. Issue: #26243, #25714 Fixes: #26281 --- Help/command/project.rst | 6 ++++ Source/cmProjectCommand.cxx | 33 ++++++++++++++-------- Tests/RunCMake/project/RunCMakeTest.cmake | 1 + .../project/SameProjectVarsSubdir-stdout.txt | 9 ++++++ Tests/RunCMake/project/SameProjectVarsSubdir.cmake | 17 +++++++++++ Tests/RunCMake/project/subdir1/CMakeLists.txt | 1 + Tests/RunCMake/project/subdir2/CMakeLists.txt | 12 ++++++++ 7 files changed, 67 insertions(+), 12 deletions(-) create mode 100644 Tests/RunCMake/project/SameProjectVarsSubdir-stdout.txt create mode 100644 Tests/RunCMake/project/SameProjectVarsSubdir.cmake create mode 100644 Tests/RunCMake/project/subdir1/CMakeLists.txt create mode 100644 Tests/RunCMake/project/subdir2/CMakeLists.txt diff --git a/Help/command/project.rst b/Help/command/project.rst index 0188291..08694fd 100644 --- a/Help/command/project.rst +++ b/Help/command/project.rst @@ -44,6 +44,12 @@ Projects should not rely on ``_SOURCE_DIR`` or ``_BINARY_DIR`` holding a particular value outside of the scope of the call to ``project()`` or one of its child scopes. +.. versionchanged:: 3.30.4 + If the variables ``_SOURCE_DIR``, + ``_BINARY_DIR``, or ``_IS_TOP_LEVEL`` are + already set as non-cache variables when ``project( ...)`` + is called, the ``project()`` command will overwrite the previous values. + Options ^^^^^^^ diff --git a/Source/cmProjectCommand.cxx b/Source/cmProjectCommand.cxx index 13070dd..da92ea6 100644 --- a/Source/cmProjectCommand.cxx +++ b/Source/cmProjectCommand.cxx @@ -8,6 +8,8 @@ #include #include +#include + #include "cmsys/RegularExpression.hxx" #include "cmExecutionStatus.h" @@ -56,17 +58,21 @@ bool cmProjectCommand(std::vector const& args, mf.SetProjectName(projectName); - mf.AddCacheDefinition(projectName + "_BINARY_DIR", - mf.GetCurrentBinaryDirectory(), + std::string varName = cmStrCat(projectName, "_BINARY_DIR"_s); + bool nonCacheVarAlreadySet = mf.IsDefinitionSet(varName); + mf.AddCacheDefinition(varName, mf.GetCurrentBinaryDirectory(), "Value Computed by CMake", cmStateEnums::STATIC); - mf.AddDefinition(projectName + "_BINARY_DIR", - mf.GetCurrentBinaryDirectory()); + if (nonCacheVarAlreadySet) { + mf.AddDefinition(varName, mf.GetCurrentBinaryDirectory()); + } - mf.AddCacheDefinition(projectName + "_SOURCE_DIR", - mf.GetCurrentSourceDirectory(), + varName = cmStrCat(projectName, "_SOURCE_DIR"_s); + nonCacheVarAlreadySet = mf.IsDefinitionSet(varName); + mf.AddCacheDefinition(varName, mf.GetCurrentSourceDirectory(), "Value Computed by CMake", cmStateEnums::STATIC); - mf.AddDefinition(projectName + "_SOURCE_DIR", - mf.GetCurrentSourceDirectory()); + if (nonCacheVarAlreadySet) { + mf.AddDefinition(varName, mf.GetCurrentSourceDirectory()); + } mf.AddDefinition("PROJECT_BINARY_DIR", mf.GetCurrentBinaryDirectory()); mf.AddDefinition("PROJECT_SOURCE_DIR", mf.GetCurrentSourceDirectory()); @@ -74,11 +80,14 @@ bool cmProjectCommand(std::vector const& args, mf.AddDefinition("PROJECT_NAME", projectName); mf.AddDefinitionBool("PROJECT_IS_TOP_LEVEL", mf.IsRootMakefile()); - mf.AddCacheDefinition(projectName + "_IS_TOP_LEVEL", - mf.IsRootMakefile() ? "ON" : "OFF", + + varName = cmStrCat(projectName, "_IS_TOP_LEVEL"_s); + nonCacheVarAlreadySet = mf.IsDefinitionSet(varName); + mf.AddCacheDefinition(varName, mf.IsRootMakefile() ? "ON" : "OFF", "Value Computed by CMake", cmStateEnums::STATIC); - mf.AddDefinition(projectName + "_IS_TOP_LEVEL", - mf.IsRootMakefile() ? "ON" : "OFF"); + if (nonCacheVarAlreadySet) { + mf.AddDefinition(varName, mf.IsRootMakefile() ? "ON" : "OFF"); + } // Set the CMAKE_PROJECT_NAME variable to be the highest-level // project name in the tree. If there are two project commands diff --git a/Tests/RunCMake/project/RunCMakeTest.cmake b/Tests/RunCMake/project/RunCMakeTest.cmake index 9c49281..e6e1bdb 100644 --- a/Tests/RunCMake/project/RunCMakeTest.cmake +++ b/Tests/RunCMake/project/RunCMakeTest.cmake @@ -45,6 +45,7 @@ run_cmake(ProjectIsTopLevel) run_cmake(ProjectIsTopLevelMultiple) run_cmake(ProjectIsTopLevelSubdirectory) run_cmake(ProjectTwice) +run_cmake(SameProjectVarsSubdir) run_cmake(VersionAndLanguagesEmpty) run_cmake(VersionEmpty) run_cmake(VersionInvalid) diff --git a/Tests/RunCMake/project/SameProjectVarsSubdir-stdout.txt b/Tests/RunCMake/project/SameProjectVarsSubdir-stdout.txt new file mode 100644 index 0000000..73dadfd --- /dev/null +++ b/Tests/RunCMake/project/SameProjectVarsSubdir-stdout.txt @@ -0,0 +1,9 @@ +(-- )? SameProjectVarsSubdir_SOURCE_DIR = [^ +]+/subdir1 + SameProjectVarsSubdir_BINARY_DIR = [^ +]+/subdir1 + SameProjectVarsSubdir_IS_TOP_LEVEL = OFF +(-- )? sub2proj_SOURCE_DIR = [^ +]+/subdir2 + sub2proj_BINARY_DIR = [^ +]+/subdir2 diff --git a/Tests/RunCMake/project/SameProjectVarsSubdir.cmake b/Tests/RunCMake/project/SameProjectVarsSubdir.cmake new file mode 100644 index 0000000..36a7960 --- /dev/null +++ b/Tests/RunCMake/project/SameProjectVarsSubdir.cmake @@ -0,0 +1,17 @@ +add_subdirectory(subdir1) + +# Simulate a situation that FetchContent_MakeAvailable() used to be able to +# create, but that should no longer be possible. If depname_SOURCE_DIR and +# depname_BINARY_DIR variables are defined as non-cache variables before the +# project(depname) call, those non-cache variables used to prevent project() +# from setting those variables itself due to CMP0126 (if set to NEW). This only +# showed up if the project(depname) call was not in the dependency's top level +# CMakeLists.txt file, but rather in a subdirectory (googletest is one example +# that used to do this). Since CMake 3.30.3, the dependency's project() call +# should set non-cache variables that will make the variable values visible +# and avoid any masking from variables set before the project() call. We want +# to verify this 3.30.3+ behavior here and in subdir2. +set(sub2proj_SOURCE_DIR ${CMAKE_CURRENT_SOURCE_DIR}) +set(sub2proj_BINARY_DIR ${CMAKE_CURRENT_BINARY_DIR}) + +add_subdirectory(subdir2) diff --git a/Tests/RunCMake/project/subdir1/CMakeLists.txt b/Tests/RunCMake/project/subdir1/CMakeLists.txt new file mode 100644 index 0000000..d6be229 --- /dev/null +++ b/Tests/RunCMake/project/subdir1/CMakeLists.txt @@ -0,0 +1 @@ +project(${RunCMake_TEST} LANGUAGES NONE) diff --git a/Tests/RunCMake/project/subdir2/CMakeLists.txt b/Tests/RunCMake/project/subdir2/CMakeLists.txt new file mode 100644 index 0000000..c28e0c9 --- /dev/null +++ b/Tests/RunCMake/project/subdir2/CMakeLists.txt @@ -0,0 +1,12 @@ +message(STATUS + " ${RunCMake_TEST}_SOURCE_DIR = ${${RunCMake_TEST}_SOURCE_DIR}\n" + " ${RunCMake_TEST}_BINARY_DIR = ${${RunCMake_TEST}_BINARY_DIR}\n" + " ${RunCMake_TEST}_IS_TOP_LEVEL = ${${RunCMake_TEST}_IS_TOP_LEVEL}" +) + +project(sub2proj LANGUAGES NONE) + +message(STATUS + " sub2proj_SOURCE_DIR = ${sub2proj_SOURCE_DIR}\n" + " sub2proj_BINARY_DIR = ${sub2proj_BINARY_DIR}" +) -- cgit v0.12