Take the qbs.install* properties into account when change tracking.

These are currently ignored, because we only look for properties that
were requested in prepare scripts. This does not typically happen for
installation parameters, but since they end up in the output artifacts,
changes to them still require re-setting up the respective build data.

Task-number: QBS-359
Change-Id: I571cb4c1a02ea790ddde2f244136c6a0fd6fb627
Reviewed-by: Joerg Bornemann <joerg.bornemann@digia.com>

7 files changed, 72 insertions, 0 deletions

diff --git a/src/lib/buildgraph/buildgraphloader.cpp b/src/lib/buildgraph/buildgraphloader.cpp
index 121cb7ca2..15e2e9d7f 100644
--- a/src/lib/buildgraph/buildgraphloader.cpp
+++ b/src/lib/buildgraph/buildgraphloader.cpp
@@ -37,6 +37,7 @@
 #include "projectbuilddata.h"
 #include "rulesevaluationcontext.h"
 #include "transformer.h"
+#include <language/artifactproperties.h>
 #include <language/language.h>
 #include <language/loader.h>
 #include <logging/translator.h>
@@ -430,6 +431,23 @@ bool BuildGraphLoader::checkProductForChanges(const ResolvedProductPtr &restored
     // TODO: Check for more stuff.
 }
 
+bool BuildGraphLoader::checkProductForInstallInfoChanges(const ResolvedProductPtr &restoredProduct,
+        const ResolvedProductPtr &newlyResolvedProduct)
+{
+    // These are not requested from rules at build time, but we still need to take
+    // them into account.
+    const QStringList specialProperties = QStringList() << QLatin1String("install")
+            << QLatin1String("installDir") << QLatin1String("installPrefix");
+    foreach (const QString &key, specialProperties) {
+        if (restoredProduct->properties->qbsPropertyValue(key)
+                != newlyResolvedProduct->properties->qbsPropertyValue(key)) {
+            m_logger.qbsDebug() << "Product property 'qbs." << key << "' changed.";
+            return true;
+        }
+    }
+    return false;
+}
+
 bool BuildGraphLoader::checkForPropertyChanges(const ResolvedProductPtr &restoredProduct,
                                                const ResolvedProductPtr &newlyResolvedProduct)
 {
@@ -437,6 +455,12 @@ bool BuildGraphLoader::checkForPropertyChanges(const ResolvedProductPtr &restore
                            "product '"  << restoredProduct->name << "'.";
     if (!restoredProduct->buildData)
         return false;
+    if (checkProductForInstallInfoChanges(restoredProduct, newlyResolvedProduct))
+        return true;
+    if (!artifactPropertyListsAreEqual(restoredProduct->artifactProperties,
+                                       newlyResolvedProduct->artifactProperties)) {
+        return true;
+    }
     QSet<TransformerConstPtr> seenTransformers;
     foreach (Artifact * const artifact, restoredProduct->buildData->artifacts) {
         const TransformerConstPtr transformer = artifact->transformer;
diff --git a/src/lib/buildgraph/buildgraphloader.h b/src/lib/buildgraph/buildgraphloader.h
index 4806daad3..fed032f72 100644
--- a/src/lib/buildgraph/buildgraphloader.h
+++ b/src/lib/buildgraph/buildgraphloader.h
@@ -83,6 +83,8 @@ private:
             QList<ResolvedProductPtr> &productsWithChangedFiles);
     bool checkProductForChanges(const ResolvedProductPtr &restoredProduct,
                                 const ResolvedProductPtr &newlyResolvedProduct);
+    bool checkProductForInstallInfoChanges(const ResolvedProductPtr &restoredProduct,
+                                           const ResolvedProductPtr &newlyResolvedProduct);
     bool checkForPropertyChanges(const ResolvedProductPtr &restoredProduct,
                                  const ResolvedProductPtr &newlyResolvedProduct);
     void onProductRemoved(const ResolvedProductPtr &product, ProjectBuildData *projectBuildData,
diff --git a/src/lib/language/artifactproperties.cpp b/src/lib/language/artifactproperties.cpp
index 61a2e27e8..ea5a36394 100644
--- a/src/lib/language/artifactproperties.cpp
+++ b/src/lib/language/artifactproperties.cpp
@@ -55,5 +55,11 @@ void ArtifactProperties::store(PersistentPool &pool) const
     pool.store(m_propertyMap);
 }
 
+bool operator==(const ArtifactProperties &ap1, const ArtifactProperties &ap2)
+{
+    return ap1.fileTagsFilter() == ap2.fileTagsFilter()
+            && ap1.propertyMap()->value() == ap2.propertyMap()->value();
+}
+
 } // namespace Internal
 } // namespace qbs
