project: Only check non-cache vars when setting project vars

The change in commit 86ad7cc886 (project: Only define non-cache vars if
already defined, 2024-09-15, v3.30.4~2^2) was meant to only check for
non-cache variables when deciding whether to set non-cache project
variables for the current call.  However, it erroneously checked for any
variable, including cache variables.  This gives the intended result on
the first run, but on subsequent runs a cache variable will exist that
did not on the first run, leading to different behavior between the two
runs.  Fix the logic to only check for a pre-existing non-cache
variable, as was originally intended.

Fixes: #26355

Help/command/project.rst

@@ -44,11 +44,27 @@ 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.3
+  ``<PROJECT-NAME>_SOURCE_DIR``, ``<PROJECT-NAME>_BINARY_DIR``, and
+  ``<PROJECT-NAME>_IS_TOP_LEVEL`` are always set as non-cache variables by
+  ``project(<PROJECT-NAME> ...)``.
+
 .. 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.
+  The variables ``<PROJECT-NAME>_SOURCE_DIR``, ``<PROJECT-NAME>_BINARY_DIR``,
+  and ``<PROJECT-NAME>_IS_TOP_LEVEL`` are only set as non-cache variables if
+  they are already set as cache or non-cache variables when
+  ``project(<PROJECT-NAME> ...)`` is called.
+  Note that this logic is flawed, as it can result in different behavior
+  between the first and subsequent runs because cache variables won't exist
+  on the first run, but they will on subsequent runs.
+
+.. versionchanged:: 3.30.5
+  The variables ``<PROJECT-NAME>_SOURCE_DIR``, ``<PROJECT-NAME>_BINARY_DIR``,
+  and ``<PROJECT-NAME>_IS_TOP_LEVEL`` are only set as non-cache variables if
+  they are already set as non-cache variables when
+  ``project(<PROJECT-NAME> ...)`` is called.
+  Unlike the flawed behavior of 3.30.4, non-cache variables will not be set
+  if only cache variables of the same name are set.
 
 Options
 ^^^^^^^

Help/release/3.30.rst

@@ -277,7 +277,23 @@ Changes made since CMake 3.30.0 include the following.
 * The :command:`project(<PROJECT-NAME>)` command now sets
   :variable:`<PROJECT-NAME>_SOURCE_DIR`, :variable:`<PROJECT-NAME>_BINARY_DIR`,
   and :variable:`<PROJECT-NAME>_IS_TOP_LEVEL` as normal variables only if they
-  are already set as normal variables when :command:`project` is invoked.
-  Cache entries by the same names are always set as before.
+  are already set as cache or non-cache variables when :command:`project` is
+  invoked.  Cache entries by the same names are always set as before.
   This refines 3.30.3's behavior change to restore behavior of nested
-  directories that call :command:`project` with the same project name.
+  directories that call :command:`project` with the same project name,
+  but the implementation in this release is flawed (this release note has
+  been retoractively updated).  It can result in different behavior between
+  the first and subsequent runs.  Do not use CMake 3.30.4 if your project
+  contains nested calls to :command:`project` with the same project name
+  and you use these variables.
+
+.. 3.30.5 (unreleased)
+
+  * The :command:`project(<PROJECT-NAME>)` command now sets
+    :variable:`<PROJECT-NAME>_SOURCE_DIR`, :variable:`<PROJECT-NAME>_BINARY_DIR`,
+    and :variable:`<PROJECT-NAME>_IS_TOP_LEVEL` as non-cache variables only if
+    they are already set as non-cache variables when :command:`project` is
+    invoked.  Cache entries by the same names are always set as before.
+    This refines 3.30.3's behavior change to restore behavior of nested
+    directories that call :command:`project` with the same project name,
+    and it addresses the bug in the implementation introduced in 3.30.4.

Source/cmProjectCommand.cxx

@@ -59,7 +59,7 @@ bool cmProjectCommand(std::vector<std::string> const& args,
   mf.SetProjectName(projectName);
 
   std::string varName = cmStrCat(projectName, "_BINARY_DIR"_s);
-  bool nonCacheVarAlreadySet = mf.IsDefinitionSet(varName);
+  bool nonCacheVarAlreadySet = mf.IsNormalDefinitionSet(varName);
   mf.AddCacheDefinition(varName, mf.GetCurrentBinaryDirectory(),
                         "Value Computed by CMake", cmStateEnums::STATIC);
   if (nonCacheVarAlreadySet) {
@@ -67,7 +67,7 @@ bool cmProjectCommand(std::vector<std::string> const& args,
   }
 
   varName = cmStrCat(projectName, "_SOURCE_DIR"_s);
-  nonCacheVarAlreadySet = mf.IsDefinitionSet(varName);
+  nonCacheVarAlreadySet = mf.IsNormalDefinitionSet(varName);
   mf.AddCacheDefinition(varName, mf.GetCurrentSourceDirectory(),
                         "Value Computed by CMake", cmStateEnums::STATIC);
   if (nonCacheVarAlreadySet) {
@@ -82,7 +82,7 @@ bool cmProjectCommand(std::vector<std::string> const& args,
   mf.AddDefinitionBool("PROJECT_IS_TOP_LEVEL", mf.IsRootMakefile());
 
   varName = cmStrCat(projectName, "_IS_TOP_LEVEL"_s);
-  nonCacheVarAlreadySet = mf.IsDefinitionSet(varName);
+  nonCacheVarAlreadySet = mf.IsNormalDefinitionSet(varName);
   mf.AddCacheDefinition(varName, mf.IsRootMakefile() ? "ON" : "OFF",
                         "Value Computed by CMake", cmStateEnums::STATIC);
   if (nonCacheVarAlreadySet) {

Tests/RunCMake/project/RunCMakeTest.cmake

@@ -45,7 +45,6 @@ run_cmake(ProjectIsTopLevel)
 run_cmake(ProjectIsTopLevelMultiple)
 run_cmake(ProjectIsTopLevelSubdirectory)
 run_cmake(ProjectTwice)
-run_cmake(SameProjectVarsSubdir)
 run_cmake(VersionAndLanguagesEmpty)
 run_cmake(VersionEmpty)
 run_cmake(VersionInvalid)
@@ -63,4 +62,10 @@ run_cmake(CMP0096-WARN)
 run_cmake(CMP0096-OLD)
 run_cmake(CMP0096-NEW)
 
+# We deliberately run these twice to verify behavior of the second CMake run
+run_cmake(SameProjectVarsSubdir)
+set(RunCMake_TEST_NO_CLEAN 1)
+run_cmake(SameProjectVarsSubdir)
+set(RunCMake_TEST_NO_CLEAN 0)
+
 run_cmake(NoMinimumRequired)