Browse Source

Fix CMAKE_PARENT_LIST_FILE after return from include() or find_package()

Fix the implementation, clarify the documentation, and add tests.

Fixes: #25026
Co-authored-by: Brad King <[email protected]>
Benjamin Buch 8 months ago
parent
commit
a9ea55f0d7

+ 8 - 2
Help/variable/CMAKE_PARENT_LIST_FILE.rst

@@ -5,5 +5,11 @@ Full path to the CMake file that included the current one.
 
 While processing a CMake file loaded by :command:`include` or
 :command:`find_package` this variable contains the full path to the file
-including it.  The top of the include stack is always the ``CMakeLists.txt``
-for the current directory.  See also :variable:`CMAKE_CURRENT_LIST_FILE`.
+including it.
+
+While processing a ``CMakeLists.txt`` file, even in subdirectories,
+this variable has the same value as :variable:`CMAKE_CURRENT_LIST_FILE`.
+While processing a :option:`cmake -P` script, this variable is not defined
+in the outermost script.
+
+See also :variable:`CMAKE_CURRENT_LIST_FILE`.

+ 41 - 23
Source/cmMakefile.cxx

@@ -100,11 +100,46 @@ class FileScopeBase
 protected:
   cmMakefile* Makefile;
 
+private:
+  std::string OldCurrent;
+  cm::optional<std::string> OldParent;
+
 public:
   FileScopeBase(cmMakefile* mf)
     : Makefile(mf)
   {
   }
+  void PushListFileVars(std::string const& newCurrent)
+  {
+    if (cmValue p = this->Makefile->GetDefinition(kCMAKE_PARENT_LIST_FILE)) {
+      this->OldParent = *p;
+    }
+    if (cmValue c = this->Makefile->GetDefinition(kCMAKE_CURRENT_LIST_FILE)) {
+      this->OldCurrent = *c;
+      this->Makefile->AddDefinition(kCMAKE_PARENT_LIST_FILE, this->OldCurrent);
+      this->Makefile->MarkVariableAsUsed(kCMAKE_PARENT_LIST_FILE);
+    }
+    this->Makefile->AddDefinition(kCMAKE_CURRENT_LIST_FILE, newCurrent);
+    this->Makefile->AddDefinition(kCMAKE_CURRENT_LIST_DIR,
+                                  cmSystemTools::GetFilenamePath(newCurrent));
+    this->Makefile->MarkVariableAsUsed(kCMAKE_CURRENT_LIST_FILE);
+    this->Makefile->MarkVariableAsUsed(kCMAKE_CURRENT_LIST_DIR);
+  }
+  void PopListFileVars()
+  {
+    if (this->OldParent) {
+      this->Makefile->AddDefinition(kCMAKE_PARENT_LIST_FILE, *this->OldParent);
+      this->Makefile->MarkVariableAsUsed(kCMAKE_PARENT_LIST_FILE);
+    } else {
+      this->Makefile->RemoveDefinition(kCMAKE_PARENT_LIST_FILE);
+    }
+    this->Makefile->AddDefinition(kCMAKE_CURRENT_LIST_FILE, this->OldCurrent);
+    this->Makefile->AddDefinition(
+      kCMAKE_CURRENT_LIST_DIR,
+      cmSystemTools::GetFilenamePath(this->OldCurrent));
+    this->Makefile->MarkVariableAsUsed(kCMAKE_CURRENT_LIST_FILE);
+    this->Makefile->MarkVariableAsUsed(kCMAKE_CURRENT_LIST_DIR);
+  }
 };
 }
 
