From 4c152752da0a896b849dfaefd2dabd591b324f38 Mon Sep 17 00:00:00 2001 From: Craig Scott Date: Sat, 17 Aug 2024 16:11:22 +1000 Subject: Help: State valid scopes for using proj_SOURCE_DIR and proj_BINARY_DIR Issue: #25714 --- Help/command/project.rst | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/Help/command/project.rst b/Help/command/project.rst index 2b93880..0188291 100644 --- a/Help/command/project.rst +++ b/Help/command/project.rst @@ -33,10 +33,17 @@ Also sets the variables: Boolean value indicating whether the project is top-level. -Further variables are set by the optional arguments described in the following. -If any of these arguments is not used, then the corresponding variables are +Further variables are set by the optional arguments described in `Options`_ +further below. Where an option is not given, its corresponding variable is set to the empty string. +Note that variables of the form ``_SOURCE_DIR`` and ``_BINARY_DIR`` +may also be set by other commands before ``project()`` is called (see the +:command:`FetchContent_MakeAvailable` command for one example). +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. + Options ^^^^^^^ -- cgit v0.12 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