Change tracking: Be more sensitive to environment changes.

Contrary to what the old code assumed, it is not enough to compare the
values of environment variables that were used while resolving the
project: A command run by a transformer might be sensitive to
environment changes as well (e.g. gcc to CPLUS_INCLUDE_PATH).
Note: This does not mean we will rebuild everything when anything in the
environment changes; it means that *if* a file is rebuilt, its command
will see the new environment instead of the old one.

Change-Id: I7916b8c40f1440564034ec3317c549ebd4e988a0
Reviewed-by: Jake Petroules <jake.petroules@theqtcompany.com>
Reviewed-by: Joerg Bornemann <joerg.bornemann@theqtcompany.com>

6 files changed, 37 insertions, 11 deletions

diff --git a/src/lib/corelib/buildgraph/buildgraphloader.cpp b/src/lib/corelib/buildgraph/buildgraphloader.cpp
index c1e1204b3..bf9f5e534 100644
--- a/src/lib/corelib/buildgraph/buildgraphloader.cpp
+++ b/src/lib/corelib/buildgraph/buildgraphloader.cpp
@@ -335,16 +335,11 @@ void BuildGraphLoader::trackProjectChanges()
 
 bool BuildGraphLoader::hasEnvironmentChanged(const TopLevelProjectConstPtr &restoredProject) const
 {
-    for (QHash<QString, QString>::ConstIterator it = restoredProject->usedEnvironment.constBegin();
-         it != restoredProject->usedEnvironment.constEnd(); ++it) {
-        const QString var = it.key();
-        const QString oldValue = it.value();
-        const QString newValue = m_environment.value(var);
-        if (newValue != oldValue) {
-            m_logger.qbsDebug() << QString::fromLatin1("Environment variable '%1' changed "
-                "from '%2' to '%3'. Must re-resolve project.").arg(var, oldValue, newValue);
-            return true;
-        }
+    if (m_environment != restoredProject->environment) {
+        m_logger.qbsDebug() << "Set of environment variables changed. Must re-resolve project.";
+        m_logger.qbsTrace() << "old: " << restoredProject->environment.toStringList() << "\nnew:"
+                            << m_environment.toStringList();
+        return true;
     }
     return false;
 }
diff --git a/src/lib/corelib/language/language.h b/src/lib/corelib/language/language.h
index 00d3963b9..8e1e90af9 100644
--- a/src/lib/corelib/language/language.h
+++ b/src/lib/corelib/language/language.h
@@ -458,7 +458,13 @@ public:
 
     QString buildDirectory; // Not saved
     QProcessEnvironment environment;
-    QHash<QString, QString> usedEnvironment; // Environment variables requested by the project while resolving.
+
+    // Environment variables requested by the project while resolving.
+    // TODO: This information is currently not used. Remove in 1.5 or use elaborate change tracking
+    //       logic where rules declare the environment variables that could influence their
+    //       behavior.
+    QHash<QString, QString> usedEnvironment;
+
     QHash<QString, QString> canonicalFilePathResults; // Results of calls to "File.canonicalFilePath()."
     QHash<QString, bool> fileExistsResults; // Results of calls to "File.exists()".
     QHash<QString, FileTime> fileLastModifiedResults; // Results of calls to "File.lastModified()".
diff --git a/tests/auto/blackbox/testdata/trackExternalProductChanges/hidden/hiddenheaderqbs.h b/tests/auto/blackbox/testdata/trackExternalProductChanges/hidden/hiddenheaderqbs.h
new file mode 100644
index 000000000..e69de29bb
--- /dev/null
+++ b/tests/auto/blackbox/testdata/trackExternalProductChanges/hidden/hiddenheaderqbs.h
diff --git a/tests/auto/blackbox/testdata/trackExternalProductChanges/including.cpp b/tests/auto/blackbox/testdata/trackExternalProductChanges/including.cpp
new file mode 100644
index 000000000..bfe738d1d
--- /dev/null
+++ b/tests/auto/blackbox/testdata/trackExternalProductChanges/including.cpp
@@ -0,0 +1,3 @@
+#include <hiddenheaderqbs.h>
+
+void f() {}
diff --git a/tests/auto/blackbox/testdata/trackExternalProductChanges/project.qbs b/tests/auto/blackbox/testdata/trackExternalProductChanges/project.qbs
index df1ddc1f9..451b0a25a 100644
--- a/tests/auto/blackbox/testdata/trackExternalProductChanges/project.qbs
+++ b/tests/auto/blackbox/testdata/trackExternalProductChanges/project.qbs
@@ -6,4 +6,10 @@ CppApplication {
     property stringList filesFromEnv: qbs.getEnv("QBS_TEST_PULL_IN_FILE_VIA_ENV")
                                       ? ["environmentChange.cpp"] : []
     files: ["main.cpp"].concat(FileList.fileList()).concat(filesFromEnv).concat(FileList.filesFromFs(qbs))
+
+    Group {
+        condition: qbs.getEnv("INCLUDE_PATH_TEST")
+        name: "file that needs help from the environment to find a header"
+        files: "including.cpp"
+    }
 }
diff --git a/tests/auto/blackbox/tst_blackbox.cpp b/tests/auto/blackbox/tst_blackbox.cpp
index b7c382d46..c90ac5618 100644
--- a/tests/auto/blackbox/tst_blackbox.cpp
+++ b/tests/auto/blackbox/tst_blackbox.cpp
@@ -843,6 +843,22 @@ void TestBlackbox::trackExternalProductChanges()
     QVERIFY(!m_qbsStdout.contains("compiling environmentChange.cpp"));
     QVERIFY(!m_qbsStdout.contains("compiling jsFileChange.cpp"));
     QVERIFY(m_qbsStdout.contains("compiling fileExists.cpp"));
+
+    rmDirR(relativeBuildDir());
+    Settings s((QString()));
+    const Profile profile(profileName(), &s);
+    const QStringList toolchainTypes = profile.value("qbs.toolchain").toStringList();
+    if (!toolchainTypes.contains("gcc"))
+        QSKIP("Need GCC-like compiler to run this test");
+    params.environment = QProcessEnvironment::systemEnvironment();
+    params.environment.insert("INCLUDE_PATH_TEST", "1");
+    params.expectFailure = true;
+    QVERIFY(runQbs(params) != 0);
+    QVERIFY2(m_qbsStderr.contains("hiddenheaderqbs.h"), m_qbsStderr.constData());
+    params.environment.insert("CPLUS_INCLUDE_PATH",
+                              QDir::toNativeSeparators(QDir::currentPath() + "/hidden"));
+    params.expectFailure = false;
+    QCOMPARE(runQbs(params), 0);
 }
 
 void TestBlackbox::trackRemoveFile()