BuildStep: Remove finished() signal and use FutureInterface to report

Remove the finished() signal that is (sometimes) used to report that
a buildstep is done and use the FutureInterface for that purpose
consistently.

Change-Id: Ibe5520b562b91f1a7f4fc73ee898b33b930029ec
Reviewed-by: Tim Jenssen <tim.jenssen@theqtcompany.com>
Reviewed-by: Christian Kandeler <christian.kandeler@theqtcompany.com>

src/plugins/android/androiddeployqtstep.cpp

@@ -417,8 +417,7 @@ void AndroidDeployQtStep::run(QFutureInterface<bool> &fi)
     if (!m_avdName.isEmpty()) {
         QString serialNumber = AndroidConfigurations::currentConfig().waitForAvd(m_avdName, fi);
         if (serialNumber.isEmpty()) {
-            fi.reportResult(false);
+            reportRunResult(fi, false);
-            emit finished();
             return;
         }
         m_serialNumber = serialNumber;
@@ -460,8 +459,7 @@ void AndroidDeployQtStep::run(QFutureInterface<bool> &fi)
                << QLatin1String("/system/") + m_libdir + QLatin1String("/libc.so")
                << QString::fromLatin1("%1/libc.so").arg(m_buildDirectory));
 
-    fi.reportResult(returnValue == Success ? true : false);
+    reportRunResult(fi, returnValue == Success);
-    fi.reportFinished();
 }
 
 void AndroidDeployQtStep::runCommand(const QString &program, const QStringList &arguments)

src/plugins/autotoolsprojectmanager/autogenstep.cpp

@@ -157,7 +157,7 @@ bool AutogenStep::init(QList<const BuildStep *> &earlierSteps)
     return AbstractProcessStep::init(earlierSteps);
 }
 
-void AutogenStep::run(QFutureInterface<bool> &interface)
+void AutogenStep::run(QFutureInterface<bool> &fi)
 {
     BuildConfiguration *bc = buildConfiguration();
 
@@ -175,13 +175,12 @@ void AutogenStep::run(QFutureInterface<bool> &interface)
 
     if (!m_runAutogen) {
         emit addOutput(tr("Configuration unchanged, skipping autogen step."), BuildStep::MessageOutput);
-        interface.reportResult(true);
+        reportRunResult(fi, true);
-        emit finished();
         return;
     }
 
     m_runAutogen = false;
-    AbstractProcessStep::run(interface);
+    AbstractProcessStep::run(fi);
 }
 
 BuildStepConfigWidget *AutogenStep::createConfigWidget()

src/plugins/autotoolsprojectmanager/autogenstep.h

@@ -91,7 +91,7 @@ public:
     explicit AutogenStep(ProjectExplorer::BuildStepList *bsl);
 
     bool init(QList<const BuildStep *> &earlierSteps) override;
-    void run(QFutureInterface<bool> &interface) override;
+    void run(QFutureInterface<bool> &fi) override;
     ProjectExplorer::BuildStepConfigWidget *createConfigWidget() override;
     bool immutable() const override;
     QString additionalArguments() const;

src/plugins/autotoolsprojectmanager/autoreconfstep.cpp

@@ -158,7 +158,7 @@ bool AutoreconfStep::init(QList<const BuildStep *> &earlierSteps)
     return AbstractProcessStep::init(earlierSteps);
 }
 
-void AutoreconfStep::run(QFutureInterface<bool> &interface)
+void AutoreconfStep::run(QFutureInterface<bool> &fi)
 {
     BuildConfiguration *bc = buildConfiguration();
 
@@ -170,13 +170,12 @@ void AutoreconfStep::run(QFutureInterface<bool> &interface)
 
     if (!m_runAutoreconf) {
         emit addOutput(tr("Configuration unchanged, skipping autoreconf step."), BuildStep::MessageOutput);
-        interface.reportResult(true);
+        reportRunResult(fi, true);
-        emit finished();
         return;
     }
 
     m_runAutoreconf = false;
-    AbstractProcessStep::run(interface);
+    AbstractProcessStep::run(fi);
 }
 
 BuildStepConfigWidget *AutoreconfStep::createConfigWidget()

src/plugins/autotoolsprojectmanager/autoreconfstep.h

