diff options
13 files changed, 74 insertions, 19 deletions
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/src/lib/corelib/language/item.cpp b/src/lib/corelib/language/item.cpp index d7ad9f5f5..ae48d1930 100644 --- a/src/lib/corelib/language/item.cpp +++ b/src/lib/corelib/language/item.cpp @@ -283,6 +283,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 aadf2516b..69e4a5fec 100644 --- a/src/lib/corelib/language/item.h +++ b/src/lib/corelib/language/item.h @@ -165,6 +165,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 431abc9fe..b46ed44a0 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(); 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 4e97ba92c..7f6d1b8cf 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 5d7d33526..8d37ad03d 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/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/tst_language.cpp b/tests/auto/language/tst_language.cpp index 6a29876dc..2fbffdc03 100644 --- a/tests/auto/language/tst_language.cpp +++ b/tests/auto/language/tst_language.cpp @@ -1614,6 +1614,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(); diff --git a/tests/auto/language/tst_language.h b/tests/auto/language/tst_language.h index 5effb05b7..a7c39cec2 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(); |