Fix change tracking for module properties requested from artifacts

... in commands. For some strange reason, this particular case was
missing.

Task-number: QBS-1049
Change-Id: I2ceb000ef5b362754f9fcbccd44b3e3c3e396e46
Reviewed-by: Denis Klychkov <kd.snake@gmail.com>
Reviewed-by: Joerg Bornemann <joerg.bornemann@qt.io>

12 files changed, 66 insertions, 9 deletions

diff --git a/src/lib/corelib/buildgraph/buildgraphloader.cpp b/src/lib/corelib/buildgraph/buildgraphloader.cpp
index d779f7b6b..1cdaa90df 100644
--- a/src/lib/corelib/buildgraph/buildgraphloader.cpp
+++ b/src/lib/corelib/buildgraph/buildgraphloader.cpp
@@ -694,26 +694,45 @@ static QVariantMap propertyMapByKind(const ResolvedProductConstPtr &product, Pro
     return QVariantMap();
 }
 
+static void invalidateTransformer(const TransformerPtr &transformer)
+{
+    const JavaScriptCommandPtr &pseudoCommand = JavaScriptCommand::create();
+    pseudoCommand->setSourceCode(QLatin1String("random stuff that will cause "
+                                               "commandsEqual() to fail"));
+    transformer->commands << pseudoCommand;
+}
+
 bool BuildGraphLoader::checkForPropertyChanges(const TransformerPtr &restoredTrafo,
         const ResolvedProductPtr &freshProduct)
 {
     // This check must come first, as it can prevent build data rescuing.
     foreach (const Property &property, restoredTrafo->propertiesRequestedInCommands) {
         if (checkForPropertyChange(property, propertyMapByKind(freshProduct, property.kind))) {
-            const JavaScriptCommandPtr &pseudoCommand = JavaScriptCommand::create();
-            pseudoCommand->setSourceCode(QLatin1String("random stuff that will cause "
-                                                       "commandsEqual() to fail"));
-            restoredTrafo->commands << pseudoCommand;
+            invalidateTransformer(restoredTrafo);
             return true;
         }
     }
 
+    QMap<QString, SourceArtifactConstPtr> artifactMap;
+    for (auto it = restoredTrafo->propertiesRequestedFromArtifactInCommands.cbegin();
+         it != restoredTrafo->propertiesRequestedFromArtifactInCommands.cend(); ++it) {
+        const SourceArtifactConstPtr artifact
+                = findSourceArtifact(freshProduct, it.key(), artifactMap);
+        if (!artifact)
+            continue;
+        foreach (const Property &property, it.value()) {
+            if (checkForPropertyChange(property, artifact->properties->value())) {
+                invalidateTransformer(restoredTrafo);
+                return true;
+            }
+        }
+    }
+
     foreach (const Property &property, restoredTrafo->propertiesRequestedInPrepareScript) {
         if (checkForPropertyChange(property, propertyMapByKind(freshProduct, property.kind)))
             return true;
     }
 
-    QMap<QString, SourceArtifactConstPtr> artifactMap;
     for (QHash<QString, PropertySet>::ConstIterator it =
          restoredTrafo->propertiesRequestedFromArtifactInPrepareScript.constBegin();
          it != restoredTrafo->propertiesRequestedFromArtifactInPrepareScript.constEnd(); ++it) {
@@ -842,6 +861,8 @@ void BuildGraphLoader::rescueOldBuildData(const ResolvedProductConstPtr &restore
                     = oldArtifact->transformer->propertiesRequestedInCommands;
             rad.propertiesRequestedFromArtifactInPrepareScript
                     = oldArtifact->transformer->propertiesRequestedFromArtifactInPrepareScript;
+            rad.propertiesRequestedFromArtifactInCommands
+                    = oldArtifact->transformer->propertiesRequestedFromArtifactInCommands;
             const ChildrenInfo &childrenInfo = childLists.value(oldArtifact);
             foreach (Artifact * const child, childrenInfo.children) {
                 rad.children << RescuableArtifactData::ChildData(child->product->name,
diff --git a/src/lib/corelib/buildgraph/executor.cpp b/src/lib/corelib/buildgraph/executor.cpp
index 946af9044..c1f3371e5 100644
--- a/src/lib/corelib/buildgraph/executor.cpp
+++ b/src/lib/corelib/buildgraph/executor.cpp
@@ -800,6 +800,8 @@ void Executor::rescueOldBuildData(Artifact *artifact, bool *childrenAdded = 0)
                 = rad.propertiesRequestedInCommands;
         artifact->transformer->propertiesRequestedFromArtifactInPrepareScript
                 = rad.propertiesRequestedFromArtifactInPrepareScript;
+        artifact->transformer->propertiesRequestedFromArtifactInCommands
+                = rad.propertiesRequestedFromArtifactInCommands;
         artifact->setTimestamp(rad.timeStamp);
         if (childrenAdded && !childrenToConnect.isEmpty())
             *childrenAdded = true;
diff --git a/src/lib/corelib/buildgraph/jscommandexecutor.cpp b/src/lib/corelib/buildgraph/jscommandexecutor.cpp
index ddbc1049d..3fdebc436 100644
--- a/src/lib/corelib/buildgraph/jscommandexecutor.cpp
+++ b/src/lib/corelib/buildgraph/jscommandexecutor.cpp
@@ -127,6 +127,8 @@ private:
         scriptEngine->setGlobalObject(scope.prototype());
         transformer->propertiesRequestedInCommands
                 += scriptEngine->propertiesRequestedInScript();
+        transformer->propertiesRequestedFromArtifactInCommands
+                = scriptEngine->propertiesRequestedFromArtifact();
         scriptEngine->clearRequestedProperties();
         if (scriptEngine->hasUncaughtException()) {
             // ### We don't know the line number of the command's sourceCode property assignment.
diff --git a/src/lib/corelib/buildgraph/rescuableartifactdata.cpp b/src/lib/corelib/buildgraph/rescuableartifactdata.cpp
index 02c3d9ea6..d73c20b2b 100644
--- a/src/lib/corelib/buildgraph/rescuableartifactdata.cpp
+++ b/src/lib/corelib/buildgraph/rescuableartifactdata.cpp
@@ -68,6 +68,7 @@ void RescuableArtifactData::load(PersistentPool &pool)
     propertiesRequestedInPrepareScript = restorePropertySet(pool);
     propertiesRequestedInCommands = restorePropertySet(pool);
     propertiesRequestedFromArtifactInPrepareScript = restorePropertyHash(pool);
+    propertiesRequestedFromArtifactInCommands = restorePropertyHash(pool);
     commands = loadCommandList(pool);
     fileTags.load(pool);
     properties = pool.loadVariantMap();
@@ -88,6 +89,7 @@ void RescuableArtifactData::store(PersistentPool &pool) const
     storePropertySet(pool, propertiesRequestedInPrepareScript);
     storePropertySet(pool, propertiesRequestedInCommands);
     storePropertyHash(pool, propertiesRequestedFromArtifactInPrepareScript);
+    storePropertyHash(pool, propertiesRequestedFromArtifactInCommands);
     storeCommandList(commands, pool);
     fileTags.store(pool);
     pool.store(properties);
diff --git a/src/lib/corelib/buildgraph/rescuableartifactdata.h b/src/lib/corelib/buildgraph/rescuableartifactdata.h
index 842228bcb..0f6d8ae40 100644
--- a/src/lib/corelib/buildgraph/rescuableartifactdata.h
+++ b/src/lib/corelib/buildgraph/rescuableartifactdata.h
@@ -81,6 +81,7 @@ public:
     PropertySet propertiesRequestedInPrepareScript;
     PropertySet propertiesRequestedInCommands;
     PropertyHash propertiesRequestedFromArtifactInPrepareScript;
+    PropertyHash propertiesRequestedFromArtifactInCommands;
 
     // Only needed for API purposes
     FileTags fileTags;
diff --git a/src/lib/corelib/buildgraph/transformer.cpp b/src/lib/corelib/buildgraph/transformer.cpp
index 87966e110..3d0eedbf3 100644
--- a/src/lib/corelib/buildgraph/transformer.cpp
+++ b/src/lib/corelib/buildgraph/transformer.cpp
@@ -264,6 +264,7 @@ void Transformer::load(PersistentPool &pool)
     propertiesRequestedInPrepareScript = restorePropertySet(pool);
     propertiesRequestedInCommands = restorePropertySet(pool);
     propertiesRequestedFromArtifactInPrepareScript = restorePropertyHash(pool);
+    propertiesRequestedFromArtifactInCommands = restorePropertyHash(pool);
     commands = loadCommandList(pool);
     pool.stream() >> alwaysRun;
 }
@@ -276,6 +277,7 @@ void Transformer::store(PersistentPool &pool) const
     storePropertySet(pool, propertiesRequestedInPrepareScript);
     storePropertySet(pool, propertiesRequestedInCommands);
     storePropertyHash(pool, propertiesRequestedFromArtifactInPrepareScript);
+    storePropertyHash(pool, propertiesRequestedFromArtifactInCommands);
     storeCommandList(commands, pool);
     pool.stream() << alwaysRun;
 }
diff --git a/src/lib/corelib/buildgraph/transformer.h b/src/lib/corelib/buildgraph/transformer.h
index d6bf8cb62..844ffc425 100644
--- a/src/lib/corelib/buildgraph/transformer.h
+++ b/src/lib/corelib/buildgraph/transformer.h
@@ -70,6 +70,7 @@ public:
     PropertySet propertiesRequestedInPrepareScript;
     PropertySet propertiesRequestedInCommands;
     QHash<QString, PropertySet> propertiesRequestedFromArtifactInPrepareScript;
+    QHash<QString, PropertySet> propertiesRequestedFromArtifactInCommands;
     bool alwaysRun;
 
     static QScriptValue translateFileConfig(QScriptEngine *scriptEngine,
diff --git a/src/lib/corelib/tools/persistence.cpp b/src/lib/corelib/tools/persistence.cpp
index 82890b688..c620e0879 100644
--- a/src/lib/corelib/tools/persistence.cpp
+++ b/src/lib/corelib/tools/persistence.cpp
@@ -50,7 +50,7 @@
 namespace qbs {
 namespace Internal {
 
-static const char QBS_PERSISTENCE_MAGIC[] = "QBSPERSISTENCE-92";
+static const char QBS_PERSISTENCE_MAGIC[] = "QBSPERSISTENCE-93";
 
 PersistentPool::PersistentPool(Logger &logger) : m_logger(logger)
 {
diff --git a/tests/auto/blackbox/testdata/propertyChanges/modules/TestModule/module.qbs b/tests/auto/blackbox/testdata/propertyChanges/modules/TestModule/module.qbs
index bac717dc6..4bdd5c7fe 100644
--- a/tests/auto/blackbox/testdata/propertyChanges/modules/TestModule/module.qbs
+++ b/tests/auto/blackbox/testdata/propertyChanges/modules/TestModule/module.qbs
@@ -7,6 +7,8 @@ Module {
         fileTags: "test-input"
     }
 
+    property string testProperty
+
     Rule {
         inputs: ['test-input']
         Artifact {
@@ -19,8 +21,9 @@ Module {
             cmd.highlight = "codegen";
             cmd.description = "Making output from input";
             cmd.sourceCode = function() {
-            // console.info('Change in source code');
-            File.copy(input.filePath, output.filePath);
+                // console.info('Change in source code');
+                console.info(input.moduleProperty("TestModule", "testProperty"));
+                File.copy(input.filePath, output.filePath);
             }
             return cmd;
         }
diff --git a/tests/auto/blackbox/testdata/propertyChanges/project.qbs b/tests/auto/blackbox/testdata/propertyChanges/project.qbs
index 8293d4ca7..a2b42a626 100644
--- a/tests/auto/blackbox/testdata/propertyChanges/project.qbs
+++ b/tests/auto/blackbox/testdata/propertyChanges/project.qbs
@@ -6,6 +6,7 @@ import qbs.TextFile
 Project {
     property var projectDefines: ["blubb2"]
     property string fileContentSuffix: "suffix 1"
+    property string testProperty: "default value"
     CppApplication {
         name: qbs.enableDebugCode ? "product 1.debug" : "product 1.release"
         cpp.defines: ["blubb1"]
diff --git a/tests/auto/blackbox/testdata/propertyChanges/ruletest.qbs b/tests/auto/blackbox/testdata/propertyChanges/ruletest.qbs
index b367e07fc..21d6f75ef 100644
--- a/tests/auto/blackbox/testdata/propertyChanges/ruletest.qbs
+++ b/tests/auto/blackbox/testdata/propertyChanges/ruletest.qbs
@@ -4,5 +4,8 @@ Product {
     name: "ruletest"
     type: "test-output"
     Depends { name: "TestModule" }
-    files: "test.in"
+    Group {
+        files: "test.in"
+        TestModule.testProperty: project.testProperty
+    }
 }
diff --git a/tests/auto/blackbox/tst_blackbox.cpp b/tests/auto/blackbox/tst_blackbox.cpp
index 454644de3..989c59245 100644
--- a/tests/auto/blackbox/tst_blackbox.cpp
+++ b/tests/auto/blackbox/tst_blackbox.cpp
@@ -1981,6 +1981,7 @@ void TestBlackbox::propertyChanges()
     QVERIFY(m_qbsStdout.contains("linking product 1.debug"));
     QVERIFY(m_qbsStdout.contains("generated.txt"));
     QVERIFY(m_qbsStdout.contains("Making output from input"));
+    QVERIFY(m_qbsStdout.contains("default value"));
     QVERIFY(m_qbsStdout.contains("Making output from other output"));
     QFile generatedFile(relativeProductBuildDir("generated text file") + "/generated.txt");
     QVERIFY(generatedFile.open(QIODevice::ReadOnly));
@@ -2175,6 +2176,24 @@ void TestBlackbox::propertyChanges()
     QCOMPARE(generatedFile.readAll(), QByteArray("prefix 2contents 2suffix 2"));
     generatedFile.close();
 
+    // Incremental build, module property used in JavaScript command changed.
+    WAIT_FOR_NEW_TIMESTAMP();
+    QVERIFY(projectFile.open(QIODevice::ReadWrite));
+    contents = projectFile.readAll();
+    contents.replace("default value", "new value");
+    projectFile.resize(0);
+    projectFile.write(contents);
+    projectFile.close();
+    QCOMPARE(runQbs(params), 0);
+    QVERIFY(!m_qbsStdout.contains("compiling source1.cpp"));
+    QVERIFY(!m_qbsStdout.contains("compiling source2.cpp"));
+    QVERIFY(!m_qbsStdout.contains("compiling source3.cpp"));
+    QVERIFY(!m_qbsStdout.contains("compiling lib.cpp"));
+    QVERIFY(!m_qbsStdout.contains("generated.txt"));
+    QVERIFY(m_qbsStdout.contains("Making output from input"));
+    QVERIFY(m_qbsStdout.contains("Making output from other output"));
+    QVERIFY(m_qbsStdout.contains("new value"));
+
     // Incremental build, prepare script of a rule in a module changed.
     WAIT_FOR_NEW_TIMESTAMP();
     QFile moduleFile("modules/TestModule/module.qbs");