Environment: Keep case of environment variables on windows

Task-number: QTCREATORBUG-18915
Change-Id: I4c389155e03dde423d1ed578409baa1f38fc26db
Reviewed-by: Eike Ziller <eike.ziller@qt.io>
Reviewed-by: Joerg Bornemann <joerg.bornemann@qt.io>

src/libs/utils/environment.cpp

@@ -57,6 +57,31 @@ public:
 
 Q_GLOBAL_STATIC(SystemEnvironment, staticSystemEnvironment)
 
+static QMap<QString, QString>::iterator findKey(QMap<QString, QString> &input, Utils::OsType osType,
+                                                const QString &key)
+{
+    const Qt::CaseSensitivity casing
+            = (osType == Utils::OsTypeWindows) ? Qt::CaseInsensitive : Qt::CaseSensitive;
+    for (auto it = input.begin(); it != input.end(); ++it) {
+        if (key.compare(it.key(), casing) == 0)
+            return it;
+    }
+    return input.end();
+}
+
+static QMap<QString, QString>::const_iterator findKey(const QMap<QString, QString> &input,
+                                                      Utils::OsType osType,
+                                                      const QString &key)
+{
+    const Qt::CaseSensitivity casing
+            = (osType == Utils::OsTypeWindows) ? Qt::CaseInsensitive : Qt::CaseSensitive;
+    for (auto it = input.constBegin(); it != input.constEnd(); ++it) {
+        if (key.compare(it.key(), casing) == 0)
+            return it;
+    }
+    return input.constEnd();
+}
+
 namespace Utils {
 
 enum : char
@@ -207,10 +232,9 @@ Environment::Environment(const QStringList &env, OsType osType) : m_osType(osTyp
     for (const QString &s : env) {
         int i = s.indexOf('=', 1);
         if (i >= 0) {
-            if (m_osType == OsTypeWindows)
+            const QString key = s.left(i);
-                m_values.insert(s.left(i).toUpper(), s.mid(i+1));
+            const QString value = s.mid(i + 1);
-            else
+            set(key, value);
-                m_values.insert(s.left(i), s.mid(i+1));
         }
     }
 }
@@ -220,10 +244,8 @@ QStringList Environment::toStringList() const
     QStringList result;
     const QMap<QString, QString>::const_iterator end = m_values.constEnd();
     for (QMap<QString, QString>::const_iterator it = m_values.constBegin(); it != end; ++it) {
-        QString entry = it.key();
+        const QString entry = it.key() + '=' + it.value();
-        entry += '=';
+        result.append(entry);
-        entry += it.value();
-        result.push_back(entry);
     }
     return result;
 }