@@ -610,10 +645,12 @@ cmMakefile::IncludeScope::IncludeScope(cmMakefile* mf,
   if (!this->NoPolicyScope) {
     this->Makefile->PushPolicy();
   }
+  this->PushListFileVars(filenametoread);
 }
 
 cmMakefile::IncludeScope::~IncludeScope()
 {
+  this->PopListFileVars();
   if (!this->NoPolicyScope) {
     // Pop the scope we pushed for the script.
     this->Makefile->PopPolicy();
@@ -628,9 +665,6 @@ cmMakefile::IncludeScope::~IncludeScope()
 bool cmMakefile::ReadDependentFile(std::string const& filename,
                                    bool noPolicyScope)
 {
-  if (cmValue def = this->GetDefinition(kCMAKE_CURRENT_LIST_FILE)) {
-    this->AddDefinition(kCMAKE_PARENT_LIST_FILE, *def);
-  }
   std::string filenametoread = cmSystemTools::CollapseFullPath(
     filename, this->GetCurrentSourceDirectory());
 
@@ -685,10 +719,12 @@ public:
     assert(this->Makefile->StateSnapshot.IsValid());
 
     this->Makefile->PushFunctionBlockerBarrier();
+    this->PushListFileVars(filenametoread);
   }
 
   ~ListFileScope()
   {
+    this->PopListFileVars();
     this->Makefile->PopSnapshot(this->ReportError);
     this->Makefile->PopFunctionBlockerBarrier(this->ReportError);
     this->Makefile->Backtrace = this->Makefile->Backtrace.Pop();
@@ -826,18 +862,6 @@ void cmMakefile::RunListFile(cmListFile const& listFile,
   // add this list file to the list of dependencies
   this->ListFiles.push_back(filenametoread);
 
-  std::string currentParentFile =
-    this->GetSafeDefinition(kCMAKE_PARENT_LIST_FILE);
-  std::string currentFile = this->GetSafeDefinition(kCMAKE_CURRENT_LIST_FILE);
-
-  this->AddDefinition(kCMAKE_CURRENT_LIST_FILE, filenametoread);
-  this->AddDefinition(kCMAKE_CURRENT_LIST_DIR,
-                      cmSystemTools::GetFilenamePath(filenametoread));
-
-  this->MarkVariableAsUsed(kCMAKE_PARENT_LIST_FILE);
-  this->MarkVariableAsUsed(kCMAKE_CURRENT_LIST_FILE);
-  this->MarkVariableAsUsed(kCMAKE_CURRENT_LIST_DIR);
-
   // Run the parsed commands.
   size_t const numberFunctions = listFile.Functions.size();
   for (size_t i = 0; i < numberFunctions; ++i) {
@@ -884,14 +908,6 @@ void cmMakefile::RunListFile(cmListFile const& listFile,
       }
     }
   }
-
-  this->AddDefinition(kCMAKE_PARENT_LIST_FILE, currentParentFile);
-  this->AddDefinition(kCMAKE_CURRENT_LIST_FILE, currentFile);
-  this->AddDefinition(kCMAKE_CURRENT_LIST_DIR,
-                      cmSystemTools::GetFilenamePath(currentFile));
-  this->MarkVariableAsUsed(kCMAKE_PARENT_LIST_FILE);
-  this->MarkVariableAsUsed(kCMAKE_CURRENT_LIST_FILE);
-  this->MarkVariableAsUsed(kCMAKE_CURRENT_LIST_DIR);
 }
 
 void cmMakefile::EnforceDirectoryLevelRules() const
@@ -1482,6 +1498,7 @@ public:
       this->Makefile->StateSnapshot.GetState()->CreatePolicyScopeSnapshot(
         this->Makefile->StateSnapshot);
     this->Makefile->PushFunctionBlockerBarrier();
+    this->PushListFileVars(currentStart);
 
     this->GG = mf->GetGlobalGenerator();
     this->CurrentMakefile = this->GG->GetCurrentMakefile();
@@ -1495,6 +1512,7 @@ public:
 
   ~BuildsystemFileScope()
   {
+    this->PopListFileVars();
     this->Makefile->PopFunctionBlockerBarrier(this->ReportError);
     this->Makefile->PopSnapshot(this->ReportError);
 #if !defined(CMAKE_BOOTSTRAP)

+ 2 - 2
Tests/RunCMake/find_package/ParentVariable-stdout.txt

@@ -1,5 +1,5 @@
 -- ParentVariable\.cmake: '[^']*/Tests/RunCMake/find_package/CMakeLists\.txt'
 -- ParentVariable/PrimaryConfig\.cmake: '[^']*/Tests/RunCMake/find_package/ParentVariable\.cmake'
 -- ParentVariable/SecondaryConfig\.cmake: '[^']*/Tests/RunCMake/find_package/ParentVariable/PrimaryConfig\.cmake'
--- ParentVariable/PrimaryConfig\.cmake: '[^']*/Tests/RunCMake/find_package/ParentVariable/PrimaryConfig\.cmake'
--- ParentVariable\.cmake: '[^']*/Tests/RunCMake/find_package/ParentVariable\.cmake'
+-- ParentVariable/PrimaryConfig\.cmake: '[^']*/Tests/RunCMake/find_package/ParentVariable\.cmake'
+-- ParentVariable\.cmake: '[^']*/Tests/RunCMake/find_package/CMakeLists\.txt'

