diff options
author | Jarek Kobus <jaroslaw.kobus@qt.io> | 2023-07-11 20:39:42 +0200 |
---|---|---|
committer | Jarek Kobus <jaroslaw.kobus@qt.io> | 2023-07-13 06:50:07 +0000 |
commit | b37b94f0e56b93f5d07789e8922dc90b371669c8 (patch) | |
tree | c357edee395a569d5d57cda0dba8de3f305e53a6 | |
parent | 66ecfb15d11ba15a07523b080ce3b4efd6b70324 (diff) |
AbstractProcessStep: De-virtualize finish() method
Provide a setDoneHook() setter instead. The hook is
introduced temporarily, as when all the subclasses
are transformed to use the task tree, the done hook
is going to be a part of the subclass' recipe.
Task-number: QTCREATORBUG-29168
Change-Id: Idbc0f8b8a32c8df2fa5ecb73ed1cbaedad99620d
Reviewed-by: hjk <hjk@qt.io>
-rw-r--r-- | src/plugins/android/androidbuildapkstep.cpp | 12 | ||||
-rw-r--r-- | src/plugins/android/androidbuildapkstep.h | 1 | ||||
-rw-r--r-- | src/plugins/cmakeprojectmanager/cmakebuildstep.cpp | 13 | ||||
-rw-r--r-- | src/plugins/cmakeprojectmanager/cmakebuildstep.h | 1 | ||||
-rw-r--r-- | src/plugins/cmakeprojectmanager/cmakeinstallstep.cpp | 9 | ||||
-rw-r--r-- | src/plugins/projectexplorer/abstractprocessstep.cpp | 18 | ||||
-rw-r--r-- | src/plugins/projectexplorer/abstractprocessstep.h | 5 | ||||
-rw-r--r-- | src/plugins/qmakeprojectmanager/qmakemakestep.cpp | 21 | ||||
-rw-r--r-- | src/plugins/remotelinux/makeinstallstep.cpp | 62 |
9 files changed, 62 insertions, 80 deletions
diff --git a/src/plugins/android/androidbuildapkstep.cpp b/src/plugins/android/androidbuildapkstep.cpp index b27730b45b..27b8614450 100644 --- a/src/plugins/android/androidbuildapkstep.cpp +++ b/src/plugins/android/androidbuildapkstep.cpp @@ -484,6 +484,11 @@ AndroidBuildApkStep::AndroidBuildApkStep(BuildStepList *parent, Utils::Id id) if (format == OutputFormat::Stderr) stdError(string); }); + + setDoneHook([this](bool success) { + if (m_openPackageLocationForRun && success) + QTimer::singleShot(0, this, &AndroidBuildApkStep::showInGraphicalShell); + }); } bool AndroidBuildApkStep::init() @@ -632,13 +637,6 @@ QWidget *AndroidBuildApkStep::createConfigWidget() return new AndroidBuildApkWidget(this); } -void AndroidBuildApkStep::finish(ProcessResult result) -{ - if (m_openPackageLocationForRun && isSuccess(result)) - QTimer::singleShot(0, this, &AndroidBuildApkStep::showInGraphicalShell); - AbstractProcessStep::finish(result); -} - bool AndroidBuildApkStep::verifyKeystorePassword() { if (!m_keystorePath.exists()) { diff --git a/src/plugins/android/androidbuildapkstep.h b/src/plugins/android/androidbuildapkstep.h index 89ddd208a5..e2c5c37647 100644 --- a/src/plugins/android/androidbuildapkstep.h +++ b/src/plugins/android/androidbuildapkstep.h @@ -63,7 +63,6 @@ private: bool init() override; void setupOutputFormatter(Utils::OutputFormatter *formatter) override; QWidget *createConfigWidget() override; - void finish(Utils::ProcessResult result) override; bool verifyKeystorePassword(); bool verifyCertificatePassword(); diff --git a/src/plugins/cmakeprojectmanager/cmakebuildstep.cpp b/src/plugins/cmakeprojectmanager/cmakebuildstep.cpp index 67ceb0002a..be65426d5c 100644 --- a/src/plugins/cmakeprojectmanager/cmakebuildstep.cpp +++ b/src/plugins/cmakeprojectmanager/cmakebuildstep.cpp @@ -253,6 +253,11 @@ CMakeBuildStep::CMakeBuildStep(BuildStepList *bsl, Id id) : connect(target(), &Target::activeRunConfigurationChanged, this, &CMakeBuildStep::updateBuildTargetsModel); + + setDoneHook([this](bool) { + updateDeploymentData(); + emit progress(100, {}); + }); } QVariantMap CMakeBuildStep::toMap() const @@ -795,14 +800,6 @@ void CMakeBuildStep::updateDeploymentData() buildSystem()->setDeploymentData(deploymentData); } -void CMakeBuildStep::finish(ProcessResult result) -{ - updateDeploymentData(); - - emit progress(100, {}); - AbstractProcessStep::finish(result); -} - // CMakeBuildStepFactory CMakeBuildStepFactory::CMakeBuildStepFactory() diff --git a/src/plugins/cmakeprojectmanager/cmakebuildstep.h b/src/plugins/cmakeprojectmanager/cmakebuildstep.h index 85ee46d953..e0f6f15d19 100644 --- a/src/plugins/cmakeprojectmanager/cmakebuildstep.h +++ b/src/plugins/cmakeprojectmanager/cmakebuildstep.h @@ -77,7 +77,6 @@ signals: private: Utils::CommandLine cmakeCommand() const; - void finish(Utils::ProcessResult result) override; bool fromMap(const QVariantMap &map) override; bool init() override; diff --git a/src/plugins/cmakeprojectmanager/cmakeinstallstep.cpp b/src/plugins/cmakeprojectmanager/cmakeinstallstep.cpp index d62a2edbb7..63ed8a8bfd 100644 --- a/src/plugins/cmakeprojectmanager/cmakeinstallstep.cpp +++ b/src/plugins/cmakeprojectmanager/cmakeinstallstep.cpp @@ -38,13 +38,12 @@ public: cmakeArguments.setDisplayStyle(StringAspect::LineEditDisplay); setCommandLineProvider([this] { return cmakeCommand(); }); + setDoneHook([this](bool) { emit progress(100, {}); }); } private: CommandLine cmakeCommand() const; - void finish(ProcessResult result) override; - void setupOutputFormatter(OutputFormatter *formatter) override; QWidget *createConfigWidget() override; @@ -83,12 +82,6 @@ CommandLine CMakeInstallStep::cmakeCommand() const return cmd; } -void CMakeInstallStep::finish(ProcessResult result) -{ - emit progress(100, {}); - AbstractProcessStep::finish(result); -} - QWidget *CMakeInstallStep::createConfigWidget() { auto updateDetails = [this] { diff --git a/src/plugins/projectexplorer/abstractprocessstep.cpp b/src/plugins/projectexplorer/abstractprocessstep.cpp index 9e7a217937..06952b8512 100644 --- a/src/plugins/projectexplorer/abstractprocessstep.cpp +++ b/src/plugins/projectexplorer/abstractprocessstep.cpp @@ -82,6 +82,7 @@ public: std::function<CommandLine()> m_commandLineProvider; std::function<FilePath()> m_workingDirectoryProvider; std::function<void(Environment &)> m_environmentModifier; + std::function<void(bool)> m_doneHook; // TODO: Remove me when all subclasses moved to Tasking bool m_ignoreReturnValue = false; bool m_lowPriority = false; std::unique_ptr<QTextDecoder> stdoutStream; @@ -131,6 +132,11 @@ void AbstractProcessStep::setUseEnglishOutput() d->m_environmentModifier = [](Environment &env) { env.setupEnglishOutput(); }; } +void AbstractProcessStep::setDoneHook(const std::function<void(bool)> &doneHook) +{ + d->m_doneHook = doneHook; +} + void AbstractProcessStep::setCommandLineProvider(const std::function<CommandLine()> &provider) { d->m_commandLineProvider = provider; @@ -350,15 +356,13 @@ void AbstractProcessStep::setDisplayedParameters(ProcessParameters *params) d->m_displayedParams = params; } -bool AbstractProcessStep::isSuccess(ProcessResult result) const -{ - return result == ProcessResult::FinishedWithSuccess - || (result == ProcessResult::FinishedWithError && d->m_ignoreReturnValue); -} - void AbstractProcessStep::finish(ProcessResult result) { - emit finished(isSuccess(result)); + const bool success = result == ProcessResult::FinishedWithSuccess + || (result == ProcessResult::FinishedWithError && d->m_ignoreReturnValue); + if (d->m_doneHook) + d->m_doneHook(success); + emit finished(success); } } // namespace ProjectExplorer diff --git a/src/plugins/projectexplorer/abstractprocessstep.h b/src/plugins/projectexplorer/abstractprocessstep.h index 02923ad2b1..e6b11981ee 100644 --- a/src/plugins/projectexplorer/abstractprocessstep.h +++ b/src/plugins/projectexplorer/abstractprocessstep.h @@ -33,6 +33,7 @@ protected: bool setupProcessParameters(ProcessParameters *params) const; bool ignoreReturnValue() const; void setIgnoreReturnValue(bool b); + void setDoneHook(const std::function<void(bool)> &doneHook); void setCommandLineProvider(const std::function<Utils::CommandLine()> &provider); void setWorkingDirectoryProvider(const std::function<Utils::FilePath()> &provider); void setEnvironmentModifier(const std::function<void(Utils::Environment &)> &modifier); @@ -46,9 +47,6 @@ protected: void doCancel() override; void setLowPriority(); void setDisplayedParameters(ProcessParameters *params); - bool isSuccess(Utils::ProcessResult result) const; - - virtual void finish(Utils::ProcessResult result); bool checkWorkingDirectory(); void setupProcess(Utils::Process *process); @@ -57,6 +55,7 @@ protected: private: void setupStreams(); + void finish(Utils::ProcessResult result); class Private; Private *d; diff --git a/src/plugins/qmakeprojectmanager/qmakemakestep.cpp b/src/plugins/qmakeprojectmanager/qmakemakestep.cpp index 47e6470f60..dccbe73a53 100644 --- a/src/plugins/qmakeprojectmanager/qmakemakestep.cpp +++ b/src/plugins/qmakeprojectmanager/qmakemakestep.cpp @@ -40,7 +40,6 @@ public: QmakeMakeStep(BuildStepList *bsl, Id id); private: - void finish(ProcessResult result) override; bool init() override; void setupOutputFormatter(OutputFormatter *formatter) override; void doRun() override; @@ -60,6 +59,15 @@ QmakeMakeStep::QmakeMakeStep(BuildStepList *bsl, Id id) setUserArguments("clean"); } supportDisablingForSubdirs(); + + setDoneHook([this](bool success) { + if (!success && !isCanceled() && m_unalignedBuildDir + && settings().warnAgainstUnalignedBuildDir()) { + const QString msg = Tr::tr("The build directory is not at the same level as the source " + "directory, which could be the reason for the build failure."); + emit addTask(BuildSystemTask(Task::Warning, msg)); + } + }); } bool QmakeMakeStep::init() @@ -219,17 +227,6 @@ void QmakeMakeStep::doRun() AbstractProcessStep::doRun(); } -void QmakeMakeStep::finish(ProcessResult result) -{ - if (!isSuccess(result) && !isCanceled() && m_unalignedBuildDir - && settings().warnAgainstUnalignedBuildDir()) { - const QString msg = Tr::tr("The build directory is not at the same level as the source " - "directory, which could be the reason for the build failure."); - emit addTask(BuildSystemTask(Task::Warning, msg)); - } - MakeStep::finish(result); -} - QStringList QmakeMakeStep::displayArguments() const { const auto bc = static_cast<QmakeBuildConfiguration *>(buildConfiguration()); diff --git a/src/plugins/remotelinux/makeinstallstep.cpp b/src/plugins/remotelinux/makeinstallstep.cpp index 44da4e9d83..2e85a3be16 100644 --- a/src/plugins/remotelinux/makeinstallstep.cpp +++ b/src/plugins/remotelinux/makeinstallstep.cpp @@ -41,7 +41,6 @@ private: bool fromMap(const QVariantMap &map) override; QWidget *createConfigWidget() override; bool init() override; - void finish(ProcessResult result) override; bool isJobCountSupported() const override { return false; } void updateCommandFromAspect(); @@ -132,6 +131,35 @@ MakeInstallStep::MakeInstallStep(BuildStepList *parent, Id id) : MakeStep(parent if (format == OutputFormat::Stderr && string.contains("target 'install'")) m_noInstallTarget = true; }); + + setDoneHook([this](bool success) { + if (success) { + const FilePath rootDir = makeCommand().withNewPath(m_installRoot().path()); // FIXME: Needed? + + m_deploymentData = DeploymentData(); + m_deploymentData.setLocalInstallRoot(rootDir); + + const int startPos = rootDir.path().length(); + + const auto appFileNames = transform<QSet<QString>>(buildSystem()->applicationTargets(), + [](const BuildTargetInfo &appTarget) { return appTarget.targetFilePath.fileName(); }); + + auto handleFile = [this, &appFileNames, startPos](const FilePath &filePath) { + const DeployableFile::Type type = appFileNames.contains(filePath.fileName()) + ? DeployableFile::TypeExecutable : DeployableFile::TypeNormal; + const QString targetDir = filePath.parentDir().path().mid(startPos); + m_deploymentData.addFile(filePath, targetDir, type); + return IterationPolicy::Continue; + }; + rootDir.iterateDirectory( + handleFile, {{}, QDir::Files | QDir::Hidden, QDirIterator::Subdirectories}); + + buildSystem()->setDeploymentData(m_deploymentData); + } else if (m_noInstallTarget && m_isCmakeProject) { + emit addTask(DeploymentTask(Task::Warning, Tr::tr("You need to add an install statement " + "to your CMakeLists.txt file for deployment to work."))); + } + }); } QWidget *MakeInstallStep::createConfigWidget() @@ -188,38 +216,6 @@ bool MakeInstallStep::init() return true; } -void MakeInstallStep::finish(ProcessResult result) -{ - if (isSuccess(result)) { - const FilePath rootDir = makeCommand().withNewPath(m_installRoot().path()); // FIXME: Needed? - - m_deploymentData = DeploymentData(); - m_deploymentData.setLocalInstallRoot(rootDir); - - const int startPos = rootDir.path().length(); - - const auto appFileNames = transform<QSet<QString>>(buildSystem()->applicationTargets(), - [](const BuildTargetInfo &appTarget) { return appTarget.targetFilePath.fileName(); }); - - auto handleFile = [this, &appFileNames, startPos](const FilePath &filePath) { - const DeployableFile::Type type = appFileNames.contains(filePath.fileName()) - ? DeployableFile::TypeExecutable - : DeployableFile::TypeNormal; - const QString targetDir = filePath.parentDir().path().mid(startPos); - m_deploymentData.addFile(filePath, targetDir, type); - return IterationPolicy::Continue; - }; - rootDir.iterateDirectory(handleFile, - {{}, QDir::Files | QDir::Hidden, QDirIterator::Subdirectories}); - - buildSystem()->setDeploymentData(m_deploymentData); - } else if (m_noInstallTarget && m_isCmakeProject) { - emit addTask(DeploymentTask(Task::Warning, Tr::tr("You need to add an install statement " - "to your CMakeLists.txt file for deployment to work."))); - } - MakeStep::finish(result); -} - void MakeInstallStep::updateCommandFromAspect() { if (m_customCommand.isChecked()) |