From d0516e847335e7f432f137bf4774a05b0c007569 Mon Sep 17 00:00:00 2001
From: Christian Kandeler <christian.kandeler@qt.io>
Date: Tue, 1 Nov 2016 17:43:37 +0100
Subject: Executor: Fix dynamic rules with generated inputs

This has never worked, as the condition that was supposed to collect
such inputs always evaluated to false.

Task-number: QBS-1029
Change-Id: Idf3f86f12fa050a3d151f4551821bf4395f715d8
Reviewed-by: Joerg Bornemann <joerg.bornemann@qt.io>
---
 src/lib/corelib/buildgraph/executor.cpp            | 12 ++++--
 .../input.txt                                      |  1 +
 .../p.qbs                                          | 50 ++++++++++++++++++++++
 tests/auto/blackbox/tst_blackbox.cpp               | 18 ++++++++
 tests/auto/blackbox/tst_blackbox.h                 |  1 +
 5 files changed, 79 insertions(+), 3 deletions(-)
 create mode 100644 tests/auto/blackbox/testdata/generated-artifact-as-input-to-dynamic-rule/input.txt
 create mode 100644 tests/auto/blackbox/testdata/generated-artifact-as-input-to-dynamic-rule/p.qbs

diff --git a/src/lib/corelib/buildgraph/executor.cpp b/src/lib/corelib/buildgraph/executor.cpp
index e0f023c3d..e774f71e7 100644
--- a/src/lib/corelib/buildgraph/executor.cpp
+++ b/src/lib/corelib/buildgraph/executor.cpp
@@ -449,9 +449,15 @@ void Executor::executeRuleNode(RuleNode *ruleNode)
         for (Artifact *artifact : filterByType<Artifact>(ruleNode->product->buildData->nodes)) {
             if (artifact->artifactType == Artifact::SourceFile)
                 continue;
-            if (artifact->timestampRetrieved && !isUpToDate(artifact)
-                    && ruleNode->rule()->acceptsAsInput(artifact)) {
-                changedInputArtifacts += artifact;
+            if (ruleNode->rule()->acceptsAsInput(artifact)) {
+                for (const Artifact * const parent : artifact->parentArtifacts()) {
+                    if (parent->transformer->rule != ruleNode->rule())
+                        continue;
+                    if (parent->timestamp() < artifact->timestamp()) {
+                        changedInputArtifacts += artifact;
+                        break;
+                    }
+                }
             }
         }
     }
diff --git a/tests/auto/blackbox/testdata/generated-artifact-as-input-to-dynamic-rule/input.txt b/tests/auto/blackbox/testdata/generated-artifact-as-input-to-dynamic-rule/input.txt
new file mode 100644
index 000000000..37081c649
--- /dev/null
+++ b/tests/auto/blackbox/testdata/generated-artifact-as-input-to-dynamic-rule/input.txt
@@ -0,0 +1 @@
+old.txt
diff --git a/tests/auto/blackbox/testdata/generated-artifact-as-input-to-dynamic-rule/p.qbs b/tests/auto/blackbox/testdata/generated-artifact-as-input-to-dynamic-rule/p.qbs
new file mode 100644
index 000000000..725966c3b
--- /dev/null
+++ b/tests/auto/blackbox/testdata/generated-artifact-as-input-to-dynamic-rule/p.qbs
@@ -0,0 +1,50 @@
+import qbs
+import qbs.File
+import qbs.TextFile
+
+Product {
+    type: ["mytype.final"]
+
+    Group {
+        files: ["input.txt"]
+        fileTags: ["mytype.in"]
+    }
+
+    Rule {
+        inputs: ["mytype.in"]
+        Artifact {
+            filePath: "output.txt"
+            fileTags: ["mytype.out"]
+        }
+        prepare: {
+            var cmd = new JavaScriptCommand();
+            cmd.description = "generating " + output.fileName;
+            cmd.sourceCode = function() { File.copy(input.filePath, output.filePath); };
+            return [cmd];
+        }
+    }
+
+    Rule {
+        inputs: ["mytype.out"]
+        outputFileTags: ["mytype.final"]
+        outputArtifacts: {
+            var file;
+            var inFile = new TextFile(input.filePath, TextFile.ReadOnly);
+            try {
+                file = inFile.readLine();
+                if (!file)
+                    throw "no file name found";
+            } finally {
+                inFile.close();
+           }
+            return [{ filePath: file, fileTags: ["mytype.final"] }];
+        }
+        prepare: {
+            var cmd = new JavaScriptCommand();
+            cmd.description = "generating " + output.fileName;
+            cmd.sourceCode = function() { File.copy(input.filePath, output.filePath); };
+            return [cmd];
+        }
+    }
+}
+
diff --git a/tests/auto/blackbox/tst_blackbox.cpp b/tests/auto/blackbox/tst_blackbox.cpp
index f791c5e86..6fea9abf6 100644
--- a/tests/auto/blackbox/tst_blackbox.cpp
+++ b/tests/auto/blackbox/tst_blackbox.cpp
@@ -4042,6 +4042,24 @@ void TestBlackbox::frameworkStructure()
     QVERIFY(!directoryExists(relativeProductBuildDir("Widget") + "/Widget.framework/PrivateHeaders"));
 }
 
+void TestBlackbox::generatedArtifactAsInputToDynamicRule()
+{
+    QDir::setCurrent(testDataDir + "/generated-artifact-as-input-to-dynamic-rule");
+    QCOMPARE(runQbs(), 0);
+    const QString oldFile = relativeProductBuildDir("p") + "/old.txt";
+    QVERIFY2(regularFileExists(oldFile), qPrintable(oldFile));
+    WAIT_FOR_NEW_TIMESTAMP();
+    QFile inputFile("input.txt");
+    QVERIFY2(inputFile.open(QIODevice::WriteOnly), qPrintable(inputFile.errorString()));
+    inputFile.resize(0);
+    inputFile.write("new.txt");
+    inputFile.close();
+    QCOMPARE(runQbs(), 0);
+    QVERIFY2(!regularFileExists(oldFile), qPrintable(oldFile));
+    const QString newFile = relativeProductBuildDir("p") + "/new.txt";
+    QVERIFY2(regularFileExists(newFile), qPrintable(oldFile));
+}
+
 static bool haveWiX(const Profile &profile)
 {
     if (profile.value("wix.toolchainInstallPath").isValid() &&
diff --git a/tests/auto/blackbox/tst_blackbox.h b/tests/auto/blackbox/tst_blackbox.h
index b5a539bb8..61bb1a23b 100644
--- a/tests/auto/blackbox/tst_blackbox.h
+++ b/tests/auto/blackbox/tst_blackbox.h
@@ -134,6 +134,7 @@ private slots:
     void exportToOutsideSearchPath();
     void fileDependencies();
     void frameworkStructure();
+    void generatedArtifactAsInputToDynamicRule();
     void groupsInModules();
     void iconset();
     void iconsetApp();
-- 
cgit v1.2.3