Sanitize error/finished handling in executor jobs and command executors.

The classes as they are now specify different signals for signaling
failure and finishing. Some of these error() signals are followed by a
finished() signal, others are not, which is just asking for trouble.
Under certain circumstances there can also be several error() signals
for one and the same process. In fact, there is almost certainly a race
condition there regarding executor job re-use.
This patch changes the design so that there is always exactly one
finished() signal that carries an error status. For some non-fatal
problems that higher-level code cannot sensibly handle, we now log a
warning instead of emitting an error signal.

Change-Id: I9e3df11564e7337ad766ca0d009303367d43c4ec
Reviewed-by: Joerg Bornemann <joerg.bornemann@digia.com>

1 files changed, 13 insertions, 20 deletions

diff --git a/src/lib/corelib/buildgraph/executorjob.cpp b/src/lib/corelib/buildgraph/executorjob.cpp
index fea181fa4..1ad82cdb5 100644
--- a/src/lib/corelib/buildgraph/executorjob.cpp
+++ b/src/lib/corelib/buildgraph/executorjob.cpp
@@ -52,14 +52,12 @@ ExecutorJob::ExecutorJob(const Logger &logger, QObject *parent)
             this, SIGNAL(reportCommandDescription(QString,QString)));
     connect(m_processCommandExecutor, SIGNAL(reportProcessResult(qbs::ProcessResult)),
             this, SIGNAL(reportProcessResult(qbs::ProcessResult)));
-    connect(m_processCommandExecutor, SIGNAL(error(qbs::ErrorInfo)),
-            this, SLOT(onCommandError(qbs::ErrorInfo)));
-    connect(m_processCommandExecutor, SIGNAL(finished()), SLOT(onCommandFinished()));
+    connect(m_processCommandExecutor, SIGNAL(finished(qbs::ErrorInfo)),
+            this, SLOT(onCommandFinished(qbs::ErrorInfo)));
     connect(m_jsCommandExecutor, SIGNAL(reportCommandDescription(QString,QString)),
             this, SIGNAL(reportCommandDescription(QString,QString)));
-    connect(m_jsCommandExecutor, SIGNAL(error(qbs::ErrorInfo)),
-            this, SLOT(onCommandError(qbs::ErrorInfo)));
-    connect(m_jsCommandExecutor, SIGNAL(finished()), SLOT(onCommandFinished()));
+    connect(m_jsCommandExecutor, SIGNAL(finished(qbs::ErrorInfo)),
+            this, SLOT(onCommandFinished(qbs::ErrorInfo)));
     reset();
 }
 
@@ -136,27 +134,22 @@ void ExecutorJob::runNextCommand()
     m_currentCommandExecutor->start(m_transformer, command.data());
 }
 
-void ExecutorJob::onCommandError(const ErrorInfo &err)
+void ExecutorJob::onCommandFinished(const ErrorInfo &err)
 {
-    m_error = err;
-    setFinished();
-}
-
-void ExecutorJob::onCommandFinished()
-{
-    if (!m_transformer)
-        return;
-    runNextCommand();
+    QBS_ASSERT(m_transformer, return);
+    if (err.hasError()) {
+        m_error = err;
+        setFinished();
+    } else {
+        runNextCommand();
+    }
 }
 
 void ExecutorJob::setFinished()
 {
     const ErrorInfo err = m_error;
     reset();
-    if (err.hasError())
-        emit error(err);
-    else
-        emit success();
+    emit finished(err);
 }
 
 void ExecutorJob::reset()