Loader: Do not error out when encountering duplicate multiplex values

In qbs, values of list properties specified in a project file are merged
with those from the profile.
As of fb52fed84a1510a7de0172e643d6fd66a780e2e8, this is also true for
the special multiplexing properties ("qbs.architectures",
"qbs.buildVariants" etc). This means it can now happen that we end up
with duplicate entries in these lists implicitly. Therefore, it no longer
makes sense to throw an error when such duplicates are encountered, as
they were not necessarily specified verbatim. Instead, we simply ignore
the duplicate value and carry on normally.

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

6 files changed, 50 insertions, 26 deletions

diff --git a/src/lib/corelib/loader/productitemmultiplexer.cpp b/src/lib/corelib/loader/productitemmultiplexer.cpp
index aec0400d2..53937aa92 100644
--- a/src/lib/corelib/loader/productitemmultiplexer.cpp
+++ b/src/lib/corelib/loader/productitemmultiplexer.cpp
@@ -181,11 +181,8 @@ MultiplexInfo ProductItemMultiplexer::Private::extractMultiplexInfo(Item *produc
         if (mappedKey.isEmpty())
             throw ErrorInfo(Tr::tr("There is no entry for '%1' in 'qbs.multiplexMap'.").arg(key));
 
-        if (!uniqueMultiplexByQbsProperties.insert(mappedKey).second) {
-            throw ErrorInfo(Tr::tr("Duplicate entry '%1' in Product.%2.")
-                                .arg(mappedKey, StringConstants::multiplexByQbsPropertiesProperty()),
-                            productItem->location());
-        }
+        if (!uniqueMultiplexByQbsProperties.insert(mappedKey).second)
+            continue;
 
         const ScopedJsValue arr(ctx, evaluator.value(qbsModuleItem, key));
         if (JS_IsUndefined(arr))
@@ -198,23 +195,20 @@ MultiplexInfo ProductItemMultiplexer::Private::extractMultiplexInfo(Item *produc
             continue;
 
         MultiplexRow mprow;
-        mprow.resize(arrlen);
+        mprow.reserve(arrlen);
         QVariantList entriesForKey;
         for (quint32 i = 0; i < arrlen; ++i) {
             const ScopedJsValue sv(ctx, JS_GetPropertyUint32(ctx, arr, i));
             const QVariant value = getJsVariant(ctx, sv);
-            if (entriesForKey.contains(value)) {
-                throw ErrorInfo(Tr::tr("Duplicate entry '%1' in qbs.%2.")
-                                    .arg(value.toString(), key), productItem->location());
-            }
+            if (entriesForKey.contains(value))
+                continue;
             entriesForKey << value;
-            mprow[i] = VariantValue::create(value);
+            mprow.push_back(VariantValue::create(value));
         }
         multiplexInfo.table = combine(multiplexInfo.table, mprow);
         multiplexInfo.properties.push_back(mappedKey);
     }
     return multiplexInfo;
-
 }
 
 MultiplexTable ProductItemMultiplexer::Private::combine(const MultiplexTable &table,
diff --git a/tests/auto/api/tst_api.cpp b/tests/auto/api/tst_api.cpp
index d85b2502a..79e4ebdae 100644
--- a/tests/auto/api/tst_api.cpp
+++ b/tests/auto/api/tst_api.cpp
@@ -1885,26 +1885,23 @@ void TestApi::multiArch()
     QFile p2ArtifactInstalled(installRoot + "/host/host-tool.output");
     QVERIFY2(p2ArtifactInstalled.exists(), qPrintable(p2ArtifactInstalled.fileName()));
 
-    // Error check: Try to build for the same profile twice.
+    // Specifying the same profile twice should not result in an attempt to multiplex.
     overriddenValues.insert("project.targetProfile", hostProfile.name());
     setupParams.setOverriddenValues(overriddenValues);
     setupJob.reset(project.setupProject(setupParams, m_logSink, nullptr));
     waitForFinished(setupJob.get());
-    QVERIFY(setupJob->error().hasError());
-    QVERIFY2(setupJob->error().toString().contains("Duplicate entry 'host' in qbs.profiles."),
-             qPrintable(setupJob->error().toString()));
+    QVERIFY(!setupJob->error().hasError());
+    QCOMPARE(int(setupJob->project().projectData().products().size()), 2);
 
-    // Error check: Try to build for the same profile twice, this time attaching
-    // the properties via the product name.
+    // The same, but this time attaching the properties via the product name.
     overriddenValues.remove(QStringLiteral("project.targetProfile"));
     overriddenValues.insert("products.p1.myProfiles",
                             targetProfile.name() + ',' + targetProfile.name());
     setupParams.setOverriddenValues(overriddenValues);
     setupJob.reset(project.setupProject(setupParams, m_logSink, nullptr));
     waitForFinished(setupJob.get());
-    QVERIFY(setupJob->error().hasError());
-    QVERIFY2(setupJob->error().toString().contains("Duplicate entry 'target' in qbs.profiles."),
-             qPrintable(setupJob->error().toString()));
+    QVERIFY(!setupJob->error().hasError());
+    QCOMPARE(int(setupJob->project().projectData().products().size()), 2);
 }
 
 struct ProductDataSelector
