ModuleLoader: Fix adjustment for multiplexed dependenciesv1.9.0

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 <joerg.bornemann@qt.io>

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<Item *> 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<Item *> 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<const ProductContext *> 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<Item *> 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<const ProductContext *> 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<Item *> 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<qbs::ProductData> products = project.projectData().products();
+    QList<qbs::ProductData> products = project.projectData().allProducts();
     qbs::ProductData product;
     ProductDataSelector selector;
     selector.name = "no-multiplexing";