From 188795fecf54b436aeb4317b777c25fb9bdf8c15 Mon Sep 17 00:00:00 2001 From: Jarek Kobus Date: Wed, 12 Jul 2023 18:10:21 +0200 Subject: [PATCH] AbstractRemoteLinuxDeployStep: Refactor tree error handling Make it behave like AbstractProcessStep. Move success / failure logging into the recipe. Make starting the task tree look the same in both classes. That's a preparation step before moving running task tree into the base BuildStep class. Task-number: QTCREATORBUG-29168 Change-Id: I2bf3e2476d3942a01efc3b06778410dea40eef5e Reviewed-by: hjk --- .../projectexplorer/abstractprocessstep.cpp | 18 ++++--- .../abstractremotelinuxdeploystep.cpp | 53 +++++++++---------- 2 files changed, 34 insertions(+), 37 deletions(-) diff --git a/src/plugins/projectexplorer/abstractprocessstep.cpp b/src/plugins/projectexplorer/abstractprocessstep.cpp index acbb57badef..13e5d00d33a 100644 --- a/src/plugins/projectexplorer/abstractprocessstep.cpp +++ b/src/plugins/projectexplorer/abstractprocessstep.cpp @@ -170,17 +170,19 @@ void AbstractProcessStep::setupOutputFormatter(OutputFormatter *formatter) void AbstractProcessStep::doRun() { + QTC_ASSERT(!d->m_taskTree, return); + 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)), {}); }); connect(d->m_taskTree.get(), &TaskTree::done, this, [this] { - emit finished(true); d->m_taskTree.release()->deleteLater(); + emit finished(true); }); connect(d->m_taskTree.get(), &TaskTree::errorOccurred, this, [this] { - emit finished(false); d->m_taskTree.release()->deleteLater(); + emit finished(false); }); d->m_taskTree->start(); } @@ -269,12 +271,12 @@ void AbstractProcessStep::setLowPriority() void AbstractProcessStep::doCancel() { - const QString message = Tr::tr("The build step was ended forcefully."); - if (d->m_taskTree) { - d->m_taskTree.reset(); - emit addOutput(message, OutputFormat::ErrorMessage); - emit finished(false); - } + if (!d->m_taskTree) + return; + + d->m_taskTree.reset(); + emit addOutput(Tr::tr("The build step was ended forcefully."), OutputFormat::ErrorMessage); + emit finished(false); } ProcessParameters *AbstractProcessStep::processParameters() diff --git a/src/plugins/remotelinux/abstractremotelinuxdeploystep.cpp b/src/plugins/remotelinux/abstractremotelinuxdeploystep.cpp index c97f9bc25e3..894f91f49c3 100644 --- a/src/plugins/remotelinux/abstractremotelinuxdeploystep.cpp +++ b/src/plugins/remotelinux/abstractremotelinuxdeploystep.cpp @@ -26,7 +26,6 @@ namespace Internal { class AbstractRemoteLinuxDeployStepPrivate { public: - bool hasError; std::function()> internalInit; DeploymentTimeInfo deployTimes; @@ -110,33 +109,31 @@ bool AbstractRemoteLinuxDeployStep::init() void AbstractRemoteLinuxDeployStep::doRun() { - d->hasError = false; - QTC_ASSERT(!d->m_taskTree, return); d->m_taskTree.reset(new TaskTree({runRecipe()})); - const auto endHandler = [this] { + const auto onDone = [this] { d->m_taskTree.release()->deleteLater(); - handleFinished(); + emit finished(true); }; - connect(d->m_taskTree.get(), &TaskTree::done, this, endHandler); - connect(d->m_taskTree.get(), &TaskTree::errorOccurred, this, endHandler); + const auto onError = [this] { + d->m_taskTree.release()->deleteLater(); + emit finished(false); + }; + connect(d->m_taskTree.get(), &TaskTree::done, this, onDone); + connect(d->m_taskTree.get(), &TaskTree::errorOccurred, this, onError); d->m_taskTree->start(); } void AbstractRemoteLinuxDeployStep::doCancel() { - if (d->hasError) - return; - - emit addOutput(Tr::tr("User requests deployment to stop; cleaning up."), - OutputFormat::NormalMessage); - d->hasError = true; - if (!d->m_taskTree) return; + d->m_taskTree.reset(); - handleFinished(); + emit addOutput(Tr::tr("User requests deployment to stop; cleaning up."), + OutputFormat::NormalMessage); + emit finished(false); } void AbstractRemoteLinuxDeployStep::addProgressMessage(const QString &message) @@ -148,7 +145,6 @@ void AbstractRemoteLinuxDeployStep::addErrorMessage(const QString &message) { emit addOutput(message, OutputFormat::ErrorMessage); emit addTask(DeploymentTask(Task::Error, message), 1); // TODO correct? - d->hasError = true; } void AbstractRemoteLinuxDeployStep::addWarningMessage(const QString &message) @@ -157,16 +153,6 @@ void AbstractRemoteLinuxDeployStep::addWarningMessage(const QString &message) emit addTask(DeploymentTask(Task::Warning, message), 1); // TODO correct? } -void AbstractRemoteLinuxDeployStep::handleFinished() -{ - if (d->hasError) - emit addOutput(Tr::tr("Deploy step failed."), OutputFormat::ErrorMessage); - else - emit addOutput(Tr::tr("Deploy step finished."), OutputFormat::NormalMessage); - - emit finished(!d->hasError); -} - void AbstractRemoteLinuxDeployStep::handleStdOutData(const QString &data) { emit addOutput(data, OutputFormat::Stdout, DontAppendNewline); @@ -188,17 +174,26 @@ GroupItem AbstractRemoteLinuxDeployStep::runRecipe() const auto canDeploy = isDeploymentPossible(); if (!canDeploy) { addErrorMessage(canDeploy.error()); - handleFinished(); return SetupResult::StopWithError; } if (!isDeploymentNecessary()) { addProgressMessage(Tr::tr("No deployment action necessary. Skipping.")); - handleFinished(); return SetupResult::StopWithDone; } return SetupResult::Continue; }; - return Group { onGroupSetup(onSetup), deployRecipe() }; + const auto onDone = [this] { + emit addOutput(Tr::tr("Deploy step finished."), OutputFormat::NormalMessage); + }; + const auto onError = [this] { + emit addOutput(Tr::tr("Deploy step failed."), OutputFormat::ErrorMessage); + }; + return Group { + onGroupSetup(onSetup), + deployRecipe(), + onGroupDone(onDone), + onGroupError(onError) + }; } } // namespace RemoteLinux