project: Only define non-cache vars if already defined

In c1ece78d11 (project: non cache <project> prefix variables are
also created, 2024-08-27), we started explicitly setting the non-cache
variable for <projectName>_SOURCE_DIR, <projectName>_BINARY_DIR,
and <projectName>_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

@@ -44,6 +44,12 @@ Projects should not rely on ``<PROJECT-NAME>_SOURCE_DIR`` or
 ``<PROJECT-NAME>_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 ``<PROJECT-NAME>_SOURCE_DIR``,
+  ``<PROJECT-NAME>_BINARY_DIR``, or ``<PROJECT-NAME>_IS_TOP_LEVEL`` are
+  already set as non-cache variables when ``project(<PROJECT-NAME> ...)``
+  is called, the ``project()`` command will overwrite the previous values.
+
 Options
 ^^^^^^^
 

Source/cmProjectCommand.cxx

@@ -8,6 +8,8 @@
 #include <limits>
 #include <utility>
 
+#include <cmext/string_view>
+
 #include "cmsys/RegularExpression.hxx"
 
 #include "cmExecutionStatus.h"
@@ -56,17 +58,21 @@ bool cmProjectCommand(std::vector<std::string> 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<std::string> 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

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)

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

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)

Tests/RunCMake/project/subdir1/CMakeLists.txt

@@ -0,0 +1 @@
+project(${RunCMake_TEST} LANGUAGES NONE)

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}"
+)