From a30e9e0f9af0e575d59b284cb9f168c71a83eb35 Mon Sep 17 00:00:00 2001 From: David Schulz Date: Wed, 11 Jul 2018 12:22:20 +0200 Subject: [PATCH] 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 --- .../projectexplorer/abstractmsvctoolchain.cpp | 34 +++---- .../projectexplorer/abstractmsvctoolchain.h | 9 +- src/plugins/projectexplorer/msvctoolchain.cpp | 95 ++++++++++++------- src/plugins/projectexplorer/msvctoolchain.h | 13 ++- 4 files changed, 90 insertions(+), 61 deletions(-) diff --git a/src/plugins/projectexplorer/abstractmsvctoolchain.cpp b/src/plugins/projectexplorer/abstractmsvctoolchain.cpp index 64de2dd7074..c47c0b43075 100644 --- a/src/plugins/projectexplorer/abstractmsvctoolchain.cpp +++ b/src/plugins/projectexplorer/abstractmsvctoolchain.cpp @@ -316,10 +316,10 @@ bool AbstractMsvcToolChain::canClone() const return true; } -bool AbstractMsvcToolChain::generateEnvironmentSettings(const Utils::Environment &env, - const QString &batchFile, - const QString &batchArgs, - QMap &envPairs) +Utils::optional AbstractMsvcToolChain::generateEnvironmentSettings(const Utils::Environment &env, + const QString &batchFile, + const QString &batchArgs, + QMap &envPairs) { const QString marker = "####################"; // Create a temporary file name for the output. Use a temporary file here @@ -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, - QCoreApplication::translate("ProjectExplorer::Internal::AbstractMsvcToolChain", - "Failed to retrieve MSVC Environment from \"%1\":\n" - "%2") - .arg(command, response.stdErr()), Constants::TASK_CATEGORY_COMPILE); - return false; + return QCoreApplication::translate("ProjectExplorer::Internal::AbstractMsvcToolChain", + "Failed to retrieve MSVC Environment from \"%1\":\n" + "%2").arg(command, response.stdErr()); } 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, - QCoreApplication::translate("ProjectExplorer::Internal::AbstractMsvcToolChain", - "Failed to retrieve MSVC Environment from \"%1\":\n" - "%2") - .arg(command, message), Constants::TASK_CATEGORY_COMPILE); - return false; + return QCoreApplication::translate("ProjectExplorer::Internal::AbstractMsvcToolChain", + "Failed to retrieve MSVC Environment from \"%1\":\n" + "%2").arg(command, message); } // 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; } /** diff --git a/src/plugins/projectexplorer/abstractmsvctoolchain.h b/src/plugins/projectexplorer/abstractmsvctoolchain.h index 0851e09cc09..7bfdf3ac467 100644 --- a/src/plugins/projectexplorer/abstractmsvctoolchain.h +++ b/src/plugins/projectexplorer/abstractmsvctoolchain.h @@ -33,6 +33,7 @@ #include #include +#include namespace ProjectExplorer { namespace Internal { @@ -71,10 +72,10 @@ public: bool operator ==(const ToolChain &) const override; - static bool generateEnvironmentSettings(const Utils::Environment &env, - const QString &batchFile, - const QString &batchArgs, - QMap &envPairs); + static Utils::optional generateEnvironmentSettings(const Utils::Environment &env, + const QString &batchFile, + const QString &batchArgs, + QMap &envPairs); protected: class WarningFlagAdder diff --git a/src/plugins/projectexplorer/msvctoolchain.cpp b/src/plugins/projectexplorer/msvctoolchain.cpp index 24491e15982..8a277fe5f1f 100644 --- a/src/plugins/projectexplorer/msvctoolchain.cpp +++ b/src/plugins/projectexplorer/msvctoolchain.cpp @@ -27,6 +27,7 @@ #include "msvcparser.h" #include "projectexplorerconstants.h" +#include "taskhub.h" #include "toolchainmanager.h" #include @@ -523,46 +524,57 @@ static QString winExpandDelayedEnvReferences(QString in, const Utils::Environmen return in; } -void MsvcToolChain::environmentModifications(QFutureInterface> &future, - QString vcvarsBat, QString varsBatArg) +void MsvcToolChain::environmentModifications( + QFutureInterface &future, + QString vcvarsBat, QString varsBatArg) { const Utils::Environment inEnv = Utils::Environment::systemEnvironment(); Utils::Environment outEnv; QMap envPairs; - if (!generateEnvironmentSettings(inEnv, vcvarsBat, varsBatArg, envPairs)) - return; + QList diff; + Utils::optional 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) { - const QString expandedValue = winExpandDelayedEnvReferences(envIter.value(), inEnv); - if (!expandedValue.isEmpty()) - outEnv.set(envIter.key(), expandedValue); - } + // Now loop through and process them + 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); + } - if (debug) { - const QStringList newVars = outEnv.toStringList(); - const QStringList oldVars = inEnv.toStringList(); - QDebug nsp = qDebug().nospace(); - foreach (const QString &n, newVars) { - if (!oldVars.contains(n)) - nsp << n << '\n'; + if (debug) { + const QStringList newVars = outEnv.toStringList(); + const QStringList oldVars = inEnv.toStringList(); + QDebug nsp = qDebug().nospace(); + foreach (const QString &n, newVars) { + if (!oldVars.contains(n)) + nsp << n << '\n'; + } + } + + 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); + } } } - QList 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); + future.reportResult({error, diff}); } -void MsvcToolChain::initEnvModWatcher(const QFuture > &future) +void MsvcToolChain::initEnvModWatcher(const QFuture &future) { - QObject::connect(&m_envModWatcher, &QFutureWatcher>::resultReadyAt, [&]() { - updateEnvironmentModifications(m_envModWatcher.result()); + QObject::connect(&m_envModWatcher, &QFutureWatcher::resultReadyAt, [&]() { + 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::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()) - result.modify(m_envModWatcher.result()); + if (m_envModWatcher.future().isFinished() && !m_envModWatcher.future().isCanceled()) { + 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 { + resultEnv.modify(result.environmentItems); + } + } } else { - result.modify(m_environmentModifications); + resultEnv.modify(m_environmentModifications); } - return result; + 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()); diff --git a/src/plugins/projectexplorer/msvctoolchain.h b/src/plugins/projectexplorer/msvctoolchain.h index e71773800eb..8cd39af2dc5 100644 --- a/src/plugins/projectexplorer/msvctoolchain.h +++ b/src/plugins/projectexplorer/msvctoolchain.h @@ -31,6 +31,8 @@ #include +#include + 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 > &future, + struct GenerateEnvResult + { + Utils::optional error; + QList environmentItems; + }; + static void environmentModifications(QFutureInterface &future, QString vcvarsBat, QString varsBatArg); - void initEnvModWatcher(const QFuture> &future); + void initEnvModWatcher(const QFuture &future); void updateEnvironmentModifications(QList modifications); mutable QList m_environmentModifications; - mutable QFutureWatcher> m_envModWatcher; + mutable QFutureWatcher m_envModWatcher; QString m_varsBatArg; // Argument };