Utils::Environment: Use expanded values

The Environment class is supposed to support values with references to
other variables, but we failed to actually expand them in most places.

Fixes: QTCREATORBUG-22687
Change-Id: I108cb59d3b4571471423455240f6f4f1cf64bf05
Reviewed-by: hjk <hjk@qt.io>

19 files changed, 58 insertions, 26 deletions

diff --git a/src/libs/clangsupport/processcreator.cpp b/src/libs/clangsupport/processcreator.cpp
index e0147bda6b..a33d28c9c4 100644
--- a/src/libs/clangsupport/processcreator.cpp
+++ b/src/libs/clangsupport/processcreator.cpp
@@ -175,7 +175,7 @@ QProcessEnvironment ProcessCreator::processEnvironment() const
     const Utils::Environment &env = m_environment;
     for (auto it = env.constBegin(); it != env.constEnd(); ++it) {
         if (env.isEnabled(it))
-            processEnvironment.insert(it.key(), env.value(it));
+            processEnvironment.insert(it.key(), env.expandedValueForKey(env.key(it)));
     }
 
     return processEnvironment;
diff --git a/src/libs/utils/environment.cpp b/src/libs/utils/environment.cpp
index 02c0329714..156f4b1561 100644
--- a/src/libs/utils/environment.cpp
+++ b/src/libs/utils/environment.cpp
@@ -46,7 +46,7 @@ QProcessEnvironment Environment::toProcessEnvironment() const
     QProcessEnvironment result;
     for (auto it = m_values.constBegin(); it != m_values.constEnd(); ++it) {
         if (it.value().second)
-            result.insert(it.key(), it.value().first);
+            result.insert(it.key(), expandedValueForKey(key(it)));
     }
     return result;
 }
@@ -184,7 +184,7 @@ QStringList Environment::appendExeExtensions(const QString &executable) const
         // Check all the executable extensions on windows:
         // PATHEXT is only used if the executable has no extension
         if (fi.suffix().isEmpty()) {
-            const QStringList extensions = value("PATHEXT").split(';');
+            const QStringList extensions = expandedValueForKey("PATHEXT").split(';');
 
             for (const QString &ext : extensions)
                 execs << executable + ext.toLower();
@@ -212,6 +212,11 @@ bool Environment::isSameExecutable(const QString &exe1, const QString &exe2) con
     return false;
 }
 
+QString Environment::expandedValueForKey(const QString &key) const
+{
+    return expandVariables(value(key));
+}
+
 FilePath Environment::searchInPath(const QString &executable,
                                    const FilePathList &additionalDirs,
                                    const PathFilter &func) const
@@ -297,7 +302,7 @@ FilePathList Environment::path() const
 
 FilePathList Environment::pathListValue(const QString &varName) const
 {
-    const QStringList pathComponents = value(varName)
+    const QStringList pathComponents = expandedValueForKey(varName)
             .split(OsSpecificAspects::pathListSeparator(m_osType), QString::SkipEmptyParts);
     return transform(pathComponents, &FilePath::fromUserInput);
 }
diff --git a/src/libs/utils/environment.h b/src/libs/utils/environment.h
index a233435c4a..0f97c2c3a8 100644
--- a/src/libs/utils/environment.h
+++ b/src/libs/utils/environment.h
@@ -76,6 +76,7 @@ public:
 
     bool isSameExecutable(const QString &exe1, const QString &exe2) const;
 
+    QString expandedValueForKey(const QString &key) const;
     QString expandVariables(const QString &input) const;
     FilePath expandVariables(const FilePath &input) const;
     QStringList expandVariables(const QStringList &input) const;
