Loader: Detect cyclic dependencies involving Depends.productTypes

These cannot be detected while the product with the bulk dependency is
being handled, so record the respective tags and throw an error if a
product with a matching type finishes after the product with the bulk
dependency.

Change-Id: I1d6d2370b1849ea9aa4602df960a4203abf5ffe0
Reviewed-by: Ivan Komissarov <ABBAPOH@gmail.com>

5 files changed, 64 insertions, 3 deletions

diff --git a/src/lib/corelib/loader/dependenciesresolver.cpp b/src/lib/corelib/loader/dependenciesresolver.cpp
index 262b875d0..38cba0aa3 100644
--- a/src/lib/corelib/loader/dependenciesresolver.cpp
+++ b/src/lib/corelib/loader/dependenciesresolver.cpp
@@ -876,8 +876,11 @@ std::optional<EvaluatedDependsItem> DependenciesResolver::evaluateDependsItem(It
     forwardParameterDeclarations(item, m_product.item->modules());
     const QVariantMap parameters = extractParameters(item);
 
+    const FileTags productTypeTags = FileTags::fromStringList(productTypes);
+    if (!productTypeTags.empty())
+        m_product.bulkDependencies.emplace_back(productTypeTags, item->location());
     return EvaluatedDependsItem{
-        item, QualifiedId::fromString(name), submodules, FileTags::fromStringList(productTypes),
+        item, QualifiedId::fromString(name), submodules, productTypeTags,
         multiplexIds, profiles, {minVersion, maxVersion}, parameters, limitToSubProject,
         fallbackMode, required};
 }
diff --git a/src/lib/corelib/loader/loaderutils.cpp b/src/lib/corelib/loader/loaderutils.cpp
index 7c9bb899a..a0949df91 100644
--- a/src/lib/corelib/loader/loaderutils.cpp
+++ b/src/lib/corelib/loader/loaderutils.cpp
@@ -314,6 +314,20 @@ std::vector<ProductContext *> TopLevelProjectContext::productsWithTypeAndConstra
     return matchingProducts;
 }
 
+std::vector<std::pair<ProductContext *, CodeLocation>>
+TopLevelProjectContext::finishedProductsWithBulkDependency(const FileTag &tag) const
+{
+    return m_reverseBulkDependencies.value(tag);
+}
+
+void TopLevelProjectContext::registerBulkDependencies(ProductContext &product)
+{
+    for (const auto &tagAndLoc : product.bulkDependencies) {
+        for (const FileTag &tag : tagAndLoc.first)
+            m_reverseBulkDependencies[tag].emplace_back(&product, tagAndLoc.second);
+    }
+}
+
 void TopLevelProjectContext::addProjectNameUsedInOverrides(const QString &name)
 {
     m_projectNamesUsedInOverrides << name;
diff --git a/src/lib/corelib/loader/loaderutils.h b/src/lib/corelib/loader/loaderutils.h
index a98f80a2a..679f60e27 100644
--- a/src/lib/corelib/loader/loaderutils.h
+++ b/src/lib/corelib/loader/loaderutils.h
@@ -180,6 +180,10 @@ public:
     TimingData timingData;
     std::unique_ptr<DependenciesContext> dependenciesContext;
 
+    // This is needed because complex cyclic dependencies that involve Depends.productTypes
+    // may only be detected after a product has already been fully resolved.
+    std::vector<std::pair<FileTags, CodeLocation>> bulkDependencies;
+
     // The keys are module prototypes, the values specify whether the module's
     // condition is true for this product.
     std::unordered_map<Item *, bool> modulePrototypeEnabledInfo;
@@ -237,6 +241,9 @@ public:
             const QString &name, const std::function<bool(ProductContext &)> &constraint);
     std::vector<ProductContext *> productsWithTypeAndConstraint(
             const FileTags &tags, const std::function<bool(ProductContext &)> &constraint);
+    std::vector<std::pair<ProductContext *, CodeLocation>>
+    finishedProductsWithBulkDependency(const FileTag &tag) const;
+    void registerBulkDependencies(ProductContext &product);
 
     void addProjectNameUsedInOverrides(const QString &name);
     const Set<QString> &projectNamesUsedInOverrides() const;
@@ -338,6 +345,7 @@ private:
     std::mutex m_moduleProvidersCacheMutex;
     QVariantMap m_localProfiles;
     ItemReaderCache m_itemReaderCache;
