From bf2a579903d44b8d856c5199044a96e803a79e1d Mon Sep 17 00:00:00 2001 From: Christian Kandeler Date: Mon, 8 Jul 2013 13:21:56 +0200 Subject: Make sure no remains of removed products stay in the build graph. Namely, artifacts in transformer inputs of (former) parents. Task-number: QBS-358 Change-Id: I19e6cf6cd50e4d99c49e2e570daf163da25a3a79 Reviewed-by: Joerg Bornemann --- src/lib/buildgraph/artifact.cpp | 32 +------------ src/lib/buildgraph/artifact.h | 4 -- src/lib/buildgraph/buildgraphloader.cpp | 11 ++--- src/lib/buildgraph/buildgraphloader.h | 2 +- src/lib/buildgraph/projectbuilddata.cpp | 56 +++++++++++++++++----- src/lib/buildgraph/projectbuilddata.h | 5 +- tests/auto/blackbox/testdata/renameProduct/lib.cpp | 3 ++ .../auto/blackbox/testdata/renameProduct/main.cpp | 6 +++ .../blackbox/testdata/renameProduct/rename.qbs | 17 +++++++ tests/auto/blackbox/tst_blackbox.cpp | 33 +++++++++++++ tests/auto/blackbox/tst_blackbox.h | 1 + 11 files changed, 114 insertions(+), 56 deletions(-) create mode 100644 tests/auto/blackbox/testdata/renameProduct/lib.cpp create mode 100644 tests/auto/blackbox/testdata/renameProduct/main.cpp create mode 100644 tests/auto/blackbox/testdata/renameProduct/rename.qbs diff --git a/src/lib/buildgraph/artifact.cpp b/src/lib/buildgraph/artifact.cpp index 9923ede18..75a284a45 100644 --- a/src/lib/buildgraph/artifact.cpp +++ b/src/lib/buildgraph/artifact.cpp @@ -28,12 +28,10 @@ ****************************************************************************/ #include "artifact.h" + #include "transformer.h" -#include "buildgraph.h" -#include #include -#include #include #include @@ -115,33 +113,5 @@ void Artifact::store(PersistentPool &pool) const << static_cast(alwaysUpdated); } -void Artifact::disconnectChildren(const Logger &logger) -{ - if (logger.traceEnabled()) { - logger.qbsTrace() << QString::fromLocal8Bit("[BG] disconnectChildren: '%1'") - .arg(relativeArtifactFileName(this)); - } - foreach (Artifact * const child, children) - child->parents.remove(this); - children.clear(); -} - -void Artifact::disconnectParents(const Logger &logger) -{ - if (logger.traceEnabled()) { - logger.qbsTrace() << QString::fromLocal8Bit("[BG] disconnectParents: '%1'") - .arg(relativeArtifactFileName(this)); - } - foreach (Artifact * const parent, parents) - parent->children.remove(this); - parents.clear(); -} - -void Artifact::disconnectAll(const Logger &logger) -{ - disconnectChildren(logger); - disconnectParents(logger); -} - } // namespace Internal } // namespace qbs diff --git a/src/lib/buildgraph/artifact.h b/src/lib/buildgraph/artifact.h index bc2a8025d..ce0aff81a 100644 --- a/src/lib/buildgraph/artifact.h +++ b/src/lib/buildgraph/artifact.h @@ -101,15 +101,11 @@ public: const QString &filePath() const { return m_filePath; } QString dirPath() const { return m_dirPath.toString(); } QString fileName() const { return m_fileName.toString(); } - void disconnectAll(const Logger &logger); private: void load(PersistentPool &pool); void store(PersistentPool &pool) const; - void disconnectChildren(const Logger &logger); - void disconnectParents(const Logger &logger); - private: QString m_filePath; QStringRef m_dirPath; diff --git a/src/lib/buildgraph/buildgraphloader.cpp b/src/lib/buildgraph/buildgraphloader.cpp index afb0ac090..ce04d91cc 100644 --- a/src/lib/buildgraph/buildgraphloader.cpp +++ b/src/lib/buildgraph/buildgraphloader.cpp @@ -324,12 +324,13 @@ void BuildGraphLoader::trackProjectChanges(const SetupProjectParameters ¶met // Products still left in the list do not exist anymore. foreach (const ResolvedProductPtr &removedProduct, allRestoredProducts) - onProductRemoved(removedProduct); + onProductRemoved(removedProduct, m_result.newlyResolvedProject->buildData.data()); CycleDetector(m_logger).visitProject(m_result.newlyResolvedProject); } -void BuildGraphLoader::onProductRemoved(const ResolvedProductPtr &product) +void BuildGraphLoader::onProductRemoved(const ResolvedProductPtr &product, + ProjectBuildData *projectBuildData) { m_logger.qbsDebug() << "[BG] product '" << product->name << "' removed."; @@ -337,10 +338,8 @@ void BuildGraphLoader::onProductRemoved(const ResolvedProductPtr &product) // delete all removed artifacts physically from the disk if (product->buildData) { - foreach (Artifact *artifact, product->buildData->artifacts) { - artifact->disconnectAll(m_logger); - removeGeneratedArtifactFromDisk(artifact, m_logger); - } + foreach (Artifact *artifact, product->buildData->artifacts) + projectBuildData->removeArtifact(artifact, projectBuildData, m_logger); } } diff --git a/src/lib/buildgraph/buildgraphloader.h b/src/lib/buildgraph/buildgraphloader.h index 6bc0b625b..999d1f205 100644 --- a/src/lib/buildgraph/buildgraphloader.h +++ b/src/lib/buildgraph/buildgraphloader.h @@ -65,7 +65,7 @@ private: void trackProjectChanges(const SetupProjectParameters ¶meters, const QString &buildGraphFilePath, const TopLevelProjectPtr &restoredProject); - void onProductRemoved(const ResolvedProductPtr &product); + void onProductRemoved(const ResolvedProductPtr &product, ProjectBuildData *projectBuildData); void onProductChanged(const ResolvedProductPtr &product, const ResolvedProductPtr &changedProduct); void removeArtifactAndExclusiveDependents(Artifact *artifact, diff --git a/src/lib/buildgraph/projectbuilddata.cpp b/src/lib/buildgraph/projectbuilddata.cpp index fbcbf4796..2c4e44935 100644 --- a/src/lib/buildgraph/projectbuilddata.cpp +++ b/src/lib/buildgraph/projectbuilddata.cpp @@ -107,8 +107,44 @@ static bool commandsEqual(const TransformerConstPtr &t1, const TransformerConstP return true; } +static void disconnectArtifactChildren(Artifact *artifact, const Logger &logger) +{ + if (logger.traceEnabled()) { + logger.qbsTrace() << QString::fromLocal8Bit("[BG] disconnectChildren: '%1'") + .arg(relativeArtifactFileName(artifact)); + } + foreach (Artifact * const child, artifact->children) + child->parents.remove(artifact); + artifact->children.clear(); +} -void ProjectBuildData::removeArtifact(Artifact *artifact, const Logger &logger) +static void disconnectArtifactParents(Artifact *artifact, ProjectBuildData *projectBuildData, + const Logger &logger) +{ + if (logger.traceEnabled()) { + logger.qbsTrace() << QString::fromLocal8Bit("[BG] disconnectParents: '%1'") + .arg(relativeArtifactFileName(artifact)); + } + foreach (Artifact * const parent, artifact->parents) { + parent->children.remove(artifact); + if (parent->transformer) { + parent->transformer->inputs.remove(artifact); + projectBuildData->artifactsThatMustGetNewTransformers += parent; + } + } + + artifact->parents.clear(); +} + +static void disconnectArtifact(Artifact *artifact, ProjectBuildData *projectBuildData, + const Logger &logger) +{ + disconnectArtifactChildren(artifact, logger); + disconnectArtifactParents(artifact, projectBuildData, logger); +} + +void ProjectBuildData::removeArtifact(Artifact *artifact, ProjectBuildData *projectBuildData, + const Logger &logger) { if (logger.traceEnabled()) logger.qbsTrace() << "[BG] remove artifact " << relativeArtifactFileName(artifact); @@ -117,20 +153,16 @@ void ProjectBuildData::removeArtifact(Artifact *artifact, const Logger &logger) artifact->product->buildData->artifacts.remove(artifact); removeFromArtifactLookupTable(artifact); artifact->product->buildData->targetArtifacts.remove(artifact); - foreach (Artifact *parent, artifact->parents) { - parent->children.remove(artifact); - if (parent->transformer) { - parent->transformer->inputs.remove(artifact); - artifactsThatMustGetNewTransformers += parent; - } - } - foreach (Artifact *child, artifact->children) - child->parents.remove(artifact); - artifact->children.clear(); - artifact->parents.clear(); + disconnectArtifact(artifact, projectBuildData, logger); + projectBuildData->artifactsThatMustGetNewTransformers -= artifact; isDirty = true; } +void ProjectBuildData::removeArtifact(Artifact *artifact, const Logger &logger) +{ + removeArtifact(artifact, artifact->topLevelProject->buildData.data(), logger); +} + void ProjectBuildData::updateNodesThatMustGetNewTransformer(const Logger &logger) { RulesEvaluationContext::Scope s(evaluationContext.data()); diff --git a/src/lib/buildgraph/projectbuilddata.h b/src/lib/buildgraph/projectbuilddata.h index 334cf8e75..637889805 100644 --- a/src/lib/buildgraph/projectbuilddata.h +++ b/src/lib/buildgraph/projectbuilddata.h @@ -59,8 +59,10 @@ public: QList lookupArtifacts(const QString &dirPath, const QString &fileName) const; QList lookupArtifacts(const Artifact *artifact) const; void insertFileDependency(Artifact *artifact); - void removeArtifact(Artifact *artifact, const Logger &logger); void updateNodesThatMustGetNewTransformer(const Logger &logger); + void removeArtifact(Artifact *artifact, const Logger &logger); + void removeArtifact(Artifact *artifact, ProjectBuildData *projectBuildData, + const Logger &logger); ArtifactList dependencyArtifacts; RulesEvaluationContextPtr evaluationContext; @@ -72,7 +74,6 @@ private: void store(PersistentPool &pool) const; void updateNodeThatMustGetNewTransformer(Artifact *artifact, const Logger &logger); -private: QHash > > m_artifactLookupTable; }; diff --git a/tests/auto/blackbox/testdata/renameProduct/lib.cpp b/tests/auto/blackbox/testdata/renameProduct/lib.cpp new file mode 100644 index 000000000..47ce43bc9 --- /dev/null +++ b/tests/auto/blackbox/testdata/renameProduct/lib.cpp @@ -0,0 +1,3 @@ +#include + +MY_EXPORT void f() { } diff --git a/tests/auto/blackbox/testdata/renameProduct/main.cpp b/tests/auto/blackbox/testdata/renameProduct/main.cpp new file mode 100644 index 000000000..6a0bac9f1 --- /dev/null +++ b/tests/auto/blackbox/testdata/renameProduct/main.cpp @@ -0,0 +1,6 @@ +void f(); + +int main() +{ + f(); +} diff --git a/tests/auto/blackbox/testdata/renameProduct/rename.qbs b/tests/auto/blackbox/testdata/renameProduct/rename.qbs new file mode 100644 index 000000000..a2fe2e22f --- /dev/null +++ b/tests/auto/blackbox/testdata/renameProduct/rename.qbs @@ -0,0 +1,17 @@ +import qbs + +Project { + CppApplication { + Depends { name: "TheLib" } + cpp.defines: "MY_EXPORT=" + files: "main.cpp" + } + + DynamicLibrary { + name: "TheLib" + Depends { name: "cpp" } + Depends { name: "Qt.core" } + cpp.defines: "MY_EXPORT=Q_DECL_EXPORT" + files: "lib.cpp" + } +} diff --git a/tests/auto/blackbox/tst_blackbox.cpp b/tests/auto/blackbox/tst_blackbox.cpp index d576952e1..79ebb7ae1 100644 --- a/tests/auto/blackbox/tst_blackbox.cpp +++ b/tests/auto/blackbox/tst_blackbox.cpp @@ -408,6 +408,39 @@ void TestBlackbox::clean() QVERIFY(QFile(depExeFilePath).exists()); } +void TestBlackbox::renameProduct() +{ + QDir::setCurrent(testDataDir + "/renameProduct"); + + // Initial run. + QCOMPARE(runQbs(), 0); + + // Rename lib and adapt Depends item. + waitForNewTimestamp(); + QFile f("rename.qbs"); + QVERIFY(f.open(QIODevice::ReadWrite)); + QByteArray contents = f.readAll(); + contents.replace("TheLib", "thelib"); + f.resize(0); + f.write(contents); + f.close(); + QCOMPARE(runQbs(), 0); + + // Rename lib and don't adapt Depends item. + waitForNewTimestamp(); + QVERIFY(f.open(QIODevice::ReadWrite)); + contents = f.readAll(); + const int libNameIndex = contents.lastIndexOf("thelib"); + QVERIFY(libNameIndex != -1); + contents.replace(libNameIndex, 6, "TheLib"); + f.resize(0); + f.write(contents); + f.close(); + QbsRunParameters params; + params.expectFailure = true; + QVERIFY(runQbs(params) != 0); +} + void TestBlackbox::subProjects() { QDir::setCurrent(testDataDir + "/subprojects"); diff --git a/tests/auto/blackbox/tst_blackbox.h b/tests/auto/blackbox/tst_blackbox.h index 743edc649..f703a51c7 100644 --- a/tests/auto/blackbox/tst_blackbox.h +++ b/tests/auto/blackbox/tst_blackbox.h @@ -104,6 +104,7 @@ private slots: void resolve_project_dry_run_data(); void resolve_project_dry_run(); void clean(); + void renameProduct(); void subProjects(); void track_qrc(); void track_qobject_change(); -- cgit v1.2.1