diff --git a/src/libs/utils/qtcprocess.cpp b/src/libs/utils/qtcprocess.cpp
index 7080d218a5..557f77fd34 100644
--- a/src/libs/utils/qtcprocess.cpp
+++ b/src/libs/utils/qtcprocess.cpp
@@ -94,7 +94,7 @@ static void envExpandWin(QString &args, const Environment *env, const QString *p
         if (prev >= 0) {
             const QString var = args.mid(prev + 1, that - prev - 1).toUpper();
             const QString val = (var == cdName && pwd && !pwd->isEmpty())
-                                    ? QDir::toNativeSeparators(*pwd) : env->value(var);
+                                    ? QDir::toNativeSeparators(*pwd) : env->expandedValueForKey(var);
             if (!val.isEmpty()) { // Empty values are impossible, so this is an existence check
                 args.replace(prev, that - prev + 1, val);
                 off = prev + val.length();
@@ -394,7 +394,7 @@ static QStringList splitArgsUnix(const QString &args, bool abortOnMeta,
                                 if (abortOnMeta)
                                     goto metaerr; // Assume this is a shell builtin
                             } else {
-                                cret += env->value(vit);
+                                cret += env->expandedValueForKey(env->key(vit));
                             }
                         }
                         if (!braced)
@@ -444,7 +444,7 @@ static QStringList splitArgsUnix(const QString &args, bool abortOnMeta,
                         if (abortOnMeta)
                             goto metaerr; // Assume this is a shell builtin
                     } else {
-                        val = env->value(vit);
+                        val = env->expandedValueForKey(env->key(vit));
                     }
                 }
                 for (int i = 0; i < val.length(); i++) {
@@ -698,7 +698,7 @@ void QtcProcess::start()
                      qPrintable(m_commandLine.executable().toString()));
         env = m_environment;
 
-        QProcess::setEnvironment(env.toStringList());
+        QProcess::setProcessEnvironment(env.toProcessEnvironment());
     } else {
         env = Environment::systemEnvironment();
     }
diff --git a/src/plugins/cmakeprojectmanager/cmakebuildstep.cpp b/src/plugins/cmakeprojectmanager/cmakebuildstep.cpp
index ecaefda599..45fd574583 100644
--- a/src/plugins/cmakeprojectmanager/cmakebuildstep.cpp
+++ b/src/plugins/cmakeprojectmanager/cmakebuildstep.cpp
@@ -192,7 +192,7 @@ bool CMakeBuildStep::init()
     pp->setMacroExpander(bc->macroExpander());
     Utils::Environment env = bc->environment();
     Utils::Environment::setupEnglishOutput(&env);
-    if (!env.value("NINJA_STATUS").startsWith(m_ninjaProgressString))
+    if (!env.expandedValueForKey("NINJA_STATUS").startsWith(m_ninjaProgressString))
         env.set("NINJA_STATUS", m_ninjaProgressString + "%o/sec] ");
     pp->setEnvironment(env);
     pp->setWorkingDirectory(bc->buildDirectory());
diff --git a/src/plugins/debugger/cdb/cdbengine.cpp b/src/plugins/debugger/cdb/cdbengine.cpp
index 124c97981b..a5554a3aa8 100644
--- a/src/plugins/debugger/cdb/cdbengine.cpp
+++ b/src/plugins/debugger/cdb/cdbengine.cpp
@@ -395,10 +395,10 @@ void CdbEngine::setupEngine()
         debugger.addArgs({"-srcpath", sourcePaths.join(';')});
 
     QStringList symbolPaths = stringListSetting(CdbSymbolPaths);
-    QString symbolPath = sp.inferior.environment.value("_NT_ALT_SYMBOL_PATH");
+    QString symbolPath = sp.inferior.environment.expandedValueForKey("_NT_ALT_SYMBOL_PATH");
     if (!symbolPath.isEmpty())
         symbolPaths += symbolPath;
-    symbolPath = sp.inferior.environment.value("_NT_SYMBOL_PATH");
+    symbolPath = sp.inferior.environment.expandedValueForKey("_NT_SYMBOL_PATH");
     if (!symbolPath.isEmpty())
         symbolPaths += symbolPath;
     debugger.addArgs({"-y", symbolPaths.join(';')});
diff --git a/src/plugins/debugger/debuggerruncontrol.cpp b/src/plugins/debugger/debuggerruncontrol.cpp
index fd83b6acc2..ec80b470cf 100644
--- a/src/plugins/debugger/debuggerruncontrol.cpp
+++ b/src/plugins/debugger/debuggerruncontrol.cpp
@@ -794,7 +794,7 @@ bool DebuggerRunTool::fixupParameters()
     for (const auto &var :
          QStringList({"DYLD_IMAGE_SUFFIX", "DYLD_LIBRARY_PATH", "DYLD_FRAMEWORK_PATH"}))
         if (rp.inferior.environment.hasKey(var))
