Ignore property assignments in Export items of disabled products

Ideally, we would also ignore Depends items in such Export items, but
that task seems to be a bit more involved.

Task-number: QBS-1250
Change-Id: Iab74873f74d5b8f0b8771c3c1866c7a1a5fa78aa
Reviewed-by: Jake Petroules <jake.petroules@qt.io>

12 files changed, 138 insertions, 1 deletions

diff --git a/src/lib/corelib/language/modulemerger.cpp b/src/lib/corelib/language/modulemerger.cpp
index 9dc066bf1..6cb42d0fb 100644
--- a/src/lib/corelib/language/modulemerger.cpp
+++ b/src/lib/corelib/language/modulemerger.cpp
@@ -176,7 +176,9 @@ Item::PropertyMap ModuleMerger::dfs(const Item::Module &m, Item::PropertyMap pro
     if (moduleInstance)
         insertProperties(&props, moduleInstance, ListProperties);
 
-    return props;
+    const bool isNonPresentModule = m.item->type() != ItemType::Product
+            && !m.item->isPresentModule();
+    return isNonPresentModule ? Item::PropertyMap() : props;
 }
 
 void ModuleMerger::mergeOutProps(Item::PropertyMap *dst, const Item::PropertyMap &src)
diff --git a/tests/auto/blackbox/testdata/exported-dependency-in-disabled-product/exported-dependency-in-disabled-product.qbs b/tests/auto/blackbox/testdata/exported-dependency-in-disabled-product/exported-dependency-in-disabled-product.qbs
new file mode 100644
index 000000000..9904502a6
--- /dev/null
+++ b/tests/auto/blackbox/testdata/exported-dependency-in-disabled-product/exported-dependency-in-disabled-product.qbs
@@ -0,0 +1,19 @@
+import qbs
+
+Project {
+    Application {
+        name: "app"
+        Depends { name: "dep"; required: false }
+        files: "main.cpp"
+    }
+    Product {
+        name: "dep"
+        condition: eval(conditionString)
+        property string conditionString
+        Depends { name: "nosuchmodule"; required: false }
+        Depends { name: "broken"; required: false }
+        Export {
+            Depends { name: "cpp" }
+        }
+    }
+}
diff --git a/tests/auto/blackbox/testdata/exported-dependency-in-disabled-product/main.cpp b/tests/auto/blackbox/testdata/exported-dependency-in-disabled-product/main.cpp
new file mode 100644
index 000000000..237c8ce18
--- /dev/null
+++ b/tests/auto/blackbox/testdata/exported-dependency-in-disabled-product/main.cpp
@@ -0,0 +1 @@
+int main() {}
diff --git a/tests/auto/blackbox/testdata/exported-dependency-in-disabled-product/modules/broken/broken.qbs b/tests/auto/blackbox/testdata/exported-dependency-in-disabled-product/modules/broken/broken.qbs
new file mode 100644
index 000000000..92f01d5d3
--- /dev/null
+++ b/tests/auto/blackbox/testdata/exported-dependency-in-disabled-product/modules/broken/broken.qbs
@@ -0,0 +1,5 @@
+import qbs
+
+Module {
+    validate: { throw "broken!"; }
+}
diff --git a/tests/auto/blackbox/testdata/exported-property-in-disabled-product/exported-property-in-disabled-product.qbs b/tests/auto/blackbox/testdata/exported-property-in-disabled-product/exported-property-in-disabled-product.qbs
new file mode 100644
index 000000000..2717466b0
--- /dev/null
+++ b/tests/auto/blackbox/testdata/exported-property-in-disabled-product/exported-property-in-disabled-product.qbs
@@ -0,0 +1,20 @@
+import qbs
+
+Project {
+    CppApplication {
+        name: "app"
+        Depends { name: "dep"; required: false }
+        files: "main.cpp"
+    }
+    Product {
+        name: "dep"
+        condition: eval(conditionString)
+        property string conditionString
+        Depends { name: "nosuchmodule"; required: false }
+        Depends { name: "broken"; required: false }
+        Export {
+            Depends { name: "cpp" }
+            cpp.dynamicLibraries: ["nosuchlib"]
+        }
+    }
+}
diff --git a/tests/auto/blackbox/testdata/exported-property-in-disabled-product/main.cpp b/tests/auto/blackbox/testdata/exported-property-in-disabled-product/main.cpp
new file mode 100644
index 000000000..237c8ce18
--- /dev/null
+++ b/tests/auto/blackbox/testdata/exported-property-in-disabled-product/main.cpp
@@ -0,0 +1 @@
+int main() {}
diff --git a/tests/auto/blackbox/testdata/exported-property-in-disabled-product/modules/broken/broken.qbs b/tests/auto/blackbox/testdata/exported-property-in-disabled-product/modules/broken/broken.qbs
new file mode 100644
index 000000000..92f01d5d3
--- /dev/null
+++ b/tests/auto/blackbox/testdata/exported-property-in-disabled-product/modules/broken/broken.qbs
@@ -0,0 +1,5 @@
+import qbs
+
+Module {
+    validate: { throw "broken!"; }
+}
diff --git a/tests/auto/blackbox/testdata/property-assignment-in-failed-module/main.cpp b/tests/auto/blackbox/testdata/property-assignment-in-failed-module/main.cpp
new file mode 100644
index 000000000..237c8ce18
--- /dev/null
+++ b/tests/auto/blackbox/testdata/property-assignment-in-failed-module/main.cpp
@@ -0,0 +1 @@
+int main() {}
diff --git a/tests/auto/blackbox/testdata/property-assignment-in-failed-module/modules/m/m.qbs b/tests/auto/blackbox/testdata/property-assignment-in-failed-module/modules/m/m.qbs
new file mode 100644
index 000000000..38b15d807
--- /dev/null
+++ b/tests/auto/blackbox/testdata/property-assignment-in-failed-module/modules/m/m.qbs
@@ -0,0 +1,11 @@
+import qbs
+
+Module {
+    property bool doFail
+    Depends { name: "cpp" }
+    cpp.dynamicLibraries: ["nosuchlib"]
+    validate: {
+        if (doFail)
+            throw "Failure!";
+    }
+}
diff --git a/tests/auto/blackbox/testdata/property-assignment-in-failed-module/property-assignment-in-failed-module.qbs b/tests/auto/blackbox/testdata/property-assignment-in-failed-module/property-assignment-in-failed-module.qbs
new file mode 100644
index 000000000..8a3a6f359
--- /dev/null
+++ b/tests/auto/blackbox/testdata/property-assignment-in-failed-module/property-assignment-in-failed-module.qbs
@@ -0,0 +1,7 @@
+import qbs
+
+CppApplication {
+    name: "app"
+    Depends { name: "m"; required: false }
+    files: "main.cpp"
+}
diff --git a/tests/auto/blackbox/tst_blackbox.cpp b/tests/auto/blackbox/tst_blackbox.cpp
index b8cea5294..2677b041e 100644
--- a/tests/auto/blackbox/tst_blackbox.cpp
+++ b/tests/auto/blackbox/tst_blackbox.cpp
@@ -2552,6 +2552,19 @@ void TestBlackbox::propertyAssignmentOnNonPresentModule()
     QVERIFY2(m_qbsStderr.isEmpty(), m_qbsStderr.constData());
 }
 
