Handle Depends.productTypes entirely in the ModuleLoader

The old approach was not compatible with product multiplexing.

Change-Id: Iac5947665c41c284fa9e177920fd4f225d353973
Reviewed-by: Joerg Bornemann <joerg.bornemann@qt.io>

5 files changed, 112 insertions, 48 deletions

diff --git a/src/lib/corelib/language/item.cpp b/src/lib/corelib/language/item.cpp
index 3e9fbb6a5..9ac12b0e6 100644
--- a/src/lib/corelib/language/item.cpp
+++ b/src/lib/corelib/language/item.cpp
@@ -365,6 +365,12 @@ void Item::addChild(Item *parent, Item *child)
     child->setParent(parent);
 }
 
+void Item::removeChild(Item *parent, Item *child)
+{
+    parent->m_children.removeOne(child);
+    child->setParent(nullptr);
+}
+
 void Item::setPropertyDeclaration(const QString &name, const PropertyDeclaration &declaration)
 {
     if (declaration.isExpired()) {
diff --git a/src/lib/corelib/language/item.h b/src/lib/corelib/language/item.h
index 97ca3ad91..6756e576d 100644
--- a/src/lib/corelib/language/item.h
+++ b/src/lib/corelib/language/item.h
@@ -135,6 +135,7 @@ public:
     void setChildren(const QList<Item *> &children) { m_children = children; }
     void setParent(Item *item) { m_parent = item; }
     static void addChild(Item *parent, Item *child);
+    static void removeChild(Item *parent, Item *child);
     void dump() const;
     bool isPresentModule() const;
     void setupForBuiltinType(Logger &logger);
diff --git a/src/lib/corelib/language/moduleloader.cpp b/src/lib/corelib/language/moduleloader.cpp
index 3df9182ee..466707a79 100644
--- a/src/lib/corelib/language/moduleloader.cpp
+++ b/src/lib/corelib/language/moduleloader.cpp
@@ -137,8 +137,6 @@ public:
         for (auto productContext : qAsConst(allProducts)) {
             auto &productDependencies = m_dependencyMap[productContext];
             for (const auto &dep : qAsConst(productContext->info.usedProducts)) {
-                if (!dep.productTypes.empty())
-                    continue;
                 QBS_CHECK(!dep.name.isEmpty());
                 const auto &deps = productsMap.value(dep.name);
                 if (dep.profile == StringConstants::star()) {
@@ -536,9 +534,10 @@ void ModuleLoader::handleTopLevelProject(ModuleLoaderResult *loadResult, Item *p
     tlp.buildDirectory = buildDirectory;
     handleProject(loadResult, &tlp, projectItem, referencedFilePaths);
     checkProjectNamesInOverrides(tlp);
-    collectProductsByName(tlp);
+    collectProductsByNameAndType(tlp);
     checkProductNamesInOverrides();
 
+    normalizeDependencies(tlp);
     adjustDependenciesForMultiplexing(tlp);
 
     for (ProjectContext * const projectContext : qAsConst(tlp.projects)) {
@@ -891,6 +890,82 @@ QList<Item *> ModuleLoader::multiplexProductItem(ProductContext *dummyContext, I
     return additionalProductItems;
 }
 
+void ModuleLoader::normalizeDependencies(const ModuleLoader::TopLevelProjectContext &tlp)
+{
+    for (const ProjectContext * const project : tlp.projects) {
+        for (const ProductContext &product : project->products)
+            normalizeDependencies(product);
+    }
+}
+
+void ModuleLoader::normalizeDependencies(const ModuleLoader::ProductContext &product)
+{
+    std::vector<Item *> dependsItemsToAdd;
+    std::vector<Item *> dependsItemsToRemove;
+    for (Item *dependsItem : product.item->children()) {
+        if (dependsItem->type() != ItemType::Depends)
+            continue;
+        bool productTypesIsSet;
+        const FileTags productTypes = m_evaluator->fileTagsValue(dependsItem,
+                StringConstants::productTypesProperty(), &productTypesIsSet);
+        if (productTypesIsSet) {
+            bool nameIsSet;
+            m_evaluator->stringValue(dependsItem, StringConstants::nameProperty(), QString(),
+                                     &nameIsSet);
+            if (nameIsSet) {
+                throw ErrorInfo(Tr::tr("The 'productTypes' and 'name' properties are mutually "
+                                       "exclusive."), dependsItem->location());
+            }
+            bool submodulesPropertySet;
+            m_evaluator->stringListValue( dependsItem, StringConstants::submodulesProperty(),
+                                          &submodulesPropertySet);
+            if (submodulesPropertySet) {
+                throw ErrorInfo(Tr::tr("The 'productTypes' and 'subModules' properties are "
+                                       "mutually exclusive."), dependsItem->location());
+            }
+            const bool limitToSubProject = m_evaluator->boolValue
+                    (dependsItem, StringConstants::limitToSubProjectProperty());
+            static const auto hasSameSubProject
+                    = [](const ProductContext &product, const ProductContext &other) {
+                for (const Item *otherParent = other.item->parent(); otherParent;
+                     otherParent = otherParent->parent()) {
+                    if (otherParent == product.item->parent())
+                        return true;
+                }
+                return false;
+            };
+            std::vector<const ProductContext *> matchingProducts;
+            for (const FileTag &typeTag : productTypes) {
+                const auto range = m_productsByType.equal_range(typeTag);
+                for (auto it = range.first; it != range.second; ++it) {
+                    if (it->second != &product
+                            && (!limitToSubProject || hasSameSubProject(product, *it->second))) {
+                        matchingProducts.push_back(it->second);
+                    }
+                }
+            }
+            if (matchingProducts.empty()) {
+                qCDebug(lcModuleLoader) << "Depends.productTypes does not match anything."
+                                        << dependsItem->location();
+                dependsItemsToRemove.push_back(dependsItem);
+                continue;
+            }
+            for (std::size_t i = 1; i < matchingProducts.size(); ++i) {
+                Item * const dependsClone = dependsItem->clone();
+                dependsClone->setProperty(StringConstants::nameProperty(),
+                                         VariantValue::create(matchingProducts.at(i)->name));
+                dependsItemsToAdd.push_back(dependsClone);
+            }
+            dependsItem->setProperty(StringConstants::nameProperty(),
+                                     VariantValue::create(matchingProducts.front()->name));
+        }
+    }
+    for (Item * const newDependsItem : dependsItemsToAdd)
+        Item::addChild(product.item, newDependsItem);
+    for (Item * const dependsItem : dependsItemsToRemove)
+        Item::removeChild(product.item, dependsItem);
+}
+
 void ModuleLoader::adjustDependenciesForMultiplexing(const TopLevelProjectContext &tlp)
 {
     for (const ProjectContext * const project : tlp.projects) {
@@ -1939,11 +2014,24 @@ ModuleLoader::ShadowProductInfo ModuleLoader::getShadowProductInfo(
                           ? product.name.mid(shadowProductPrefix().size()) : QString());
 }
 
-void ModuleLoader::collectProductsByName(const TopLevelProjectContext &topLevelProject)
+void ModuleLoader::collectProductsByNameAndType(const TopLevelProjectContext &topLevelProject)
 {
     for (ProjectContext * const project : topLevelProject.projects) {
-        for (ProductContext &product : project->products)
+        for (ProductContext &product : project->products) {
             m_productsByName.insert({ product.name, &product });
+            try {
+                // Load the qbs module here already, in case it is needed in the type
+                // property.
+                product.item->addModule(loadBaseModule(&product, product.item));
+                const FileTags productTags
+                        = m_evaluator->fileTagsValue(product.item, StringConstants::typeProperty());
+                for (const FileTag &tag : productTags)
+                    m_productsByType.insert({ tag, &product});
+            } catch (const ErrorInfo &e) {
+                qCDebug(lcModuleLoader) << "product" << product.name << "has complex type "
+                                           " and won't get an entry in the type map";
+            }
+        }
     }
 }
 
@@ -2192,7 +2280,11 @@ void ModuleLoader::adjustDefiningItemsInGroupModuleInstances(const Item::Module
 void ModuleLoader::resolveDependencies(DependsContext *dependsContext, Item *item,
                                        ProductContext *productContext)
 {
-    const Item::Module baseModule = loadBaseModule(dependsContext->product, item);
+    if (!productContext) {
+        // For products, we already did this in collectProductsByNameAndType().
+        const Item::Module baseModule = loadBaseModule(dependsContext->product, item);
+        item->addModule(baseModule);
+    }
     // Resolve all Depends items.
     ItemModuleList loadedModules;
     QList<Item *> dependsItemPerLoadedModule;
@@ -2224,7 +2316,6 @@ void ModuleLoader::resolveDependencies(DependsContext *dependsContext, Item *ite
         lastDependsItem = dependsItem;
     }
 
-    item->addModule(baseModule);
     for (int i = 0; i < loadedModules.size(); ++i) {
         Item::Module &module = loadedModules[i];
         mergeParameters(module.parameters, extractParameters(dependsItemPerLoadedModule.at(i)));
@@ -2267,38 +2358,12 @@ void ModuleLoader::resolveDependsItem(DependsContext *dependsContext, Item *pare
         qCDebug(lcModuleLoader) << "Depends item disabled, ignoring.";
         return;
     }
-    bool productTypesIsSet;
-    const FileTags productTypes = m_evaluator->fileTagsValue(dependsItem,
-            StringConstants::productTypesProperty(), &productTypesIsSet);
     bool nameIsSet;
     const QString name = m_evaluator->stringValue(dependsItem, StringConstants::nameProperty(),
                                                   QString(), &nameIsSet);
     bool submodulesPropertySet;
     const QStringList submodules = m_evaluator->stringListValue(
                 dependsItem, StringConstants::submodulesProperty(), &submodulesPropertySet);
-    if (productTypesIsSet) {
-        if (nameIsSet) {
-            throw ErrorInfo(Tr::tr("The 'productTypes' and 'name' properties are mutually "
-                                   "exclusive."), dependsItem->location());
-        }
-        if (submodulesPropertySet) {
-            throw ErrorInfo(Tr::tr("The 'productTypes' and 'subModules' properties are mutually "
-                                   "exclusive."), dependsItem->location());
-        }
-        if (productTypes.empty()) {
-            qCDebug(lcModuleLoader) << "Ignoring Depends item with empty productTypes list.";
-            return;
-        }
-
-        // TODO: We could also filter by the "profiles" property. This would required a refactoring
-        //       (Dependency needs a list of profiles and the multiplexing must happen later).
-        ModuleLoaderResult::ProductInfo::Dependency dependency;
-        dependency.productTypes = productTypes;
-        dependency.limitToSubProject
-                = m_evaluator->boolValue(dependsItem, StringConstants::limitToSubProjectProperty());
-        productResults->push_back(dependency);
-        return;
-    }
     if (submodules.empty() && submodulesPropertySet) {
         qCDebug(lcModuleLoader) << "Ignoring Depends item with empty submodules list.";
         return;
diff --git a/src/lib/corelib/language/moduleloader.h b/src/lib/corelib/language/moduleloader.h
index f559105b5..35ea26e70 100644
--- a/src/lib/corelib/language/moduleloader.h
+++ b/src/lib/corelib/language/moduleloader.h
@@ -85,7 +85,6 @@ struct ModuleLoaderResult
     {
         struct Dependency
         {
-            FileTags productTypes;
             QString name;
             QString profile; // "*" <=> Match all profiles.
             QString multiplexConfigurationId;
@@ -236,9 +235,12 @@ private:
     static MultiplexTable combine(const MultiplexTable &table, const MultiplexRow &values);
     MultiplexInfo extractMultiplexInfo(Item *productItem, Item *qbsModuleItem);
     QList<Item *> multiplexProductItem(ProductContext *dummyContext, Item *productItem);
+    void normalizeDependencies(const TopLevelProjectContext &tlp);
+    void normalizeDependencies(const ProductContext &product);
     void adjustDependenciesForMultiplexing(const TopLevelProjectContext &tlp);
     void adjustDependenciesForMultiplexing(const ProductContext &product);
 
+
     void prepareProduct(ProjectContext *projectContext, Item *productItem);
     void setupProductDependencies(ProductContext *productContext);
     void handleProduct(ProductContext *productContext);
@@ -338,7 +340,7 @@ private:
     void handleProductError(const ErrorInfo &error, ProductContext *productContext);
     QualifiedIdSet gatherModulePropertiesSetInGroup(const Item *group);
     Item *loadItemFromFile(const QString &filePath);
-    void collectProductsByName(const TopLevelProjectContext &topLevelProject);
+    void collectProductsByNameAndType(const TopLevelProjectContext &topLevelProject);
 
     void handleProfileItems(Item *item, ProjectContext *projectContext);
     std::vector<Item *> collectProfileItems(Item *item, ProjectContext *projectContext);
@@ -384,6 +386,7 @@ private:
     QVariantMap m_storedProfiles;
     QVariantMap m_localProfiles;
     std::multimap<QString, const ProductContext *> m_productsByName;
+    std::multimap<FileTag, const ProductContext *> m_productsByType;
     SetupProjectParameters m_parameters;
     std::unique_ptr<Settings> m_settings;
     Version m_qbsVersion;
diff --git a/src/lib/corelib/language/projectresolver.cpp b/src/lib/corelib/language/projectresolver.cpp
index 174c9d5bd..33fee79b7 100644
--- a/src/lib/corelib/language/projectresolver.cpp
+++ b/src/lib/corelib/language/projectresolver.cpp
@@ -1322,19 +1322,8 @@ ProjectResolver::ProductDependencyInfos ProjectResolver::getProductDependencies(
     ProductDependencyInfos result;
     result.dependencies.reserve(productInfo.usedProducts.size());
     for (const auto &dependency : productInfo.usedProducts) {
-        QBS_CHECK(dependency.name.isEmpty() != dependency.productTypes.empty());
-        if (!dependency.productTypes.empty()) {
-            for (const FileTag &tag : dependency.productTypes) {
-                const QList<ResolvedProductPtr> productsForTag = m_productsByType.value(tag);
-                for (const ResolvedProductPtr &p : productsForTag) {
-                    if (p == product || !p->enabled
-                            || (dependency.limitToSubProject && !product->isInParentProject(p))) {
-                        continue;
-                    }
-                    result.dependencies.emplace_back(p, dependency.parameters);
-                }
-            }
-        } else if (dependency.profile == StringConstants::star()) {
+        QBS_CHECK(!dependency.name.isEmpty());
+        if (dependency.profile == StringConstants::star()) {
             for (const ResolvedProductPtr &p : qAsConst(m_productsByName)) {
                 if (p->name != dependency.name || p == product || !p->enabled
                         || (dependency.limitToSubProject && !product->isInParentProject(p))) {