-            rp.debugger.environment.set(var, rp.inferior.environment.value(var));
+            rp.debugger.environment.set(var, rp.inferior.environment.expandedValueForKey(var));
 
     // validate debugger if C++ debugging is enabled
     if (rp.isCppDebugging() && !rp.validationErrors.isEmpty()) {
diff --git a/src/plugins/projectexplorer/buildconfiguration.cpp b/src/plugins/projectexplorer/buildconfiguration.cpp
index c7d0ce24f4..6874b051c7 100644
--- a/src/plugins/projectexplorer/buildconfiguration.cpp
+++ b/src/plugins/projectexplorer/buildconfiguration.cpp
@@ -80,7 +80,7 @@ BuildConfiguration::BuildConfiguration(Target *target, Core::Id id)
 
     expander->registerPrefix(Constants::VAR_CURRENTBUILD_ENV,
                              tr("Variables in the current build environment"),
-                             [this](const QString &var) { return environment().value(var); });
+                             [this](const QString &var) { return environment().expandedValueForKey(var); });
 
     updateCacheAndEmitEnvironmentChanged();
     connect(target, &Target::kitChanged,
diff --git a/src/plugins/projectexplorer/makestep.cpp b/src/plugins/projectexplorer/makestep.cpp
index cfa7a414bf..dece53ad6f 100644
--- a/src/plugins/projectexplorer/makestep.cpp
+++ b/src/plugins/projectexplorer/makestep.cpp
@@ -231,7 +231,7 @@ bool MakeStep::makeflagsJobCountMismatch() const
     const Utils::Environment env = environment(buildConfiguration());
     if (!env.hasKey(MAKEFLAGS))
         return false;
-    Utils::optional<int> makeFlagsJobCount = argsJobCount(env.value(MAKEFLAGS));
+    Utils::optional<int> makeFlagsJobCount = argsJobCount(env.expandedValueForKey(MAKEFLAGS));
     return makeFlagsJobCount.has_value() && *makeFlagsJobCount != m_userJobCount;
 }
 
@@ -240,7 +240,7 @@ bool MakeStep::makeflagsContainsJobCount() const
     const Utils::Environment env = environment(buildConfiguration());
     if (!env.hasKey(MAKEFLAGS))
         return false;
-    return argsJobCount(env.value(MAKEFLAGS)).has_value();
+    return argsJobCount(env.expandedValueForKey(MAKEFLAGS)).has_value();
 }
 
 bool MakeStep::userArgsContainsJobCount() const
@@ -258,7 +258,7 @@ Utils::Environment MakeStep::environment(BuildConfiguration *bc) const
         const ToolChain *tc = tcs.isEmpty() ? nullptr : tcs.constFirst();
         if (tc && tc->targetAbi().os() == Abi::WindowsOS
                 && tc->targetAbi().osFlavor() != Abi::WindowsMSysFlavor) {
-            env.set(MAKEFLAGS, 'L' + env.value(MAKEFLAGS));
+            env.set(MAKEFLAGS, 'L' + env.expandedValueForKey(MAKEFLAGS));
         }
     }
     return env;
diff --git a/src/plugins/projectexplorer/msvctoolchain.cpp b/src/plugins/projectexplorer/msvctoolchain.cpp
index de97f3f49b..c90799a5e0 100644
--- a/src/plugins/projectexplorer/msvctoolchain.cpp
+++ b/src/plugins/projectexplorer/msvctoolchain.cpp
@@ -700,7 +700,7 @@ static QString winExpandDelayedEnvReferences(QString in, const Utils::Environmen
         if (nextPos == -1)
             break;
         const QString var = in.mid(pos + 1, nextPos - pos - 1);
-        const QString replacement = env.value(var.toUpper());
+        const QString replacement = env.expandedValueForKey(var.toUpper());
         in.replace(pos, nextPos + 1 - pos, replacement);
         pos += replacement.size();
     }
diff --git a/src/plugins/projectexplorer/projectexplorer.cpp b/src/plugins/projectexplorer/projectexplorer.cpp
index 06b31a9c1c..5a73e5c075 100644
--- a/src/plugins/projectexplorer/projectexplorer.cpp
+++ b/src/plugins/projectexplorer/projectexplorer.cpp
@@ -1686,7 +1686,7 @@ bool ProjectExplorerPlugin::initialize(const QStringList &arguments, QString *er
                              BuildConfiguration::tr("Variables in the current build environment"),
                              [](const QString &var) {
                                  if (BuildConfiguration *bc = activeBuildConfiguration())
-                                     return bc->environment().value(var);
+                                     return bc->environment().expandedValueForKey(var);
                                  return QString();
                              });
 
