From 9077f10d46909259effab143dc3d0e448e19ee8a Mon Sep 17 00:00:00 2001 From: Christian Kandeler Date: Fri, 1 Sep 2017 12:47:02 +0200 Subject: ModuleLoader: Fix adjustment for multiplexed dependencies We erroneously gave the responsible function only a local project view instead of the global one, meaning dependencies could fail across subprojects. Change-Id: I18155df39cb981aed36e204ea2a85ed26a7eb6c7 Reviewed-by: Joerg Bornemann --- src/lib/corelib/language/moduleloader.cpp | 132 ++++++++------- src/lib/corelib/language/moduleloader.h | 4 +- .../api/testdata/multiplexing/multiplexing.qbs | 178 +++++++++++---------- tests/auto/api/tst_api.cpp | 2 +- 4 files changed, 169 insertions(+), 147 deletions(-) diff --git a/src/lib/corelib/language/moduleloader.cpp b/src/lib/corelib/language/moduleloader.cpp index e861f2dfd..232c91a2c 100755 --- a/src/lib/corelib/language/moduleloader.cpp +++ b/src/lib/corelib/language/moduleloader.cpp @@ -523,6 +523,8 @@ void ModuleLoader::handleTopLevelProject(ModuleLoaderResult *loadResult, Item *p TopLevelProjectContext tlp; tlp.buildDirectory = buildDirectory; handleProject(loadResult, &tlp, projectItem, referencedFilePaths); + collectProductsByName(tlp); + adjustDependenciesForMultiplexing(tlp); for (ProjectContext * const projectContext : qAsConst(tlp.projects)) { m_reader->setExtraSearchPathsStack(projectContext->searchPathsStack); @@ -661,7 +663,6 @@ void ModuleLoader::handleProject(ModuleLoaderResult *loadResult, break; } } - adjustDependenciesForMultiplexing(projectContext); m_reader->popExtraSearchPaths(); } @@ -858,76 +859,79 @@ QList ModuleLoader::multiplexProductItem(ProductContext *dummyContext, I return additionalProductItems; } -void ModuleLoader::adjustDependenciesForMultiplexing(const ProjectContext &projectContext) +void ModuleLoader::adjustDependenciesForMultiplexing(const TopLevelProjectContext &tlp) { - for (const ProductContext &product : projectContext.products) - m_productsByName.insert({ product.name, &product }); + for (const ProjectContext * const project : tlp.projects) { + for (const ProductContext &product : project->products) + adjustDependenciesForMultiplexing(product); + } +} - const QString multiplexConfigurationIdKey = QStringLiteral("multiplexConfigurationId"); - for (const ProductContext &product : projectContext.products) { - std::vector additionalDependsItems; - for (Item *dependsItem : product.item->children()) { - if (dependsItem->type() != ItemType::Depends) - continue; - const QString name = m_evaluator->stringValue(dependsItem, QStringLiteral("name")); - const bool productIsMultiplexed = !product.multiplexConfigurationId.isEmpty(); - if (name == product.name) { - QBS_CHECK(!productIsMultiplexed); // This product must be an aggregator. - continue; - } - const auto productRange = m_productsByName.equal_range(name); - std::vector dependencies; - bool hasNonMultiplexedDependency = false; - for (auto it = productRange.first; it != productRange.second; ++it) { - if (!it->second->multiplexConfigurationId.isEmpty()) { - dependencies.push_back(it->second); - if (productIsMultiplexed) - break; - } else { - hasNonMultiplexedDependency = true; +void ModuleLoader::adjustDependenciesForMultiplexing(const ModuleLoader::ProductContext &product) +{ + static const QString multiplexConfigurationIdKey = QStringLiteral("multiplexConfigurationId"); + std::vector additionalDependsItems; + for (Item *dependsItem : product.item->children()) { + if (dependsItem->type() != ItemType::Depends) + continue; + const QString name = m_evaluator->stringValue(dependsItem, QStringLiteral("name")); + const bool productIsMultiplexed = !product.multiplexConfigurationId.isEmpty(); + if (name == product.name) { + QBS_CHECK(!productIsMultiplexed); // This product must be an aggregator. + continue; + } + const auto productRange = m_productsByName.equal_range(name); + std::vector dependencies; + bool hasNonMultiplexedDependency = false; + for (auto it = productRange.first; it != productRange.second; ++it) { + if (!it->second->multiplexConfigurationId.isEmpty()) { + dependencies.push_back(it->second); + if (productIsMultiplexed) break; - } + } else { + hasNonMultiplexedDependency = true; + break; } + } - // These are the allowed cases: - // (1) Normal dependency with no multiplexing whatsoever. - // (2) Both product and dependency are multiplexed. - // (3) The product is not multiplexed, but the dependency is. - // (3a) The dependency has an aggregator. We want to depend on the aggregator. - // (3b) The dependency does not have an aggregator. We want to depend on all the - // multiplexed variants. - // (4) The product is multiplexed, but the dependency is not. This case is implicitly - // handled, because we don't have to adapt any Depends items. - - // (1) and (3a) - if (!productIsMultiplexed && hasNonMultiplexedDependency) - continue; + // These are the allowed cases: + // (1) Normal dependency with no multiplexing whatsoever. + // (2) Both product and dependency are multiplexed. + // (3) The product is not multiplexed, but the dependency is. + // (3a) The dependency has an aggregator. We want to depend on the aggregator. + // (3b) The dependency does not have an aggregator. We want to depend on all the + // multiplexed variants. + // (4) The product is multiplexed, but the dependency is not. This case is implicitly + // handled, because we don't have to adapt any Depends items. - for (std::size_t i = 0; i < dependencies.size(); ++i) { - const QString depMultiplexId = dependencies.at(i)->multiplexConfigurationId; - if (i == 0) { - if (productIsMultiplexed) { // (2) - dependsItem->setProperty(multiplexConfigurationIdKey, - product.item->property(multiplexConfigurationIdKey)); - break; - } - // (3b) + // (1) and (3a) + if (!productIsMultiplexed && hasNonMultiplexedDependency) + continue; + + for (std::size_t i = 0; i < dependencies.size(); ++i) { + const QString depMultiplexId = dependencies.at(i)->multiplexConfigurationId; + if (i == 0) { + if (productIsMultiplexed) { // (2) dependsItem->setProperty(multiplexConfigurationIdKey, - VariantValue::create(depMultiplexId)); - } else { - // (3b) - Item * const newDependsItem = dependsItem->clone(); - newDependsItem->setProperty(multiplexConfigurationIdKey, - VariantValue::create(depMultiplexId)); - dependsItem->setProperty(QStringLiteral("profiles"), - VariantValue::create(QStringLiteral("*"))); - additionalDependsItems.push_back(newDependsItem); + product.item->property(multiplexConfigurationIdKey)); + break; } + // (3b) + dependsItem->setProperty(multiplexConfigurationIdKey, + VariantValue::create(depMultiplexId)); + } else { + // (3b) + Item * const newDependsItem = dependsItem->clone(); + newDependsItem->setProperty(multiplexConfigurationIdKey, + VariantValue::create(depMultiplexId)); + dependsItem->setProperty(QStringLiteral("profiles"), + VariantValue::create(QStringLiteral("*"))); + additionalDependsItems.push_back(newDependsItem); } } - for (Item * const newDependsItem : additionalDependsItems) - Item::addChild(product.item, newDependsItem); } + for (Item * const newDependsItem : additionalDependsItems) + Item::addChild(product.item, newDependsItem); } void ModuleLoader::prepareProduct(ProjectContext *projectContext, Item *productItem) @@ -1611,6 +1615,14 @@ Item *ModuleLoader::loadItemFromFile(const QString &filePath) return item; } +void ModuleLoader::collectProductsByName(const TopLevelProjectContext &topLevelProject) +{ + for (ProjectContext * const project : topLevelProject.projects) { + for (ProductContext &product : project->products) + m_productsByName.insert({ product.name, &product }); + } +} + void ModuleLoader::propagateModulesFromParent(ProductContext *productContext, Item *groupItem, const ModuleDependencies &reverseDepencencies) { diff --git a/src/lib/corelib/language/moduleloader.h b/src/lib/corelib/language/moduleloader.h index 0e9445f1c..aeea34ad0 100644 --- a/src/lib/corelib/language/moduleloader.h +++ b/src/lib/corelib/language/moduleloader.h @@ -230,7 +230,8 @@ private: static MultiplexTable combine(const MultiplexTable &table, const MultiplexRow &values); MultiplexInfo extractMultiplexInfo(Item *productItem, Item *qbsModuleItem); QList multiplexProductItem(ProductContext *dummyContext, Item *productItem); - void adjustDependenciesForMultiplexing(const ProjectContext &projectContext); + void adjustDependenciesForMultiplexing(const TopLevelProjectContext &tlp); + void adjustDependenciesForMultiplexing(const ProductContext &product); void prepareProduct(ProjectContext *projectContext, Item *productItem); void setupProductDependencies(ProductContext *productContext); @@ -320,6 +321,7 @@ private: void handleProductError(const ErrorInfo &error, ProductContext *productContext); QualifiedIdSet gatherModulePropertiesSetInGroup(const Item *group); Item *loadItemFromFile(const QString &filePath); + void collectProductsByName(const TopLevelProjectContext &topLevelProject); ItemPool *m_pool; Logger &m_logger; diff --git a/tests/auto/api/testdata/multiplexing/multiplexing.qbs b/tests/auto/api/testdata/multiplexing/multiplexing.qbs index 9027743b4..3d099eb7a 100644 --- a/tests/auto/api/testdata/multiplexing/multiplexing.qbs +++ b/tests/auto/api/testdata/multiplexing/multiplexing.qbs @@ -2,93 +2,101 @@ import qbs import qbs.TextFile Project { - Product { - name: "no-multiplexing" - type: ["reversed-text"] - files: ["foo.txt"] - } - Product { - name: "multiplex-without-aggregator-2" - type: ["reversed-text"] - files: ["foo.txt"] - multiplexByQbsProperties: ["architectures"] - qbs.architectures: ["TRS-80", "C64"] - } - Product { - name: "multiplex-with-export" - type: ["reversed-text"] - files: ["foo.txt"] - multiplexByQbsProperties: ["architectures"] - qbs.architectures: ["TRS-80", "C64"] - Export { Depends { name: "multiplex-without-aggregator-2" } } - } - Product { - name: "nonmultiplex-with-export" - type: ["reversed-text"] - files: ["foo.txt"] - Export { Depends { name: "multiplex-without-aggregator-2" } } - } - Product { - name: "nonmultiplex-exporting-aggregation" - type: ["reversed-text"] - files: ["foo.txt"] - Export { Depends { name: "multiplex-with-aggregator-2" } } - } - Product { - name: "multiplex-using-export" - type: ["reversed-text"] - files: ["foo.txt"] - multiplexByQbsProperties: ["architectures"] - qbs.architectures: ["TRS-80", "C64"] - Depends { name: "multiplex-with-export" } - } - Product { - name: "multiplex-without-aggregator-2-depend-on-non-multiplexed" - type: ["reversed-text"] - files: ["foo.txt"] - multiplexByQbsProperties: ["architectures"] - qbs.architectures: ["TRS-80", "C64"] - Depends { name: "no-multiplexing" } - } - Product { - name: "multiplex-without-aggregator-4" - type: ["reversed-text"] - files: ["foo.txt"] - multiplexByQbsProperties: ["architectures", "buildVariants"] - qbs.architectures: ["TRS-80", "C64"] - qbs.buildVariants: ["debug", "release"] - } - Product { - name: "multiplex-with-aggregator-2" - type: ["reversed-text"] - files: ["foo.txt"] - aggregate: true - multiplexByQbsProperties: ["architectures"] - qbs.architectures: ["TRS-80", "C64"] - qbs.architecture: "Atari ST" - } - Product { - name: "multiplex-with-aggregator-2-dependent" - Depends { name: "multiplex-with-aggregator-2" } - type: ["something"] - files: ["foo.txt"] - } - Product { - name: "non-multiplexed-with-dependencies-on-multiplexed" - Depends { name: "multiplex-without-aggregator-2" } - } - Product { - name: "non-multiplexed-with-dependencies-on-multiplexed-via-export1" - Depends { name: "multiplex-with-export" } - } - Product { - name: "non-multiplexed-with-dependencies-on-multiplexed-via-export2" - Depends { name: "nonmultiplex-with-export" } + Project { + name: "subproject 1" + Product { + name: "multiplex-using-export" + type: ["reversed-text"] + files: ["foo.txt"] + multiplexByQbsProperties: ["architectures"] + qbs.architectures: ["TRS-80", "C64"] + Depends { name: "multiplex-with-export" } + } + Product { + name: "multiplex-without-aggregator-2-depend-on-non-multiplexed" + type: ["reversed-text"] + files: ["foo.txt"] + multiplexByQbsProperties: ["architectures"] + qbs.architectures: ["TRS-80", "C64"] + Depends { name: "no-multiplexing" } + } + Product { + name: "multiplex-with-aggregator-2" + type: ["reversed-text"] + files: ["foo.txt"] + aggregate: true + multiplexByQbsProperties: ["architectures"] + qbs.architectures: ["TRS-80", "C64"] + qbs.architecture: "Atari ST" + } + Product { + name: "multiplex-with-aggregator-2-dependent" + Depends { name: "multiplex-with-aggregator-2" } + type: ["something"] + files: ["foo.txt"] + } + Product { + name: "non-multiplexed-with-dependencies-on-multiplexed" + Depends { name: "multiplex-without-aggregator-2" } + } + Product { + name: "non-multiplexed-with-dependencies-on-multiplexed-via-export1" + Depends { name: "multiplex-with-export" } + } + Product { + name: "non-multiplexed-with-dependencies-on-multiplexed-via-export2" + Depends { name: "nonmultiplex-with-export" } + } + Product { + name: "non-multiplexed-with-dependencies-on-aggregation-via-export" + Depends { name: "nonmultiplex-exporting-aggregation" } + } } - Product { - name: "non-multiplexed-with-dependencies-on-aggregation-via-export" - Depends { name: "nonmultiplex-exporting-aggregation" } + + Project { + name: "subproject 2" + Product { + name: "no-multiplexing" + type: ["reversed-text"] + files: ["foo.txt"] + } + Product { + name: "multiplex-without-aggregator-2" + type: ["reversed-text"] + files: ["foo.txt"] + multiplexByQbsProperties: ["architectures"] + qbs.architectures: ["TRS-80", "C64"] + } + Product { + name: "multiplex-with-export" + type: ["reversed-text"] + files: ["foo.txt"] + multiplexByQbsProperties: ["architectures"] + qbs.architectures: ["TRS-80", "C64"] + Export { Depends { name: "multiplex-without-aggregator-2" } } + } + Product { + name: "nonmultiplex-with-export" + type: ["reversed-text"] + files: ["foo.txt"] + Export { Depends { name: "multiplex-without-aggregator-2" } } + } + Product { + name: "nonmultiplex-exporting-aggregation" + type: ["reversed-text"] + files: ["foo.txt"] + Export { Depends { name: "multiplex-with-aggregator-2" } } + } + Product { + name: "multiplex-without-aggregator-4" + type: ["reversed-text"] + files: ["foo.txt"] + multiplexByQbsProperties: ["architectures", "buildVariants"] + qbs.architectures: ["TRS-80", "C64"] + qbs.buildVariants: ["debug", "release"] + } } + Product { name: "aggregate-with-dependencies-on-aggregation-via-export" Depends { name: "nonmultiplex-exporting-aggregation" } diff --git a/tests/auto/api/tst_api.cpp b/tests/auto/api/tst_api.cpp index 0b46f04ba..71d8117c0 100644 --- a/tests/auto/api/tst_api.cpp +++ b/tests/auto/api/tst_api.cpp @@ -1693,7 +1693,7 @@ void TestApi::multiplexing() waitForFinished(setupJob.get()); QVERIFY2(!setupJob->error().hasError(), qPrintable(setupJob->error().toString())); qbs::Project project = setupJob->project(); - QList products = project.projectData().products(); + QList products = project.projectData().allProducts(); qbs::ProductData product; ProductDataSelector selector; selector.name = "no-multiplexing"; -- cgit v1.2.3