From a239f23a987a063852c8f29040ef4eaeaebf3b9c Mon Sep 17 00:00:00 2001 From: Kyle Edwards Date: Tue, 21 Dec 2021 17:12:51 -0500 Subject: 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 | 36 +++++++----- Source/cmCMakePresetsGraph.h | 37 ++++++++++-- Source/cmCMakePresetsGraphReadJSON.cxx | 66 +++++++++++++++++++--- .../CMakePresets/UserInheritance-stderr.txt | 2 +- 4 files changed, 112 insertions(+), 29 deletions(-) diff --git a/Source/cmCMakePresetsGraph.cxx b/Source/cmCMakePresetsGraph.cxx index ebc243b..a8aad12 100644 --- a/Source/cmCMakePresetsGraph.cxx +++ b/Source/cmCMakePresetsGraph.cxx @@ -3,6 +3,7 @@ #include "cmCMakePresetsGraph.h" #include +#include #include #include #include @@ -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 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( diff --git a/Source/cmCMakePresetsGraph.h b/Source/cmCMakePresetsGraph.h index 937b281..c27a611 100644 --- a/Source/cmCMakePresetsGraph.h +++ b/Source/cmCMakePresetsGraph.h @@ -8,6 +8,7 @@ #include #include #include +#include #include #include @@ -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 ReachableFiles; + }; + class Preset { public: @@ -77,7 +88,7 @@ public: std::string Name; std::vector Inherits; bool Hidden; - bool User; + File* OriginFile; std::string DisplayName; std::string Description; @@ -321,12 +332,11 @@ public: std::vector TestPresetOrder; std::string SourceDir; - int Version; - int UserVersion; + std::vector> 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& inProgressFiles, + File*& file); void ClearPresets(); }; diff --git a/Source/cmCMakePresetsGraphReadJSON.cxx b/Source/cmCMakePresetsGraphReadJSON.cxx index 3e002fe..aa5c9d4 100644 --- a/Source/cmCMakePresetsGraphReadJSON.cxx +++ b/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 #include #include #include +#include #include #include @@ -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& 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 = 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; } diff --git a/Tests/RunCMake/CMakePresets/UserInheritance-stderr.txt b/Tests/RunCMake/CMakePresets/UserInheritance-stderr.txt index 213215a..5ad8b4b 100644 --- a/Tests/RunCMake/CMakePresets/UserInheritance-stderr.txt +++ b/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$ -- cgit v0.12