diff --git a/tests/auto/language/testdata/erroneous/duplicate-multiplex-value.qbs b/tests/auto/language/testdata/duplicate-multiplex-value.qbs
index 24b246604..24b246604 100644
--- a/tests/auto/language/testdata/erroneous/duplicate-multiplex-value.qbs
+++ b/tests/auto/language/testdata/duplicate-multiplex-value.qbs
diff --git a/tests/auto/language/testdata/erroneous/duplicate-multiplex-value2.qbs b/tests/auto/language/testdata/duplicate-multiplex-value2.qbs
index d6c057a9e..d6c057a9e 100644
--- a/tests/auto/language/testdata/erroneous/duplicate-multiplex-value2.qbs
+++ b/tests/auto/language/testdata/duplicate-multiplex-value2.qbs
diff --git a/tests/auto/language/tst_language.cpp b/tests/auto/language/tst_language.cpp
index eda7bf566..c97b83cc1 100644
--- a/tests/auto/language/tst_language.cpp
+++ b/tests/auto/language/tst_language.cpp
@@ -626,6 +626,42 @@ void TestLanguage::dottedNames()
     }
 }
 
+void TestLanguage::duplicateMultiplexValues_data()
+{
+    QTest::addColumn<bool>("dummy");
+    QTest::newRow("duplicate-multiplex-value") << true;
+    QTest::newRow("duplicate-multiplex-value2") << true;
+}
+
+void TestLanguage::duplicateMultiplexValues()
+{
+    bool exceptionCaught = false;
+    try {
+        SetupProjectParameters params = defaultParameters;
+        const QString filePath = testDataDir() + QLatin1Char('/')
+            + QString::fromLocal8Bit(QTest::currentDataTag()) + QLatin1String(".qbs");
+        params.setProjectFilePath(filePath);
+        TopLevelProjectPtr project = m_resolver->resolve(params);
+        QVERIFY(project);
+        const std::vector<ResolvedProductPtr> products = project->allProducts();
+        QCOMPARE(products.size(), 2);
+        bool x86 = false;
+        bool arm = false;
+        for (const ResolvedProductPtr &p : products) {
+            if (p->moduleProperties->moduleProperty("qbs", "architecture").toString() == "x86")
+                x86 = true;
+            else if (p->moduleProperties->moduleProperty("qbs", "architecture").toString() == "arm")
+                arm = true;
+        }
+        QVERIFY(x86);
+        QVERIFY(arm);
+    } catch (const ErrorInfo &e) {
+        exceptionCaught = true;
+        qDebug() << e.toString();
+    }
+    QVERIFY(!exceptionCaught);
+}
+
 void TestLanguage::emptyJsFile()
 {
     bool exceptionCaught = false;
@@ -937,11 +973,6 @@ void TestLanguage::erroneousFiles_data()
     QTest::newRow("dependency-profile-mismatch-2")
             << "dependency-profile-mismatch-2.qbs:15:9 Dependency from product 'main' to "
                "product 'dep' not fulfilled. There are no eligible multiplex candidates.";
-    QTest::newRow("duplicate-multiplex-value")
-            << "duplicate-multiplex-value.qbs:1:1.*Duplicate entry 'x86' in qbs.architectures.";
-    QTest::newRow("duplicate-multiplex-value2")
-            << "duplicate-multiplex-value2.qbs:1:1.*Duplicate entry 'architecture' in "
-               "Product.multiplexByQbsProperties.";
     QTest::newRow("invalid-references")
             << "invalid-references.qbs:2:17.*Cannot open '.*nosuchproject.qbs'";
     QTest::newRow("missing-js-file")
diff --git a/tests/auto/language/tst_language.h b/tests/auto/language/tst_language.h
index 0a59c48b1..1229157b2 100644
--- a/tests/auto/language/tst_language.h
+++ b/tests/auto/language/tst_language.h
@@ -95,6 +95,8 @@ private slots:
     void disabledSubProject();
     void dottedNames_data();
     void dottedNames();
+    void duplicateMultiplexValues_data();
+    void duplicateMultiplexValues();
     void emptyJsFile();
     void enumerateProjectProperties();
     void evalErrorInNonPresentModule_data();