+ 1 - 1
Tests/RunCMake/include/ParentVariableRoot-stdout.txt

@@ -1,5 +1,5 @@
 -- CMakeLists\.txt: '[^']*/Tests/RunCMake/include/CMakeLists\.txt'
 -- ParentVariableRoot/include1\.cmake: '[^']*/Tests/RunCMake/include/CMakeLists\.txt'
 -- ParentVariableRoot/include2\.cmake: '[^']*/Tests/RunCMake/include/ParentVariableRoot/include1\.cmake'
--- ParentVariableRoot/include1\.cmake: '[^']*/Tests/RunCMake/include/ParentVariableRoot/include1\.cmake'
+-- ParentVariableRoot/include1\.cmake: '[^']*/Tests/RunCMake/include/CMakeLists\.txt'
 -- CMakeLists\.txt: '[^']*/Tests/RunCMake/include/CMakeLists\.txt'

+ 2 - 2
Tests/RunCMake/include/ParentVariableScript-stdout.txt

@@ -1,5 +1,5 @@
 -- ParentVariableScript\.cmake: ''
 -- ParentVariableScript/include1\.cmake: '[^']*/Tests/RunCMake/include/ParentVariableScript\.cmake'
 -- ParentVariableScript/include2\.cmake: '[^']*/Tests/RunCMake/include/ParentVariableScript/include1\.cmake'
--- ParentVariableScript/include1\.cmake: '[^']*/Tests/RunCMake/include/ParentVariableScript/include1\.cmake'
--- ParentVariableScript\.cmake: '[^']*/Tests/RunCMake/include/ParentVariableScript\.cmake'
+-- ParentVariableScript/include1\.cmake: '[^']*/Tests/RunCMake/include/ParentVariableScript\.cmake'
+-- ParentVariableScript\.cmake: ''

+ 3 - 0
Tests/RunCMake/include/ParentVariableScript.cmake

@@ -3,4 +3,7 @@ if(DEFINED CMAKE_PARENT_LIST_FILE)
 endif()
 message(STATUS "ParentVariableScript.cmake: '${CMAKE_PARENT_LIST_FILE}'")
 include("${CMAKE_CURRENT_LIST_DIR}/ParentVariableScript/include1.cmake")
+if(DEFINED CMAKE_PARENT_LIST_FILE)
+  message(SEND_ERROR "`CMAKE_PARENT_LIST_FILE` is not expected to be set here")
+endif()
 message(STATUS "ParentVariableScript.cmake: '${CMAKE_PARENT_LIST_FILE}'")

+ 2 - 2
Tests/RunCMake/include/ParentVariableSubDir-stdout.txt

@@ -1,7 +1,7 @@
 -- ParentVariableSubDir/CMakeLists\.txt: '[^']*/Tests/RunCMake/include/ParentVariableSubDir/CMakeLists\.txt'
 -- ParentVariableSubDir/include1\.cmake: '[^']*/Tests/RunCMake/include/ParentVariableSubDir/CMakeLists\.txt'
 -- ParentVariableSubDir/Inc/include2.cmake: '[^']*/Tests/RunCMake/include/ParentVariableSubDir/include1\.cmake'
--- ParentVariableSubDir/include1\.cmake: '[^']*/Tests/RunCMake/include/ParentVariableSubDir/include1\.cmake'
+-- ParentVariableSubDir/include1\.cmake: '[^']*/Tests/RunCMake/include/ParentVariableSubDir/CMakeLists\.txt'
 -- ParentVariableSubDir/Inc/CMakeLists\.txt: '[^']*/Tests/RunCMake/include/ParentVariableSubDir/include1\.cmake'
--- ParentVariableSubDir/include1\.cmake: '[^']*/Tests/RunCMake/include/ParentVariableSubDir/include1\.cmake'
+-- ParentVariableSubDir/include1\.cmake: '[^']*/Tests/RunCMake/include/ParentVariableSubDir/CMakeLists\.txt'
 -- ParentVariableSubDir/CMakeLists\.txt: '[^']*/Tests/RunCMake/include/ParentVariableSubDir/CMakeLists\.txt'