ProjectExplorer: only add Tasks from the main thread

If something fails while extracting the build env from msvc vavars
scripts we want to add task hub entries to inform the user about a most
probably unusable toolchain. As the msvc toolchain detection is threaded
nowadays and the TaskHub is not thread safe we need to propagate the
error message to the main thread before adding the Task entries.

Change-Id: I5c67d3b8d58b22ea895afb6f22ee73f19fa52f17
Reviewed-by: Eike Ziller <eike.ziller@qt.io>

src/plugins/projectexplorer/abstractmsvctoolchain.cpp

@@ -316,7 +316,7 @@ bool AbstractMsvcToolChain::canClone() const
     return true;
 }
 
-bool AbstractMsvcToolChain::generateEnvironmentSettings(const Utils::Environment &env,
+Utils::optional<QString> AbstractMsvcToolChain::generateEnvironmentSettings(const Utils::Environment &env,
                                                                             const QString &batchFile,
                                                                             const QString &batchArgs,
                                                                             QMap<QString, QString> &envPairs)
@@ -342,7 +342,7 @@ bool AbstractMsvcToolChain::generateEnvironmentSettings(const Utils::Environment
     saver.write("@echo " + marker.toLocal8Bit() + "\r\n");
     if (!saver.finalize()) {
         qWarning("%s: %s", Q_FUNC_INFO, qPrintable(saver.errorString()));
-        return false;
+        return QString();
     }
 
     Utils::SynchronousProcess run;
@@ -369,12 +369,9 @@ bool AbstractMsvcToolChain::generateEnvironmentSettings(const Utils::Environment
 
     QString command = QDir::toNativeSeparators(batchFile);
     if (!response.stdErr().isEmpty()) {
-        TaskHub::addTask(Task::Error,
+        return QCoreApplication::translate("ProjectExplorer::Internal::AbstractMsvcToolChain",
-                         QCoreApplication::translate("ProjectExplorer::Internal::AbstractMsvcToolChain",
                                            "Failed to retrieve MSVC Environment from \"%1\":\n"
-                                                     "%2")
+                                           "%2").arg(command, response.stdErr());
-                         .arg(command, response.stdErr()), Constants::TASK_CATEGORY_COMPILE);
-        return false;
     }
 
     if (response.result != Utils::SynchronousProcessResponse::Finished) {
@@ -382,12 +379,9 @@ bool AbstractMsvcToolChain::generateEnvironmentSettings(const Utils::Environment
         qWarning().noquote() << message;
         if (!batchArgs.isEmpty())
             command += ' ' + batchArgs;
-        TaskHub::addTask(Task::Error,
+        return QCoreApplication::translate("ProjectExplorer::Internal::AbstractMsvcToolChain",
-                         QCoreApplication::translate("ProjectExplorer::Internal::AbstractMsvcToolChain",
                                            "Failed to retrieve MSVC Environment from \"%1\":\n"
-                                                     "%2")
+                                           "%2").arg(command, message);
-                         .arg(command, message), Constants::TASK_CATEGORY_COMPILE);
-        return false;
     }
 
     // The SDK/MSVC scripts do not return exit codes != 0. Check on stdout.
@@ -398,13 +392,13 @@ bool AbstractMsvcToolChain::generateEnvironmentSettings(const Utils::Environment
     const int start = stdOut.indexOf(marker);
     if (start == -1) {
         qWarning("Could not find start marker in stdout output.");
-        return false;
+        return QString();
     }
 
     const int end = stdOut.indexOf(marker, start + 1);
     if (end == -1) {
         qWarning("Could not find end marker in stdout output.");
-        return false;
+        return QString();
     }
 
     const QString output = stdOut.mid(start, end - start);
@@ -418,7 +412,7 @@ bool AbstractMsvcToolChain::generateEnvironmentSettings(const Utils::Environment
         }
     }
 
-    return true;
+    return Utils::nullopt;
 }
 
 /**

src/plugins/projectexplorer/abstractmsvctoolchain.h

@@ -33,6 +33,7 @@
 
 #include <utils/environment.h>
 #include <utils/fileutils.h>
+#include <utils/optional.h>
 
 namespace ProjectExplorer {
 namespace Internal {
@@ -71,7 +72,7 @@ public:
 
     bool operator ==(const ToolChain &) const override;
 
-    static bool generateEnvironmentSettings(const Utils::Environment &env,
+    static Utils::optional<QString> generateEnvironmentSettings(const Utils::Environment &env,
                                                                 const QString &batchFile,
                                                                 const QString &batchArgs,
                                                                 QMap<QString, QString> &envPairs);

src/plugins/projectexplorer/msvctoolchain.cpp

@@ -27,6 +27,7 @@
 
 #include "msvcparser.h"
 #include "projectexplorerconstants.h"
+#include "taskhub.h"
 #include "toolchainmanager.h"
 
 #include <utils/algorithm.h>
@@ -523,17 +524,20 @@ static QString winExpandDelayedEnvReferences(QString in, const Utils::Environmen
     return in;
 }
 
-void MsvcToolChain::environmentModifications(QFutureInterface<QList<Utils::EnvironmentItem>> &future,
+void MsvcToolChain::environmentModifications(
+        QFutureInterface<MsvcToolChain::GenerateEnvResult> &future,
         QString vcvarsBat, QString varsBatArg)
 {
     const Utils::Environment inEnv = Utils::Environment::systemEnvironment();
     Utils::Environment outEnv;
     QMap<QString, QString> envPairs;
-    if (!generateEnvironmentSettings(inEnv, vcvarsBat, varsBatArg, envPairs))
+    QList<Utils::EnvironmentItem> diff;
-        return;
+    Utils::optional<QString> error = generateEnvironmentSettings(inEnv, vcvarsBat,
+                                                                 varsBatArg, envPairs);
+    if (!error) {
 
         // Now loop through and process them
-    for (auto envIter = envPairs.cbegin(), eend = envPairs.cend(); envIter != eend; ++envIter) {
+        for (auto envIter = envPairs.cbegin(), end = envPairs.cend(); envIter != end; ++envIter) {
             const QString expandedValue = winExpandDelayedEnvReferences(envIter.value(), inEnv);
             if (!expandedValue.isEmpty())
                 outEnv.set(envIter.key(), expandedValue);
@@ -549,20 +553,28 @@ void MsvcToolChain::environmentModifications(QFutureInterface<QList<Utils::Envir
             }
         }
 
-    QList<Utils::EnvironmentItem> diff = inEnv.diff(outEnv, true);
+        diff = inEnv.diff(outEnv, true);
         for (int i = diff.size() - 1; i >= 0; --i) {
             if (diff.at(i).name.startsWith(QLatin1Char('='))) { // Exclude "=C:", "=EXITCODE"
                 diff.removeAt(i);
             }
         }
-
-    future.reportResult(diff);
     }
 
-void MsvcToolChain::initEnvModWatcher(const QFuture<QList<Utils::EnvironmentItem> > &future)
+    future.reportResult({error, diff});
+}
+
+void MsvcToolChain::initEnvModWatcher(const QFuture<GenerateEnvResult> &future)
 {
-    QObject::connect(&m_envModWatcher, &QFutureWatcher<QList<Utils::EnvironmentItem>>::resultReadyAt, [&]() {
+    QObject::connect(&m_envModWatcher, &QFutureWatcher<GenerateEnvResult>::resultReadyAt, [&]() {
-        updateEnvironmentModifications(m_envModWatcher.result());
+        const GenerateEnvResult &result = m_envModWatcher.result();
+        if (result.error) {
+            const QString &errorMessage = *result.error;
+            if (!errorMessage.isEmpty())
+                TaskHub::addTask(Task::Error, errorMessage, Constants::TASK_CATEGORY_COMPILE);
+        } else {
+            updateEnvironmentModifications(result.environmentItems);
+        }
     });
     m_envModWatcher.setFuture(future);
 }
@@ -578,15 +590,23 @@ void MsvcToolChain::updateEnvironmentModifications(QList<Utils::EnvironmentItem>
 
 Utils::Environment MsvcToolChain::readEnvironmentSetting(const Utils::Environment& env) const
 {
-    Utils::Environment result = env;
+    Utils::Environment resultEnv = env;
     if (m_environmentModifications.isEmpty()) {
         m_envModWatcher.waitForFinished();
-        if (m_envModWatcher.future().isFinished() && !m_envModWatcher.future().isCanceled())
+        if (m_envModWatcher.future().isFinished() && !m_envModWatcher.future().isCanceled()) {
-            result.modify(m_envModWatcher.result());
+            const GenerateEnvResult &result = m_envModWatcher.result();
+            if (result.error) {
+                const QString &errorMessage = *result.error;
+                if (!errorMessage.isEmpty())
+                    TaskHub::addTask(Task::Error, errorMessage, Constants::TASK_CATEGORY_COMPILE);
             } else {
-        result.modify(m_environmentModifications);
+                resultEnv.modify(result.environmentItems);
             }
-    return result;
+        }
+    } else {
+        resultEnv.modify(m_environmentModifications);
+    }
+    return resultEnv;
 }
 
 // --------------------------------------------------------------------------
@@ -608,7 +628,14 @@ MsvcToolChain::MsvcToolChain(const MsvcToolChain &other)
         initEnvModWatcher(other.m_envModWatcher.future());
     } else if (m_environmentModifications.isEmpty() && other.m_envModWatcher.future().isFinished()
                && !other.m_envModWatcher.future().isCanceled()) {
-        m_environmentModifications = other.m_envModWatcher.result();
+        const GenerateEnvResult &result = m_envModWatcher.result();
+        if (result.error) {
+            const QString &errorMessage = *result.error;
+            if (!errorMessage.isEmpty())
+                TaskHub::addTask(Task::Error, errorMessage, Constants::TASK_CATEGORY_COMPILE);
+        } else {
+            updateEnvironmentModifications(result.environmentItems);
+        }
     }
 
     setDisplayName(other.displayName());

src/plugins/projectexplorer/msvctoolchain.h

@@ -31,6 +31,8 @@
 
 #include <QFutureWatcher>
 
+#include <utils/optional.h>
+
 QT_FORWARD_DECLARE_CLASS(QLabel)
 QT_FORWARD_DECLARE_CLASS(QVersionNumber)
 
@@ -89,13 +91,18 @@ protected:
                                 const Utils::Environment &env) const override;
 
 private:
-    static void environmentModifications(QFutureInterface<QList<Utils::EnvironmentItem> > &future,
+    struct GenerateEnvResult
+    {
+        Utils::optional<QString> error;
+        QList<Utils::EnvironmentItem> environmentItems;
+    };
+    static void environmentModifications(QFutureInterface<GenerateEnvResult> &future,
                                          QString vcvarsBat, QString varsBatArg);
-    void initEnvModWatcher(const QFuture<QList<Utils::EnvironmentItem>> &future);
+    void initEnvModWatcher(const QFuture<GenerateEnvResult> &future);
     void updateEnvironmentModifications(QList<Utils::EnvironmentItem> modifications);
 
     mutable QList<Utils::EnvironmentItem> m_environmentModifications;
-    mutable QFutureWatcher<QList<Utils::EnvironmentItem>> m_envModWatcher;
+    mutable QFutureWatcher<GenerateEnvResult> m_envModWatcher;
 
     QString m_varsBatArg; // Argument
 };