@@ -91,7 +91,7 @@ public:
     explicit AutoreconfStep(ProjectExplorer::BuildStepList *bsl);
 
     bool init(QList<const BuildStep *> &earlierSteps) override;
-    void run(QFutureInterface<bool> &interface) override;
+    void run(QFutureInterface<bool> &fi) override;
     ProjectExplorer::BuildStepConfigWidget *createConfigWidget() override;
     bool immutable() const override;
     QString additionalArguments() const;

src/plugins/autotoolsprojectmanager/configurestep.cpp

@@ -171,7 +171,7 @@ bool ConfigureStep::init(QList<const BuildStep *> &earlierSteps)
     return AbstractProcessStep::init(earlierSteps);
 }
 
-void ConfigureStep::run(QFutureInterface<bool>& interface)
+void ConfigureStep::run(QFutureInterface<bool>& fi)
 {
     BuildConfiguration *bc = buildConfiguration();
 
@@ -187,13 +187,12 @@ void ConfigureStep::run(QFutureInterface<bool>& interface)
 
     if (!m_runConfigure) {
         emit addOutput(tr("Configuration unchanged, skipping configure step."), BuildStep::MessageOutput);
-        interface.reportResult(true);
+        reportRunResult(fi, true);
-        emit finished();
         return;
     }
 
     m_runConfigure = false;
-    AbstractProcessStep::run(interface);
+    AbstractProcessStep::run(fi);
 }
 
 BuildStepConfigWidget *ConfigureStep::createConfigWidget()

src/plugins/autotoolsprojectmanager/configurestep.h

@@ -90,7 +90,7 @@ public:
     explicit ConfigureStep(ProjectExplorer::BuildStepList *bsl);
 
     bool init(QList<const BuildStep *> &earlierSteps) override;
-    void run(QFutureInterface<bool> &interface) override;
+    void run(QFutureInterface<bool> &fi) override;
     ProjectExplorer::BuildStepConfigWidget *createConfigWidget() override;
     bool immutable() const override;
     QString additionalArguments() const;

src/plugins/baremetal/baremetalgdbcommandsdeploystep.cpp

@@ -83,8 +83,7 @@ void BareMetalGdbCommandsDeployStep::ctor()
 
 void BareMetalGdbCommandsDeployStep::run(QFutureInterface<bool> &fi)
 {
-    fi.reportResult(true);
+    reportRunResult(fi, true);
-    emit finished();
 }
 
 bool BareMetalGdbCommandsDeployStep::fromMap(const QVariantMap &map)

src/plugins/cmakeprojectmanager/cmakebuildstep.cpp

@@ -236,9 +236,7 @@ void CMakeBuildStep::run(QFutureInterface<bool> &fi)
         m_runTrigger = connect(bc, &CMakeBuildConfiguration::dataAvailable,
                                this, [this, &fi]() { runImpl(fi); });
         m_errorTrigger = connect(bc, &CMakeBuildConfiguration::errorOccured,
-                                 this, [this, &fi]() {
+                                 this, [this, &fi]() { reportRunResult(fi, false); });
-            fi.reportResult(false);
-        });
     } else {
         runImpl(fi);
     }

src/plugins/ios/iosdeploystep.cpp

@@ -117,9 +117,8 @@ void IosDeployStep::run(QFutureInterface<bool> &fi)
         if (iossimulator().isNull())
             TaskHub::addTask(Task::Error, tr("Deployment failed. No iOS device found."),
                              ProjectExplorer::Constants::TASK_CATEGORY_DEPLOYMENT);
-        m_futureInterface.reportResult(!iossimulator().isNull());
+        reportRunResult(m_futureInterface, !iossimulator().isNull());
         cleanup();
-        emit finished();
         return;
     }
     m_transferStatus = TransferInProgress;
@@ -179,7 +178,7 @@ void IosDeployStep::handleDidTransferApp(IosToolHandler *handler, const QString
                              tr("Deployment failed. The settings in the Devices window of Xcode might be incorrect."),
                              ProjectExplorer::Constants::TASK_CATEGORY_DEPLOYMENT);
     }
-    m_futureInterface.reportResult(status == IosToolHandler::Success);
+    reportRunResult(m_futureInterface, status == IosToolHandler::Success);
 }
 
 void IosDeployStep::handleFinished(IosToolHandler *handler)
