From d4540126185efe4a8017280a0e1d1481a33c44cc Mon Sep 17 00:00:00 2001 From: Christian Kandeler Date: Tue, 29 Nov 2016 16:34:45 +0100 Subject: 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 Reviewed-by: Joerg Bornemann --- src/lib/corelib/buildgraph/buildgraphloader.cpp | 31 ++++++++++++++++++---- src/lib/corelib/buildgraph/executor.cpp | 2 ++ src/lib/corelib/buildgraph/jscommandexecutor.cpp | 2 ++ .../corelib/buildgraph/rescuableartifactdata.cpp | 2 ++ src/lib/corelib/buildgraph/rescuableartifactdata.h | 1 + src/lib/corelib/buildgraph/transformer.cpp | 2 ++ src/lib/corelib/buildgraph/transformer.h | 1 + src/lib/corelib/tools/persistence.cpp | 2 +- .../propertyChanges/modules/TestModule/module.qbs | 7 +++-- .../blackbox/testdata/propertyChanges/project.qbs | 1 + .../blackbox/testdata/propertyChanges/ruletest.qbs | 5 +++- tests/auto/blackbox/tst_blackbox.cpp | 19 +++++++++++++ 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 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 artifactMap; for (QHash::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 propertiesRequestedFromArtifactInPrepareScript; + QHash 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"); -- cgit v1.2.3