@@ -239,20 +261,25 @@ QProcessEnvironment Environment::toProcessEnvironment() const
 
 void Environment::set(const QString &key, const QString &value)
 {
-    m_values.insert(m_osType == OsTypeWindows ? key.toUpper() : key, value);
+    auto it = findKey(m_values, m_osType, key);
+    if (it == m_values.end())
+        m_values.insert(key, value);
+    else
+        it.value() = value;
 }
 
 void Environment::unset(const QString &key)
 {
-    m_values.remove(m_osType == OsTypeWindows ? key.toUpper() : key);
+    auto it = findKey(m_values, m_osType, key);
+    if (it != m_values.end())
+        m_values.erase(it);
 }
 
 void Environment::appendOrSet(const QString &key, const QString &value, const QString &sep)
 {
-    const QString &_key = m_osType == OsTypeWindows ? key.toUpper() : key;
+    auto it = findKey(m_values, m_osType, key);
-    QMap<QString, QString>::iterator it = m_values.find(_key);
     if (it == m_values.end()) {
-        m_values.insert(_key, value);
+        m_values.insert(key, value);
     } else {
         // Append unless it is already there
         const QString toAppend = sep + value;
@@ -263,10 +290,9 @@ void Environment::appendOrSet(const QString &key, const QString &value, const QS
 
 void Environment::prependOrSet(const QString&key, const QString &value, const QString &sep)
 {
-    const QString &_key = m_osType == OsTypeWindows ? key.toUpper() : key;
+    auto it = findKey(m_values, m_osType, key);
-    QMap<QString, QString>::iterator it = m_values.find(_key);
     if (it == m_values.end()) {
-        m_values.insert(_key, value);
+        m_values.insert(key, value);
     } else {
         // Prepend unless it is already there
         const QString toPrepend = value + sep;
@@ -563,7 +589,7 @@ QString Environment::expandVariables(const QString &input) const
         for (int vStart = -1, i = 0; i < result.length(); ) {
             if (result.at(i++) == '%') {
                 if (vStart > 0) {
-                    const_iterator it = m_values.constFind(result.mid(vStart, i - vStart - 1).toUpper());
+                    const_iterator it = findKey(m_values, m_osType, result.mid(vStart, i - vStart - 1));
                     if (it != m_values.constEnd()) {
                         result.replace(vStart - 1, i - vStart + 1, *it);
                         i = vStart - 1 + it->length();

tests/auto/environment/tst_environment.cpp

@@ -38,6 +38,23 @@ private slots:
     void environment_data();
     void environment();
 
+    void environmentSetup_data();
+    void environmentSetup();
+
+    void environmentSetWindows();
+    void environmentSetWindowsFuzzy();
+    void environmentSetUnix();
+
+    void environmentSetNewWindows();
+    void environmentSetNewUnix();
+
+    void environmentUnsetWindows();
+    void environmentUnsetWindowsFuzzy();
+    void environmentUnsetUnix();
+
+    void environmentUnsetUnknownWindows();
+    void environmentUnsetUnknownUnix();
+
 private:
     Environment env;
 };
@@ -98,6 +115,138 @@ void tst_Environment::environment()
     QCOMPARE(env.expandVariables(in), out);
 }
 
+void tst_Environment::environmentSetup_data()
+{
+    QTest::addColumn<int>("osType");
+    QTest::addColumn<QStringList>("in");
+    QTest::addColumn<QStringList>("out");
+
+    QTest::newRow("EmptyWin")
+            << static_cast<int>(Utils::OsTypeWindows) << QStringList() << QStringList();
+    QTest::newRow("EmptyLinux")
+            << static_cast<int>(Utils::OsTypeLinux) << QStringList() << QStringList();
+
+    QTest::newRow("SimpleWin")
+            << static_cast<int>(Utils::OsTypeWindows) << QStringList({"Foo=bar"}) << QStringList({"Foo=bar"});
+    QTest::newRow("EmptyLinux")
+            << static_cast<int>(Utils::OsTypeLinux) << QStringList({"Foo=bar"}) << QStringList({"Foo=bar"});
+
+    QTest::newRow("MultiWin")
+            << static_cast<int>(Utils::OsTypeWindows)
+            << QStringList({"Foo=bar", "Hi=HO"}) << QStringList({"Foo=bar", "Hi=HO"});
+    QTest::newRow("MultiLinux")
+            << static_cast<int>(Utils::OsTypeLinux)
+            << QStringList({"Foo=bar", "Hi=HO"}) << QStringList({"Foo=bar", "Hi=HO"});
+
+    QTest::newRow("DuplicateWin")
+            << static_cast<int>(Utils::OsTypeWindows)
+            << QStringList({"Foo=bar", "FOO=HO"}) << QStringList({"Foo=HO"});
+    QTest::newRow("DuplicateLinux")
+            << static_cast<int>(Utils::OsTypeLinux)
+            << QStringList({"Foo=bar", "FOO=HO"}) << QStringList({"FOO=HO", "Foo=bar"});
+}
+
+void tst_Environment::environmentSetup()
+{
+    QFETCH(int, osType);
+    QFETCH(QStringList, in);
+    QFETCH(QStringList, out);
+
+    Environment env(in, static_cast<Utils::OsType>(osType));
+
+    QCOMPARE(env.toStringList(), out);
+}
+
+void tst_Environment::environmentSetWindows()
+{
+    Environment env(QStringList({"Foo=bar", "Hi=HO"}), Utils::OsTypeWindows);
+
+    env.set("Foo", "baz");
+
+    QCOMPARE(env.toStringList(), QStringList({"Foo=baz", "Hi=HO"}));
+}
+
+void tst_Environment::environmentSetWindowsFuzzy()
+{
+    Environment env(QStringList({"Foo=bar", "Hi=HO"}), Utils::OsTypeWindows);
+
+    env.set("FOO", "baz");
+
+    QCOMPARE(env.toStringList(), QStringList({"Foo=baz", "Hi=HO"}));
+}
+
+void tst_Environment::environmentSetUnix()
+{
+    Environment env(QStringList({"Foo=bar", "Hi=HO"}), Utils::OsTypeLinux);
+
+    env.set("Foo", "baz");
+
+    QCOMPARE(env.toStringList(), QStringList({"Foo=baz", "Hi=HO"}));
+}
+
+void tst_Environment::environmentSetNewWindows()
+{
+    Environment env(QStringList({"Foo=bar", "Hi=HO"}), Utils::OsTypeWindows);
+
+    env.set("bar", "baz");
+
+    QCOMPARE(env.toStringList(), QStringList({"Foo=bar", "Hi=HO", "bar=baz"}));
+}
+
+void tst_Environment::environmentSetNewUnix()
+{
+    Environment env(QStringList({"Foo=bar", "Hi=HO"}), Utils::OsTypeLinux);
+
+    env.set("bar", "baz");
+
+    QCOMPARE(env.toStringList(), QStringList({"Foo=bar", "Hi=HO", "bar=baz"}));
+}
+
+void tst_Environment::environmentUnsetWindows()
+{
+    Environment env(QStringList({"Foo=bar", "Hi=HO"}), Utils::OsTypeWindows);
+
+    env.unset("Foo");
+
+    QCOMPARE(env.toStringList(), QStringList({"Hi=HO"}));
+}
+
+void tst_Environment::environmentUnsetWindowsFuzzy()
+{
+    Environment env(QStringList({"Foo=bar", "Hi=HO"}), Utils::OsTypeWindows);
+
+    env.unset("FOO");
+
+    QCOMPARE(env.toStringList(), QStringList({"Hi=HO"}));
+}
+
+void tst_Environment::environmentUnsetUnix()
+{
+    Environment env(QStringList({"Foo=bar", "Hi=HO"}), Utils::OsTypeLinux);
+
+    env.unset("Foo");
+
+    QCOMPARE(env.toStringList(), QStringList({"Hi=HO"}));
+}
+
+void tst_Environment::environmentUnsetUnknownWindows()
+{
+    Environment env(QStringList({"Foo=bar", "Hi=HO"}), Utils::OsTypeWindows);
+
+    env.unset("baz");
+
+    QCOMPARE(env.toStringList(), QStringList({"Foo=bar", "Hi=HO"}));
+}
+
+void tst_Environment::environmentUnsetUnknownUnix()
+{
+    Environment env(QStringList({"Foo=bar", "Hi=HO"}), Utils::OsTypeLinux);
+
+    env.unset("baz");
+
+    QCOMPARE(env.toStringList(), QStringList({"Foo=bar", "Hi=HO"}));
+}
+
 QTEST_MAIN(tst_Environment)
 
 #include "tst_environment.moc"