diff options
30 files changed, 157 insertions, 31 deletions
diff --git a/share/qbs/imports/qbs/Probes/qbs-pkg-config-probe.js b/share/qbs/imports/qbs/Probes/qbs-pkg-config-probe.js index ed75a027c..1fb3b26f5 100644 --- a/share/qbs/imports/qbs/Probes/qbs-pkg-config-probe.js +++ b/share/qbs/imports/qbs/Probes/qbs-pkg-config-probe.js @@ -87,7 +87,7 @@ function configure( result.packages = []; result.packagesByModuleName = {}; result.brokenPackages = []; - result.qtInfos = {}; + result.qtInfos = []; var options = {}; options.libDirs = libDirs; diff --git a/share/qbs/module-providers/__fallback/fallback.qbs b/share/qbs/module-providers/__fallback/fallback.qbs index 52a3f080a..349af3a52 100644 --- a/share/qbs/module-providers/__fallback/fallback.qbs +++ b/share/qbs/module-providers/__fallback/fallback.qbs @@ -46,6 +46,8 @@ Module { property string theName: FileInfo.completeBaseName(filePath) + readonly property bool __fallback: true // Hack, please look away + Probes.PkgConfigProbe { id: pkgConfigProbe condition: pkgconfig.present diff --git a/share/qbs/module-providers/qbspkgconfig.qbs b/share/qbs/module-providers/qbspkgconfig.qbs index 0c6c99293..45309c862 100644 --- a/share/qbs/module-providers/qbspkgconfig.qbs +++ b/share/qbs/module-providers/qbspkgconfig.qbs @@ -152,7 +152,7 @@ ModuleProvider { if (moduleName.startsWith("Qt")) { function setupQt(packageName, qtInfos) { - if (qtInfos === undefined) + if (qtInfos === undefined || qtInfos.length === 0) return []; var qtProviderDir = FileInfo.joinPaths(path, "Qt"); return SetupQt.doSetup(packageName, qtInfos, outputBaseDir, qtProviderDir); diff --git a/src/lib/corelib/language/item.cpp b/src/lib/corelib/language/item.cpp index 16d570499..29fa1417d 100644 --- a/src/lib/corelib/language/item.cpp +++ b/src/lib/corelib/language/item.cpp @@ -284,6 +284,11 @@ bool Item::isPresentModule() const return v && v->type() == Value::JSSourceValueType; } +bool Item::isFallbackModule() const +{ + return hasProperty(QLatin1String("__fallback")); +} + void Item::setupForBuiltinType(DeprecationWarningMode deprecationMode, Logger &logger) { assertModuleLocked(); diff --git a/src/lib/corelib/language/item.h b/src/lib/corelib/language/item.h index 9ee826d6d..d0dde98c4 100644 --- a/src/lib/corelib/language/item.h +++ b/src/lib/corelib/language/item.h @@ -182,6 +182,7 @@ public: static void removeChild(Item *parent, Item *child); void dump() const; bool isPresentModule() const; + bool isFallbackModule() const; void setupForBuiltinType(DeprecationWarningMode deprecationMode, Logger &logger); void copyProperty(const QString &propertyName, Item *target) const; void overrideProperties( diff --git a/src/lib/corelib/loader/dependenciesresolver.cpp b/src/lib/corelib/loader/dependenciesresolver.cpp index 514105e2c..e49af1600 100644 --- a/src/lib/corelib/loader/dependenciesresolver.cpp +++ b/src/lib/corelib/loader/dependenciesresolver.cpp @@ -360,13 +360,6 @@ HandleDependency DependenciesResolver::handleResolvedDependencies() if (dependency.name.toString() == StringConstants::qbsModule()) throw e; - // This can happen when a property is set unconditionally on a non-required, - // non-present dependency. We allow this for user convenience. - if (!dependency.requiredLocally) { - state.pendingResolvedDependencies.pop(); - continue; - } - // See QBS-1338 for why we do not abort handling the product. state.pendingResolvedDependencies.pop(); Item::Modules &modules = m_product.item->modules(); @@ -1001,21 +994,30 @@ void DependenciesResolver::checkForModuleNamePrefixCollision( return; for (const Item::Module &m : m_product.item->modules()) { - if (m.name.length() == dependency.name.length() - || m.name.front() != dependency.name.front()) { + if (m.name.length() == dependency.name.length()) continue; - } + QualifiedId shortName; QualifiedId longName; - if (m.name < dependency.name) { + if (m.name.length() < dependency.name.length()) { shortName = m.name; longName = dependency.name; } else { shortName = dependency.name; longName = m.name; } - throw ErrorInfo(Tr::tr("The name of module '%1' is equal to the first component of the " - "name of module '%2', which is not allowed") + const auto isPrefix = [&] { + for (int i = 0; i < shortName.length(); ++i) { + if (shortName.at(i) != longName.at(i)) + return false; + } + return true; + }; + if (!isPrefix()) + continue; + + throw ErrorInfo(Tr::tr("The name of module '%1' is a prefix of the name of module '%2', " + "which is not allowed") .arg(shortName.toString(), longName.toString()), dependency.location()); } } diff --git a/src/lib/corelib/loader/moduleinstantiator.cpp b/src/lib/corelib/loader/moduleinstantiator.cpp index ab67bc270..1c0359217 100644 --- a/src/lib/corelib/loader/moduleinstantiator.cpp +++ b/src/lib/corelib/loader/moduleinstantiator.cpp @@ -135,6 +135,16 @@ void ModuleInstantiator::exchangePlaceholderItem(Item *loadingItem, Item *module return; } + if (!moduleItemForItemValues->isPresentModule()) + return; + + // This will yield false negatives for the case where there is an invalid property attached + // for a module that is actually found by pkg-config via the fallback provider. + // However, this is extremely rare compared to the case where the presence of the fallback + // module simply indicates "not present". + if (moduleItemForItemValues->isFallbackModule()) + return; + // If the old and the new items are the same, it means the existing item value already // pointed to a module instance (rather than a placeholder). // This can happen in two cases: diff --git a/tests/auto/blackbox/testdata/property-assignment-on-non-present-module/property-assignment-on-non-present-module.qbs b/tests/auto/blackbox/testdata/property-assignment-on-non-present-module/property-assignment-on-non-present-module.qbs deleted file mode 100644 index a01d6c561..000000000 --- a/tests/auto/blackbox/testdata/property-assignment-on-non-present-module/property-assignment-on-non-present-module.qbs +++ /dev/null @@ -1,4 +0,0 @@ -Product { - Depends { name: "nein"; required: false } - nein.doch: "ohhh!" -} diff --git a/tests/auto/blackbox/tst_blackbox.cpp b/tests/auto/blackbox/tst_blackbox.cpp index 0b93ee094..cac8f6dc7 100644 --- a/tests/auto/blackbox/tst_blackbox.cpp +++ b/tests/auto/blackbox/tst_blackbox.cpp @@ -3480,13 +3480,6 @@ void TestBlackbox::productProperties() QVERIFY(regularFileExists(relativeExecutableFilePath("blubb_user"))); } -void TestBlackbox::propertyAssignmentOnNonPresentModule() -{ - QDir::setCurrent(testDataDir + "/property-assignment-on-non-present-module"); - QCOMPARE(runQbs(), 0); - QVERIFY2(m_qbsStderr.isEmpty(), m_qbsStderr.constData()); -} - void TestBlackbox::propertyAssignmentInFailedModule() { QDir::setCurrent(testDataDir + "/property-assignment-in-failed-module"); diff --git a/tests/auto/blackbox/tst_blackbox.h b/tests/auto/blackbox/tst_blackbox.h index 503d627a6..b1ec6877f 100644 --- a/tests/auto/blackbox/tst_blackbox.h +++ b/tests/auto/blackbox/tst_blackbox.h @@ -248,7 +248,6 @@ private slots: void productDependenciesByType(); void productInExportedModule(); void productProperties(); - void propertyAssignmentOnNonPresentModule(); void propertyAssignmentInFailedModule(); void propertyChanges(); void propertyEvaluationContext(); diff --git a/tests/auto/blackbox/tst_blackboxqt.cpp b/tests/auto/blackbox/tst_blackboxqt.cpp index d0f0a3831..b083a97e8 100644 --- a/tests/auto/blackbox/tst_blackboxqt.cpp +++ b/tests/auto/blackbox/tst_blackboxqt.cpp @@ -396,6 +396,20 @@ void TestBlackboxQt::pkgconfigQt_data() << QStringList({"moduleProviders.qbspkgconfig.sysroot:/some/fake/sysroot"}) << false; } +void TestBlackboxQt::pkgconfigNoQt() +{ + QDir::setCurrent(testDataDir + "/pkgconfig-qt"); + rmDirR(relativeBuildDir()); + QbsRunParameters params("build", {"-f", "pkgconfig-qt.qbs"}); + params.arguments << "moduleProviders.qbspkgconfig.libDirs:nonexistent"; + params.expectFailure = true; + + QCOMPARE(runQbs(params) == 0, false); + QVERIFY2(m_qbsStderr.contains("Dependency 'Qt.core' not found for product 'p'"), m_qbsStderr); + // QBS-1777: basic check for JS exceptions in case of missing Qt + QVERIFY2(!m_qbsStderr.contains("Error executing provider for module 'Qt.core'"), m_qbsStderr); +} + void TestBlackboxQt::pluginMetaData() { QDir::setCurrent(testDataDir + "/plugin-meta-data"); diff --git a/tests/auto/blackbox/tst_blackboxqt.h b/tests/auto/blackbox/tst_blackboxqt.h index 270ca3ec0..da395b7d4 100644 --- a/tests/auto/blackbox/tst_blackboxqt.h +++ b/tests/auto/blackbox/tst_blackboxqt.h @@ -62,6 +62,7 @@ private slots: void pkgconfig(); void pkgconfigQt(); void pkgconfigQt_data(); + void pkgconfigNoQt(); void pluginMetaData(); void pluginSupport_data(); void pluginSupport(); diff --git a/tests/auto/language/testdata/invalid-prop-on-non-required-module/invalid-prop-on-non-required-module.qbs b/tests/auto/language/testdata/invalid-prop-on-non-required-module/invalid-prop-on-non-required-module.qbs new file mode 100644 index 000000000..80ae6ad93 --- /dev/null +++ b/tests/auto/language/testdata/invalid-prop-on-non-required-module/invalid-prop-on-non-required-module.qbs @@ -0,0 +1,19 @@ +Project { + property bool useExistingModule + + Product { + name: "a" + condition: project.useExistingModule + Depends { name: "deploader" } + Depends { name: "dep" } + dep.nosuchprop: true + } + + Product { + name: "b" + condition: !project.useExistingModule + Depends { name: "deploader" } + Depends { name: "random"; required: false } + random.nosuchprop: true + } +} diff --git a/tests/auto/language/testdata/invalid-prop-on-non-required-module/modules/dep/dep.qbs b/tests/auto/language/testdata/invalid-prop-on-non-required-module/modules/dep/dep.qbs new file mode 100644 index 000000000..5e45122de --- /dev/null +++ b/tests/auto/language/testdata/invalid-prop-on-non-required-module/modules/dep/dep.qbs @@ -0,0 +1 @@ +Module {} diff --git a/tests/auto/language/testdata/invalid-prop-on-non-required-module/modules/deploader/deploader.qbs b/tests/auto/language/testdata/invalid-prop-on-non-required-module/modules/deploader/deploader.qbs new file mode 100644 index 000000000..15a1b5309 --- /dev/null +++ b/tests/auto/language/testdata/invalid-prop-on-non-required-module/modules/deploader/deploader.qbs @@ -0,0 +1,7 @@ +Module { + // This indirection exists to properly model QBS-1776. + // "deploader" corresponds to "bundle", and "dep" corresponds to "codesign" + Depends { condition: project.useExistingModule; name: "dep"; required: false } + + Depends { condition: !project.useExistingModule; name: "random"; required: false } +} diff --git a/tests/auto/language/testdata/module-name-collisions/complex-collision.qbs b/tests/auto/language/testdata/module-name-collisions/complex-collision.qbs new file mode 100644 index 000000000..cd55aa946 --- /dev/null +++ b/tests/auto/language/testdata/module-name-collisions/complex-collision.qbs @@ -0,0 +1,4 @@ +Product { + Depends { name: "prefix1.middle1" } + Depends { name: "prefix1.middle1.suffix1" } +} diff --git a/tests/auto/language/testdata/module-name-collisions/modules/prefix1/middle1/middle1.qbs b/tests/auto/language/testdata/module-name-collisions/modules/prefix1/middle1/middle1.qbs new file mode 100644 index 000000000..5e45122de --- /dev/null +++ b/tests/auto/language/testdata/module-name-collisions/modules/prefix1/middle1/middle1.qbs @@ -0,0 +1 @@ +Module {} diff --git a/tests/auto/language/testdata/module-name-collisions/modules/prefix1/middle1/suffix1/suffix1.qbs b/tests/auto/language/testdata/module-name-collisions/modules/prefix1/middle1/suffix1/suffix1.qbs new file mode 100644 index 000000000..5e45122de --- /dev/null +++ b/tests/auto/language/testdata/module-name-collisions/modules/prefix1/middle1/suffix1/suffix1.qbs @@ -0,0 +1 @@ +Module {} diff --git a/tests/auto/language/testdata/module-name-collisions/modules/prefix1/middle1/suffix2/suffix2.qbs b/tests/auto/language/testdata/module-name-collisions/modules/prefix1/middle1/suffix2/suffix2.qbs new file mode 100644 index 000000000..5e45122de --- /dev/null +++ b/tests/auto/language/testdata/module-name-collisions/modules/prefix1/middle1/suffix2/suffix2.qbs @@ -0,0 +1 @@ +Module {} diff --git a/tests/auto/language/testdata/module-name-collisions/modules/prefix1/middle2/suffix/suffix.qbs b/tests/auto/language/testdata/module-name-collisions/modules/prefix1/middle2/suffix/suffix.qbs new file mode 100644 index 000000000..5e45122de --- /dev/null +++ b/tests/auto/language/testdata/module-name-collisions/modules/prefix1/middle2/suffix/suffix.qbs @@ -0,0 +1 @@ +Module {} diff --git a/tests/auto/language/testdata/erroneous/modules/prefix1/prefix1.qbs b/tests/auto/language/testdata/module-name-collisions/modules/prefix1/prefix1.qbs index 84957060c..84957060c 100644 --- a/tests/auto/language/testdata/erroneous/modules/prefix1/prefix1.qbs +++ b/tests/auto/language/testdata/module-name-collisions/modules/prefix1/prefix1.qbs diff --git a/tests/auto/language/testdata/erroneous/modules/prefix1/suffix/suffix.qbs b/tests/auto/language/testdata/module-name-collisions/modules/prefix1/suffix/suffix.qbs index 218a4feb7..218a4feb7 100644 --- a/tests/auto/language/testdata/erroneous/modules/prefix1/suffix/suffix.qbs +++ b/tests/auto/language/testdata/module-name-collisions/modules/prefix1/suffix/suffix.qbs diff --git a/tests/auto/language/testdata/module-name-collisions/modules/prefix2/prefix2.qbs b/tests/auto/language/testdata/module-name-collisions/modules/prefix2/prefix2.qbs new file mode 100644 index 000000000..a5aaa6f8b --- /dev/null +++ b/tests/auto/language/testdata/module-name-collisions/modules/prefix2/prefix2.qbs @@ -0,0 +1,3 @@ +Module { + Depends { name: "prefix2.suffix" } +} diff --git a/tests/auto/language/testdata/module-name-collisions/modules/prefix2/suffix/suffix.qbs b/tests/auto/language/testdata/module-name-collisions/modules/prefix2/suffix/suffix.qbs new file mode 100644 index 000000000..84957060c --- /dev/null +++ b/tests/auto/language/testdata/module-name-collisions/modules/prefix2/suffix/suffix.qbs @@ -0,0 +1,2 @@ +Module { +} diff --git a/tests/auto/language/testdata/module-name-collisions/no-collision1.qbs b/tests/auto/language/testdata/module-name-collisions/no-collision1.qbs new file mode 100644 index 000000000..e5a94ad66 --- /dev/null +++ b/tests/auto/language/testdata/module-name-collisions/no-collision1.qbs @@ -0,0 +1,4 @@ +Product { + Depends { name: "prefix1.middle1.suffix1" } + Depends { name: "prefix1.middle1.suffix2" } +} diff --git a/tests/auto/language/testdata/module-name-collisions/no-collision2.qbs b/tests/auto/language/testdata/module-name-collisions/no-collision2.qbs new file mode 100644 index 000000000..664ec1729 --- /dev/null +++ b/tests/auto/language/testdata/module-name-collisions/no-collision2.qbs @@ -0,0 +1,4 @@ +Product { + Depends { name: "prefix1.middle1" } + Depends { name: "prefix1.middle2.suffix" } +} diff --git a/tests/auto/language/testdata/erroneous/same-module-prefix1.qbs b/tests/auto/language/testdata/module-name-collisions/simple-collision1.qbs index 8aba31c2a..8aba31c2a 100644 --- a/tests/auto/language/testdata/erroneous/same-module-prefix1.qbs +++ b/tests/auto/language/testdata/module-name-collisions/simple-collision1.qbs diff --git a/tests/auto/language/testdata/erroneous/same-module-prefix2.qbs b/tests/auto/language/testdata/module-name-collisions/simple-collision2.qbs index 6679091c2..6679091c2 100644 --- a/tests/auto/language/testdata/erroneous/same-module-prefix2.qbs +++ b/tests/auto/language/testdata/module-name-collisions/simple-collision2.qbs diff --git a/tests/auto/language/tst_language.cpp b/tests/auto/language/tst_language.cpp index 343b059c9..7af4356cb 100644 --- a/tests/auto/language/tst_language.cpp +++ b/tests/auto/language/tst_language.cpp @@ -956,10 +956,6 @@ void TestLanguage::erroneousFiles_data() "cannot read property 'includes' of undefined"; QTest::newRow("misused-inherited-property") << "Binding to non-item property"; QTest::newRow("undeclared_property_in_Properties_item") << "Item 'blubb' is not declared"; - QTest::newRow("same-module-prefix1") << "The name of module 'prefix1' is equal to the first " - "component of the name of module 'prefix1.suffix'"; - QTest::newRow("same-module-prefix2") << "The name of module 'prefix2' is equal to the first " - "component of the name of module 'prefix2.suffix'"; QTest::newRow("conflicting-properties-in-export-items") << "Export item in inherited item redeclares property 'theProp' with different type."; QTest::newRow("invalid-property-option") @@ -1614,6 +1610,33 @@ void TestLanguage::invalidOverrides_data() << "products.MyOtherProduct.cpp.useRPaths" << QString(); } +void TestLanguage::invalidPropOnNonRequiredModule_data() +{ + QTest::addColumn<bool>("useExistingModule"); + QTest::addColumn<bool>("errorExpected"); + + QTest::newRow("existing module") << true << true; + QTest::newRow("non-existing module") << false << false; +} + +void TestLanguage::invalidPropOnNonRequiredModule() +{ + QFETCH(bool, useExistingModule); + QFETCH(bool, errorExpected); + + try { + defaultParameters.setOverriddenValues( + {std::make_pair("project.useExistingModule", useExistingModule)}); + resolveProject("invalid-prop-on-non-required-module"); + QVERIFY(!errorExpected); + } catch (const ErrorInfo &e) { + const QString errorString = e.toString(); + QVERIFY2(errorExpected, qPrintable(errorString)); + QVERIFY2(errorString.contains("Property 'nosuchprop' is not declared"), + qPrintable(errorString)); + } +} + void TestLanguage::itemPrototype() { FileContextPtr fileContext = FileContext::create(); @@ -1738,6 +1761,34 @@ void TestLanguage::moduleMergingVariantValues() QCOMPARE(exceptionCaught, false); } +void TestLanguage::moduleNameCollisions_data() +{ + QTest::addColumn<QString>("projectFile"); + QTest::addColumn<bool>("collisionExpected"); + + QTest::newRow("simple collision (one order)") << "simple-collision1.qbs" << true; + QTest::newRow("simple collision (other order)") << "simple-collision2.qbs" << true; + QTest::newRow("collision with more components") << "complex-collision.qbs" << true; + QTest::newRow("no collision (same length)") << "no-collision1.qbs" << false; + QTest::newRow("no collision (different length)") << "no-collision2.qbs" << false; +} + +void TestLanguage::moduleNameCollisions() +{ + QFETCH(QString, projectFile); + QFETCH(bool, collisionExpected); + + try { + const QString compositeProjectFilePath = QString("module-name-collisions/") + projectFile; + QVERIFY(resolveProject(qPrintable(compositeProjectFilePath))); + QVERIFY(!collisionExpected); + } catch (const ErrorInfo &e) { + const QString errorString = e.toString(); + QVERIFY2(collisionExpected, qPrintable(errorString)); + QVERIFY2(errorString.contains("not allowed"), qPrintable(errorString)); + } +} + void TestLanguage::moduleParameters_data() { QTest::addColumn<QVariantMap>("inputProperties"); diff --git a/tests/auto/language/tst_language.h b/tests/auto/language/tst_language.h index 5effb05b7..870b7d1f8 100644 --- a/tests/auto/language/tst_language.h +++ b/tests/auto/language/tst_language.h @@ -110,6 +110,8 @@ private slots: void invalidBindingInDisabledItem(); void invalidOverrides(); void invalidOverrides_data(); + void invalidPropOnNonRequiredModule_data(); + void invalidPropOnNonRequiredModule(); void itemPrototype(); void itemScope(); void jsExtensions(); @@ -117,6 +119,8 @@ private slots: void jsImportUsedInMultipleScopes(); void localProfileAsTopLevelProfile(); void moduleMergingVariantValues(); + void moduleNameCollisions_data(); + void moduleNameCollisions(); void moduleParameters_data(); void moduleParameters(); void modulePrioritizationBySearchPath_data(); |