+void TestBlackbox::propertyAssignmentInFailedModule()
+{
+    QDir::setCurrent(testDataDir + "/property-assignment-in-failed-module");
+    QCOMPARE(runQbs(QbsRunParameters("resolve", QStringList("modules.m.doFail:false"))), 0);
+    QbsRunParameters failParams;
+    failParams.expectFailure = true;
+    QVERIFY(runQbs(failParams) != 0);
+    QCOMPARE(runQbs(QbsRunParameters("resolve", QStringList("modules.m.doFail:true"))), 0);
+    QVERIFY2(m_qbsStdout.contains("Resolving"), m_qbsStdout.constData());
+    QEXPECT_FAIL(0, "circular dependency between module merging and validation", Continue);
+    QCOMPARE(runQbs(failParams), 0);
+}
+
 void TestBlackbox::propertyChanges()
 {
     QDir::setCurrent(testDataDir + "/propertyChanges");
@@ -3008,6 +3021,53 @@ void TestBlackbox::escapedLinkerFlags()
     QVERIFY2(m_qbsStderr.contains("Encountered escaped linker flag"), m_qbsStderr.constData());
 }
 
+void TestBlackbox::exportedDependencyInDisabledProduct()
+{
+    QDir::setCurrent(testDataDir + "/exported-dependency-in-disabled-product");
+    QFETCH(QString, depCondition);
+    QFETCH(bool, compileExpected);
+    rmDirR(relativeBuildDir());
+    const QString propertyArg = "products.dep.conditionString:" + depCondition;
+    QCOMPARE(runQbs(QStringList(propertyArg)), 0);
+    QEXPECT_FAIL("dependency directly disabled", "QBS-1250", Continue);
+    QEXPECT_FAIL("dependency disabled via non-present module", "QBS-1250", Continue);
+    QEXPECT_FAIL("dependency disabled via failed module", "QBS-1250", Continue);
+    QCOMPARE(m_qbsStdout.contains("compiling"), compileExpected);
+}
+
+void TestBlackbox::exportedDependencyInDisabledProduct_data()
+{
+    QTest::addColumn<QString>("depCondition");
+    QTest::addColumn<bool>("compileExpected");
+    QTest::newRow("dependency enabled") << "true" << true;
+    QTest::newRow("dependency directly disabled") << "false" << false;
+    QTest::newRow("dependency disabled via non-present module") << "nosuchmodule.present" << false;
+    QTest::newRow("dependency disabled via failed module") << "broken.present" << false;
+}
+
+void TestBlackbox::exportedPropertyInDisabledProduct()
+{
+    QDir::setCurrent(testDataDir + "/exported-property-in-disabled-product");
+    QFETCH(QString, depCondition);
+    QFETCH(bool, successExpected);
+    const QString propertyArg = "products.dep.conditionString:" + depCondition;
+    QCOMPARE(runQbs(QbsRunParameters("resolve", QStringList(propertyArg))), 0);
+    QVERIFY2(m_qbsStdout.contains("Resolving"), m_qbsStdout.constData());
+    QbsRunParameters buildParams;
+    buildParams.expectFailure = !successExpected;
+    QCOMPARE(runQbs(buildParams) == 0, successExpected);
+}
+
+void TestBlackbox::exportedPropertyInDisabledProduct_data()
+{
+    QTest::addColumn<QString>("depCondition");
+    QTest::addColumn<bool>("successExpected");
+    QTest::newRow("dependency enabled") << "true" << false;
+    QTest::newRow("dependency directly disabled") << "false" << true;
+    QTest::newRow("dependency disabled via non-present module") << "nosuchmodule.present" << true;
+    QTest::newRow("dependency disabled via failed module") << "broken.present" << true;
+}
+
 void TestBlackbox::systemRunPaths()
 {
     const SettingsPtr s = settings();
diff --git a/tests/auto/blackbox/tst_blackbox.h b/tests/auto/blackbox/tst_blackbox.h
index 6386491ac..b31495ad0 100644
--- a/tests/auto/blackbox/tst_blackbox.h
+++ b/tests/auto/blackbox/tst_blackbox.h
@@ -87,6 +87,10 @@ private slots:
     void erroneousFiles();
     void errorInfo();
     void escapedLinkerFlags();
+    void exportedDependencyInDisabledProduct();
+    void exportedDependencyInDisabledProduct_data();
+    void exportedPropertyInDisabledProduct();
+    void exportedPropertyInDisabledProduct_data();
     void exportRule();
     void exportToOutsideSearchPath();
     void externalLibs();
@@ -161,6 +165,7 @@ private slots:
     void productDependenciesByType();
     void productProperties();
     void propertyAssignmentOnNonPresentModule();
+    void propertyAssignmentInFailedModule();
     void propertyChanges();
     void propertyPrecedence();
     void properQuoting();