From c18c082406152ede2bcb39c423ca22fbc9864cf1 Mon Sep 17 00:00:00 2001 From: Christian Kandeler Date: Thu, 7 Jun 2018 16:56:12 +0200 Subject: Change tracking: Do not rescue outdated child artifacts If a former child artifact is not in the list anymore after the rules applicator has run and it was not originally added as a result of scanning, then it is no longer a child artifact. We used to add these artifacts back to the list of children, so that an existing parent/child connection persisted even after changing the rule input tags in a way that the child would not match anymore. Change-Id: I700f515bbe5732bdb9251d48e3fc5ad53fb181cc Reviewed-by: Oswald Buddenhagen Reviewed-by: Joerg Bornemann --- src/lib/corelib/buildgraph/executor.cpp | 21 ++++++----- .../changed-rule-inputs/changed-rule-inputs.qbs | 42 ++++++++++++++++++++++ tests/auto/blackbox/tst_blackbox.cpp | 29 +++++++++++++++ tests/auto/blackbox/tst_blackbox.h | 1 + 4 files changed, 84 insertions(+), 9 deletions(-) create mode 100644 tests/auto/blackbox/testdata/changed-rule-inputs/changed-rule-inputs.qbs diff --git a/src/lib/corelib/buildgraph/executor.cpp b/src/lib/corelib/buildgraph/executor.cpp index 24f484ad2..8713b9b41 100644 --- a/src/lib/corelib/buildgraph/executor.cpp +++ b/src/lib/corelib/buildgraph/executor.cpp @@ -771,8 +771,7 @@ void Executor::rescueOldBuildData(Artifact *artifact, bool *childrenAdded = 0) return; qCDebug(lcBuildGraph) << "Attempting to rescue data of artifact" << artifact->fileName(); - typedef std::pair ChildArtifactData; - QList childrenToConnect; + std::vector childrenToConnect; bool canRescue = artifact->transformer->commands == rad.commands; if (canRescue) { ResolvedProductPtr pseudoProduct = ResolvedProduct::create(); @@ -796,10 +795,15 @@ void Executor::rescueOldBuildData(Artifact *artifact, bool *childrenAdded = 0) removeGeneratedArtifactFromDisk(cd.childFilePath, m_logger); } } - // TODO: Shouldn't addedByScanner always be true here? Otherwise the child would be - // in the list already, no? + if (!cd.addedByScanner) { + // If an artifact has disappeared from the list of children, the commands + // might need to run again. + canRescue = false; + qCDebug(lcBuildGraph) << "Former child artifact" << cd.childFilePath << + "is no longer in the list of children"; + } if (canRescue) - childrenToConnect.push_back({child, cd.addedByScanner}); + childrenToConnect.push_back(child); } for (const QString &depPath : rad.fileDependencies) { const QList depList = m_project->buildData->lookupFiles(depPath); @@ -865,10 +869,9 @@ void Executor::rescueOldBuildData(Artifact *artifact, bool *childrenAdded = 0) artifact->knownOutOfDate = artifact->knownOutOfDate || rad.knownOutOfDate; if (childrenAdded && !childrenToConnect.empty()) *childrenAdded = true; - for (const ChildArtifactData &cad : qAsConst(childrenToConnect)) { - safeConnect(artifact, cad.first); - if (cad.second) - artifact->childrenAddedByScanner << cad.first; + for (Artifact * const child : childrenToConnect) { + safeConnect(artifact, child); + artifact->childrenAddedByScanner << child; } qCDebug(lcBuildGraph) << "Data was rescued."; } else { diff --git a/tests/auto/blackbox/testdata/changed-rule-inputs/changed-rule-inputs.qbs b/tests/auto/blackbox/testdata/changed-rule-inputs/changed-rule-inputs.qbs new file mode 100644 index 000000000..8aef7b9b6 --- /dev/null +++ b/tests/auto/blackbox/testdata/changed-rule-inputs/changed-rule-inputs.qbs @@ -0,0 +1,42 @@ +import qbs + +Project { + Product { + name: "p1" + type: "p1" + Rule { + alwaysRun: true + multiplex: true + Artifact { + fileTags: "p1" + filePath: "p1-dummy" + } + prepare: { + var cmd = new JavaScriptCommand(); + cmd.description = "generating " + output.fileName; + cmd.sourceCode = function() {}; + return cmd; + } + } + } + Product { + name: "p2" + type: "p2" + Depends { name: "p1" } + Rule { + requiresInputs: false + multiplex: true + inputsFromDependencies: "p1" + Artifact { + fileTags: "p2" + filePath: "p2-dummy" + } + prepare: { + var cmd = new JavaScriptCommand(); + cmd.description = "generating " + output.fileName; + cmd.sourceCode = function() {}; + return cmd; + } + } + } +} diff --git a/tests/auto/blackbox/tst_blackbox.cpp b/tests/auto/blackbox/tst_blackbox.cpp index 0c3f8b1bc..6987ae662 100644 --- a/tests/auto/blackbox/tst_blackbox.cpp +++ b/tests/auto/blackbox/tst_blackbox.cpp @@ -707,6 +707,35 @@ void TestBlackbox::changedFiles() QVERIFY2(m_qbsStdout.contains("file1.cpp"), m_qbsStdout.constData()); } +void TestBlackbox::changedRuleInputs() +{ + QDir::setCurrent(testDataDir + "/changed-rule-inputs"); + + // Initial build. + QCOMPARE(runQbs(), 0); + QVERIFY2(m_qbsStdout.contains("generating p1-dummy"), m_qbsStdout.constData()); + QVERIFY2(m_qbsStdout.contains("generating p2-dummy"), m_qbsStdout.constData()); + + // Re-build: p1 is always regenerated, and p2 has a dependency on it. + QCOMPARE(runQbs(), 0); + QVERIFY2(m_qbsStdout.contains("generating p1-dummy"), m_qbsStdout.constData()); + QVERIFY2(m_qbsStdout.contains("generating p2-dummy"), m_qbsStdout.constData()); + + // Remove the dependency. p2 gets re-generated one last time, because its set of + // inputs changed. + WAIT_FOR_NEW_TIMESTAMP(); + REPLACE_IN_FILE("changed-rule-inputs.qbs", "inputsFromDependencies: \"p1\"", + "inputsFromDependencies: \"p3\""); + QCOMPARE(runQbs(), 0); + QVERIFY2(m_qbsStdout.contains("generating p1-dummy"), m_qbsStdout.constData()); + QVERIFY2(m_qbsStdout.contains("generating p2-dummy"), m_qbsStdout.constData()); + + // Now the artifacts are no longer connected, and p2 must not get rebuilt anymore. + QCOMPARE(runQbs(), 0); + QVERIFY2(m_qbsStdout.contains("generating p1-dummy"), m_qbsStdout.constData()); + QVERIFY2(!m_qbsStdout.contains("generating p2-dummy"), m_qbsStdout.constData()); +} + void TestBlackbox::changeInDisabledProduct() { QDir::setCurrent(testDataDir + "/change-in-disabled-product"); diff --git a/tests/auto/blackbox/tst_blackbox.h b/tests/auto/blackbox/tst_blackbox.h index 70a950a84..35ba2c5b7 100644 --- a/tests/auto/blackbox/tst_blackbox.h +++ b/tests/auto/blackbox/tst_blackbox.h @@ -57,6 +57,7 @@ private slots: void buildGraphVersions(); void changedFiles_data(); void changedFiles(); + void changedRuleInputs(); void changeInDisabledProduct(); void changeInImportedFile(); void changeTrackingAndMultiplexing(); -- cgit v1.2.3