From 7aff31171eb3e756fc6ac6ff4bb4db6dcc96e42a Mon Sep 17 00:00:00 2001 From: Christian Kandeler Date: Mon, 15 Jul 2013 10:50:15 +0200 Subject: Handle "indirect" changes to a product's file list. If the list of source files in a product changed without the respective file having been touched (e.g. because the list was dependent on an environment variable), we re-resolved the product, but we did not touch the build data, so the new file was not compiled. Change-Id: Ib63020339aa0bbc2a858ed22f938ab76fb0611c5 Reviewed-by: Joerg Bornemann --- src/lib/buildgraph/buildgraphloader.cpp | 66 ++++++++++++---------- src/lib/buildgraph/buildgraphloader.h | 6 +- .../environmentChange.cpp | 1 + .../trackExternalProductChanges/fileList.js | 1 + .../trackExternalProductChanges/jsFileChange.cpp | 1 + .../testdata/trackExternalProductChanges/main.cpp | 1 + .../trackExternalProductChanges/project.qbs | 6 ++ tests/auto/blackbox/tst_blackbox.cpp | 35 ++++++++++++ tests/auto/blackbox/tst_blackbox.h | 1 + 9 files changed, 84 insertions(+), 34 deletions(-) create mode 100644 tests/auto/blackbox/testdata/trackExternalProductChanges/environmentChange.cpp create mode 100644 tests/auto/blackbox/testdata/trackExternalProductChanges/fileList.js create mode 100644 tests/auto/blackbox/testdata/trackExternalProductChanges/jsFileChange.cpp create mode 100644 tests/auto/blackbox/testdata/trackExternalProductChanges/main.cpp create mode 100644 tests/auto/blackbox/testdata/trackExternalProductChanges/project.qbs diff --git a/src/lib/buildgraph/buildgraphloader.cpp b/src/lib/buildgraph/buildgraphloader.cpp index 943944858..4d62e55ae 100644 --- a/src/lib/buildgraph/buildgraphloader.cpp +++ b/src/lib/buildgraph/buildgraphloader.cpp @@ -168,19 +168,21 @@ void BuildGraphLoader::trackProjectChanges(const SetupProjectParameters ¶met { const FileTime buildGraphTimeStamp = FileInfo(buildGraphFilePath).lastModified(); QSet buildSystemFiles = restoredProject->buildSystemFiles; - const QList allRestoredProjects - = restoredProject->allSubProjects() << restoredProject; - - bool reResolvingNecessary = hasProjectFileChanged(allRestoredProjects, buildGraphTimeStamp, - buildSystemFiles); - if (!reResolvingNecessary) - reResolvingNecessary = hasEnvironmentChanged(restoredProject); QList allRestoredProducts = restoredProject->allProducts(); QList changedProducts; - reResolvingNecessary |= hasProductFileChanged(allRestoredProducts, buildGraphTimeStamp, - buildSystemFiles, changedProducts); - if (!reResolvingNecessary) - reResolvingNecessary = hasBuildSystemFileChanged(buildSystemFiles, buildGraphTimeStamp); + bool reResolvingNecessary = hasProductFileChanged(allRestoredProducts, buildGraphTimeStamp, + buildSystemFiles, changedProducts); + + // "External" changes, e.g. in the environment or in a JavaScript file, + // can make the list of source files in a product change without the respective file + // having been touched. In such a case, the build data for that product will have to be set up + // anew. + bool allProductsPotentiallyChanged = false; + if (hasBuildSystemFileChanged(buildSystemFiles, buildGraphTimeStamp) + || hasEnvironmentChanged(restoredProject)) { + reResolvingNecessary = true; + allProductsPotentiallyChanged = true; + } if (!reResolvingNecessary) return; @@ -215,6 +217,8 @@ void BuildGraphLoader::trackProjectChanges(const SetupProjectParameters ¶met } } + if (allProductsPotentiallyChanged) + checkAllProductsForChanges(allRestoredProducts, freshProductsByName, changedProducts); foreach (const ResolvedProductPtr &product, changedProducts) { ResolvedProductPtr freshProduct = freshProductsByName.value(product->name); if (!freshProduct) @@ -281,26 +285,6 @@ bool BuildGraphLoader::hasEnvironmentChanged(const TopLevelProjectConstPtr &rest return false; } -bool BuildGraphLoader::hasProjectFileChanged(const QList &restoredProjects, - const FileTime &referenceTime, - QSet &remainingBuildSystemFiles) const -{ - foreach (const ResolvedProjectConstPtr &p, restoredProjects) { - const QString fileName = p->location.fileName(); - const FileInfo fi(fileName); - remainingBuildSystemFiles.remove(fileName); - if (!fi.exists()) { - m_logger.qbsDebug() << "A sub-project was removed, must re-resolve project"; - return true; - } - if (referenceTime < fi.lastModified()) { - m_logger.qbsDebug() << "A project file changed, must re-resolve project."; - return true; - } - } - return false; -} - bool BuildGraphLoader::hasProductFileChanged(const QList &restoredProducts, const FileTime &referenceTime, QSet &remainingBuildSystemFiles, QList &changedProducts) @@ -314,6 +298,7 @@ bool BuildGraphLoader::hasProductFileChanged(const QList &re m_logger.qbsDebug() << "A product was removed, must re-resolve project"; hasChanged = true; } else if (referenceTime < pfi.lastModified()) { + m_logger.qbsDebug() << "A product was changed, must re-resolve project"; changedProducts += product; } else { foreach (const GroupPtr &group, product->groups) { @@ -348,6 +333,25 @@ bool BuildGraphLoader::hasBuildSystemFileChanged(const QSet &buildSyste return false; } +void BuildGraphLoader::checkAllProductsForChanges(const QList &restoredProducts, + const QMap &newlyResolvedProductsByName, + QList &changedProducts) +{ + foreach (const ResolvedProductPtr &restoredProduct, restoredProducts) { + if (changedProducts.contains(restoredProduct)) + continue; + const ResolvedProductPtr newlyResolvedProduct + = newlyResolvedProductsByName.value(restoredProduct->name); + if (!newlyResolvedProduct) + continue; + if (newlyResolvedProduct->allFiles() == restoredProduct->allFiles()) + continue; + m_logger.qbsDebug() << "Product '" << restoredProduct->name + << "' was changed, must re-resolve project"; + changedProducts << restoredProduct; + } +} + void BuildGraphLoader::onProductRemoved(const ResolvedProductPtr &product, ProjectBuildData *projectBuildData) { diff --git a/src/lib/buildgraph/buildgraphloader.h b/src/lib/buildgraph/buildgraphloader.h index cd769583b..8cbe5e354 100644 --- a/src/lib/buildgraph/buildgraphloader.h +++ b/src/lib/buildgraph/buildgraphloader.h @@ -67,15 +67,15 @@ private: const QString &buildGraphFilePath, const TopLevelProjectPtr &restoredProject); bool hasEnvironmentChanged(const TopLevelProjectConstPtr &restoredProject) const; - bool hasProjectFileChanged(const QList &restoredProjects, - const FileTime &referenceTime, - QSet &remainingBuildSystemFiles) const; bool hasProductFileChanged(const QList &restoredProducts, const FileTime &referenceTime, QSet &remainingBuildSystemFiles, QList &changedProducts); bool hasBuildSystemFileChanged(const QSet &buildSystemFiles, const FileTime &referenceTime); + void checkAllProductsForChanges(const QList &restoredProducts, + const QMap &newlyResolvedProductsByName, + QList &changedProducts); void onProductRemoved(const ResolvedProductPtr &product, ProjectBuildData *projectBuildData); void onProductChanged(const ResolvedProductPtr &product, const ResolvedProductPtr &changedProduct); diff --git a/tests/auto/blackbox/testdata/trackExternalProductChanges/environmentChange.cpp b/tests/auto/blackbox/testdata/trackExternalProductChanges/environmentChange.cpp new file mode 100644 index 000000000..dd3c37880 --- /dev/null +++ b/tests/auto/blackbox/testdata/trackExternalProductChanges/environmentChange.cpp @@ -0,0 +1 @@ +void environmentChange() { } diff --git a/tests/auto/blackbox/testdata/trackExternalProductChanges/fileList.js b/tests/auto/blackbox/testdata/trackExternalProductChanges/fileList.js new file mode 100644 index 000000000..0d540d82e --- /dev/null +++ b/tests/auto/blackbox/testdata/trackExternalProductChanges/fileList.js @@ -0,0 +1 @@ +function fileList() { return []; } diff --git a/tests/auto/blackbox/testdata/trackExternalProductChanges/jsFileChange.cpp b/tests/auto/blackbox/testdata/trackExternalProductChanges/jsFileChange.cpp new file mode 100644 index 000000000..a23a409ae --- /dev/null +++ b/tests/auto/blackbox/testdata/trackExternalProductChanges/jsFileChange.cpp @@ -0,0 +1 @@ +void jsFileChange() { } diff --git a/tests/auto/blackbox/testdata/trackExternalProductChanges/main.cpp b/tests/auto/blackbox/testdata/trackExternalProductChanges/main.cpp new file mode 100644 index 000000000..8b8d58de0 --- /dev/null +++ b/tests/auto/blackbox/testdata/trackExternalProductChanges/main.cpp @@ -0,0 +1 @@ +int main() { } diff --git a/tests/auto/blackbox/testdata/trackExternalProductChanges/project.qbs b/tests/auto/blackbox/testdata/trackExternalProductChanges/project.qbs new file mode 100644 index 000000000..cfd897447 --- /dev/null +++ b/tests/auto/blackbox/testdata/trackExternalProductChanges/project.qbs @@ -0,0 +1,6 @@ +import qbs +import "fileList.js" as FileList + +CppApplication { + files: ["main.cpp"].concat(FileList.fileList()).concat(qbs.getenv("QBS_TEST_PULL_IN_FILE_VIA_ENV") ? ["environmentChange.cpp"] : []) +} diff --git a/tests/auto/blackbox/tst_blackbox.cpp b/tests/auto/blackbox/tst_blackbox.cpp index dd9719b09..8d2f6dbdb 100644 --- a/tests/auto/blackbox/tst_blackbox.cpp +++ b/tests/auto/blackbox/tst_blackbox.cpp @@ -589,6 +589,41 @@ void TestBlackbox::trackAddFile() QCOMPARE(unchangedObjectFileTime1, unchangedObjectFileTime2); } +void TestBlackbox::trackExternalProductChanges() +{ + QDir::setCurrent(testDataDir + "/trackExternalProductChanges"); + QCOMPARE(runQbs(), 0); + QVERIFY(m_qbsStdout.contains("compiling main.cpp")); + QVERIFY(!m_qbsStdout.contains("compiling environmentChange.cpp")); + QVERIFY(!m_qbsStdout.contains("compiling jsFileChange.cpp")); + + QbsRunParameters params; + params.environment.insert("QBS_TEST_PULL_IN_FILE_VIA_ENV", "1"); + QCOMPARE(runQbs(params), 0); + QVERIFY(!m_qbsStdout.contains("compiling main.cpp")); + QVERIFY(m_qbsStdout.contains("compiling environmentChange.cpp")); + QVERIFY(!m_qbsStdout.contains("compiling jsFileChange.cpp")); + + rmDirR(buildDir); + QCOMPARE(runQbs(), 0); + QVERIFY(m_qbsStdout.contains("compiling main.cpp")); + QVERIFY(!m_qbsStdout.contains("compiling environmentChange.cpp")); + QVERIFY(!m_qbsStdout.contains("compiling jsFileChange.cpp")); + + waitForNewTimestamp(); + QFile jsFile("fileList.js"); + QVERIFY(jsFile.open(QIODevice::ReadWrite)); + QByteArray jsCode = jsFile.readAll(); + jsCode.replace("[]", "['jsFileChange.cpp']"); + jsFile.resize(0); + jsFile.write(jsCode); + jsFile.close(); + QCOMPARE(runQbs(), 0); + QVERIFY(!m_qbsStdout.contains("compiling main.cpp")); + QVERIFY(!m_qbsStdout.contains("compiling environmentChange.cpp")); + QVERIFY(m_qbsStdout.contains("compiling jsFileChange.cpp")); +} + void TestBlackbox::trackRemoveFile() { QProcess process; diff --git a/tests/auto/blackbox/tst_blackbox.h b/tests/auto/blackbox/tst_blackbox.h index b9d19da96..153e0aae6 100644 --- a/tests/auto/blackbox/tst_blackbox.h +++ b/tests/auto/blackbox/tst_blackbox.h @@ -111,6 +111,7 @@ private slots: void track_qrc(); void track_qobject_change(); void trackAddFile(); + void trackExternalProductChanges(); void trackRemoveFile(); void trackAddFileTag(); void trackRemoveFileTag(); -- cgit v1.2.1