From 5c1973d4b74e0823b4f4fedc896ac342520ae852 Mon Sep 17 00:00:00 2001 From: Christian Kandeler Date: Tue, 16 May 2023 10:47:26 +0200 Subject: 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 --- src/lib/corelib/loader/productitemmultiplexer.cpp | 18 ++++------ tests/auto/api/tst_api.cpp | 15 ++++---- .../testdata/duplicate-multiplex-value.qbs | 6 ++++ .../testdata/duplicate-multiplex-value2.qbs | 6 ++++ .../erroneous/duplicate-multiplex-value.qbs | 6 ---- .../erroneous/duplicate-multiplex-value2.qbs | 6 ---- tests/auto/language/tst_language.cpp | 41 +++++++++++++++++++--- tests/auto/language/tst_language.h | 2 ++ 8 files changed, 62 insertions(+), 38 deletions(-) create mode 100644 tests/auto/language/testdata/duplicate-multiplex-value.qbs create mode 100644 tests/auto/language/testdata/duplicate-multiplex-value2.qbs delete mode 100644 tests/auto/language/testdata/erroneous/duplicate-multiplex-value.qbs delete mode 100644 tests/auto/language/testdata/erroneous/duplicate-multiplex-value2.qbs 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/duplicate-multiplex-value.qbs b/tests/auto/language/testdata/duplicate-multiplex-value.qbs new file mode 100644 index 000000000..24b246604 --- /dev/null +++ b/tests/auto/language/testdata/duplicate-multiplex-value.qbs @@ -0,0 +1,6 @@ +Product { + name: "p" + multiplexByQbsProperties: "architectures" + aggregate: false + qbs.architectures: ["x86", "arm", "x86"] +} diff --git a/tests/auto/language/testdata/duplicate-multiplex-value2.qbs b/tests/auto/language/testdata/duplicate-multiplex-value2.qbs new file mode 100644 index 000000000..d6c057a9e --- /dev/null +++ b/tests/auto/language/testdata/duplicate-multiplex-value2.qbs @@ -0,0 +1,6 @@ +Product { + name: "p" + multiplexByQbsProperties: ["architectures", "buildVariants", "architectures"] + aggregate: false + qbs.architectures: ["x86", "arm"] +} diff --git a/tests/auto/language/testdata/erroneous/duplicate-multiplex-value.qbs b/tests/auto/language/testdata/erroneous/duplicate-multiplex-value.qbs deleted file mode 100644 index 24b246604..000000000 --- a/tests/auto/language/testdata/erroneous/duplicate-multiplex-value.qbs +++ /dev/null @@ -1,6 +0,0 @@ -Product { - name: "p" - multiplexByQbsProperties: "architectures" - aggregate: false - qbs.architectures: ["x86", "arm", "x86"] -} diff --git a/tests/auto/language/testdata/erroneous/duplicate-multiplex-value2.qbs b/tests/auto/language/testdata/erroneous/duplicate-multiplex-value2.qbs deleted file mode 100644 index d6c057a9e..000000000 --- a/tests/auto/language/testdata/erroneous/duplicate-multiplex-value2.qbs +++ /dev/null @@ -1,6 +0,0 @@ -Product { - name: "p" - multiplexByQbsProperties: ["architectures", "buildVariants", "architectures"] - aggregate: false - qbs.architectures: ["x86", "arm"] -} 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("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 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(); -- cgit v1.2.1