From 667a8edc7a082cc465f47fe8c826e92df0618b14 Mon Sep 17 00:00:00 2001 From: Karim Abdelrahman Date: Wed, 16 Apr 2025 16:26:11 +0300 Subject: [PATCH 1/4] McuSupport: fix optional packages MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit MCU plugin didn't have to handle "optional" property in MCU kit descriptions. I believe this is mainly because "optional" was always "false" for all the previous MCU targets. A newly introduced kit, with one package set "optional" to "true", manifested the issue. The user couldn't create a kit without providing a valid path to that optional package. The user should be able to leave the optional package path empty or provide a valid path. This change introduces a new Groupbox widget to display the optional packages. An optional package with empty path is considered valid. Task-number: QTCREATORBUG-32777 Change-Id: I5f27952efc77bc5e4351ab4c013669eb6056810f Reviewed-by: Sivert Krøvel Reviewed-by: Alessandro Portale --- src/plugins/mcusupport/mcuabstractpackage.h | 1 + src/plugins/mcusupport/mcupackage.cpp | 38 +++++++++++++------ src/plugins/mcusupport/mcupackage.h | 5 +++ .../mcusupport/mcusupportoptionspage.cpp | 23 ++++++++++- src/plugins/mcusupport/mcusupportsdk.cpp | 2 + src/plugins/mcusupport/mcutargetdescription.h | 1 + src/plugins/mcusupport/mcutargetfactory.cpp | 1 + src/plugins/mcusupport/test/packagemock.h | 1 + src/plugins/mcusupport/test/unittest.cpp | 3 ++ 9 files changed, 63 insertions(+), 12 deletions(-) diff --git a/src/plugins/mcusupport/mcuabstractpackage.h b/src/plugins/mcusupport/mcuabstractpackage.h index 98d69144aa8..751d312133d 100644 --- a/src/plugins/mcusupport/mcuabstractpackage.h +++ b/src/plugins/mcusupport/mcuabstractpackage.h @@ -34,6 +34,7 @@ public: virtual QString label() const = 0; virtual QString cmakeVariableName() const = 0; virtual QString environmentVariableName() const = 0; + virtual bool isOptional() const = 0; virtual bool isAddToSystemPath() const = 0; virtual QStringList versions() const = 0; diff --git a/src/plugins/mcusupport/mcupackage.cpp b/src/plugins/mcusupport/mcupackage.cpp index ff66956a532..15c5ef9b202 100644 --- a/src/plugins/mcusupport/mcupackage.cpp +++ b/src/plugins/mcusupport/mcupackage.cpp @@ -41,6 +41,7 @@ McuPackage::McuPackage(const SettingsHandler::Ptr &settingsHandler, const QStringList &versions, const QString &downloadUrl, const McuPackageVersionDetector *versionDetector, + const bool optional, const bool addToSystemPath, const Utils::PathChooser::Kind &valueType, const bool allowNewerVersionKey) @@ -53,6 +54,7 @@ McuPackage::McuPackage(const SettingsHandler::Ptr &settingsHandler, , m_cmakeVariableName(cmakeVarName) , m_environmentVariableName(envVarName) , m_downloadUrl(downloadUrl) + , m_optional(optional) , m_addToSystemPath(addToSystemPath) , m_valueType(valueType) { @@ -93,6 +95,11 @@ QString McuPackage::environmentVariableName() const return m_environmentVariableName; } +bool McuPackage::isOptional() const +{ + return m_optional; +} + bool McuPackage::isAddToSystemPath() const { return m_addToSystemPath; @@ -190,25 +197,34 @@ McuPackage::Status McuPackage::status() const return m_status; } +bool McuPackage::isOptionalAndEmpty() const +{ + return m_status == Status::EmptyPath && isOptional(); +} + bool McuPackage::isValidStatus() const { return m_status == Status::ValidPackage || m_status == Status::ValidPackageMismatchedVersion - || m_status == Status::ValidPackageVersionNotDetected; + || m_status == Status::ValidPackageVersionNotDetected || isOptionalAndEmpty(); } void McuPackage::updateStatusUi() { - switch (m_status) { - case Status::ValidPackage: + if (isOptionalAndEmpty()) { m_infoLabel->setType(InfoLabel::Ok); - break; - case Status::ValidPackageMismatchedVersion: - case Status::ValidPackageVersionNotDetected: - m_infoLabel->setType(InfoLabel::Warning); - break; - default: - m_infoLabel->setType(InfoLabel::NotOk); - break; + } else { + switch (m_status) { + case Status::ValidPackage: + m_infoLabel->setType(InfoLabel::Ok); + break; + case Status::ValidPackageMismatchedVersion: + case Status::ValidPackageVersionNotDetected: + m_infoLabel->setType(InfoLabel::Warning); + break; + default: + m_infoLabel->setType(InfoLabel::NotOk); + break; + } } m_infoLabel->setText(statusText()); } diff --git a/src/plugins/mcusupport/mcupackage.h b/src/plugins/mcusupport/mcupackage.h index 7b9ef646506..b3366d1db87 100644 --- a/src/plugins/mcusupport/mcupackage.h +++ b/src/plugins/mcusupport/mcupackage.h @@ -39,6 +39,7 @@ public: const QString &downloadUrl = {}, const McuPackageVersionDetector *versionDetector = nullptr, const bool addToPath = false, + const bool optional = false, const Utils::PathChooser::Kind &valueType = Utils::PathChooser::Kind::ExistingDirectory, const bool allowNewerVersionKey = false); @@ -50,6 +51,7 @@ public: QString label() const override; QString cmakeVariableName() const override; QString environmentVariableName() const override; + bool isOptional() const override; bool isAddToSystemPath() const override; QStringList versions() const override; @@ -77,6 +79,8 @@ private: void updatePath(); void updateStatusUi(); + bool isOptionalAndEmpty() const; + SettingsHandler::Ptr settingsHandler; Utils::PathChooser *m_fileChooser = nullptr; @@ -95,6 +99,7 @@ private: const QString m_cmakeVariableName; const QString m_environmentVariableName; const QString m_downloadUrl; + const bool m_optional; const bool m_addToSystemPath; const Utils::PathChooser::Kind m_valueType; diff --git a/src/plugins/mcusupport/mcusupportoptionspage.cpp b/src/plugins/mcusupport/mcusupportoptionspage.cpp index d1f0f3f86c9..348a0d2435a 100644 --- a/src/plugins/mcusupport/mcusupportoptionspage.cpp +++ b/src/plugins/mcusupport/mcusupportoptionspage.cpp @@ -55,8 +55,10 @@ private: QMap m_packageWidgets; QMap m_mcuTargetPacketWidgets; QFormLayout *m_packagesLayout = nullptr; + QFormLayout *m_optionalPackagesLayout = nullptr; QGroupBox *m_qtForMCUsSdkGroupBox = nullptr; QGroupBox *m_packagesGroupBox = nullptr; + QGroupBox *m_optionalPackagesGroupBox = nullptr; QGroupBox *m_mcuTargetsGroupBox = nullptr; QComboBox *m_mcuTargetsComboBox = nullptr; QGroupBox *m_kitCreationGroupBox = nullptr; @@ -122,6 +124,14 @@ McuSupportOptionsWidget::McuSupportOptionsWidget(McuSupportOptions &options, m_packagesGroupBox->setLayout(m_packagesLayout); } + { + m_optionalPackagesGroupBox = new QGroupBox(Tr::tr("Optional")); + m_optionalPackagesGroupBox->setFlat(true); + mainLayout->addWidget(m_optionalPackagesGroupBox); + m_optionalPackagesLayout = new QFormLayout; + m_optionalPackagesGroupBox->setLayout(m_optionalPackagesLayout); + } + { m_mcuTargetsInfoLabel = new Utils::InfoLabel; mainLayout->addWidget(m_mcuTargetsInfoLabel); @@ -187,6 +197,10 @@ void McuSupportOptionsWidget::updateStatus() const bool ready = valid && mcuTarget; m_mcuTargetsGroupBox->setVisible(ready); m_packagesGroupBox->setVisible(ready && !mcuTarget->packages().isEmpty()); + m_optionalPackagesGroupBox->setVisible( + ready && std::ranges::any_of(mcuTarget->packages(), [](McuPackagePtr p) { + return p->isOptional(); + })); m_kitCreationGroupBox->setVisible(ready); m_mcuTargetsInfoLabel->setVisible(valid && m_options.sdkRepository.mcuTargets.isEmpty()); if (m_mcuTargetsInfoLabel->isVisible()) { @@ -266,6 +280,10 @@ void McuSupportOptionsWidget::showMcuTargetPackages() m_packagesLayout->removeRow(0); } + while (m_optionalPackagesLayout->rowCount() > 0) { + m_optionalPackagesLayout->removeRow(0); + } + std::set packages; for (const auto &package : mcuTarget->packages()) { @@ -285,7 +303,10 @@ void McuSupportOptionsWidget::showMcuTargetPackages() package->setPath(macroExpander->expand(package->defaultPath())); } }); - m_packagesLayout->addRow(package->label(), packageWidget); + if (package->isOptional()) + m_optionalPackagesLayout->addRow(package->label(), packageWidget); + else + m_packagesLayout->addRow(package->label(), packageWidget); packageWidget->show(); } diff --git a/src/plugins/mcusupport/mcusupportsdk.cpp b/src/plugins/mcusupport/mcusupportsdk.cpp index 39082010da5..2710462b91d 100644 --- a/src/plugins/mcusupport/mcusupportsdk.cpp +++ b/src/plugins/mcusupport/mcusupportsdk.cpp @@ -61,6 +61,7 @@ McuPackagePtr createQtForMCUsPackage(const SettingsHandler::Ptr &settingsHandler {}, // versions {}, // downloadUrl nullptr, // versionDetector + false, // optional false, // addToPath Utils::PathChooser::Kind::ExistingDirectory, // valueType true)}; // useNewestVersionKey @@ -706,6 +707,7 @@ static PackageDescription parsePackage(const QJsonObject &cmakeEntry) detectionPaths, versions, parseVersionDetection(cmakeEntry), + cmakeEntry["optional"].toBool(), cmakeEntry["addToSystemPath"].toBool(), parseLineEditType(cmakeEntry["type"])}; } diff --git a/src/plugins/mcusupport/mcutargetdescription.h b/src/plugins/mcusupport/mcutargetdescription.h index 3b6d783320a..296de46f4fe 100644 --- a/src/plugins/mcusupport/mcutargetdescription.h +++ b/src/plugins/mcusupport/mcutargetdescription.h @@ -33,6 +33,7 @@ struct PackageDescription Utils::FilePaths detectionPaths; QStringList versions; VersionDetection versionDetection; + bool optional; bool shouldAddToSystemPath; Utils::PathChooser::Kind type; }; //struct PackageDescription diff --git a/src/plugins/mcusupport/mcutargetfactory.cpp b/src/plugins/mcusupport/mcutargetfactory.cpp index 120e2ce89ca..a392d1f3b3b 100644 --- a/src/plugins/mcusupport/mcutargetfactory.cpp +++ b/src/plugins/mcusupport/mcutargetfactory.cpp @@ -137,6 +137,7 @@ McuPackagePtr McuTargetFactory::createPackage(const PackageDescription &pkgDesc) pkgDesc.versions, {}, createVersionDetection(pkgDesc.versionDetection), + pkgDesc.optional, pkgDesc.shouldAddToSystemPath, pkgDesc.type}}; } diff --git a/src/plugins/mcusupport/test/packagemock.h b/src/plugins/mcusupport/test/packagemock.h index ab2e425ef54..037063165da 100644 --- a/src/plugins/mcusupport/test/packagemock.h +++ b/src/plugins/mcusupport/test/packagemock.h @@ -29,6 +29,7 @@ public: MOCK_METHOD(bool, isValidStatus, (), (const)); MOCK_METHOD(QString, cmakeVariableName, (), (const)); MOCK_METHOD(QString, environmentVariableName, (), (const)); + MOCK_METHOD(bool, isOptional, (), (const)); MOCK_METHOD(bool, isAddToSystemPath, (), (const)); MOCK_METHOD(bool, writeToSettings, (), (const)); MOCK_METHOD(void, readFromSettings, ()); diff --git a/src/plugins/mcusupport/test/unittest.cpp b/src/plugins/mcusupport/test/unittest.cpp index f45fd668e6d..eba2d80be14 100644 --- a/src/plugins/mcusupport/test/unittest.cpp +++ b/src/plugins/mcusupport/test/unittest.cpp @@ -196,6 +196,7 @@ const PackageDescription {}, VersionDetection{}, false, + false, Utils::PathChooser::Kind::ExistingDirectory}; const McuTargetDescription::Platform platformDescription{id, @@ -851,6 +852,7 @@ void McuSupportTest::test_useFallbackPathForToolchainWhenPathFromSettingsIsNotAv {}, VersionDetection{}, false, + false, Utils::PathChooser::Kind::ExistingDirectory}; McuTargetDescription::Toolchain toolchainDescription{armGcc, {}, compilerDescription, {}}; @@ -875,6 +877,7 @@ void McuSupportTest::test_usePathFromSettingsForToolchainPath() {}, VersionDetection{}, false, + false, Utils::PathChooser::Kind::ExistingDirectory}; McuTargetDescription::Toolchain toolchainDescription{armGcc, {}, compilerDescription, {}}; From 69976a70ee98cf300275edf3c45e84691bd711dc Mon Sep 17 00:00:00 2001 From: Christian Stenger Date: Thu, 17 Apr 2025 14:35:14 +0200 Subject: [PATCH 2/4] SquishTests: Fix access to undefined variable MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Amends 29ec942455190878b6b2e32395c5ccac8566aa7a. Change-Id: I82d343e2e5a077d8db265ef83c98fbb490bb58ab Reviewed-by: Jukka Nokso Reviewed-by: Robert Löhning --- tests/system/suite_editors/tst_generic_highlighter/test.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/system/suite_editors/tst_generic_highlighter/test.py b/tests/system/suite_editors/tst_generic_highlighter/test.py index 0d96e4e99bf..2643403c89a 100644 --- a/tests/system/suite_editors/tst_generic_highlighter/test.py +++ b/tests/system/suite_editors/tst_generic_highlighter/test.py @@ -126,7 +126,7 @@ def displayHintForHighlighterDefinition(fileName, patterns, added): if hasSuffix(fileName, patterns): return not added test.warning("Got an unexpected suffix.", "Filename: %s, Patterns: %s" - % (fileName, str(patterns + lPatterns))) + % (fileName, str(patterns))) return False def main(): From 916e369f89ac52a7205578d73568d5953949bb94 Mon Sep 17 00:00:00 2001 From: Eike Ziller Date: Wed, 23 Apr 2025 08:46:43 +0200 Subject: [PATCH 3/4] Git: Fix potential crash in instant blame MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When clicking links in the tool tip. The connection was guarded with the label, but was accessing data from the BlameMark. We got a report for a crash that looks like the mark was already gone at this point. It is safer to capture the relevant data explicitly. Change-Id: I16aa30a37c4221c4bf3caf90692a660737be3870 Reviewed-by: André Hartmann --- src/plugins/git/instantblame.cpp | 67 +++++++++++++++++--------------- 1 file changed, 35 insertions(+), 32 deletions(-) diff --git a/src/plugins/git/instantblame.cpp b/src/plugins/git/instantblame.cpp index 928952cfee5..22be00d901a 100644 --- a/src/plugins/git/instantblame.cpp +++ b/src/plugins/git/instantblame.cpp @@ -69,42 +69,45 @@ bool BlameMark::addToolTipContent(QLayout *target) const auto textLabel = new QLabel; textLabel->setText(toolTip()); target->addWidget(textLabel); - QObject::connect(textLabel, &QLabel::linkActivated, textLabel, [this](const QString &link) { - qCInfo(log) << "Link activated with target:" << link; - const QString hash = (link == "blameParent") ? m_info.hash + "^" : m_info.hash; + QObject::connect( + textLabel, &QLabel::linkActivated, textLabel, [info = m_info](const QString &link) { + qCInfo(log) << "Link activated with target:" << link; + const QString hash = (link == "blameParent") ? info.hash + "^" : info.hash; - if (link.startsWith("blame") || link == "showFile") { - const VcsBasePluginState state = currentState(); - QTC_ASSERT(state.hasTopLevel(), return); - const Utils::FilePath path = state.topLevel(); + if (link.startsWith("blame") || link == "showFile") { + const VcsBasePluginState state = currentState(); + QTC_ASSERT(state.hasTopLevel(), return); + const Utils::FilePath path = state.topLevel(); - const QString originalFileName = m_info.originalFileName; - if (link.startsWith("blame")) { - qCInfo(log).nospace().noquote() << "Blaming: \"" << path << "/" << originalFileName - << "\":" << m_info.originalLine << " @ " << hash; - gitClient().annotate(path, originalFileName, m_info.originalLine, hash); + const QString originalFileName = info.originalFileName; + if (link.startsWith("blame")) { + qCInfo(log).nospace().noquote() + << "Blaming: \"" << path << "/" << originalFileName + << "\":" << info.originalLine << " @ " << hash; + gitClient().annotate(path, originalFileName, info.originalLine, hash); + } else { + qCInfo(log).nospace().noquote() + << "Showing file: \"" << path << "/" << originalFileName << "\" @ " << hash; + + const auto fileName = Utils::FilePath::fromString(originalFileName); + gitClient().openShowEditor(path, hash, fileName); + } + } else if (link == "logLine") { + const VcsBasePluginState state = currentState(); + QTC_ASSERT(state.hasFile(), return); + + qCInfo(log).nospace().noquote() + << "Showing log for: \"" << info.filePath << "\" line:" << info.line; + + const QString lineArg + = QString("-L %1,%1:%2").arg(info.line).arg(state.relativeCurrentFile()); + gitClient().log(state.currentFileTopLevel(), {}, true, {lineArg, "--no-patch"}); } else { - qCInfo(log).nospace().noquote() << "Showing file: \"" << path << "/" - << originalFileName << "\" @ " << hash; - - const auto fileName = Utils::FilePath::fromString(originalFileName); - gitClient().openShowEditor(path, hash, fileName); + qCInfo(log).nospace().noquote() + << "Showing commit: " << hash << " for " << info.filePath; + gitClient().show(info.filePath, hash); } - } else if (link == "logLine") { - const VcsBasePluginState state = currentState(); - QTC_ASSERT(state.hasFile(), return); - - qCInfo(log).nospace().noquote() << "Showing log for: \"" << m_info.filePath - << "\" line:" << m_info.line; - - const QString lineArg = QString("-L %1,%1:%2") - .arg(m_info.line).arg(state.relativeCurrentFile()); - gitClient().log(state.currentFileTopLevel(), {}, true, {lineArg, "--no-patch"}); - } else { - qCInfo(log).nospace().noquote() << "Showing commit: " << hash << " for " << m_info.filePath; - gitClient().show(m_info.filePath, hash); - } - }); + }); return true; } From d2d4dcfb56e75b959f29052e78c61b1ff46b663e Mon Sep 17 00:00:00 2001 From: Jarek Kobus Date: Wed, 23 Apr 2025 08:50:48 +0200 Subject: [PATCH 4/4] Android: Avoid event processing when executing manager command Task-number: QTCREATORBUG-32849 Change-Id: I650265d18b826f37dfe48e0d1cb9e81e09c2981e Reviewed-by: Alessandro Portale --- src/plugins/android/androidsdkmanager.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/plugins/android/androidsdkmanager.cpp b/src/plugins/android/androidsdkmanager.cpp index 8725db518a9..d9add29555b 100644 --- a/src/plugins/android/androidsdkmanager.cpp +++ b/src/plugins/android/androidsdkmanager.cpp @@ -504,7 +504,7 @@ static bool sdkManagerCommand(const QStringList &args, QString *output) proc.setCommand({AndroidConfig::sdkManagerToolPath(), newArgs}); qCDebug(sdkManagerLog).noquote() << "Running SDK Manager command (sync):" << proc.commandLine().toUserOutput(); - proc.runBlocking(60s, EventLoopMode::On); + proc.runBlocking(60s); if (output) *output = proc.allOutput(); return proc.result() == ProcessResult::FinishedWithSuccess;