diff --git a/src/lib/language/artifactproperties.h b/src/lib/language/artifactproperties.h
index f28218ae2..8bb7d6225 100644
--- a/src/lib/language/artifactproperties.h
+++ b/src/lib/language/artifactproperties.h
@@ -58,6 +58,11 @@ private:
     PropertyMapPtr m_propertyMap;
 };
 
+bool operator==(const ArtifactProperties &ap1, const ArtifactProperties &ap2);
+inline bool operator!=(const ArtifactProperties &ap1, const ArtifactProperties &ap2) {
+    return !(ap1 == ap2);
+}
+
 } // namespace Internal
 } // namespace qbs
 
diff --git a/src/lib/language/language.cpp b/src/lib/language/language.cpp
index 79a0cd260..6e5462930 100644
--- a/src/lib/language/language.cpp
+++ b/src/lib/language/language.cpp
@@ -988,6 +988,9 @@ template<typename T> bool listsAreEqual(const QList<T> &l1, const QList<T> &l2)
 QString keyFromElem(const SourceArtifactPtr &sa) { return sa->absoluteFilePath; }
 QString keyFromElem(const ResolvedTransformerConstPtr &t) { return t->transform->sourceCode; }
 QString keyFromElem(const RulePtr &r) { return r->toString(); }
+QString keyFromElem(const ArtifactPropertiesPtr &ap) {
+    return ap->fileTagsFilter().toStringList().join(QLatin1String(","));
+}
 
 bool operator==(const SourceArtifact &sa1, const SourceArtifact &sa2)
 {
@@ -1070,5 +1073,11 @@ uint qHash(const RuleArtifact::Binding &b)
     return qHash(qMakePair(b.code, b.name.join(QLatin1String(","))));
 }
 
+bool artifactPropertyListsAreEqual(const QList<ArtifactPropertiesPtr> &l1,
+                                   const QList<ArtifactPropertiesPtr> &l2)
+{
+    return listsAreEqual(l1, l2);
+}
+
 } // namespace Internal
 } // namespace qbs
diff --git a/src/lib/language/language.h b/src/lib/language/language.h
index 77f0425e4..904db3460 100644
--- a/src/lib/language/language.h
+++ b/src/lib/language/language.h
@@ -448,6 +448,9 @@ private:
     QVariantMap m_buildConfiguration;
 };
 
+bool artifactPropertyListsAreEqual(const QList<ArtifactPropertiesPtr> &l1,
+                                   const QList<ArtifactPropertiesPtr> &l2);
+
 } // namespace Internal
 } // namespace qbs
 
diff --git a/tests/auto/blackbox/tst_blackbox.cpp b/tests/auto/blackbox/tst_blackbox.cpp
index 0700df165..c796d3e61 100644
--- a/tests/auto/blackbox/tst_blackbox.cpp
+++ b/tests/auto/blackbox/tst_blackbox.cpp
@@ -1456,6 +1456,29 @@ void TestBlackbox::installedApp()
             + HostOsInfo::appendExecutableSuffix(QLatin1String("/usr/bin/installedApp"))));
     QVERIFY(!addedFile.exists());
 
+    // Check whether changing install parameters on the product causes re-installation.
+    QFile projectFile("installed_artifact.qbs");
+    QVERIFY(projectFile.open(QIODevice::ReadWrite));
+    QByteArray content = projectFile.readAll();
+    content.replace("qbs.installPrefix: \"/usr\"", "qbs.installPrefix: '/usr/local'");
+    waitForNewTimestamp();
+    projectFile.resize(0);
+    projectFile.write(content);
+    QVERIFY(projectFile.flush());
+    QCOMPARE(runQbs(QbsRunParameters(QStringList("install"))), 0);
+    QVERIFY(QFile::exists(defaultInstallRoot
+            + HostOsInfo::appendExecutableSuffix(QLatin1String("/usr/local/bin/installedApp"))));
+
+    // 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();
+    QCOMPARE(runQbs(QbsRunParameters(QStringList("install"))), 0);
+    QVERIFY(QFile::exists(defaultInstallRoot
+            + HostOsInfo::appendExecutableSuffix(QLatin1String("/usr/local/custom/installedApp"))));
+
     rmDirR(buildDir);
     QbsRunParameters params;
     params.arguments << "install" << "--no-build";