@@ -189,7 +188,7 @@ void IosDeployStep::handleFinished(IosToolHandler *handler)
         m_transferStatus = TransferFailed;
         TaskHub::addTask(Task::Error, tr("Deployment failed."),
                          ProjectExplorer::Constants::TASK_CATEGORY_DEPLOYMENT);
-        m_futureInterface.reportResult(false);
+        reportRunResult(m_futureInterface, false);
         break;
     case NoTransfer:
     case TransferOk:
@@ -199,7 +198,6 @@ void IosDeployStep::handleFinished(IosToolHandler *handler)
     cleanup();
     handler->deleteLater();
     // move it when result is reported? (would need care to avoid problems with concurrent runs)
-    emit finished();
 }
 
 void IosDeployStep::handleErrorMsg(IosToolHandler *handler, const QString &msg)

src/plugins/projectexplorer/abstractprocessstep.cpp

@@ -187,8 +187,7 @@ void AbstractProcessStep::run(QFutureInterface<bool> &fi)
             emit addOutput(tr("Could not create directory \"%1\"")
                            .arg(QDir::toNativeSeparators(wd.absolutePath())),
                            BuildStep::ErrorMessageOutput);
-            fi.reportResult(false);
+            reportRunResult(fi, false);
-            emit finished();
             return;
         }
     }
@@ -196,8 +195,7 @@ void AbstractProcessStep::run(QFutureInterface<bool> &fi)
     QString effectiveCommand = m_param.effectiveCommand();
     if (!QFileInfo::exists(effectiveCommand)) {
         processStartupFailed();
-        fi.reportResult(false);
+        reportRunResult(fi, false);
-        emit finished();
         return;
     }
 
@@ -221,8 +219,7 @@ void AbstractProcessStep::run(QFutureInterface<bool> &fi)
         processStartupFailed();
         delete m_process;
         m_process = nullptr;
-        fi.reportResult(false);
+        reportRunResult(fi, false);
-        emit finished();
         return;
     }
     processStarted();
