From 56be5df0efd02471e59cc3872d8fe46f17fd2424 Mon Sep 17 00:00:00 2001 From: Christian Kandeler Date: Tue, 14 Jan 2025 14:41:12 +0100 Subject: [PATCH] ProjectExplorer: Always assume a BuildInfo has a factory The purpose of a BuildInfo is to create a BuildConfiguration via its factory. It's confusing if code treats a null factory as a valid condition. Change-Id: I2efd45c2f8f82b927aa300370fc39d991a5f8fa2 Reviewed-by: hjk --- src/plugins/projectexplorer/project.cpp | 11 ++-- .../projectexplorer/projectimporter.cpp | 4 ++ .../projectexplorer/targetsetupwidget.cpp | 57 +++++++++---------- src/plugins/qtsupport/qtprojectimporter.cpp | 27 ++++++++- 4 files changed, 59 insertions(+), 40 deletions(-) diff --git a/src/plugins/projectexplorer/project.cpp b/src/plugins/projectexplorer/project.cpp index 25e712b66bc..b4e5cdee4c3 100644 --- a/src/plugins/projectexplorer/project.cpp +++ b/src/plugins/projectexplorer/project.cpp @@ -1097,6 +1097,8 @@ void Project::setup(const QList &infoList) BuildConfiguration *Project::setup(const BuildInfo &info) { + QTC_ASSERT(info.factory, return nullptr); + Kit *k = KitManager::kit(info.kitId); if (!k) return nullptr; @@ -1109,12 +1111,9 @@ BuildConfiguration *Project::setup(const BuildInfo &info) QTC_ASSERT(t, return nullptr); - BuildConfiguration *bc = nullptr; - if (info.factory) { - bc = info.factory->create(t, info); - if (bc) - t->addBuildConfiguration(bc); - } + BuildConfiguration * const bc = info.factory->create(t, info); + if (bc) + t->addBuildConfiguration(bc); if (newTarget) { newTarget->updateDefaultDeployConfigurations(); newTarget->updateDefaultRunConfigurations(); diff --git a/src/plugins/projectexplorer/projectimporter.cpp b/src/plugins/projectexplorer/projectimporter.cpp index c384cc3b561..01a4e99b652 100644 --- a/src/plugins/projectexplorer/projectimporter.cpp +++ b/src/plugins/projectexplorer/projectimporter.cpp @@ -138,6 +138,10 @@ const QList ProjectImporter::import(const Utils::FilePath &importPath } auto factory = BuildConfigurationFactory::find(k, projectFilePath()); + if (!factory) { + qCDebug(log) << "No factory for kit" << k->displayName(); + continue; + } for (BuildInfo i : infoList) { i.displayName = Tr::tr("%1 (imported)").arg(i.displayName); i.kitId = k->id(); diff --git a/src/plugins/projectexplorer/targetsetupwidget.cpp b/src/plugins/projectexplorer/targetsetupwidget.cpp index b227c4750b3..d46b86611ea 100644 --- a/src/plugins/projectexplorer/targetsetupwidget.cpp +++ b/src/plugins/projectexplorer/targetsetupwidget.cpp @@ -129,32 +129,30 @@ void TargetSetupWidget::addBuildInfo(const BuildInfo &info, bool isImport) store.isEnabled = info.enabledByDefault; ++m_selected; - if (info.factory) { - store.checkbox = new QCheckBox; - store.checkbox->setText(info.displayName); - store.checkbox->setChecked(store.isEnabled); - store.checkbox->setAttribute(Qt::WA_LayoutUsesWidgetRect); - m_newBuildsLayout->addWidget(store.checkbox, pos * 2, 0); + store.checkbox = new QCheckBox; + store.checkbox->setText(info.displayName); + store.checkbox->setChecked(store.isEnabled); + store.checkbox->setAttribute(Qt::WA_LayoutUsesWidgetRect); + m_newBuildsLayout->addWidget(store.checkbox, pos * 2, 0); - store.pathChooser = new PathChooser(); - store.pathChooser->setExpectedKind(PathChooser::Directory); - store.pathChooser->setFilePath(info.buildDirectory); - if (!info.showBuildDirConfigWidget) - store.pathChooser->setVisible(false); - store.pathChooser->setHistoryCompleter("TargetSetup.BuildDir.History"); - store.pathChooser->setReadOnly(isImport); - m_newBuildsLayout->addWidget(store.pathChooser, pos * 2, 1); + store.pathChooser = new PathChooser(); + store.pathChooser->setExpectedKind(PathChooser::Directory); + store.pathChooser->setFilePath(info.buildDirectory); + if (!info.showBuildDirConfigWidget) + store.pathChooser->setVisible(false); + store.pathChooser->setHistoryCompleter("TargetSetup.BuildDir.History"); + store.pathChooser->setReadOnly(isImport); + m_newBuildsLayout->addWidget(store.pathChooser, pos * 2, 1); - store.issuesLabel = new QLabel; - store.issuesLabel->setIndent(32); - m_newBuildsLayout->addWidget(store.issuesLabel, pos * 2 + 1, 0, 1, 2); - store.issuesLabel->setVisible(false); + store.issuesLabel = new QLabel; + store.issuesLabel->setIndent(32); + m_newBuildsLayout->addWidget(store.issuesLabel, pos * 2 + 1, 0, 1, 2); + store.issuesLabel->setVisible(false); - connect(store.checkbox, &QAbstractButton::toggled, this, - [this, checkBox = store.checkbox](bool b) { checkBoxToggled(checkBox, b); }); - connect(store.pathChooser, &PathChooser::rawPathChanged, this, - [this, pathChooser = store.pathChooser] { pathChanged(pathChooser); }); - } + connect(store.checkbox, &QAbstractButton::toggled, this, + [this, checkBox = store.checkbox](bool b) { checkBoxToggled(checkBox, b); }); + connect(store.pathChooser, &PathChooser::rawPathChanged, this, + [this, pathChooser = store.pathChooser] { pathChanged(pathChooser); }); store.hasIssues = false; m_infoStore.emplace_back(std::move(store)); @@ -283,8 +281,7 @@ void TargetSetupWidget::clear() void TargetSetupWidget::updateDefaultBuildDirectories() { for (const BuildInfo &buildInfo : buildInfoList(m_kit, m_projectPath)) { - if (!buildInfo.factory) - continue; + QTC_ASSERT(buildInfo.factory, continue); bool found = false; for (BuildInfoStore &buildInfoStore : m_infoStore) { if (buildInfoStore.buildInfo.typeName == buildInfo.typeName) { @@ -348,16 +345,14 @@ void TargetSetupWidget::reportIssues(int index) QPair TargetSetupWidget::findIssues(const BuildInfo &info) { - if (m_projectPath.isEmpty() || !info.factory) + QTC_ASSERT(info.factory, return std::make_pair(Task::Unknown, QString())); + if (m_projectPath.isEmpty()) return {Task::Unknown, {}}; - Tasks issues; - if (info.factory) - issues = info.factory->reportIssues(m_kit, m_projectPath, info.buildDirectory); - + const Tasks issues = info.factory->reportIssues(m_kit, m_projectPath, info.buildDirectory); QString text; Task::TaskType highestType = Task::Unknown; - for (const Task &t : std::as_const(issues)) { + for (const Task &t : issues) { if (!text.isEmpty()) text.append(QLatin1String("
")); // set severity: diff --git a/src/plugins/qtsupport/qtprojectimporter.cpp b/src/plugins/qtsupport/qtprojectimporter.cpp index 613f636484d..80e35f61295 100644 --- a/src/plugins/qtsupport/qtprojectimporter.cpp +++ b/src/plugins/qtsupport/qtprojectimporter.cpp @@ -14,6 +14,7 @@ #include #include #include +#include #include #include @@ -108,6 +109,8 @@ Kit *QtProjectImporter::createTemporaryKit(const QtVersionData &versionData, #if WITH_TESTS +#include +#include #include #include @@ -117,6 +120,23 @@ Kit *QtProjectImporter::createTemporaryKit(const QtVersionData &versionData, namespace QtSupport::Internal { +class TestBuildConfigFactory : public BuildConfigurationFactory +{ +public: + TestBuildConfigFactory() + { + for (ExtensionSystem::PluginSpec * const spec : ExtensionSystem::PluginManager::plugins()) { + if (spec->id() == "qmakeprojectmanager") { + if (spec->state() == ExtensionSystem::PluginSpec::Running) + return; + break; + } + } + registerBuildConfiguration("QtSupport.Test"); + setSupportedProjectMimeTypeName(Utils::Constants::PROFILE_MIMETYPE); + } +}; + struct DirectoryData { DirectoryData(const QString &ip, Kit *k = nullptr, bool ink = false, @@ -162,6 +182,7 @@ protected: void deleteDirectoryData(void *directoryData) const override; private: + const TestBuildConfigFactory m_bcFactory; const QList m_testData; mutable Utils::FilePath m_path; mutable QVector m_deletedTestData; @@ -443,7 +464,7 @@ void QtProjectImporterTest::testQtProjectImporter_oneProject() // Finally set up importer: // Copy the directoryData so that importer is free to delete it later. - TestQtProjectImporter importer(tempDir1.path(), + TestQtProjectImporter importer(tempDir1.filePath("test.pro"), Utils::transform(testData, [](DirectoryData *i) { return static_cast(new DirectoryData(*i)); })); @@ -456,7 +477,7 @@ void QtProjectImporterTest::testQtProjectImporter_oneProject() const QList buildInfo = importer.import(Utils::FilePath::fromString(appDir), true); // VALIDATE: Basic TestImporter state: - QCOMPARE(importer.projectFilePath(), tempDir1.path()); + QCOMPARE(importer.projectFilePath(), tempDir1.filePath("test.pro")); QCOMPARE(importer.allDeleted(), true); // VALIDATE: Result looks reasonable: @@ -560,7 +581,7 @@ void QtProjectImporterTest::testQtProjectImporter_oneProject() QCOMPARE(newKitId, newKitIdAfterImport); // VALIDATE: Importer state - QCOMPARE(importer.projectFilePath(), tempDir1.path()); + QCOMPARE(importer.projectFilePath(), tempDir1.filePath("test.pro")); QCOMPARE(importer.allDeleted(), true); if (kitIsPersistent) {