From d8c000b6c01787205bd11a475a7b9b0e168451b0 Mon Sep 17 00:00:00 2001 From: Christian Kandeler Date: Fri, 10 Aug 2018 13:26:53 +0200 Subject: Fix potential inconsistency in Artifact data If safeConnect() returns false, we must not add that "child" to the list of children added by scanners. Change-Id: Icbb1b2d14d99f002e2370ee2bda25daafb0398b2 Reviewed-by: Joerg Bornemann --- src/lib/corelib/buildgraph/executor.cpp | 4 ++-- src/lib/corelib/buildgraph/inputartifactscanner.cpp | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) (limited to 'src') diff --git a/src/lib/corelib/buildgraph/executor.cpp b/src/lib/corelib/buildgraph/executor.cpp index e18e9f699..d2c245c8a 100644 --- a/src/lib/corelib/buildgraph/executor.cpp +++ b/src/lib/corelib/buildgraph/executor.cpp @@ -872,8 +872,8 @@ void Executor::rescueOldBuildData(Artifact *artifact, bool *childrenAdded = 0) if (childrenAdded && !childrenToConnect.empty()) *childrenAdded = true; for (Artifact * const child : childrenToConnect) { - safeConnect(artifact, child); - artifact->childrenAddedByScanner << child; + if (safeConnect(artifact, child)) + artifact->childrenAddedByScanner << child; } qCDebug(lcBuildGraph) << "Data was rescued."; } else { diff --git a/src/lib/corelib/buildgraph/inputartifactscanner.cpp b/src/lib/corelib/buildgraph/inputartifactscanner.cpp index d183b1879..e96f6fdde 100644 --- a/src/lib/corelib/buildgraph/inputartifactscanner.cpp +++ b/src/lib/corelib/buildgraph/inputartifactscanner.cpp @@ -344,8 +344,8 @@ void InputArtifactScanner::handleDependency(ResolvedDependency &dependency) } else { if (m_artifact->children.contains(artifactDependency)) return; - safeConnect(m_artifact, artifactDependency); - m_artifact->childrenAddedByScanner += artifactDependency; + if (safeConnect(m_artifact, artifactDependency)) + m_artifact->childrenAddedByScanner += artifactDependency; m_newDependencyAdded = true; } } -- cgit v1.2.3 From 89689cac88ffe880942c7acb59fc3374a34cbd6c Mon Sep 17 00:00:00 2001 From: Christian Kandeler Date: Wed, 29 Aug 2018 18:02:10 +0200 Subject: Store product names in ExportedModule ... rather than product pointers. ExportedModule objects can be stored in Transformers, which potentially outlive the referenced products. Alternatively, we could update the product pointers during change tracking, but that would be tedious and error-prone. [ChangeLog] Fixed possible crash on storing a build graph after re- resolving. Change-Id: I09bcf638a17da410198524858eb4c1bda59bebcb Reviewed-by: Christian Stenger --- src/lib/corelib/buildgraph/buildgraph.cpp | 22 +++++++++++++++++++--- src/lib/corelib/language/language.cpp | 8 +------- src/lib/corelib/language/language.h | 2 +- src/lib/corelib/language/projectresolver.cpp | 13 ++++++------- src/lib/corelib/tools/persistence.cpp | 2 +- 5 files changed, 28 insertions(+), 19 deletions(-) (limited to 'src') diff --git a/src/lib/corelib/buildgraph/buildgraph.cpp b/src/lib/corelib/buildgraph/buildgraph.cpp index f12c27ab6..e9767b942 100644 --- a/src/lib/corelib/buildgraph/buildgraph.cpp +++ b/src/lib/corelib/buildgraph/buildgraph.cpp @@ -222,9 +222,25 @@ private: QScriptValue result = engine->newArray(); quint32 idx = 0; const bool exportCase = depType == DependencyType::Exported; - const std::vector &productDeps = (exportCase - ? product->exportedModule.productDependencies - : product->dependencies); + std::vector productDeps; + if (exportCase) { + if (!product->exportedModule.productDependencies.empty()) { + const auto allProducts = product->topLevelProject()->allProducts(); + const auto getProductForName = [&allProducts](const QString &name) { + const auto cmp = [name](const ResolvedProductConstPtr &p) { + return p->uniqueName() == name; + }; + const auto it = std::find_if(allProducts.cbegin(), allProducts.cend(), cmp); + QBS_ASSERT(it != allProducts.cend(), return ResolvedProductPtr()); + return *it; + }; + std::transform(product->exportedModule.productDependencies.cbegin(), + product->exportedModule.productDependencies.cend(), + std::back_inserter(productDeps), getProductForName); + } + } else { + productDeps = product->dependencies; + } for (const ResolvedProductPtr &dependency : qAsConst(productDeps)) { QScriptValue obj = engine->newObject(engine->productPropertyScriptClass()); obj.setPrototype(setupProductScriptValue(static_cast(engine), diff --git a/src/lib/corelib/language/language.cpp b/src/lib/corelib/language/language.cpp index 56c310272..5351ba80c 100644 --- a/src/lib/corelib/language/language.cpp +++ b/src/lib/corelib/language/language.cpp @@ -979,11 +979,6 @@ bool operator==(const ExportedItem &i1, const ExportedItem &i2) bool operator==(const ExportedModule &m1, const ExportedModule &m2) { - static const auto cmpProductsByName = []( - const ResolvedProductConstPtr &p1, - const ResolvedProductConstPtr &p2) { - return p1->name == p2->name; - }; static const auto depMapsEqual = [](const QMap &m1, const QMap &m2) { if (m1.size() != m2.size()) @@ -1003,8 +998,7 @@ bool operator==(const ExportedModule &m1, const ExportedModule &m2) && m1.m_properties == m2.m_properties && m1.importStatements == m2.importStatements && m1.productDependencies.size() == m2.productDependencies.size() - && std::equal(m1.productDependencies.cbegin(), m1.productDependencies.cend(), - m2.productDependencies.cbegin(), cmpProductsByName) + && m1.productDependencies == m2.productDependencies && depMapsEqual(m1.dependencyParameters, m2.dependencyParameters); } diff --git a/src/lib/corelib/language/language.h b/src/lib/corelib/language/language.h index 05fcfcf46..571681241 100644 --- a/src/lib/corelib/language/language.h +++ b/src/lib/corelib/language/language.h @@ -540,7 +540,7 @@ public: QVariantMap propertyValues; QVariantMap modulePropertyValues; std::vector children; - std::vector productDependencies; + std::vector productDependencies; std::vector moduleDependencies; std::vector m_properties; QMap dependencyParameters; diff --git a/src/lib/corelib/language/projectresolver.cpp b/src/lib/corelib/language/projectresolver.cpp index 348858f2b..97015dae4 100644 --- a/src/lib/corelib/language/projectresolver.cpp +++ b/src/lib/corelib/language/projectresolver.cpp @@ -911,19 +911,18 @@ void ProjectResolver::collectExportedProductDependencies() if (!contains(directDepNames, dep.product->name)) continue; - if (!contains(exportingProduct->exportedModule.productDependencies, dep.product)) - exportingProduct->exportedModule.productDependencies.push_back(dep.product); + if (!contains(exportingProduct->exportedModule.productDependencies, + dep.product->uniqueName())) { + exportingProduct->exportedModule.productDependencies.push_back( + dep.product->uniqueName()); + } if (!dep.parameters.isEmpty()) { exportingProduct->exportedModule.dependencyParameters.insert(dep.product, dep.parameters); } } auto &productDeps = exportingProduct->exportedModule.productDependencies; - static const auto cmpFunc = [](const ResolvedProductConstPtr &p1, - const ResolvedProductConstPtr &p2) { - return p1->uniqueName() < p2->uniqueName(); - }; - std::sort(productDeps.begin(), productDeps.end(), cmpFunc); + std::sort(productDeps.begin(), productDeps.end()); } } diff --git a/src/lib/corelib/tools/persistence.cpp b/src/lib/corelib/tools/persistence.cpp index e705cb2da..996c8415d 100644 --- a/src/lib/corelib/tools/persistence.cpp +++ b/src/lib/corelib/tools/persistence.cpp @@ -49,7 +49,7 @@ namespace qbs { namespace Internal { -static const char QBS_PERSISTENCE_MAGIC[] = "QBSPERSISTENCE-119"; +static const char QBS_PERSISTENCE_MAGIC[] = "QBSPERSISTENCE-120"; NoBuildGraphError::NoBuildGraphError(const QString &filePath) : ErrorInfo(Tr::tr("Build graph not found for configuration '%1'. Expected location was '%2'.") -- cgit v1.2.3