@@ -237,7 +234,7 @@ void AbstractProcessStep::cleanUp()
 {
     // The process has finished, leftover data is read in processFinished
     processFinished(m_process->exitCode(), m_process->exitStatus());
-    bool returnValue = processSucceeded(m_process->exitCode(), m_process->exitStatus()) || m_ignoreReturnValue;
+    const bool returnValue = processSucceeded(m_process->exitCode(), m_process->exitStatus()) || m_ignoreReturnValue;
 
     // Clean up output parsers
     if (m_outputParserChain) {
@@ -245,12 +242,13 @@ void AbstractProcessStep::cleanUp()
         m_outputParserChain = nullptr;
     }
 
+    // Clean up process
     delete m_process;
     m_process = nullptr;
-    m_futureInterface->reportResult(returnValue);
-    m_futureInterface = nullptr;
 
-    emit finished();
+    // Report result
+    reportRunResult(*m_futureInterface, returnValue);
+    m_futureInterface = nullptr;
 }
 
 /*!

src/plugins/projectexplorer/buildmanager.cpp

@@ -64,18 +64,21 @@ static QString msgProgress(int progress, int total)
     return BuildManager::tr("Finished %1 of %n steps", 0, total).arg(progress);
 }
 
-struct BuildManagerPrivate
+class BuildManagerPrivate
 {
-    BuildManagerPrivate();
+public:
-
+    Internal::CompileOutputWindow *m_outputWindow = nullptr;
-    Internal::CompileOutputWindow *m_outputWindow;
+    Internal::TaskWindow *m_taskWindow = nullptr;
-    TaskHub *m_taskHub;
-    Internal::TaskWindow *m_taskWindow;
 
     QList<BuildStep *> m_buildQueue;
     QList<bool> m_enabledState;
     QStringList m_stepNames;
-    bool m_running;
+    int m_progress = 0;
+    int m_maxProgress = 0;
+    bool m_running = false;
+    // is set to true while canceling, so that nextBuildStep knows that the BuildStep finished because of canceling
+    bool m_skipDisabled = false;
+    bool m_canceling = false;
     QFutureWatcher<bool> m_watcher;
     QFutureInterface<bool> m_futureInterfaceForAysnc;
     BuildStep *m_currentBuildStep;
@@ -84,31 +87,16 @@ struct BuildManagerPrivate
     QHash<Project *, int>  m_activeBuildSteps;
     QHash<Target *, int> m_activeBuildStepsPerTarget;
     QHash<ProjectConfiguration *, int> m_activeBuildStepsPerProjectConfiguration;
-    Project *m_previousBuildStepProject;
+    Project *m_previousBuildStepProject = nullptr;
-    // is set to true while canceling, so that nextBuildStep knows that the BuildStep finished because of canceling
-    bool m_skipDisabled;
-    bool m_canceling;
 
     // Progress reporting to the progress manager
-    int m_progress;
+    QFutureInterface<void> *m_progressFutureInterface = nullptr;
-    int m_maxProgress;
-    QFutureInterface<void> *m_progressFutureInterface;
     QFutureWatcher<void> m_progressWatcher;
     QPointer<FutureProgress> m_futureProgress;
 
     QElapsedTimer m_elapsed;
 };
 
-BuildManagerPrivate::BuildManagerPrivate() :
-    m_running(false)
-  , m_previousBuildStepProject(0)
-  , m_skipDisabled(false)
-  , m_canceling(false)
-  , m_maxProgress(0)
-  , m_progressFutureInterface(0)
-{
-}
-
 static BuildManagerPrivate *d = nullptr;
 static BuildManager *m_instance = nullptr;
 
@@ -360,16 +348,10 @@ void BuildManager::addToOutputWindow(const QString &string, BuildStep::OutputFor
     d->m_outputWindow->appendText(stringToWrite, format);
 }
 
-void BuildManager::buildStepFinishedAsync()
-{
-    disconnect(d->m_currentBuildStep, &BuildStep::finished,
-               this, &BuildManager::buildStepFinishedAsync);
-    d->m_futureInterfaceForAysnc = QFutureInterface<bool>();
-    nextBuildQueue();
-}
-
 void BuildManager::nextBuildQueue()
 {
+    d->m_futureInterfaceForAysnc = QFutureInterface<bool>();
+
     d->m_outputWindow->flush();
     if (d->m_canceling) {
         d->m_canceling = false;
@@ -391,8 +373,10 @@ void BuildManager::nextBuildQueue()
     d->m_progressFutureInterface->setProgressValueAndText(d->m_progress*100, msgProgress(d->m_progress, d->m_maxProgress));
     decrementActiveBuildSteps(d->m_currentBuildStep);
 
-    bool result = d->m_skipDisabled || d->m_watcher.result();
+    const bool success = d->m_skipDisabled || d->m_watcher.result();
-    if (!result) {
+    if (success) {
+        nextStep();
+    } else {
         // Build Failure
         Target *t = d->m_currentBuildStep->target();
         const QString projectName = d->m_currentBuildStep->project()->displayName();
@@ -406,12 +390,9 @@ void BuildManager::nextBuildQueue()
         addToOutputWindow(tr("When executing step \"%1\"").arg(d->m_currentBuildStep->displayName()), BuildStep::ErrorOutput);
         // NBS TODO fix in qtconcurrent
         d->m_progressFutureInterface->setProgressValueAndText(d->m_progress*100, tr("Error while building/deploying project %1 (kit: %2)").arg(projectName, targetName));
-    }
 
-    if (result)
-        nextStep();
-    else
         clearBuildQueue();
+    }
 }
 
 void BuildManager::progressChanged()
@@ -463,8 +444,6 @@ void BuildManager::nextStep()
         }
 
         if (d->m_currentBuildStep->runInGuiThread()) {
-            connect(d->m_currentBuildStep, &BuildStep::finished,
-                    m_instance, &BuildManager::buildStepFinishedAsync);
             d->m_watcher.setFuture(d->m_futureInterfaceForAysnc.future());
             d->m_currentBuildStep->run(d->m_futureInterfaceForAysnc);
         } else {

src/plugins/projectexplorer/buildmanager.h

@@ -84,7 +84,6 @@ private:
     static void addToOutputWindow(const QString &string, BuildStep::OutputFormat format,
                            BuildStep::OutputNewlineSetting newlineSettings = BuildStep::DoAppendNewline);
 
-    void buildStepFinishedAsync();
     static void nextBuildQueue();
     static void progressChanged();
     static void progressTextChanged();

src/plugins/projectexplorer/buildstep.cpp

@@ -160,6 +160,12 @@ Project *BuildStep::project() const
     return target()->project();
 }
 
+void BuildStep::reportRunResult(QFutureInterface<bool> &fi, bool success)
+{
+    fi.reportResult(success);
+    fi.reportFinished();
+}
+
 /*!
     If this function returns \c true, the user cannot delete this build step for
     this target and the user is prevented from changing the order in which

src/plugins/projectexplorer/buildstep.h

@@ -75,6 +75,8 @@ public:
     enum OutputFormat { NormalOutput, ErrorOutput, MessageOutput, ErrorMessageOutput };
     enum OutputNewlineSetting { DoAppendNewline, DontAppendNewline };
 
+    static void reportRunResult(QFutureInterface<bool> &fi, bool success);
+
 signals:
     /// Adds a \p task to the Issues pane.
     /// Do note that for linking compile output with tasks, you should first emit the task
@@ -86,8 +88,6 @@ signals:
     void addOutput(const QString &string, ProjectExplorer::BuildStep::OutputFormat format,
         ProjectExplorer::BuildStep::OutputNewlineSetting newlineSetting = DoAppendNewline);
 
-    void finished();
-
     void enabledChanged();
 
 private:

src/plugins/projectexplorer/devicesupport/devicecheckbuildstep.cpp

@@ -85,7 +85,7 @@ bool DeviceCheckBuildStep::init(QList<const BuildStep *> &earlierSteps)
 
 void DeviceCheckBuildStep::run(QFutureInterface<bool> &fi)
 {
-    fi.reportResult(true);
+    reportRunResult(fi, true);
 }
 
 BuildStepConfigWidget *DeviceCheckBuildStep::createConfigWidget()

src/plugins/qbsprojectmanager/qbsbuildstep.cpp

@@ -408,8 +408,7 @@ void QbsBuildStep::build()
     m_job = qbsProject()->build(options, m_products, error);
     if (!m_job) {
         emit addOutput(error, ErrorMessageOutput);
-        m_fi->reportResult(false);
+        reportRunResult(*m_fi, false);
-        emit finished();
         return;
     }
 
@@ -430,14 +429,12 @@ void QbsBuildStep::build()
 void QbsBuildStep::finish()
 {
     QTC_ASSERT(m_fi, return);
-    m_fi->reportResult(m_lastWasSuccess);
+    reportRunResult(*m_fi, m_lastWasSuccess);
     m_fi = 0; // do not delete, it is not ours
     if (m_job) {
         m_job->deleteLater();
         m_job = 0;
     }
-
-    emit finished();
 }
 
 QbsProject *QbsBuildStep::qbsProject() const

src/plugins/qbsprojectmanager/qbscleanstep.cpp

@@ -100,8 +100,7 @@ void QbsCleanStep::run(QFutureInterface<bool> &fi)
     m_job = pro->clean(options);
 
     if (!m_job) {
-        m_fi->reportResult(false);
+        reportRunResult(*m_fi, false);
-        emit finished();
         return;
     }
 
@@ -175,12 +174,10 @@ void QbsCleanStep::cleaningDone(bool success)
     }
 
     QTC_ASSERT(m_fi, return);
-    m_fi->reportResult(success);
+    reportRunResult(*m_fi, success);
     m_fi = 0; // do not delete, it is not ours
     m_job->deleteLater();
     m_job = 0;
-
-    emit finished();
 }
 
 void QbsCleanStep::handleTaskStarted(const QString &desciption, int max)

src/plugins/qbsprojectmanager/qbsinstallstep.cpp

@@ -93,8 +93,7 @@ void QbsInstallStep::run(QFutureInterface<bool> &fi)
     m_job = pro->install(m_qbsInstallOptions);
 
     if (!m_job) {
-        m_fi->reportResult(false);
+        reportRunResult(*m_fi, false);
-        emit finished();
         return;
     }
 
@@ -193,12 +192,10 @@ void QbsInstallStep::installDone(bool success)
     }
 
     QTC_ASSERT(m_fi, return);
-    m_fi->reportResult(success);
+    reportRunResult(*m_fi, success);
     m_fi = 0; // do not delete, it is not ours
     m_job->deleteLater();
     m_job = 0;
-
-    emit finished();
 }
 
 void QbsInstallStep::handleTaskStarted(const QString &desciption, int max)

src/plugins/qmakeandroidsupport/androidpackageinstallationstep.cpp

@@ -105,8 +105,7 @@ void AndroidPackageInstallationStep::run(QFutureInterface<bool> &fi)
             emit addOutput(tr("Removing directory %1").arg(dir), MessageOutput);
             if (!Utils::FileUtils::removeRecursively(androidDir, &error)) {
                 emit addOutput(error, ErrorOutput);
-                fi.reportResult(false);
+                reportRunResult(fi, false);
-                emit finished();
                 return;
             }
         }

src/plugins/qmakeandroidsupport/qmakeandroidbuildapkstep.cpp

@@ -269,8 +269,7 @@ void QmakeAndroidBuildApkStep::run(QFutureInterface<bool> &fi)
 {
     if (m_skipBuilding) {
         emit addOutput(tr("No application .pro file found, not building an APK."), BuildStep::ErrorMessageOutput);
-        fi.reportResult(true);
+        reportRunResult(fi, true);
-        emit finished();
         return;
     }
     AndroidBuildApkStep::run(fi);

src/plugins/qmakeprojectmanager/makestep.cpp

@@ -265,16 +265,15 @@ bool MakeStep::init(QList<const BuildStep *> &earlierSteps)
 void MakeStep::run(QFutureInterface<bool> & fi)
 {
     if (m_scriptTarget) {
-        fi.reportResult(true);
+        reportRunResult(fi, true);
-        emit finished();
         return;
     }
 
     if (!QFileInfo::exists(m_makeFileToCheck)) {
         if (!ignoreReturnValue())
             emit addOutput(tr("Cannot find Makefile. Check your build settings."), BuildStep::MessageOutput);
-        fi.reportResult(ignoreReturnValue());
+        const bool success = ignoreReturnValue();
-        emit finished();
+        reportRunResult(fi, success);
         return;
     }
 

src/plugins/qmakeprojectmanager/qmakestep.cpp

@@ -250,14 +250,13 @@ bool QMakeStep::init(QList<const BuildStep *> &earlierSteps)
 void QMakeStep::run(QFutureInterface<bool> &fi)
 {
     if (m_scriptTemplate) {
-        fi.reportResult(true);
+        reportRunResult(fi, true);
         return;
     }
 
     if (!m_needToRunQMake) {
         emit addOutput(tr("Configuration unchanged, skipping qmake step."), BuildStep::MessageOutput);
-        fi.reportResult(true);
+        reportRunResult(fi, true);
-        emit finished();
         return;
     }
 
@@ -610,7 +609,7 @@ void QMakeStepConfigWidget::askForRebuild()
     question->setText(tr("The option will only take effect if the project is recompiled. Do you want to recompile now?"));
     question->setStandardButtons(QMessageBox::Yes | QMessageBox::No);
     question->setModal(true);
-    connect(question, SIGNAL(finished(int)), this, SLOT(recompileMessageBoxFinished(int)));
+    connect(question, &QDialog::finished, this, &QMakeStepConfigWidget::recompileMessageBoxFinished);
     question->show();
 }
 

src/plugins/remotelinux/abstractremotelinuxdeploystep.cpp

@@ -149,8 +149,7 @@ void AbstractRemoteLinuxDeployStep::handleFinished()
     else
         emit addOutput(tr("Deploy step finished."), MessageOutput);
     disconnect(deployService(), 0, this, 0);
-    d->future.reportResult(!d->hasError);
+    reportRunResult(d->future, !d->hasError);
-    emit finished();
 }
 
 void AbstractRemoteLinuxDeployStep::handleStdOutData(const QString &data)

src/plugins/remotelinux/tarpackagecreationstep.cpp

@@ -145,7 +145,7 @@ void TarPackageCreationStep::run(QFutureInterface<bool> &fi)
         emit addOutput(tr("Packaging finished successfully."), MessageOutput);
     else
         emit addOutput(tr("Packaging failed."), ErrorMessageOutput);
-    fi.reportResult(success);
+    reportRunResult(fi, success);
 }
 
 void TarPackageCreationStep::setIgnoreMissingFiles(bool ignoreMissingFiles)