Take source artifact properties into account when change tracking.

When properties on source files change, rules might have to be re-
applied and the installation of the respective source file may
have to be re-done. The current code catches none of that.

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

6 files changed, 40 insertions, 10 deletions

diff --git a/src/lib/buildgraph/buildgraphloader.cpp b/src/lib/buildgraph/buildgraphloader.cpp
index 8d8b68887..a75e83b02 100644
--- a/src/lib/buildgraph/buildgraphloader.cpp
+++ b/src/lib/buildgraph/buildgraphloader.cpp
@@ -552,7 +552,7 @@ void BuildGraphLoader::onProductFileListChanged(const ResolvedProductPtr &restor
             continue;
         }
 
-        // TODO: overrideFileTags and properties have to be checked for changes as well.
+        // TODO: overrideFileTags has to be checked for changes as well.
         if (changedArtifact->fileTags != a->fileTags) {
             // artifact's filetags have changed
             m_logger.qbsDebug() << "[BG] filetags have changed for artifact '"
@@ -579,6 +579,16 @@ void BuildGraphLoader::onProductFileListChanged(const ResolvedProductPtr &restor
                 }
             }
         }
+
+        if (changedArtifact->properties->value() != a->properties->value()) {
+            m_logger.qbsDebug() << "[BG] properties have changed for artifact '"
+                                << a->absoluteFilePath << "'";
+            Artifact * const oldArtifact
+                    = lookupArtifact(restoredProduct, oldBuildData, a->absoluteFilePath, true);
+            QBS_CHECK(oldArtifact);
+            removeArtifactAndExclusiveDependents(oldArtifact, &artifactsToRemove);
+            addedArtifacts += createArtifact(newlyResolvedProduct, changedArtifact, m_logger);
+        }
     }
 
     // apply rules for new artifacts
diff --git a/tests/auto/api/tst_api.cpp b/tests/auto/api/tst_api.cpp
index 599957235..199227b22 100644
--- a/tests/auto/api/tst_api.cpp
+++ b/tests/auto/api/tst_api.cpp
@@ -129,7 +129,7 @@ void TestApi::installableFiles()
     installOptions.setInstallRoot(QLatin1String("/tmp"));
     QList<qbs::InstallableFile> installableFiles
             = project.installableFilesForProduct(product, installOptions);
-    QCOMPARE(installableFiles.count(), 1);
+    QCOMPARE(installableFiles.count(), 2);
     qbs::InstallableFile const application = installableFiles.first();
     QVERIFY(application.isExecutable());
     QString expectedTargetFilePath
