Handle "indirect" changes to a product's file list.

If the list of source files in a product changed without the respective
file having been touched (e.g. because the list was dependent on an
environment variable), we re-resolved the product, but we did not touch
the build data, so the new file was not compiled.

Change-Id: Ib63020339aa0bbc2a858ed22f938ab76fb0611c5
Reviewed-by: Joerg Bornemann <joerg.bornemann@digia.com>

9 files changed, 84 insertions, 34 deletions

diff --git a/src/lib/buildgraph/buildgraphloader.cpp b/src/lib/buildgraph/buildgraphloader.cpp
index 943944858..4d62e55ae 100644
--- a/src/lib/buildgraph/buildgraphloader.cpp
+++ b/src/lib/buildgraph/buildgraphloader.cpp
@@ -168,19 +168,21 @@ void BuildGraphLoader::trackProjectChanges(const SetupProjectParameters &paramet
 {
     const FileTime buildGraphTimeStamp = FileInfo(buildGraphFilePath).lastModified();
     QSet<QString> buildSystemFiles = restoredProject->buildSystemFiles;
-    const QList<ResolvedProjectPtr> allRestoredProjects
-            = restoredProject->allSubProjects() << restoredProject;
-
-    bool reResolvingNecessary = hasProjectFileChanged(allRestoredProjects, buildGraphTimeStamp,
-                                                      buildSystemFiles);
-    if (!reResolvingNecessary)
-        reResolvingNecessary = hasEnvironmentChanged(restoredProject);
     QList<ResolvedProductPtr> allRestoredProducts = restoredProject->allProducts();
     QList<ResolvedProductPtr> changedProducts;
-    reResolvingNecessary |= hasProductFileChanged(allRestoredProducts, buildGraphTimeStamp,
-                                                  buildSystemFiles, changedProducts);
-    if (!reResolvingNecessary)
-        reResolvingNecessary = hasBuildSystemFileChanged(buildSystemFiles, buildGraphTimeStamp);
+    bool reResolvingNecessary = hasProductFileChanged(allRestoredProducts, buildGraphTimeStamp,
+                                                      buildSystemFiles, changedProducts);
+
+    // "External" changes, e.g. in the environment or in a JavaScript file,
+    // can make the list of source files in a product change without the respective file
+    // having been touched. In such a case, the build data for that product will have to be set up
+    // anew.
+    bool allProductsPotentiallyChanged = false;
+    if (hasBuildSystemFileChanged(buildSystemFiles, buildGraphTimeStamp)
+            || hasEnvironmentChanged(restoredProject)) {
+        reResolvingNecessary = true;
+        allProductsPotentiallyChanged = true;
+    }
 
    if (!reResolvingNecessary)
        return;
@@ -215,6 +217,8 @@ void BuildGraphLoader::trackProjectChanges(const SetupProjectParameters &paramet
         }
     }
 
+    if (allProductsPotentiallyChanged)
+        checkAllProductsForChanges(allRestoredProducts, freshProductsByName, changedProducts);
     foreach (const ResolvedProductPtr &product, changedProducts) {
         ResolvedProductPtr freshProduct = freshProductsByName.value(product->name);
         if (!freshProduct)
@@ -281,26 +285,6 @@ bool BuildGraphLoader::hasEnvironmentChanged(const TopLevelProjectConstPtr &rest
     return false;
 }
 
-bool BuildGraphLoader::hasProjectFileChanged(const QList<ResolvedProjectPtr> &restoredProjects,
-                                             const FileTime &referenceTime,
-                                             QSet<QString> &remainingBuildSystemFiles) const
-{
-    foreach (const ResolvedProjectConstPtr &p, restoredProjects) {
-        const QString fileName = p->location.fileName();
-        const FileInfo fi(fileName);
-        remainingBuildSystemFiles.remove(fileName);
-        if (!fi.exists()) {
-            m_logger.qbsDebug() << "A sub-project was removed, must re-resolve project";
-            return true;
-        }
-        if (referenceTime < fi.lastModified()) {
-            m_logger.qbsDebug() << "A project file changed, must re-resolve project.";
-            return true;
-        }
-    }
-    return false;
-}
-
 bool BuildGraphLoader::hasProductFileChanged(const QList<ResolvedProductPtr> &restoredProducts,
         const FileTime &referenceTime, QSet<QString> &remainingBuildSystemFiles,
         QList<ResolvedProductPtr> &changedProducts)
@@ -314,6 +298,7 @@ bool BuildGraphLoader::hasProductFileChanged(const QList<ResolvedProductPtr> &re
             m_logger.qbsDebug() << "A product was removed, must re-resolve project";
             hasChanged = true;
         } else if (referenceTime < pfi.lastModified()) {
+            m_logger.qbsDebug() << "A product was changed, must re-resolve project";
             changedProducts += product;
         } else {
             foreach (const GroupPtr &group, product->groups) {
@@ -348,6 +333,25 @@ bool BuildGraphLoader::hasBuildSystemFileChanged(const QSet<QString> &buildSyste
     return false;
 }
 
+void BuildGraphLoader::checkAllProductsForChanges(const QList<ResolvedProductPtr> &restoredProducts,
+        const QMap<QString, ResolvedProductPtr> &newlyResolvedProductsByName,
+        QList<ResolvedProductPtr> &changedProducts)
+{
+    foreach (const ResolvedProductPtr &restoredProduct, restoredProducts) {
+        if (changedProducts.contains(restoredProduct))
+            continue;
+        const ResolvedProductPtr newlyResolvedProduct
+                = newlyResolvedProductsByName.value(restoredProduct->name);
+        if (!newlyResolvedProduct)
+            continue;
+        if (newlyResolvedProduct->allFiles() == restoredProduct->allFiles())
+            continue;
+        m_logger.qbsDebug() << "Product '" << restoredProduct->name
+                            << "' was changed, must re-resolve project";
+        changedProducts << restoredProduct;
+    }
+}
+
 void BuildGraphLoader::onProductRemoved(const ResolvedProductPtr &product,
                                         ProjectBuildData *projectBuildData)
 {
diff --git a/src/lib/buildgraph/buildgraphloader.h b/src/lib/buildgraph/buildgraphloader.h
index cd769583b..8cbe5e354 100644
--- a/src/lib/buildgraph/buildgraphloader.h
+++ b/src/lib/buildgraph/buildgraphloader.h
@@ -67,15 +67,15 @@ private:
                              const QString &buildGraphFilePath,
                              const TopLevelProjectPtr &restoredProject);
     bool hasEnvironmentChanged(const TopLevelProjectConstPtr &restoredProject) const;
-    bool hasProjectFileChanged(const QList<ResolvedProjectPtr> &restoredProjects,
-                               const FileTime &referenceTime,
-                               QSet<QString> &remainingBuildSystemFiles) const;
     bool hasProductFileChanged(const QList<ResolvedProductPtr> &restoredProducts,
                                const FileTime &referenceTime,
                                QSet<QString> &remainingBuildSystemFiles,
                                QList<ResolvedProductPtr> &changedProducts);
     bool hasBuildSystemFileChanged(const QSet<QString> &buildSystemFiles,
                                    const FileTime &referenceTime);
+    void checkAllProductsForChanges(const QList<ResolvedProductPtr> &restoredProducts,
+            const QMap<QString, ResolvedProductPtr> &newlyResolvedProductsByName,
+            QList<ResolvedProductPtr> &changedProducts);
     void onProductRemoved(const ResolvedProductPtr &product, ProjectBuildData *projectBuildData);
     void onProductChanged(const ResolvedProductPtr &product,
                           const ResolvedProductPtr &changedProduct);
diff --git a/tests/auto/blackbox/testdata/trackExternalProductChanges/environmentChange.cpp b/tests/auto/blackbox/testdata/trackExternalProductChanges/environmentChange.cpp
new file mode 100644
index 000000000..dd3c37880
--- /dev/null
+++ b/tests/auto/blackbox/testdata/trackExternalProductChanges/environmentChange.cpp
@@ -0,0 +1 @@
+void environmentChange() { }
diff --git a/tests/auto/blackbox/testdata/trackExternalProductChanges/fileList.js b/tests/auto/blackbox/testdata/trackExternalProductChanges/fileList.js
new file mode 100644
index 000000000..0d540d82e
--- /dev/null
+++ b/tests/auto/blackbox/testdata/trackExternalProductChanges/fileList.js
@@ -0,0 +1 @@
+function fileList() { return []; }
diff --git a/tests/auto/blackbox/testdata/trackExternalProductChanges/jsFileChange.cpp b/tests/auto/blackbox/testdata/trackExternalProductChanges/jsFileChange.cpp
new file mode 100644
index 000000000..a23a409ae
--- /dev/null
+++ b/tests/auto/blackbox/testdata/trackExternalProductChanges/jsFileChange.cpp
@@ -0,0 +1 @@
+void jsFileChange() { }
diff --git a/tests/auto/blackbox/testdata/trackExternalProductChanges/main.cpp b/tests/auto/blackbox/testdata/trackExternalProductChanges/main.cpp
new file mode 100644
index 000000000..8b8d58de0
--- /dev/null
+++ b/tests/auto/blackbox/testdata/trackExternalProductChanges/main.cpp
@@ -0,0 +1 @@
+int main() { }
diff --git a/tests/auto/blackbox/testdata/trackExternalProductChanges/project.qbs b/tests/auto/blackbox/testdata/trackExternalProductChanges/project.qbs
new file mode 100644
index 000000000..cfd897447
--- /dev/null
+++ b/tests/auto/blackbox/testdata/trackExternalProductChanges/project.qbs
@@ -0,0 +1,6 @@
+import qbs
+import "fileList.js" as FileList
+
+CppApplication {
+    files: ["main.cpp"].concat(FileList.fileList()).concat(qbs.getenv("QBS_TEST_PULL_IN_FILE_VIA_ENV") ? ["environmentChange.cpp"] : [])
+}
diff --git a/tests/auto/blackbox/tst_blackbox.cpp b/tests/auto/blackbox/tst_blackbox.cpp
index dd9719b09..8d2f6dbdb 100644
--- a/tests/auto/blackbox/tst_blackbox.cpp
+++ b/tests/auto/blackbox/tst_blackbox.cpp
@@ -589,6 +589,41 @@ void TestBlackbox::trackAddFile()
     QCOMPARE(unchangedObjectFileTime1, unchangedObjectFileTime2);
 }
 
+void TestBlackbox::trackExternalProductChanges()
+{
+    QDir::setCurrent(testDataDir + "/trackExternalProductChanges");
+    QCOMPARE(runQbs(), 0);
+    QVERIFY(m_qbsStdout.contains("compiling main.cpp"));
+    QVERIFY(!m_qbsStdout.contains("compiling environmentChange.cpp"));
+    QVERIFY(!m_qbsStdout.contains("compiling jsFileChange.cpp"));
+
+    QbsRunParameters params;
+    params.environment.insert("QBS_TEST_PULL_IN_FILE_VIA_ENV", "1");
+    QCOMPARE(runQbs(params), 0);
+    QVERIFY(!m_qbsStdout.contains("compiling main.cpp"));
+    QVERIFY(m_qbsStdout.contains("compiling environmentChange.cpp"));
+    QVERIFY(!m_qbsStdout.contains("compiling jsFileChange.cpp"));
+
+    rmDirR(buildDir);
+    QCOMPARE(runQbs(), 0);
+    QVERIFY(m_qbsStdout.contains("compiling main.cpp"));
+    QVERIFY(!m_qbsStdout.contains("compiling environmentChange.cpp"));
+    QVERIFY(!m_qbsStdout.contains("compiling jsFileChange.cpp"));
+
+    waitForNewTimestamp();
+    QFile jsFile("fileList.js");
+    QVERIFY(jsFile.open(QIODevice::ReadWrite));
+    QByteArray jsCode = jsFile.readAll();
+    jsCode.replace("[]", "['jsFileChange.cpp']");
+    jsFile.resize(0);
+    jsFile.write(jsCode);
+    jsFile.close();
+    QCOMPARE(runQbs(), 0);
+    QVERIFY(!m_qbsStdout.contains("compiling main.cpp"));
+    QVERIFY(!m_qbsStdout.contains("compiling environmentChange.cpp"));
+    QVERIFY(m_qbsStdout.contains("compiling jsFileChange.cpp"));
+}
+
 void TestBlackbox::trackRemoveFile()
 {
     QProcess process;
diff --git a/tests/auto/blackbox/tst_blackbox.h b/tests/auto/blackbox/tst_blackbox.h
index b9d19da96..153e0aae6 100644
--- a/tests/auto/blackbox/tst_blackbox.h
+++ b/tests/auto/blackbox/tst_blackbox.h
@@ -111,6 +111,7 @@ private slots:
     void track_qrc();
     void track_qobject_change();
     void trackAddFile();
+    void trackExternalProductChanges();
     void trackRemoveFile();
     void trackAddFileTag();
     void trackRemoveFileTag();