+    QHash<FileTag, std::vector<std::pair<ProductContext *, CodeLocation>>> m_reverseBulkDependencies;
 
     // For fast look-up when resolving Depends.productTypes.
     // The contract is that it contains fully handled, error-free, enabled products.
diff --git a/src/lib/corelib/loader/productsresolver.cpp b/src/lib/corelib/loader/productsresolver.cpp
index 8d1ea99af..997a4dc57 100644
--- a/src/lib/corelib/loader/productsresolver.cpp
+++ b/src/lib/corelib/loader/productsresolver.cpp
@@ -46,6 +46,7 @@
 #include <language/language.h>
 #include <language/scriptengine.h>
 #include <logging/categories.h>
+#include <logging/translator.h>
 #include <tools/profiling.h>
 #include <tools/progressobserver.h>
 #include <tools/setupprojectparameters.h>
@@ -112,6 +113,7 @@ private:
     void waitForBulkDependency(const ProductWithLoaderState &product);
     void unblockProductsWaitingForDependency(ProductContext &finishedProduct);
     void postProcess();
+    void checkForMissedBulkDependencies(const ProductContext &product);
 
     static int dependsItemCount(const Item *item);
     static int dependsItemCount(ProductContext &product);
@@ -470,6 +472,8 @@ void ProductsResolver::handleFinishedThreads()
             if (!product.name.startsWith(StringConstants::shadowProductPrefix()))
                 m_finishedProducts.push_back(&product);
             topLevelProject.timingData() += product.timingData;
+            checkForMissedBulkDependencies(product);
+            topLevelProject.registerBulkDependencies(product);
             unblockProductsWaitingForDependency(product);
         }
     }
@@ -541,6 +545,40 @@ void ProductsResolver::postProcess()
         project->warningsEncountered << loaderState->logger().warnings();
 }
 
+void ProductsResolver::checkForMissedBulkDependencies(const ProductContext &product)
+{
+    if (!product.product || !product.product->enabled || !product.bulkDependencies.empty())
+        return;
+    for (const FileTag &tag : product.product->fileTags) {
+        for (const auto &[p, location]
+             : m_loaderState.topLevelProject().finishedProductsWithBulkDependency(tag)) {
+            if (!p->product->enabled)
+                continue;
+            if (p->name == product.name)
+                continue;
+            ErrorInfo e;
+            e.append(Tr::tr("Cyclic dependencies detected:"));
+            e.append(Tr::tr("First product is '%1'.")
+                     .arg(product.displayName()), product.item->location());
+            e.append(Tr::tr("Second product is '%1'.")
+                     .arg(p->displayName()), p->item->location());
+            e.append(Tr::tr("Dependency from %1 to %2 was established via Depends.productTypes.")
+                     .arg(p->displayName(), product.displayName()), location);
+            if (m_loaderState.parameters().productErrorMode() == ErrorHandlingMode::Strict)
+                throw e;
+            m_loaderState.logger().printWarning(e);
+            m_loaderState.logger().printWarning(
+                        ErrorInfo(Tr::tr("Product '%1' had errors and was disabled.")
+                                  .arg(product.displayName()), product.item->location()));
+            m_loaderState.logger().printWarning(
+                        ErrorInfo(Tr::tr("Product '%1' had errors and was disabled.")
+                                  .arg(p->displayName()), p->item->location()));
+            product.product->enabled = false;
+            p->product->enabled = false;
+        }
+    }
+}
+
 int ProductsResolver::dependsItemCount(const Item *item)
 {
     int count = 0;
diff --git a/tests/auto/language/tst_language.cpp b/tests/auto/language/tst_language.cpp
index 01685157a..d732b211c 100644
--- a/tests/auto/language/tst_language.cpp
+++ b/tests/auto/language/tst_language.cpp
@@ -1038,8 +1038,6 @@ void TestLanguage::erroneousFiles()
     }
     QEXPECT_FAIL("undeclared_property_in_Properties_item", "Too expensive to check", Continue);
     QEXPECT_FAIL("original-in-export-item3", "Too expensive to check", Continue);
-    QEXPECT_FAIL("dependency_cycle3", "FIXME: Depends.productTypes is tricky", Continue);
-    QEXPECT_FAIL("dependency_cycle3a", "FIXME: Depends.productTypes is tricky", Continue);
     QVERIFY(!"No error thrown on invalid input.");
 }