From 3be0b263a87c4289f26ad778829c2b38643178a5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sivert=20Kr=C3=B8vel?= Date: Thu, 30 May 2024 17:19:10 +0200 Subject: [PATCH] McuSupport: Read and write user settings with non-versioned key The settings keys are versioned to avoid the situation where the installer removes a settings key which may still be in use. See QTCREATORBUG-29194 for details. When reading from user settings, it makes more sense to use the non-versioned keys, especially in the case of the main Qt for MCUs SDK package path. With this patch, the plain settings key is used to read and write except for when determining the default value of a path in the McuPackage constructor, where we want to also look in the system scope settings for the values written by the installer, which may be version specific Task-number: QTCREATORBUG-30810 Change-Id: Ib1e2be170cb7da24b6e4534b59f702b894556d8c Reviewed-by: Yasser Grimes Reviewed-by: Eike Ziller --- src/plugins/mcusupport/mcupackage.cpp | 18 ++++++++++++++---- src/plugins/mcusupport/mcupackage.h | 2 +- src/plugins/mcusupport/settingshandler.cpp | 16 ++++++++++++++-- 3 files changed, 29 insertions(+), 7 deletions(-) diff --git a/src/plugins/mcusupport/mcupackage.cpp b/src/plugins/mcusupport/mcupackage.cpp index 2ff913c680e..efcff77352b 100644 --- a/src/plugins/mcusupport/mcupackage.cpp +++ b/src/plugins/mcusupport/mcupackage.cpp @@ -43,11 +43,11 @@ McuPackage::McuPackage(const SettingsHandler::Ptr &settingsHandler, const McuPackageVersionDetector *versionDetector, const bool addToSystemPath, const Utils::PathChooser::Kind &valueType, - const bool useNewestVersionKey) + const bool allowNewerVersionKey) : settingsHandler(settingsHandler) , m_label(label) , m_detectionPaths(detectionPaths) - , m_settingsKey(settingsHandler->getVersionedKey(settingsKey, QSettings::SystemScope, versions, useNewestVersionKey)) + , m_settingsKey(settingsKey) , m_versionDetector(versionDetector) , m_versions(versions) , m_cmakeVariableName(cmakeVarName) @@ -56,8 +56,18 @@ McuPackage::McuPackage(const SettingsHandler::Ptr &settingsHandler, , m_addToSystemPath(addToSystemPath) , m_valueType(valueType) { - m_defaultPath = settingsHandler->getPath(m_settingsKey, QSettings::SystemScope, defaultPath); - m_path = settingsHandler->getPath(m_settingsKey, QSettings::UserScope, m_defaultPath); + // The installer writes versioned keys as well as the plain key as found in the kits. + // Use the versioned key in case the plain key was removed by an uninstall operation + const Utils::Key versionedKey = settingsHandler->getVersionedKey(settingsKey, + QSettings::SystemScope, + versions, + allowNewerVersionKey); + m_defaultPath = settingsHandler->getPath(versionedKey, QSettings::SystemScope, defaultPath); + m_path = settingsHandler->getPath(m_settingsKey, QSettings::UserScope, ""); + // The user settings may have been written with a versioned key in older versions of QtCreator + if (m_path.isEmpty()) { + m_path = settingsHandler->getPath(versionedKey, QSettings::UserScope, m_defaultPath); + } if (m_path.isEmpty()) { m_path = FilePath::fromUserInput(qtcEnvironmentVariable(m_environmentVariableName)); } diff --git a/src/plugins/mcusupport/mcupackage.h b/src/plugins/mcusupport/mcupackage.h index a74c08bcf2d..7b9ef646506 100644 --- a/src/plugins/mcusupport/mcupackage.h +++ b/src/plugins/mcusupport/mcupackage.h @@ -41,7 +41,7 @@ public: const bool addToPath = false, const Utils::PathChooser::Kind &valueType = Utils::PathChooser::Kind::ExistingDirectory, - const bool useNewestVersionKey = false); + const bool allowNewerVersionKey = false); ~McuPackage() override = default; diff --git a/src/plugins/mcusupport/settingshandler.cpp b/src/plugins/mcusupport/settingshandler.cpp index 1ecec030b45..939ba125d1c 100644 --- a/src/plugins/mcusupport/settingshandler.cpp +++ b/src/plugins/mcusupport/settingshandler.cpp @@ -105,12 +105,24 @@ FilePath SettingsHandler::getPath(const Key &settingsKey, bool SettingsHandler::write(const Key &settingsKey, const FilePath &path, - const FilePath &defaultPath) const + const FilePath &maybeDefaultPath) const { const FilePath savedPath = packagePathFromSettings(settingsKey, *Core::ICore::settings(QSettings::UserScope), - defaultPath); + maybeDefaultPath); const Key key = Key(Constants::SETTINGS_GROUP) + '/' + Constants::SETTINGS_KEY_PACKAGE_PREFIX + settingsKey; + + FilePath defaultPath = maybeDefaultPath; + if (path == maybeDefaultPath) { + // If the installer has overwritten the non-versioned key with an older version than the + // newest versioned key, and the user wants to manually return to the newest installed + // version, the defaultPath will match the desired path, and the settings object will + // assume it can simply remove the key instead of writing a new value to it. + // To work around this, pretend like the default value is the value found from the global scope + defaultPath = packagePathFromSettings(settingsKey, + *Core::ICore::settings(QSettings::SystemScope), + maybeDefaultPath);; + } Core::ICore::settings()->setValueWithDefault(key, path.toUserOutput(), defaultPath.toUserOutput());