vkconfig: March 2025 SDK fixes

- Clean up environment variables - Add ignoring invalid layer manifest when invalid until the file is modified Change-Id: I76b17ea279ce0b0445e4373cb19606a72de2644c
LunarG · Mar 3, 2025 · 0a23713 · 0a23713
1 parent fafc544
commit 0a23713
Show file tree

Hide file tree

Showing 17 changed files with 103 additions and 61 deletions.
diff --git a/vkconfig_core/executable.cpp b/vkconfig_core/executable.cpp
@@ -170,7 +170,7 @@ Executable::Executable(const DefaultExecutable& default_executable) {
     // initially will be set to the users home folder across all OS's. This is highly visible
     // in the application launcher and should not present a usability issue. The developer can
     // easily change this later to anywhere they like.
-    options.log_file = std::string("${VK_HOME}/") + default_executable.name + ".txt";
+    options.log_file = std::string("${VULKAN_HOME}/") + default_executable.name + ".txt";
 
     this->path = default_paths.executable_path;
     this->options_list.push_back(options);
@@ -180,7 +180,7 @@ Executable::Executable(const DefaultExecutable& default_executable) {
 Executable::Executable(const Path& executable_path) {
     ExecutableOptions options;
     options.working_folder = executable_path.AbsoluteDir();
-    options.log_file = std::string("${VK_HOME}/") + executable_path.Basename() + ".txt";
+    options.log_file = std::string("${VULKAN_HOME}/") + executable_path.Basename() + ".txt";
 
     this->path = executable_path;
     this->options_list.push_back(options);

diff --git a/vkconfig_core/layer.cpp b/vkconfig_core/layer.cpp
@@ -195,7 +195,7 @@ SettingMeta* Layer::Instantiate(SettingMetaSet& meta_set, const std::string& key
 }
 
 LayerLoadStatus Layer::Load(const Path& full_path_to_file, LayerType type, bool request_validate_manifest,
-                            const std::map<Path, std::string>& layers_validated, ConfiguratorMode configurator_mode) {
+                            const std::map<Path, LayerStatus>& layers_found, ConfiguratorMode configurator_mode) {
     this->type = type;  // Set layer type, no way to know this from the json file
 
     if (full_path_to_file.Empty()) {
@@ -211,6 +211,14 @@ LayerLoadStatus Layer::Load(const Path& full_path_to_file, LayerType type, bool
     file.close();
 
     this->manifest_path = full_path_to_file;
+    this->last_modified = full_path_to_file.LastModified();
+
+    auto it = layers_found.find(full_path_to_file.AbsolutePath().c_str());
+    if (it != layers_found.end()) {
+        if (it->second.disabled && it->second.last_modified == this->last_modified) {
+            return LAYER_LOAD_FAILED;
+        }
+    }
 
     // Convert the text to a JSON document & validate it.
     // It does need to be a valid json formatted file.
@@ -258,8 +266,6 @@ LayerLoadStatus Layer::Load(const Path& full_path_to_file, LayerType type, bool
 
     const QJsonObject& json_layer_object = ReadObject(json_root_object, "layer");
 
-    const std::string& last_modified = full_path_to_file.LastModified();
-
     this->key = ReadStringValue(json_layer_object, "name");
 
     if (this->key == "VK_LAYER_LUNARG_override") {
@@ -271,16 +277,14 @@ LayerLoadStatus Layer::Load(const Path& full_path_to_file, LayerType type, bool
     JsonValidator validator;
 
     std::string cached_last_modified;
-    auto it = layers_validated.find(this->manifest_path.AbsolutePath());
-    if (it != layers_validated.end()) {
-        cached_last_modified = it->second;
+    if (it != layers_found.end()) {
+        cached_last_modified = it->second.last_modified;
     }
-    const bool should_validate =
-        request_validate_manifest && last_modified != cached_last_modified && !this->manifest_path.IsBuiltIn();
+
+    const bool should_validate = request_validate_manifest && ((last_modified != cached_last_modified) || !it->second.validated);
     const bool is_valid = should_validate ? validator.Check(json_text) : true;
 
     if (!is_valid) {
-        this->validated_last_modified.clear();
         switch (configurator_mode) {
             default: {
             } break;
@@ -308,8 +312,6 @@ LayerLoadStatus Layer::Load(const Path& full_path_to_file, LayerType type, bool
             } break;
         }
         return LAYER_LOAD_INVALID;
-    } else if (request_validate_manifest) {
-        this->validated_last_modified = last_modified;
     }
 
     const QJsonValue& json_library_path_value = json_layer_object.value("library_path");
@@ -435,6 +437,10 @@ void Layer::AddSettingsSet(SettingMetaSet& settings, const SettingMeta* parent,
 
         const std::string desc = ReadStringValue(json_setting, "description");
         const SettingType type = GetSettingType(ReadStringValue(json_setting, "type").c_str());
+        if (type == SETTING_INVALID) {
+            continue;
+        }
+
         SettingView view = SETTING_VIEW_STANDARD;
         if (json_setting.value("view") != QJsonValue::Undefined) {
             view = GetSettingView(ReadStringValue(json_setting, "view").c_str());

diff --git a/vkconfig_core/layer.h b/vkconfig_core/layer.h
@@ -36,6 +36,12 @@
 #include <vector>
 #include <string>
 
+struct LayerStatus {
+    std::string last_modified;
+    bool validated = false;
+    bool disabled = false;
+};
+
 struct LayersPathInfo {
     Path path;
     LayerType type = LAYER_TYPE_EXPLICIT;
@@ -58,6 +64,10 @@ enum LayerLoadStatus {
     LAYER_LOAD_LAST = LAYER_LOAD_IGNORED,
 };
 
+inline bool IsDisabled(LayerLoadStatus status) {
+    return status == LAYER_LOAD_FAILED || status == LAYER_LOAD_INVALID || status == LAYER_LOAD_IGNORED;
+}
+
 enum { LAYER_LOAD_COUNT = LAYER_LOAD_LAST - LAYER_LOAD_FIRST + 1 };
 
 class Layer {
@@ -88,7 +98,7 @@ class Layer {
     Path binary_path;
     Version api_version;
     std::string implementation_version;
-    std::string validated_last_modified;
+    std::string last_modified;
     StatusType status;
     std::string description;
     std::string introduction;
@@ -107,7 +117,7 @@ class Layer {
     std::vector<LayerPreset> presets;
 
     LayerLoadStatus Load(const Path& full_path_to_file, LayerType type, bool request_validate_manifest,
-                         const std::map<Path, std::string>& layers_validated, ConfiguratorMode configurator_mode);
+                         const std::map<Path, LayerStatus>& layers_found, ConfiguratorMode configurator_mode);
 
    private:
     Layer& operator=(const Layer&) = delete;

diff --git a/vkconfig_core/layer_manager.cpp b/vkconfig_core/layer_manager.cpp
@@ -188,15 +188,21 @@ bool LayerManager::Load(const QJsonObject &json_root_object, ConfiguratorMode co
             this->validate_manifests = json_layers_object.value("validate_manifests").toBool();
         }
 
-        if (json_layers_object.value("validated") != QJsonValue::Undefined) {
-            const QJsonObject &json_layers_validated_object = json_layers_object.value("validated").toObject();
-            const QStringList &json_layers_validated_keys = json_layers_validated_object.keys();
-
-            for (int i = 0, n = json_layers_validated_keys.length(); i < n; ++i) {
-                const Path &manifest_path = json_layers_validated_keys[i].toStdString();
-                const std::string &last_modified =
-                    json_layers_validated_object.value(json_layers_validated_keys[i]).toString().toStdString();
-                this->layers_validated.insert(std::make_pair(manifest_path, last_modified));
+        if (json_layers_object.value("found") != QJsonValue::Undefined) {
+            const QJsonObject &json_layers_found_object = json_layers_object.value("found").toObject();
+            const QStringList &json_layers_found_keys = json_layers_found_object.keys();
+
+            for (int i = 0, n = json_layers_found_keys.length(); i < n; ++i) {
+                const QJsonObject &json_status_object = json_layers_found_object.value(json_layers_found_keys[i]).toObject();
+
+                LayerStatus layer_status;
+                layer_status.last_modified = json_status_object.value("last_modified").toString().toStdString();
+                layer_status.validated = json_status_object.value("validated").toBool();
+                layer_status.disabled = json_status_object.value("disabled").toBool();
+
+                const Path &manifest_path = json_layers_found_keys[i].toStdString();
+
+                this->layers_found.insert(std::make_pair(manifest_path, layer_status));
             }
         }
 
@@ -219,11 +225,13 @@ bool LayerManager::Load(const QJsonObject &json_root_object, ConfiguratorMode co
 }
 
 bool LayerManager::Save(QJsonObject &json_root_object) const {
-    QJsonObject json_layers_paths_object;
-    for (std::size_t i = 0, n = this->available_layers.size(); i < n; ++i) {
-        const Layer &layer = this->available_layers[i];
-
-        json_layers_paths_object.insert(layer.manifest_path.AbsolutePath().c_str(), layer.validated_last_modified.c_str());
+    QJsonObject json_layers_status_object;
+    for (auto it = this->layers_found.begin(); it != this->layers_found.end(); ++it) {
+        QJsonObject json_layer_status_object;
+        json_layer_status_object.insert("last_modified", it->second.last_modified.c_str());
+        json_layer_status_object.insert("validated", it->second.validated);
+        json_layer_status_object.insert("disabled", it->second.disabled);
+        json_layers_status_object.insert(it->first.AbsolutePath().c_str(), json_layer_status_object);
     }
 
     QJsonObject json_paths_object;
@@ -238,7 +246,7 @@ bool LayerManager::Save(QJsonObject &json_root_object) const {
     QJsonObject json_layers_object;
     json_layers_object.insert("validate_manifests", this->validate_manifests);
     json_layers_object.insert("last_layers_path", this->last_layers_path.RelativePath().c_str());
-    json_layers_object.insert("validated", json_layers_paths_object);
+    json_layers_object.insert("found", json_layers_status_object);
     json_layers_object.insert("paths", json_paths_object);
 
     json_root_object.insert("layers", json_layers_object);
@@ -303,7 +311,7 @@ std::string LayerManager::Log() const {
 
 void LayerManager::InitSystemPaths() {
     this->available_layers.clear();
-    this->layers_validated.clear();
+    this->layers_found.clear();
 
     this->paths[LAYERS_PATHS_IMPLICIT_SYSTEM] = GetImplicitLayerPaths();
 
@@ -424,7 +432,7 @@ const Layer *LayerManager::FindLastModified(const std::string &layer_name, const
             continue;
         }
         if (result != nullptr) {
-            if (result->validated_last_modified > this->available_layers[i].validated_last_modified) {
+            if (result->last_modified > this->available_layers[i].last_modified) {
                 continue;
             }
         }
@@ -490,28 +498,41 @@ LayerLoadStatus LayerManager::LoadLayer(const Path &layer_path, LayerType type,
     Layer *already_loaded_layer = this->FindFromManifest(layer_path, true);
     if (already_loaded_layer != nullptr) {
         // Already loaded
-        auto it = this->layers_validated.find(layer_path);
-        if (it != layers_validated.end()) {
-            if (last_modified == it->second) {
+        auto it = this->layers_found.find(layer_path);
+        if (it != layers_found.end()) {
+            if (last_modified == it->second.last_modified) {
                 return LAYER_LOAD_UNMODIFIED;
             }
         }
 
         // Modified to reload
         LayerLoadStatus status =
-            already_loaded_layer->Load(layer_path, type, this->validate_manifests, this->layers_validated, configurator_mode);
+            already_loaded_layer->Load(layer_path, type, this->validate_manifests, this->layers_found, configurator_mode);
         if (status == LAYER_LOAD_ADDED) {
-            it->second = already_loaded_layer->validated_last_modified;
+            it->second.last_modified = already_loaded_layer->last_modified;
             return LAYER_LOAD_RELOADED;
         } else {
+            it->second.disabled = IsDisabled(status);
             return status;
         }
     } else {
         Layer layer;
-        LayerLoadStatus status = layer.Load(layer_path, type, this->validate_manifests, this->layers_validated, configurator_mode);
+        LayerLoadStatus status = layer.Load(layer_path, type, this->validate_manifests, this->layers_found, configurator_mode);
         if (status == LAYER_LOAD_ADDED) {
             this->available_layers.push_back(layer);
-            this->layers_validated.insert(std::make_pair(layer.manifest_path, layer.validated_last_modified));
+        }
+
+        auto it = this->layers_found.find(layer_path);
+        if (it != layers_found.end()) {
+            it->second.disabled = IsDisabled(status);
+            it->second.validated = this->validate_manifests && !it->second.disabled;
+            it->second.last_modified = layer.last_modified;
+        } else {
+            LayerStatus found;
+            found.disabled = IsDisabled(status);
+            found.validated = this->validate_manifests && !found.disabled;
+            found.last_modified = layer.last_modified;
+            this->layers_found.insert(std::make_pair(layer.manifest_path, found));
         }
 
         return status;

diff --git a/vkconfig_core/layer_manager.h b/vkconfig_core/layer_manager.h
@@ -70,5 +70,5 @@ class LayerManager : public Serialize {
     void InitSystemPaths();
     void UpdateLayersEnabled(const LayersPathInfo& path_info);
 
-    std::map<Path, std::string> layers_validated;
+    std::map<Path, LayerStatus> layers_found;
 };
diff --git a/vkconfig_core/setting.cpp b/vkconfig_core/setting.cpp
@@ -44,7 +44,6 @@ SettingType GetSettingType(const char* token) {
         if (ToUpperCase(token) == GetToken(type)) return type;
     }
 
-    assert(0);  // Unknown token
     return static_cast<SettingType>(-1);
 }
 

diff --git a/vkconfig_core/setting.h b/vkconfig_core/setting.h
@@ -39,6 +39,7 @@ enum SettingInputError {
 };
 
 enum SettingType {  // Enum value can't be changed
+    SETTING_INVALID = -1,
     SETTING_STRING = 0,
     SETTING_INT,
     SETTING_FLOAT,

diff --git a/vkconfig_core/test/test_executable.cpp b/vkconfig_core/test/test_executable.cpp
@@ -56,7 +56,7 @@ TEST(test_executable, ctor_path) {
     EXPECT_TRUE(!options->working_folder.Empty());
     EXPECT_EQ(true, options->args.empty());
     EXPECT_EQ(true, options->envs.empty());
-    EXPECT_STREQ(Path("${VK_HOME}/vkcube.txt").RelativePath().c_str(), options->log_file.RelativePath().c_str());
+    EXPECT_STREQ(Path("${VULKAN_HOME}/vkcube.txt").RelativePath().c_str(), options->log_file.RelativePath().c_str());
 }
 
 TEST(test_executable, remove_last) {
@@ -187,7 +187,7 @@ TEST(test_executable, DefaultExecutable) {
 #endif
     EXPECT_STREQ(options->args[0].c_str(), "--suppress_popups");
     EXPECT_TRUE(options->envs.empty());
-    EXPECT_STREQ(options->log_file.RelativePath().c_str(), Path("${VK_HOME}/vkcube.txt").RelativePath().c_str());
+    EXPECT_STREQ(options->log_file.RelativePath().c_str(), Path("${VULKAN_HOME}/vkcube.txt").RelativePath().c_str());
 
     qputenv("VULKAN_SDK", saved.c_str());
 }
diff --git a/vkconfig_core/test/test_executable_manager.cpp b/vkconfig_core/test/test_executable_manager.cpp
@@ -39,17 +39,17 @@ TEST(test_executable_manager, reset_default_applications_sdk_found) {
         EXPECT_TRUE(executables[0].path.RelativePath().find("${VULKAN_BIN}") != std::string::npos);
         const std::vector<ExecutableOptions>& options0 = executables[0].GetOptions();
         EXPECT_TRUE(options0[0].working_folder.RelativePath().find("${VULKAN_BIN}") != std::string::npos);
-        EXPECT_TRUE(options0[0].log_file.RelativePath().find("${VK_HOME}") != std::string::npos);
+        EXPECT_TRUE(options0[0].log_file.RelativePath().find("${VULKAN_HOME}") != std::string::npos);
 
         EXPECT_TRUE(executables[1].path.RelativePath().find("${VULKAN_BIN}") != std::string::npos);
         const std::vector<ExecutableOptions>& options1 = executables[1].GetOptions();
         EXPECT_TRUE(options1[0].working_folder.RelativePath().find("${VULKAN_BIN}") != std::string::npos);
-        EXPECT_TRUE(options1[0].log_file.RelativePath().find("${VK_HOME}") != std::string::npos);
+        EXPECT_TRUE(options1[0].log_file.RelativePath().find("${VULKAN_HOME}") != std::string::npos);
 
         EXPECT_TRUE(executables[2].path.RelativePath().find("${VULKAN_BIN}") != std::string::npos);
         const std::vector<ExecutableOptions>& options2 = executables[2].GetOptions();
         EXPECT_TRUE(options2[0].working_folder.RelativePath().find("${VULKAN_BIN}") != std::string::npos);
-        EXPECT_TRUE(options2[0].log_file.RelativePath().find("${VK_HOME}") != std::string::npos);
+        EXPECT_TRUE(options2[0].log_file.RelativePath().find("${VULKAN_HOME}") != std::string::npos);
     }
 }
 
@@ -66,17 +66,17 @@ TEST(test_executable_manager, reset_default_applications_no_sdk) {
     EXPECT_TRUE(executables[0].path.RelativePath().find("vkcube") != std::string::npos);
     const std::vector<ExecutableOptions>& options0 = executables[0].GetOptions();
     EXPECT_TRUE(options0[0].working_folder.Empty());
-    EXPECT_TRUE(options0[0].log_file.RelativePath().find("${VK_HOME}") != std::string::npos);
+    EXPECT_TRUE(options0[0].log_file.RelativePath().find("${VULKAN_HOME}") != std::string::npos);
 
     EXPECT_TRUE(executables[1].path.RelativePath().find("vkcubepp") != std::string::npos);
     const std::vector<ExecutableOptions>& options1 = executables[1].GetOptions();
     EXPECT_TRUE(options1[0].working_folder.Empty());
-    EXPECT_TRUE(options1[0].log_file.RelativePath().find("${VK_HOME}") != std::string::npos);
+    EXPECT_TRUE(options1[0].log_file.RelativePath().find("${VULKAN_HOME}") != std::string::npos);
 
     EXPECT_TRUE(executables[2].path.RelativePath().find("vulkaninfo") != std::string::npos);
     const std::vector<ExecutableOptions>& options2 = executables[2].GetOptions();
     EXPECT_TRUE(options2[0].working_folder.Empty());
-    EXPECT_TRUE(options2[0].log_file.RelativePath().find("${VK_HOME}") != std::string::npos);
+    EXPECT_TRUE(options2[0].log_file.RelativePath().find("${VULKAN_HOME}") != std::string::npos);
 }
 
 TEST(test_executable_manager, active_executable) {

diff --git a/vkconfig_core/test/test_layer.cpp b/vkconfig_core/test/test_layer.cpp
@@ -36,7 +36,7 @@ static SettingMetaString* InstantiateString(Layer& layer, const std::string& key
     return static_cast<SettingMetaString*>(layer.Instantiate(layer.settings, key, SETTING_STRING));
 }
 
-static std::map<Path, std::string> Dummy() { return std::map<Path, std::string>(); }
+static std::map<Path, LayerStatus> Dummy() { return std::map<Path, LayerStatus>(); }
 
 TEST(test_layer, collect_settings) {
     Layer layer;
-Original file line number
+Diff line change
@@ Expand Up / @@ -44,7 +44,6 @@ SettingType GetSettingType(const char* token) { @@
             if (ToUpperCase(token) == GetToken(type)) return type;
         }
-        assert(0);  // Unknown token
         return static_cast<SettingType>(-1);
     }
@@ Expand Down @@