Refactor: Generalize file graph in CMakePresets

Before this refactoring, presets had a simple flag that marked them
as "user" or "not user", and checking the file graph of two files
was as simple as checking this flag. This only allowed for two files
in the graph. Generalize the code to allow for arbitrarily many files
in the graph.

Source/cmCMakePresetsGraph.cxx

@@ -3,6 +3,7 @@
 #include "cmCMakePresetsGraph.h"
 
 #include <algorithm>
+#include <cassert>
 #include <cstdlib>
 #include <functional>
 #include <iostream>
@@ -106,8 +107,8 @@ ReadFileResult VisitPreset(
     }
 
     auto& parentPreset = parent->second.Unexpanded;
-    if (!preset.User && parentPreset.User) {
-      return ReadFileResult::USER_PRESET_INHERITANCE;
+    if (!preset.OriginFile->ReachableFiles.count(parentPreset.OriginFile)) {
+      return ReadFileResult::PRESET_UNREACHABLE_FROM_FILE;
     }
 
     auto result = VisitPreset(parentPreset, presets, cycleStatus, graph);
@@ -876,23 +877,28 @@ cmCMakePresetsGraph::ReadProjectPresetsInternal(bool allowNoFiles)
 {
   bool haveOneFile = false;
 
+  File* file;
   std::string filename = GetUserFilename(this->SourceDir);
+  std::vector<File*> inProgressFiles;
   if (cmSystemTools::FileExists(filename)) {
-    auto result = this->ReadJSONFile(filename, true);
+    auto result = this->ReadJSONFile(filename, RootType::User,
+                                     ReadReason::Root, inProgressFiles, file);
     if (result != ReadFileResult::READ_OK) {
       return result;
     }
     haveOneFile = true;
-  }
-
-  filename = GetFilename(this->SourceDir);
-  if (cmSystemTools::FileExists(filename)) {
-    auto result = this->ReadJSONFile(filename, false);
-    if (result != ReadFileResult::READ_OK) {
-      return result;
+  } else {
+    filename = GetFilename(this->SourceDir);
+    if (cmSystemTools::FileExists(filename)) {
+      auto result = this->ReadJSONFile(
+        filename, RootType::Project, ReadReason::Root, inProgressFiles, file);
+      if (result != ReadFileResult::READ_OK) {
+        return result;
+      }
+      haveOneFile = true;
     }
-    haveOneFile = true;
   }
+  assert(inProgressFiles.empty());
 
   if (!haveOneFile) {
     return allowNoFiles ? ReadFileResult::READ_OK
@@ -983,8 +989,8 @@ const char* cmCMakePresetsGraph::ResultToString(ReadFileResult result)
       return "Duplicate presets";
     case ReadFileResult::CYCLIC_PRESET_INHERITANCE:
       return "Cyclic preset inheritance";
-    case ReadFileResult::USER_PRESET_INHERITANCE:
-      return "Project preset inherits from user preset";
+    case ReadFileResult::PRESET_UNREACHABLE_FROM_FILE:
+      return "Inherited preset is unreachable from preset's file";
     case ReadFileResult::INVALID_MACRO_EXPANSION:
       return "Invalid macro expansion";
     case ReadFileResult::BUILD_TEST_PRESETS_UNSUPPORTED:
@@ -1002,6 +1008,8 @@ const char* cmCMakePresetsGraph::ResultToString(ReadFileResult result)
     case ReadFileResult::TOOLCHAIN_FILE_UNSUPPORTED:
       return "File version must be 3 or higher for toolchainFile preset "
              "support.";
+    case ReadFileResult::CYCLIC_INCLUDE:
+      return "Cyclic include among preset files";
   }
 
   return "Unknown error";
@@ -1016,6 +1024,8 @@ void cmCMakePresetsGraph::ClearPresets()
   this->ConfigurePresetOrder.clear();
   this->BuildPresetOrder.clear();
   this->TestPresetOrder.clear();
+
+  this->Files.clear();
 }
 
 void cmCMakePresetsGraph::PrintPresets(

Source/cmCMakePresetsGraph.h

@@ -8,6 +8,7 @@
 #include <map>
 #include <memory>
 #include <string>
+#include <unordered_set>
 #include <utility>
 #include <vector>
 
@@ -32,7 +33,7 @@ public:
     INVALID_VARIABLE,
     DUPLICATE_PRESETS,
     CYCLIC_PRESET_INHERITANCE,
-    USER_PRESET_INHERITANCE,
+    PRESET_UNREACHABLE_FROM_FILE,
     INVALID_MACRO_EXPANSION,
     BUILD_TEST_PRESETS_UNSUPPORTED,
     INVALID_CONFIGURE_PRESET,
@@ -40,6 +41,7 @@ public:
     INVALID_CONDITION,
     CONDITION_UNSUPPORTED,
     TOOLCHAIN_FILE_UNSUPPORTED,
+    CYCLIC_INCLUDE,
   };
 
   enum class ArchToolsetStrategy
@@ -57,6 +59,15 @@ public:
 
   class Condition;
 
+  class File
+  {
+  public:
+    std::string Filename;
+    int Version;
+
+    std::unordered_set<File*> ReachableFiles;
+  };
+
   class Preset
   {
   public:
@@ -77,7 +88,7 @@ public:
     std::string Name;
     std::vector<std::string> Inherits;
     bool Hidden;
-    bool User;
+    File* OriginFile;
     std::string DisplayName;
     std::string Description;
 
@@ -321,12 +332,11 @@ public:
   std::vector<std::string> TestPresetOrder;
 
   std::string SourceDir;
-  int Version;
-  int UserVersion;
+  std::vector<std::unique_ptr<File>> Files;
 
   int GetVersion(const Preset& preset) const
   {
-    return preset.User ? this->UserVersion : this->Version;
+    return preset.OriginFile->Version;
   }
 
   static std::string GetFilename(const std::string& sourceDir);
@@ -372,7 +382,22 @@ public:
   void PrintAllPresets() const;
 
 private:
+  enum class RootType
+  {
+    Project,
+    User,
+  };
+
+  enum class ReadReason
+  {
+    Root,
+    Included,
+  };
+
   ReadFileResult ReadProjectPresetsInternal(bool allowNoFiles);
-  ReadFileResult ReadJSONFile(const std::string& filename, bool user);
+  ReadFileResult ReadJSONFile(const std::string& filename, RootType rootType,
+                              ReadReason readReason,
+                              std::vector<File*>& inProgressFiles,
+                              File*& file);
   void ClearPresets();
 };

Source/cmCMakePresetsGraphReadJSON.cxx

@@ -1,8 +1,10 @@
 /* Distributed under the OSI-approved BSD 3-Clause License.  See accompanying
    file Copyright.txt or https://cmake.org/licensing for details.  */
+#include <algorithm>
 #include <functional>
 #include <map>
 #include <string>
+#include <unordered_set>
 #include <utility>
 #include <vector>
 
@@ -18,6 +20,8 @@
 #include "cmCMakePresetsGraph.h"
 #include "cmCMakePresetsGraphInternal.h"
 #include "cmJSONHelpers.h"
+#include "cmStringAlgorithms.h"
+#include "cmSystemTools.h"
 #include "cmVersion.h"
 
 namespace {
@@ -406,8 +410,21 @@ cmCMakePresetsGraph::ReadFileResult EnvironmentMapHelper(
 }
 
 cmCMakePresetsGraph::ReadFileResult cmCMakePresetsGraph::ReadJSONFile(
-  const std::string& filename, bool user)
+  const std::string& filename, RootType rootType, ReadReason readReason,
+  std::vector<File*>& inProgressFiles, File*& file)
 {
+  for (auto const& f : this->Files) {
+    if (cmSystemTools::SameFile(filename, f->Filename)) {
+      file = f.get();
+      auto fileIt =
+        std::find(inProgressFiles.begin(), inProgressFiles.end(), file);
+      if (fileIt != inProgressFiles.end()) {
+        return cmCMakePresetsGraph::ReadFileResult::CYCLIC_INCLUDE;
+      }
+      return cmCMakePresetsGraph::ReadFileResult::READ_OK;
+    }
+  }
+
   cmsys::ifstream fin(filename.c_str());
   if (!fin) {
     return ReadFileResult::FILE_NOT_FOUND;
@@ -430,11 +447,6 @@ cmCMakePresetsGraph::ReadFileResult cmCMakePresetsGraph::ReadJSONFile(
   if (v < MIN_VERSION || v > MAX_VERSION) {
     return ReadFileResult::UNRECOGNIZED_VERSION;
   }
-  if (user) {
-    this->UserVersion = v;
-  } else {
-    this->Version = v;
-  }
 
   // Support for build and test presets added in version 2.
   if (v < 2 &&
@@ -460,8 +472,16 @@ cmCMakePresetsGraph::ReadFileResult cmCMakePresetsGraph::ReadJSONFile(
     return ReadFileResult::UNRECOGNIZED_CMAKE_VERSION;
   }
 
+  auto filePtr = cm::make_unique<File>();
+  file = filePtr.get();
+  this->Files.emplace_back(std::move(filePtr));
+  inProgressFiles.emplace_back(file);
+  file->Filename = filename;
+  file->Version = v;
+  file->ReachableFiles.insert(file);
+
   for (auto& preset : presets.ConfigurePresets) {
-    preset.User = user;
+    preset.OriginFile = file;
     if (preset.Name.empty()) {
       return ReadFileResult::INVALID_PRESET;
     }
@@ -494,7 +514,7 @@ cmCMakePresetsGraph::ReadFileResult cmCMakePresetsGraph::ReadJSONFile(
   }
 
   for (auto& preset : presets.BuildPresets) {
-    preset.User = user;
+    preset.OriginFile = file;
     if (preset.Name.empty()) {
       return ReadFileResult::INVALID_PRESET;
     }
@@ -515,7 +535,7 @@ cmCMakePresetsGraph::ReadFileResult cmCMakePresetsGraph::ReadJSONFile(
   }
 
   for (auto& preset : presets.TestPresets) {
-    preset.User = user;
+    preset.OriginFile = file;
     if (preset.Name.empty()) {
       return ReadFileResult::INVALID_PRESET;
     }
@@ -535,5 +555,33 @@ cmCMakePresetsGraph::ReadFileResult cmCMakePresetsGraph::ReadJSONFile(
     this->TestPresetOrder.push_back(preset.Name);
   }
 
+  auto const includeFile = [this, &inProgressFiles, file](
+                             const std::string& include, RootType rootType2,
+                             ReadReason readReason2) -> ReadFileResult {
+    ReadFileResult r;
+    File* includedFile;
+    if ((r = this->ReadJSONFile(include, rootType2, readReason2,
+                                inProgressFiles, includedFile)) !=
+        ReadFileResult::READ_OK) {
+      return r;
+    }
+
+    file->ReachableFiles.insert(includedFile->ReachableFiles.begin(),
+                                includedFile->ReachableFiles.end());
+    return ReadFileResult::READ_OK;
+  };
+
+  if (rootType == RootType::User && readReason == ReadReason::Root) {
+    auto cmakePresetsFilename = GetFilename(this->SourceDir);
+    if (cmSystemTools::FileExists(cmakePresetsFilename)) {
+      if ((result = includeFile(cmakePresetsFilename, RootType::Project,
+                                ReadReason::Root)) !=
+          ReadFileResult::READ_OK) {
+        return result;
+      }
+    }
+  }
+
+  inProgressFiles.pop_back();
   return ReadFileResult::READ_OK;
 }

Tests/RunCMake/CMakePresets/UserInheritance-stderr.txt

@@ -1,2 +1,2 @@
 ^CMake Error: Could not read presets from [^
-]*/Tests/RunCMake/CMakePresets/UserInheritance: Project preset inherits from user preset$
+]*/Tests/RunCMake/CMakePresets/UserInheritance: Inherited preset is unreachable from preset's file$