From 47ac604aea470a2b3e4c30c640c2ed367668d2ad Mon Sep 17 00:00:00 2001 From: Jarek Kobus Date: Wed, 12 Jul 2023 16:30:33 +0200 Subject: [PATCH] AbstractProcessStep: Use task tree for all subclasses Introduce AbstractProcessStep::runRecipe() virtual method with the default implementation. Task-number: QTCREATORBUG-29168 Change-Id: Iac75f4c38f8ee91ad8ac9324bb27881a3722911f Reviewed-by: Reviewed-by: hjk --- src/plugins/android/androidbuildapkstep.cpp | 4 +- src/plugins/android/androidbuildapkstep.h | 2 +- .../androidpackageinstallationstep.cpp | 6 +- .../autotoolsprojectmanager/autogenstep.cpp | 6 +- .../autoreconfstep.cpp | 6 +- .../autotoolsprojectmanager/configurestep.cpp | 6 +- .../cmakeprojectmanager/cmakebuildstep.cpp | 5 +- .../cmakeprojectmanager/cmakebuildstep.h | 2 +- .../projectexplorer/abstractprocessstep.cpp | 81 +++++-------------- .../projectexplorer/abstractprocessstep.h | 16 +--- .../python/pysidebuildconfiguration.cpp | 5 +- src/plugins/python/pysidebuildconfiguration.h | 2 +- .../qmakeprojectmanager/qmakemakestep.cpp | 11 ++- src/plugins/qmakeprojectmanager/qmakestep.cpp | 4 +- src/plugins/qmakeprojectmanager/qmakestep.h | 2 +- src/plugins/remotelinux/makeinstallstep.cpp | 6 +- 16 files changed, 61 insertions(+), 103 deletions(-) diff --git a/src/plugins/android/androidbuildapkstep.cpp b/src/plugins/android/androidbuildapkstep.cpp index 5556db1c940..9b6e8d4dddf 100644 --- a/src/plugins/android/androidbuildapkstep.cpp +++ b/src/plugins/android/androidbuildapkstep.cpp @@ -696,7 +696,7 @@ static bool copyFileIfNewer(const FilePath &sourceFilePath, return true; } -void AndroidBuildApkStep::doRun() +Tasking::GroupItem AndroidBuildApkStep::runRecipe() { using namespace Tasking; @@ -854,7 +854,7 @@ void AndroidBuildApkStep::doRun() onGroupDone(onDone), defaultProcessTask() }; - runTaskTree(root); + return root; } void AndroidBuildApkStep::reportWarningOrError(const QString &message, Task::TaskType type) diff --git a/src/plugins/android/androidbuildapkstep.h b/src/plugins/android/androidbuildapkstep.h index e2c5c376470..fd68a4ae2cd 100644 --- a/src/plugins/android/androidbuildapkstep.h +++ b/src/plugins/android/androidbuildapkstep.h @@ -66,7 +66,7 @@ private: bool verifyKeystorePassword(); bool verifyCertificatePassword(); - void doRun() override; + Tasking::GroupItem runRecipe() final; void stdError(const QString &output); void reportWarningOrError(const QString &message, ProjectExplorer::Task::TaskType type); diff --git a/src/plugins/android/androidpackageinstallationstep.cpp b/src/plugins/android/androidpackageinstallationstep.cpp index bd3ddfb7f0b..eaa8dc84162 100644 --- a/src/plugins/android/androidpackageinstallationstep.cpp +++ b/src/plugins/android/androidpackageinstallationstep.cpp @@ -46,7 +46,7 @@ public: private: bool init() final; void setupOutputFormatter(OutputFormatter *formatter) final; - void doRun() final; + Tasking::GroupItem runRecipe() final; void reportWarningOrError(const QString &message, ProjectExplorer::Task::TaskType type); @@ -120,7 +120,7 @@ void AndroidPackageInstallationStep::setupOutputFormatter(OutputFormatter *forma AbstractProcessStep::setupOutputFormatter(formatter); } -void AndroidPackageInstallationStep::doRun() +Tasking::GroupItem AndroidPackageInstallationStep::runRecipe() { using namespace Tasking; @@ -171,7 +171,7 @@ void AndroidPackageInstallationStep::doRun() return SetupResult::Continue; }; - runTaskTree({onGroupSetup(onSetup), defaultProcessTask()}); + return Group { onGroupSetup(onSetup), defaultProcessTask() }; } void AndroidPackageInstallationStep::reportWarningOrError(const QString &message, diff --git a/src/plugins/autotoolsprojectmanager/autogenstep.cpp b/src/plugins/autotoolsprojectmanager/autogenstep.cpp index a6121de9276..ec54184478b 100644 --- a/src/plugins/autotoolsprojectmanager/autogenstep.cpp +++ b/src/plugins/autotoolsprojectmanager/autogenstep.cpp @@ -40,7 +40,7 @@ public: AutogenStep(BuildStepList *bsl, Id id); private: - void doRun() final; + Tasking::GroupItem runRecipe() final; bool m_runAutogen = false; StringAspect m_arguments{this}; @@ -70,7 +70,7 @@ AutogenStep::AutogenStep(BuildStepList *bsl, Id id) : AbstractProcessStep(bsl, i }); } -void AutogenStep::doRun() +Tasking::GroupItem AutogenStep::runRecipe() { using namespace Tasking; @@ -96,7 +96,7 @@ void AutogenStep::doRun() }; const auto onDone = [this] { m_runAutogen = false; }; - runTaskTree({onGroupSetup(onSetup), onGroupDone(onDone), defaultProcessTask()}); + return Group { onGroupSetup(onSetup), onGroupDone(onDone), defaultProcessTask() }; } // AutogenStepFactory diff --git a/src/plugins/autotoolsprojectmanager/autoreconfstep.cpp b/src/plugins/autotoolsprojectmanager/autoreconfstep.cpp index 3d545a6e754..2afc690387e 100644 --- a/src/plugins/autotoolsprojectmanager/autoreconfstep.cpp +++ b/src/plugins/autotoolsprojectmanager/autoreconfstep.cpp @@ -60,7 +60,8 @@ public: }); } - void doRun() override +private: + Tasking::GroupItem runRecipe() final { using namespace Tasking; @@ -79,10 +80,9 @@ public: }; const auto onDone = [this] { m_runAutoreconf = false; }; - runTaskTree({onGroupSetup(onSetup), onGroupDone(onDone), defaultProcessTask()}); + return Group { onGroupSetup(onSetup), onGroupDone(onDone), defaultProcessTask() }; } -private: bool m_runAutoreconf = false; StringAspect arguments{this}; }; diff --git a/src/plugins/autotoolsprojectmanager/configurestep.cpp b/src/plugins/autotoolsprojectmanager/configurestep.cpp index 58515990182..14c7a3eefa6 100644 --- a/src/plugins/autotoolsprojectmanager/configurestep.cpp +++ b/src/plugins/autotoolsprojectmanager/configurestep.cpp @@ -63,7 +63,7 @@ public: } private: - void doRun() final; + Tasking::GroupItem runRecipe() final; CommandLine getCommandLine(const QString &arguments) { @@ -74,7 +74,7 @@ private: StringAspect arguments{this}; }; -void ConfigureStep::doRun() +Tasking::GroupItem ConfigureStep::runRecipe() { using namespace Tasking; @@ -100,7 +100,7 @@ void ConfigureStep::doRun() }; const auto onDone = [this] { m_runConfigure = false; }; - runTaskTree({onGroupSetup(onSetup), onGroupDone(onDone), defaultProcessTask()}); + return Group { onGroupSetup(onSetup), onGroupDone(onDone), defaultProcessTask() }; } // ConfigureStepFactory diff --git a/src/plugins/cmakeprojectmanager/cmakebuildstep.cpp b/src/plugins/cmakeprojectmanager/cmakebuildstep.cpp index 7c4539a5dc4..3ce1198852d 100644 --- a/src/plugins/cmakeprojectmanager/cmakebuildstep.cpp +++ b/src/plugins/cmakeprojectmanager/cmakebuildstep.cpp @@ -347,7 +347,7 @@ void CMakeBuildStep::setupOutputFormatter(Utils::OutputFormatter *formatter) CMakeAbstractProcessStep::setupOutputFormatter(formatter); } -void CMakeBuildStep::doRun() +Tasking::GroupItem CMakeBuildStep::runRecipe() { using namespace Tasking; @@ -380,8 +380,7 @@ void CMakeBuildStep::doRun() onGroupDone(onEnd), onGroupError(onEnd) }; - - runTaskTree(root); + return root; } QString CMakeBuildStep::defaultBuildTarget() const diff --git a/src/plugins/cmakeprojectmanager/cmakebuildstep.h b/src/plugins/cmakeprojectmanager/cmakebuildstep.h index 12d9a847129..89b0cbd3b5a 100644 --- a/src/plugins/cmakeprojectmanager/cmakebuildstep.h +++ b/src/plugins/cmakeprojectmanager/cmakebuildstep.h @@ -81,7 +81,7 @@ private: bool init() override; void setupOutputFormatter(Utils::OutputFormatter *formatter) override; - void doRun() override; + Tasking::GroupItem runRecipe() final; QWidget *createConfigWidget() override; Utils::FilePath cmakeExecutable() const; diff --git a/src/plugins/projectexplorer/abstractprocessstep.cpp b/src/plugins/projectexplorer/abstractprocessstep.cpp index 00c345bd5da..acbb57badef 100644 --- a/src/plugins/projectexplorer/abstractprocessstep.cpp +++ b/src/plugins/projectexplorer/abstractprocessstep.cpp @@ -3,14 +3,11 @@ #include "abstractprocessstep.h" -#include "buildconfiguration.h" -#include "buildstep.h" #include "processparameters.h" #include "projectexplorer.h" #include "projectexplorersettings.h" #include "projectexplorertr.h" -#include #include #include #include @@ -75,7 +72,6 @@ public: Private(AbstractProcessStep *q) : q(q) {} AbstractProcessStep *q; - std::unique_ptr m_process; std::unique_ptr m_taskTree; ProcessParameters m_param; ProcessParameters *m_displayedParams = &m_param; @@ -84,8 +80,8 @@ public: std::function m_environmentModifier; bool m_ignoreReturnValue = false; bool m_lowPriority = false; - std::unique_ptr stdoutStream; - std::unique_ptr stderrStream; + std::unique_ptr stdOutDecoder; + std::unique_ptr stdErrDecoder; OutputFormatter *outputFormatter = nullptr; }; @@ -148,12 +144,15 @@ void AbstractProcessStep::setWorkingDirectoryProvider(const std::functionm_process || d->m_taskTree) + if (d->m_taskTree) return false; if (!setupProcessParameters(processParameters())) return false; + d->stdOutDecoder = std::make_unique(buildEnvironment().hasKey("VSLANG") + ? QTextCodec::codecForName("UTF-8") : QTextCodec::codecForLocale()); + d->stdErrDecoder = std::make_unique(QTextCodec::codecForLocale()); return true; } @@ -171,29 +170,19 @@ void AbstractProcessStep::setupOutputFormatter(OutputFormatter *formatter) void AbstractProcessStep::doRun() { - setupStreams(); - - d->m_process.reset(new Process); - if (!setupProcess(*d->m_process.get())) { - d->m_process.reset(); - finish(ProcessResult::StartFailed); - return; - } - connect(d->m_process.get(), &Process::done, this, [this] { - handleProcessDone(*d->m_process); - const ProcessResult result = d->outputFormatter->hasFatalErrors() - ? ProcessResult::FinishedWithError : d->m_process->result(); - d->m_process.release()->deleteLater(); - finish(result); + d->m_taskTree.reset(new TaskTree({runRecipe()})); + connect(d->m_taskTree.get(), &TaskTree::progressValueChanged, this, [this](int value) { + emit progress(qRound(double(value) * 100 / std::max(d->m_taskTree->progressMaximum(), 1)), {}); }); - d->m_process->start(); -} - -void AbstractProcessStep::setupStreams() -{ - d->stdoutStream = std::make_unique(buildEnvironment().hasKey("VSLANG") - ? QTextCodec::codecForName("UTF-8") : QTextCodec::codecForLocale()); - d->stderrStream = std::make_unique(QTextCodec::codecForLocale()); + connect(d->m_taskTree.get(), &TaskTree::done, this, [this] { + emit finished(true); + d->m_taskTree.release()->deleteLater(); + }); + connect(d->m_taskTree.get(), &TaskTree::errorOccurred, this, [this] { + emit finished(false); + d->m_taskTree.release()->deleteLater(); + }); + d->m_taskTree->start(); } GroupItem AbstractProcessStep::defaultProcessTask() @@ -234,11 +223,11 @@ bool AbstractProcessStep::setupProcess(Process &process) process.setLowPriority(); connect(&process, &Process::readyReadStandardOutput, this, [this, &process] { - emit addOutput(d->stdoutStream->toUnicode(process.readAllRawStandardOutput()), + emit addOutput(d->stdOutDecoder->toUnicode(process.readAllRawStandardOutput()), OutputFormat::Stdout, DontAppendNewline); }); connect(&process, &Process::readyReadStandardError, this, [this, &process] { - emit addOutput(d->stderrStream->toUnicode(process.readAllRawStandardError()), + emit addOutput(d->stdErrDecoder->toUnicode(process.readAllRawStandardError()), OutputFormat::Stderr, DontAppendNewline); }); connect(&process, &Process::started, this, [this] { @@ -273,25 +262,6 @@ void AbstractProcessStep::handleProcessDone(const Process &process) } } -void AbstractProcessStep::runTaskTree(const Group &recipe) -{ - setupStreams(); - - d->m_taskTree.reset(new TaskTree(recipe)); - connect(d->m_taskTree.get(), &TaskTree::progressValueChanged, this, [this](int value) { - emit progress(qRound(double(value) * 100 / std::max(d->m_taskTree->progressMaximum(), 1)), {}); - }); - connect(d->m_taskTree.get(), &TaskTree::done, this, [this] { - emit finished(true); - d->m_taskTree.release()->deleteLater(); - }); - connect(d->m_taskTree.get(), &TaskTree::errorOccurred, this, [this] { - emit finished(false); - d->m_taskTree.release()->deleteLater(); - }); - d->m_taskTree->start(); -} - void AbstractProcessStep::setLowPriority() { d->m_lowPriority = true; @@ -300,11 +270,6 @@ void AbstractProcessStep::setLowPriority() void AbstractProcessStep::doCancel() { const QString message = Tr::tr("The build step was ended forcefully."); - if (d->m_process) { - emit addOutput(message, OutputFormat::ErrorMessage); - d->m_process.reset(); - finish(ProcessResult::TerminatedAbnormally); - } if (d->m_taskTree) { d->m_taskTree.reset(); emit addOutput(message, OutputFormat::ErrorMessage); @@ -352,11 +317,9 @@ void AbstractProcessStep::setDisplayedParameters(ProcessParameters *params) d->m_displayedParams = params; } -void AbstractProcessStep::finish(ProcessResult result) +GroupItem AbstractProcessStep::runRecipe() { - const bool success = result == ProcessResult::FinishedWithSuccess - || (result == ProcessResult::FinishedWithError && d->m_ignoreReturnValue); - emit finished(success); + return Group { ignoreReturnValue() ? finishAllAndDone : stopOnError, defaultProcessTask() }; } } // namespace ProjectExplorer diff --git a/src/plugins/projectexplorer/abstractprocessstep.h b/src/plugins/projectexplorer/abstractprocessstep.h index 899440ce27d..7b5209a0234 100644 --- a/src/plugins/projectexplorer/abstractprocessstep.h +++ b/src/plugins/projectexplorer/abstractprocessstep.h @@ -5,18 +5,12 @@ #include "buildstep.h" -#include - namespace Utils { class CommandLine; class Process; -enum class ProcessResult; } -namespace Tasking { -class Group; -class GroupItem; -} +namespace Tasking { class GroupItem; } namespace ProjectExplorer { class ProcessParameters; @@ -45,19 +39,17 @@ protected: bool init() override; void setupOutputFormatter(Utils::OutputFormatter *formatter) override; - void doRun() override; - void doCancel() override; + void doRun() final; + void doCancel() final; void setLowPriority(); void setDisplayedParameters(ProcessParameters *params); Tasking::GroupItem defaultProcessTask(); bool setupProcess(Utils::Process &process); void handleProcessDone(const Utils::Process &process); - void runTaskTree(const Tasking::Group &recipe); private: - void setupStreams(); - void finish(Utils::ProcessResult result); + virtual Tasking::GroupItem runRecipe(); class Private; Private *d; diff --git a/src/plugins/python/pysidebuildconfiguration.cpp b/src/plugins/python/pysidebuildconfiguration.cpp index 3993363fcd6..8525e248c4f 100644 --- a/src/plugins/python/pysidebuildconfiguration.cpp +++ b/src/plugins/python/pysidebuildconfiguration.cpp @@ -59,7 +59,7 @@ void PySideBuildStep::updatePySideProjectPath(const FilePath &pySideProjectPath) m_pysideProject.setValue(pySideProjectPath); } -void PySideBuildStep::doRun() +Tasking::GroupItem PySideBuildStep::runRecipe() { using namespace Tasking; @@ -69,10 +69,9 @@ void PySideBuildStep::doRun() return SetupResult::Continue; }; - runTaskTree({onGroupSetup(onSetup), defaultProcessTask()}); + return Group { onGroupSetup(onSetup), defaultProcessTask() }; } - // PySideBuildConfiguration class PySideBuildConfiguration : public BuildConfiguration diff --git a/src/plugins/python/pysidebuildconfiguration.h b/src/plugins/python/pysidebuildconfiguration.h index e686b9c2220..58ae930ea87 100644 --- a/src/plugins/python/pysidebuildconfiguration.h +++ b/src/plugins/python/pysidebuildconfiguration.h @@ -17,7 +17,7 @@ public: void updatePySideProjectPath(const Utils::FilePath &pySideProjectPath); private: - void doRun() override; + Tasking::GroupItem runRecipe() final; Utils::FilePathAspect m_pysideProject{this}; }; diff --git a/src/plugins/qmakeprojectmanager/qmakemakestep.cpp b/src/plugins/qmakeprojectmanager/qmakemakestep.cpp index 9d949a09923..b79f79b7a6c 100644 --- a/src/plugins/qmakeprojectmanager/qmakemakestep.cpp +++ b/src/plugins/qmakeprojectmanager/qmakemakestep.cpp @@ -42,7 +42,7 @@ public: private: bool init() override; void setupOutputFormatter(OutputFormatter *formatter) override; - void doRun() override; + Tasking::GroupItem runRecipe() final; QStringList displayArguments() const override; bool m_scriptTarget = false; @@ -200,7 +200,7 @@ void QmakeMakeStep::setupOutputFormatter(OutputFormatter *formatter) AbstractProcessStep::setupOutputFormatter(formatter); } -void QmakeMakeStep::doRun() +Tasking::GroupItem QmakeMakeStep::runRecipe() { using namespace Tasking; @@ -226,7 +226,12 @@ void QmakeMakeStep::doRun() } }; - runTaskTree({onGroupSetup(onSetup), onGroupError(onError), defaultProcessTask()}); + return Group { + ignoreReturnValue() ? finishAllAndDone : stopOnError, + onGroupSetup(onSetup), + onGroupError(onError), + defaultProcessTask() + }; } QStringList QmakeMakeStep::displayArguments() const diff --git a/src/plugins/qmakeprojectmanager/qmakestep.cpp b/src/plugins/qmakeprojectmanager/qmakestep.cpp index 94b93354347..7117aa8ffcb 100644 --- a/src/plugins/qmakeprojectmanager/qmakestep.cpp +++ b/src/plugins/qmakeprojectmanager/qmakestep.cpp @@ -265,7 +265,7 @@ void QMakeStep::setupOutputFormatter(OutputFormatter *formatter) AbstractProcessStep::setupOutputFormatter(formatter); } -void QMakeStep::doRun() +Tasking::GroupItem QMakeStep::runRecipe() { using namespace Tasking; @@ -308,7 +308,7 @@ void QMakeStep::doRun() if (m_runMakeQmake) processList << ProcessTask(setupMakeQMake, onProcessDone, onProcessDone); - runTaskTree(Group(processList)); + return Group(processList); } void QMakeStep::setForced(bool b) diff --git a/src/plugins/qmakeprojectmanager/qmakestep.h b/src/plugins/qmakeprojectmanager/qmakestep.h index d977eedd904..366bbacf60b 100644 --- a/src/plugins/qmakeprojectmanager/qmakestep.h +++ b/src/plugins/qmakeprojectmanager/qmakestep.h @@ -101,7 +101,6 @@ public: QmakeBuildSystem *qmakeBuildSystem() const; bool init() override; void setupOutputFormatter(Utils::OutputFormatter *formatter) override; - void doRun() override; QWidget *createConfigWidget() override; void setForced(bool b); @@ -138,6 +137,7 @@ protected: bool fromMap(const QVariantMap &map) override; private: + Tasking::GroupItem runRecipe() final; // slots for handling buildconfiguration/step signals void qtVersionChanged(); void qmakeBuildConfigChanged(); diff --git a/src/plugins/remotelinux/makeinstallstep.cpp b/src/plugins/remotelinux/makeinstallstep.cpp index 1e44933f270..2749a292a61 100644 --- a/src/plugins/remotelinux/makeinstallstep.cpp +++ b/src/plugins/remotelinux/makeinstallstep.cpp @@ -41,7 +41,7 @@ private: bool fromMap(const QVariantMap &map) override; QWidget *createConfigWidget() override; bool init() override; - void doRun() override; + Tasking::GroupItem runRecipe() final; bool isJobCountSupported() const override { return false; } void updateCommandFromAspect(); @@ -188,7 +188,7 @@ bool MakeInstallStep::init() return true; } -void MakeInstallStep::doRun() +Tasking::GroupItem MakeInstallStep::runRecipe() { using namespace Tasking; @@ -222,7 +222,7 @@ void MakeInstallStep::doRun() } }; - runTaskTree({onGroupDone(onDone), onGroupError(onError), defaultProcessTask()}); + return Group { onGroupDone(onDone), onGroupError(onError), defaultProcessTask() }; } void MakeInstallStep::updateCommandFromAspect()