diff --git a/src/plugins/projectexplorer/runconfiguration.cpp b/src/plugins/projectexplorer/runconfiguration.cpp
index 285b35c371..457c15cfed 100644
--- a/src/plugins/projectexplorer/runconfiguration.cpp
+++ b/src/plugins/projectexplorer/runconfiguration.cpp
@@ -190,7 +190,7 @@ RunConfiguration::RunConfiguration(Target *target, Core::Id id)
     expander->registerPrefix("CurrentRun:Env", tr("Variables in the current run environment"),
                              [this](const QString &var) {
         const auto envAspect = aspect<EnvironmentAspect>();
-        return envAspect ? envAspect->environment().value(var) : QString();
+        return envAspect ? envAspect->environment().expandedValueForKey(var) : QString();
     });
 
     expander->registerVariable(Constants::VAR_CURRENTRUN_WORKINGDIR,
diff --git a/src/plugins/qmakeprojectmanager/qmakeproject.cpp b/src/plugins/qmakeprojectmanager/qmakeproject.cpp
index 3473564c92..b0e482a694 100644
--- a/src/plugins/qmakeprojectmanager/qmakeproject.cpp
+++ b/src/plugins/qmakeprojectmanager/qmakeproject.cpp
@@ -649,7 +649,7 @@ QtSupport::ProFileReader *QmakeProject::createProFileReader(const QmakeProFile *
 
         Environment::const_iterator eit = env.constBegin(), eend = env.constEnd();
         for (; eit != eend; ++eit)
-            m_qmakeGlobals->environment.insert(env.key(eit), env.value(eit));
+            m_qmakeGlobals->environment.insert(env.key(eit), env.expandedValueForKey(env.key(eit)));
 
         m_qmakeGlobals->setCommandLineArguments(rootProFile()->buildDir().toString(), qmakeArgs);
 
diff --git a/src/plugins/qmljstools/qmljsmodelmanager.cpp b/src/plugins/qmljstools/qmljsmodelmanager.cpp
index 2d996f4985..b33846faf9 100644
--- a/src/plugins/qmljstools/qmljsmodelmanager.cpp
+++ b/src/plugins/qmljstools/qmljsmodelmanager.cpp
@@ -111,7 +111,7 @@ ModelManagerInterface::ProjectInfo ModelManager::defaultProjectInfoForProject(
             // Append QML2_IMPORT_PATH if it is defined in build configuration.
             // It enables qmlplugindump to correctly dump custom plugins or other dependent
             // plugins that are not installed in default Qt qml installation directory.
-            projectInfo.qmlDumpEnvironment.appendOrSet("QML2_IMPORT_PATH", bc->environment().value("QML2_IMPORT_PATH"), ":");
+            projectInfo.qmlDumpEnvironment.appendOrSet("QML2_IMPORT_PATH", bc->environment().expandedValueForKey("QML2_IMPORT_PATH"), ":");
         }
     }
     if (!setPreferDump && qtVersion)
diff --git a/src/plugins/qnx/qnxdeviceprocess.cpp b/src/plugins/qnx/qnxdeviceprocess.cpp
index 0fff4b3585..2b6a0c85c4 100644
--- a/src/plugins/qnx/qnxdeviceprocess.cpp
+++ b/src/plugins/qnx/qnxdeviceprocess.cpp
@@ -59,8 +59,10 @@ QString QnxDeviceProcess::fullCommandLine(const Runnable &runnable) const
         fullCommandLine += QString::fromLatin1("cd %1 ; ").arg(QtcProcess::quoteArg(runnable.workingDirectory));
 
     const Environment env = runnable.environment;
-    for (auto it = env.constBegin(); it != env.constEnd(); ++it)
-        fullCommandLine += QString::fromLatin1("%1='%2' ").arg(env.key(it)).arg(env.value(it));
+    for (auto it = env.constBegin(); it != env.constEnd(); ++it) {
+        fullCommandLine += QString::fromLatin1("%1='%2' ")
+                .arg(env.key(it)).arg(env.expandedValueForKey(env.key(it)));
+    }
 
     fullCommandLine += QString::fromLatin1("%1 & echo $! > %2").arg(cmd).arg(m_pidFile);
 
diff --git a/src/plugins/qnx/qnxtoolchain.cpp b/src/plugins/qnx/qnxtoolchain.cpp
index 6ad191fbea..158d70e494 100644
--- a/src/plugins/qnx/qnxtoolchain.cpp
+++ b/src/plugins/qnx/qnxtoolchain.cpp
@@ -113,7 +113,7 @@ std::unique_ptr<ToolChainConfigWidget> QnxToolChain::createConfigurationWidget()
 
 void QnxToolChain::addToEnvironment(Environment &env) const
 {
-    if (env.value("QNX_HOST").isEmpty() || env.value("QNX_TARGET").isEmpty())
+    if (env.expandedValueForKey("QNX_HOST").isEmpty() || env.expandedValueForKey("QNX_TARGET").isEmpty())
         setQnxEnvironment(env, QnxUtils::qnxEnvironment(m_sdpPath));
 
     GccToolChain::addToEnvironment(env);
diff --git a/src/plugins/remotelinux/linuxdeviceprocess.cpp b/src/plugins/remotelinux/linuxdeviceprocess.cpp
index 3a1e88d19e..c16a12baee 100644
--- a/src/plugins/remotelinux/linuxdeviceprocess.cpp
+++ b/src/plugins/remotelinux/linuxdeviceprocess.cpp
@@ -94,7 +94,7 @@ QString LinuxDeviceProcess::fullCommandLine(const Runnable &runnable) const
 
     const Environment &env = runnable.environment;
     for (auto it = env.constBegin(); it != env.constEnd(); ++it)
-        cmd.addArgs(env.key(it) + "='" + env.value(it) + '\'', CommandLine::Raw);
+        cmd.addArgs(env.key(it) + "='" + env.expandedValueForKey(env.key(it)) + '\'', CommandLine::Raw);
 
     if (!runInTerminal())
         cmd.addArg("exec");
diff --git a/src/plugins/remotelinux/makeinstallstep.cpp b/src/plugins/remotelinux/makeinstallstep.cpp
index 283d9bff4a..e5636c7ea7 100644
--- a/src/plugins/remotelinux/makeinstallstep.cpp
+++ b/src/plugins/remotelinux/makeinstallstep.cpp
@@ -140,7 +140,7 @@ bool MakeInstallStep::init()
         Environment env = processParameters()->environment();
         for (auto it = cmd.environment.constBegin(); it != cmd.environment.constEnd(); ++it) {
             if (cmd.environment.isEnabled(it))
-                env.set(it.key(), cmd.environment.value(it));
+                env.set(it.key(), cmd.environment.expandedValueForKey(cmd.environment.key(it)));
         }
         processParameters()->setEnvironment(env);
     }
diff --git a/tests/auto/environment/tst_environment.cpp b/tests/auto/environment/tst_environment.cpp
index 570f43a75c..74177b5edd 100644
--- a/tests/auto/environment/tst_environment.cpp
+++ b/tests/auto/environment/tst_environment.cpp
@@ -57,6 +57,9 @@ private slots:
     void environmentUnsetUnknownWindows();
     void environmentUnsetUnknownUnix();
 
+    void expansion_data();
+    void expansion();
+
     void find_data();
     void find();
 
@@ -252,6 +255,27 @@ void tst_Environment::environmentUnsetUnknownUnix()
     QCOMPARE(env.toStringList(), QStringList({"Foo=bar", "Hi=HO"}));
 }
 
+void tst_Environment::expansion_data()
+{
+    QTest::addColumn<Utils::OsType>("osType");
+    QTest::addColumn<QString>("eu");
+    QTest::addColumn<QString>("ew");
+
+    QTest::newRow("win") << Utils::OsTypeWindows << "${v}" << "blubb";
+    QTest::newRow("lin") << Utils::OsTypeLinux << "blubb" << "%v%";
+}
+
+void tst_Environment::expansion()
+{
+    QFETCH(Utils::OsType, osType);
+    QFETCH(QString, eu);
+    QFETCH(QString, ew);
+
+    const Environment env(QStringList{"eu=${v}", "ew=%v%", "v=blubb"}, osType);
+    QCOMPARE(env.expandedValueForKey("eu"), eu);
+    QCOMPARE(env.expandedValueForKey("ew"), ew);
+}
+
 void tst_Environment::find_data()
 {
     QTest::addColumn<Utils::OsType>("osType");