diff --git a/tests/auto/blackbox/testdata/installed_artifact/installed_artifact.qbs b/tests/auto/blackbox/testdata/installed_artifact/installed_artifact.qbs
index 106ccdf54..81562a70b 100644
--- a/tests/auto/blackbox/testdata/installed_artifact/installed_artifact.qbs
+++ b/tests/auto/blackbox/testdata/installed_artifact/installed_artifact.qbs
@@ -4,7 +4,11 @@ Application {
     name: "installedApp"
     type: "application"
     Depends { name: "cpp" }
-    files: "main.cpp"
+    Group {
+        files: "main.cpp"
+        qbs.install: true
+        qbs.installDir: "src"
+    }
     qbs.installPrefix: "/usr"
     Group {
         fileTagsFilter: "application"
diff --git a/tests/auto/blackbox/testdata/trackExternalProductChanges/fileList.js b/tests/auto/blackbox/testdata/trackExternalProductChanges/fileList.js
index 0d540d82e..d5c40cde1 100644
--- a/tests/auto/blackbox/testdata/trackExternalProductChanges/fileList.js
+++ b/tests/auto/blackbox/testdata/trackExternalProductChanges/fileList.js
@@ -1 +1,6 @@
 function fileList() { return []; }
+
+function filesFromEnv(qbs) { return qbs.getenv("QBS_TEST_PULL_IN_FILE_VIA_ENV") ? ["environmentChange.cpp"] : []; }
+
+function filesFromFs(qbs) { return File.exists(path + "/fileExists.cpp") ? ["fileExists.cpp"] : []; }
+
diff --git a/tests/auto/blackbox/testdata/trackExternalProductChanges/project.qbs b/tests/auto/blackbox/testdata/trackExternalProductChanges/project.qbs
index 8ed267fa5..c3eb9b0b7 100644
--- a/tests/auto/blackbox/testdata/trackExternalProductChanges/project.qbs
+++ b/tests/auto/blackbox/testdata/trackExternalProductChanges/project.qbs
@@ -3,8 +3,5 @@ import qbs.File
 import "fileList.js" as FileList
 
 CppApplication {
-    property pathList filesFromEnv: qbs.getenv("QBS_TEST_PULL_IN_FILE_VIA_ENV") ? ["environmentChange.cpp"] : []
-    property pathList filesFromJs: FileList.fileList()
-    property pathList filesFromFs: File.exists(path + "/fileExists.cpp") ? ["fileExists.cpp"] : []
-    files: ["main.cpp"].concat(filesFromJs).concat(filesFromEnv).concat(filesFromFs)
+    files: ["main.cpp"].concat(FileList.fileList()).concat(FileList.filesFromEnv(qbs)).concat(FileList.filesFromFs(qbs))
 }
diff --git a/tests/auto/blackbox/tst_blackbox.cpp b/tests/auto/blackbox/tst_blackbox.cpp
index e96286856..f6f68d849 100644
--- a/tests/auto/blackbox/tst_blackbox.cpp
+++ b/tests/auto/blackbox/tst_blackbox.cpp
@@ -698,7 +698,7 @@ void TestBlackbox::trackExternalProductChanges()
     QFile jsFile("fileList.js");
     QVERIFY(jsFile.open(QIODevice::ReadWrite));
     QByteArray jsCode = jsFile.readAll();
-    jsCode.replace("[]", "['jsFileChange.cpp']");
+    jsCode.replace("return []", "return ['jsFileChange.cpp']");
     jsFile.resize(0);
     jsFile.write(jsCode);
     jsFile.close();
@@ -1201,7 +1201,10 @@ void TestBlackbox::propertyChanges()
     QVERIFY(m_qbsStdout.contains("compiling source2.cpp"));
     QVERIFY(m_qbsStdout.contains("compiling source3.cpp"));
     QVERIFY(!m_qbsStdout.contains("generated.txt"));
-    QVERIFY(!m_qbsStdout.contains("Making output from input"));
+
+    // Not actually necessary, but qbs cannot know that, since a property change is potentially
+    // relevant to all rules.
+    QVERIFY(m_qbsStdout.contains("Making output from input"));
 
     // Incremental build, non-essential dependency removed.
     waitForNewTimestamp();
@@ -1482,6 +1485,7 @@ void TestBlackbox::installedApp()
     QCOMPARE(runQbs(QbsRunParameters(QStringList("install") << "--remove-first")), 0);
     QVERIFY(QFile::exists(defaultInstallRoot
             + HostOsInfo::appendExecutableSuffix(QLatin1String("/usr/bin/installedApp"))));
+    QVERIFY(QFile::exists(defaultInstallRoot + QLatin1String("/usr/src/main.cpp")));
     QVERIFY(!addedFile.exists());
 
     // Check whether changing install parameters on the product causes re-installation.
@@ -1496,17 +1500,27 @@ void TestBlackbox::installedApp()
     QCOMPARE(runQbs(QbsRunParameters(QStringList("install"))), 0);
     QVERIFY(QFile::exists(defaultInstallRoot
             + HostOsInfo::appendExecutableSuffix(QLatin1String("/usr/local/bin/installedApp"))));
+    QVERIFY(QFile::exists(defaultInstallRoot + QLatin1String("/usr/local/src/main.cpp")));
 
     // Check whether changing install parameters on the artifact causes re-installation.
     content.replace("qbs.installDir: \"bin\"", "qbs.installDir: 'custom'");
     waitForNewTimestamp();
     projectFile.resize(0);
     projectFile.write(content);
-    projectFile.close();
+    QVERIFY(projectFile.flush());
     QCOMPARE(runQbs(QbsRunParameters(QStringList("install"))), 0);
     QVERIFY(QFile::exists(defaultInstallRoot
             + HostOsInfo::appendExecutableSuffix(QLatin1String("/usr/local/custom/installedApp"))));
 
+    // Check whether changing install parameters on a source file causes re-installation.
+    content.replace("qbs.installDir: \"src\"", "qbs.installDir: 'source'");
+    waitForNewTimestamp();
+    projectFile.resize(0);
+    projectFile.write(content);
+    projectFile.close();
+    QCOMPARE(runQbs(QbsRunParameters(QStringList("install"))), 0);
+    QVERIFY(QFile::exists(defaultInstallRoot + QLatin1String("/usr/local/source/main.cpp")));
+
     rmDirR(buildDir);
     QbsRunParameters params;
     params.arguments << "install" << "--no-build";