Rewrite ModuleLoader

===================
Problem description
===================
The ModuleLoader class as it exists before this patch has a number of
serious problems:
  - It instantiates modules over and over again everywhere a
    Depends item appears. The different instances are later
    merged in a hopelessly obscure and error-prone way.
  - It seriously pollutes the module scopes so that sloppily written
    project files appear to work even though they shouldn't.
  - Dependencies between products are handled twice: Once using the
    normal module instantiation code (with the Export item
    acting like a Module item), and also with a parallel mechanism
    that does strange, seemingly redundant things especially regarding
    transitive dependencies, which appear to introduce enormous
    run-time overhead. It is also split up between ModuleLoader and
    ProjectResolver, adding even more confusion.
  - The code is messy, seriously under-documented and hardly
    understood even by its original authors. It presents a huge obstacle
    to potential contributors.

=================
Patch description
=================
- Each module is instantiated once per product. Property values are
  merged on the fly. Special handling for dependencies between
  products are kept to the absolutely required minimum.
- There are no more extra passes for establishing inter-product
  dependencies. Instead, whenever an unhandled dependency is
  encountered, processing the current product is paused and resumed
  once the dependency is ready, with the product state getting saved
  and restored in between so no work is done twice.
- The ModuleLoader class now really only locates and loads modules.
  The new main class is called ProjectTreeBuilder, and we have split
  off small, self-contained pieces wherever possible. This process
  will be continued in follow-up patches (see next section).

=======
Outlook
=======
The ProjectTreeBuilder ist still too large and should be split up
further into small, easily digestible parts with clear responsibilities,
until the top-level code becomes tidy and self-documenting. In the end,
it can probably be merged with ProjectResolver and Loader.
However, this first requires the tight coupling between
ProjectTreeBuilder/ModuleProviderLoader/ProbesResolver/ProjectResolver
to be broken; otherwise we can't produce clean interfaces. As this would
involve touching a lot of otherwise unrelated code, it is out of scope
for this patch.

=================
Benchmarking data
=================

We first present wall-time clock results gathered by running
"qbs resolve --log-time" for qbs itself and Qt Creator on macOS
and Windows. The numbers are the average of several runs, with
outliers removed.
Then the same for a simple QML project using a static Qt on Linux
(this case is special because our current way of handling plugins
causes a huge amount of modules to be loaded).
Finally, we show the output of the qbs_benchmarker tool for
resolving qbs and Qt Creator on Linux.
The data shows a speed-up that is hardly noticeable for simple
projects, but increases sharply with project complexity.
This suggests that our new way of resolving does not suffer
anymore from the non-linear slowdown when the number
of dependencies gets large.

Resolving qbs on Windows:
  Before this patch: ModuleLoader 3.6s, ProjectResolver 870ms
  With this patch: ProjectTreeBuilder 3.6s, ProjectResolver 840ms
Resolving Qt Creator on Windows:
  Before this patch: ModuleLoader 17s, ProjectResolver 6.8s
  With this patch: ProjectTreeBuilder 10.0s, ProjectResolver 6.5s
Resolving qbs on macOS:
  Before this patch: ModuleLoader 4.0s, ProjectResolver 2.3s
  With this patch: ProjectTreeBuilder 4.0s, ProjectResolver 2.3s
Resolving Qt Creator on macOS:
  Before this patch: ModuleLoader 32.0s, ProjectResolver 15.6s
  With this patch: ProjectTreeBuilder 23.0s, ProjectResolver 15.3s

Note that the above numbers are for an initial resolve, so they include
the time for running Probes. The speed-up for re-resolving (with cached
Probes) is even higher, in particular on macOS, where Probes take
excessively long.

Resolving with static Qt on Linux (QBS-1521):
  Before this patch: ModuleLoader 36s, ProjectResolver 18s
  With this patch: ProjectTreeBuilder 1.5s, ProjectResolver 14s

Output of qbs_benchmarker for resolving qbs on Linux:
    Old instruction count: 10029744668
    New instruction count: 9079802661
    Relative change: -10 %
    Old peak memory usage: 69881840 Bytes
    New peak memory usage: 82434624 Bytes
    Relative change: +17 %

Output of qbs_benchmarker for resolving Qt Creator on Linux:
    Old instruction count: 87364681688
    New instruction count: 53634332869
    Relative change: -39 %
    Old peak memory usage: 578458840 Bytes
    New peak memory usage: 567271960 Bytes
    Relative change: -2 %

I don't know where the increased memory footprint for a small project
comes from, but since it goes away for larger projects, it doesn't seem
worth investigating.

Fixes: QBS-1037
Task-number: QBS-1521
Change-Id: Ieeebce8a7ff68cdffc15d645e2342ece2426fa94
Reviewed-by: Ivan Komissarov <ABBAPOH@gmail.com>
Reviewed-by: Joerg Bornemann <joerg.bornemann@qt.io>

10 files changed, 119 insertions, 28 deletions

diff --git a/tests/auto/api/tst_api.cpp b/tests/auto/api/tst_api.cpp
index 28bbe4638..d85b2502a 100644
--- a/tests/auto/api/tst_api.cpp
+++ b/tests/auto/api/tst_api.cpp
@@ -1434,7 +1434,7 @@ void TestApi::infiniteLoopResolving()
                                                                               m_logSink, nullptr));
     QTimer::singleShot(1000, setupJob.get(), &qbs::AbstractJob::cancel);
     QVERIFY(waitForFinished(setupJob.get(), testTimeoutInMsecs()));
-    QVERIFY2(setupJob->error().toString().toLower().contains("interrupted"),
+    QVERIFY2(setupJob->error().toString().toLower().contains("cancel"),
              qPrintable(setupJob->error().toString()));
 }
 
@@ -2711,12 +2711,13 @@ void TestApi::restoredWarnings()
     waitForFinished(job.get());
     QVERIFY2(!job->error().hasError(), qPrintable(job->error().toString()));
     job.reset(nullptr);
-    QCOMPARE(toSet(m_logSink->warnings).size(), 2);
+    QCOMPARE(toSet(m_logSink->warnings).size(), 3);
     const auto beforeErrors = m_logSink->warnings;
     for (const qbs::ErrorInfo &e : beforeErrors) {
         const QString msg = e.toString();
         QVERIFY2(msg.contains("Superfluous version")
-                 || msg.contains("Property 'blubb' is not declared"),
+                 || msg.contains("Property 'blubb' is not declared")
+                 || msg.contains("Product 'theProduct' had errors and was disabled"),
                  qPrintable(msg));
     }
     m_logSink->warnings.clear();
@@ -2726,7 +2727,7 @@ void TestApi::restoredWarnings()
     waitForFinished(job.get());
     QVERIFY2(!job->error().hasError(), qPrintable(job->error().toString()));
     job.reset(nullptr);
-    QCOMPARE(toSet(m_logSink->warnings).size(), 2);
+    QCOMPARE(toSet(m_logSink->warnings).size(), 3);
     m_logSink->warnings.clear();
 
     // Re-resolving with changes: Errors come from the re-resolving, stored ones must be suppressed.
@@ -2737,13 +2738,14 @@ void TestApi::restoredWarnings()
     waitForFinished(job.get());
     QVERIFY2(!job->error().hasError(), qPrintable(job->error().toString()));
     job.reset(nullptr);
-    QCOMPARE(toSet(m_logSink->warnings).size(), 3); // One more for the additional group
+    QCOMPARE(toSet(m_logSink->warnings).size(), 4); // One more for the additional group
     const auto afterErrors = m_logSink->warnings;
     for (const qbs::ErrorInfo &e : afterErrors) {
         const QString msg = e.toString();
         QVERIFY2(msg.contains("Superfluous version")
                  || msg.contains("Property 'blubb' is not declared")
-                 || msg.contains("blubb.cpp' does not exist"),
+                 || msg.contains("blubb.cpp' does not exist")
+                 || msg.contains("Product 'theProduct' had errors and was disabled"),
                  qPrintable(msg));
     }
     m_logSink->warnings.clear();
diff --git a/tests/auto/blackbox/testdata/conflicting-property-values/conflicting-property-values.qbs b/tests/auto/blackbox/testdata/conflicting-property-values/conflicting-property-values.qbs
new file mode 100644
index 000000000..23b6ee5a3
--- /dev/null
+++ b/tests/auto/blackbox/testdata/conflicting-property-values/conflicting-property-values.qbs
@@ -0,0 +1,41 @@
+Project {
+    Product {
+        name: "low"
+        Export { property string prop: "low"; property string prop2: "low" }
+    }
+    Product {
+        name: "higher1"
+        Export { Depends { name: "low" } low.prop: "higher1" }
+    }
+    Product {
+        name: "higher2"
+        Export { Depends { name: "low" } low.prop: "higher2" }
+    }
+    Product {
+        name: "highest1"
+        Export {
+            Depends { name: "low" }
+            Depends { name: "higher1" }
+            Depends { name: "higher2" }
+            low.prop: "highest1"
+            low.prop2: "highest"
+        }
+    }
+    Product {
+        name: "highest2"
+        Export {
+            Depends { name: "low" }
+            Depends { name: "higher1" }
+            Depends { name: "higher2" }
+            low.prop: "highest2"
+            low.prop2: "highest"
+        }
+    }
+    Product {
+        name: "toplevel"
+        Depends { name: "highest1" }
+        Depends { name: "highest2" }
+        low.prop: name
+        property bool dummy: { console.info("final prop value: " + low.prop); }
+    }
+}
diff --git a/tests/auto/blackbox/testdata/exports-qbs/lib.qbs b/tests/auto/blackbox/testdata/exports-qbs/lib.qbs
index 951b0fa74..01f89a221 100644
--- a/tests/auto/blackbox/testdata/exports-qbs/lib.qbs
+++ b/tests/auto/blackbox/testdata/exports-qbs/lib.qbs
@@ -52,7 +52,7 @@ DynamicLibrary {
             condition: true
             prefixMapping: [{
                 prefix: includeDir,
-                replacement: FileInfo.joinPaths(qbs.installPrefix, exportingProduct.headersInstallDir)
+                replacement: FileInfo.joinPaths(exportingProduct.qbs.installPrefix, exportingProduct.headersInstallDir)
             }]
         }
     }
diff --git a/tests/auto/blackbox/testdata/qbs-module-properties-in-providers/qbs-module-properties-in-providers.qbs b/tests/auto/blackbox/testdata/qbs-module-properties-in-providers/qbs-module-properties-in-providers.qbs
index a338a220d..c2fc58299 100644
--- a/tests/auto/blackbox/testdata/qbs-module-properties-in-providers/qbs-module-properties-in-providers.qbs
+++ b/tests/auto/blackbox/testdata/qbs-module-properties-in-providers/qbs-module-properties-in-providers.qbs
@@ -4,12 +4,12 @@ Project {
 
     Profile {
         name: "profile1"
-        qbs.sysroot: "sysroot1"
+        qbs.sysroot: "/sysroot1"
     }
 
     Profile {
         name: "profile2"
-        qbs.sysroot: "sysroot2"
+        qbs.sysroot: "/sysroot2"
     }
 
     Product {
diff --git a/tests/auto/blackbox/tst_blackbox.cpp b/tests/auto/blackbox/tst_blackbox.cpp
index fd81dd494..d7c15fab6 100644
--- a/tests/auto/blackbox/tst_blackbox.cpp
+++ b/tests/auto/blackbox/tst_blackbox.cpp
@@ -1961,6 +1961,42 @@ void TestBlackbox::conanfileProbe()
     QCOMPARE(actualResults, expectedResults);
 }
 
+void TestBlackbox::conflictingPropertyValues_data()
+{
+    QTest::addColumn<bool>("overrideInProduct");
+    QTest::newRow("don't override in product") << false;
+    QTest::newRow("override in product") << true;
+}
+
+void TestBlackbox::conflictingPropertyValues()
+{
+    QFETCH(bool, overrideInProduct);
+
+    QDir::setCurrent(testDataDir + "/conflicting-property-values");
+    if (overrideInProduct)
+        REPLACE_IN_FILE("conflicting-property-values.qbs", "// low.prop: name", "low.prop: name");
+    else
+        REPLACE_IN_FILE("conflicting-property-values.qbs", "low.prop: name", "// low.prop: name");
+    WAIT_FOR_NEW_TIMESTAMP();
+    QCOMPARE(runQbs(QString("resolve")), 0);
+    if (overrideInProduct) {
+        // Binding in product itself overrides everything else, module-level conflicts
+        // are irrelevant.
+        QVERIFY2(m_qbsStdout.contains("final prop value: toplevel"), m_qbsStdout.constData());
+        QVERIFY2(m_qbsStderr.isEmpty(), m_qbsStderr.constData());
+    } else {
+        // Only the conflicts in the highest-level modules are reported, lower-level conflicts
+        // are irrelevant.
+        // prop2 does not cause a conflict, because the values are the same.
+        QVERIFY2(m_qbsStdout.contains("final prop value: highest"), m_qbsStdout.constData());
+        QVERIFY2(m_qbsStderr.contains("Conflicting scalar values for property 'prop'"),
+                 m_qbsStderr.constData());
+        QVERIFY2(m_qbsStderr.count("values.qbs") == 2, m_qbsStderr.constData());
+        QVERIFY2(m_qbsStderr.contains("values.qbs:20:23"), m_qbsStderr.constData());
+        QVERIFY2(m_qbsStderr.contains("values.qbs:30:23"), m_qbsStderr.constData());
+    }
+}
+
 void TestBlackbox::cpuFeatures()
 {
     QDir::setCurrent(testDataDir + "/cpu-features");
@@ -3447,7 +3483,7 @@ void TestBlackbox::propertyAssignmentInFailedModule()
     QVERIFY(runQbs(failParams) != 0);
     QCOMPARE(runQbs(QbsRunParameters("resolve", QStringList("modules.m.doFail:true"))), 0);
     QVERIFY2(m_qbsStdout.contains("Resolving"), m_qbsStdout.constData());
-    QEXPECT_FAIL(nullptr, "circular dependency between module merging and validation", Continue);
+    failParams.expectFailure = false;
     QCOMPARE(runQbs(failParams), 0);
 }
 
@@ -5574,11 +5610,7 @@ void TestBlackbox::propertyPrecedence()
     switchProfileContents(profile.p, s.get(), false);
     switchFileContents(nonleafFile, true);
     QCOMPARE(runQbs(resolveParams), 0);
-    QVERIFY2(m_qbsStderr.contains("WARNING: Conflicting scalar values at")
-             && m_qbsStderr.contains("nonleaf.qbs:4:22")
-             && m_qbsStderr.contains("dep.qbs:6:26"),
-             m_qbsStderr.constData());
-    QCOMPARE(runQbs(params), 0);
+    QVERIFY2(m_qbsStderr.isEmpty(), m_qbsStderr.constData()); QCOMPARE(runQbs(params), 0);
     QVERIFY2(m_qbsStdout.contains("scalar prop: export\n")
              && m_qbsStdout.contains("list prop: [\"export\",\"nonleaf\",\"leaf\"]\n"),
              m_qbsStdout.constData());
@@ -5586,10 +5618,7 @@ void TestBlackbox::propertyPrecedence()
     // Case 8: [cmdline=0,prod=0,export=1,nonleaf=1,profile=1]
     switchProfileContents(profile.p, s.get(), true);
     QCOMPARE(runQbs(resolveParams), 0);
-    QVERIFY2(m_qbsStderr.contains("WARNING: Conflicting scalar values at")
-             && m_qbsStderr.contains("nonleaf.qbs:4:22")
-             && m_qbsStderr.contains("dep.qbs:6:26"),
-             m_qbsStderr.constData());
+    QVERIFY2(m_qbsStderr.isEmpty(), m_qbsStderr.constData());
     QCOMPARE(runQbs(params), 0);
     QVERIFY2(m_qbsStdout.contains("scalar prop: export\n")
              && m_qbsStdout.contains("list prop: [\"export\",\"nonleaf\",\"profile\"]\n"),
@@ -6177,11 +6206,11 @@ void TestBlackbox::qbsModulePropertiesInProviders()
     QCOMPARE(m_qbsStdout.count("Running setup script for qbsmetatestmodule"), 2);
 
     // Check that products get correct values from modules
-    QVERIFY2(m_qbsStdout.contains(("product1.qbsmetatestmodule.prop: sysroot1")), m_qbsStdout);
-    QVERIFY2(m_qbsStdout.contains(("product1.qbsmetatestmodule.prop: sysroot2")), m_qbsStdout);
+    QVERIFY2(m_qbsStdout.contains(("product1.qbsmetatestmodule.prop: /sysroot1")), m_qbsStdout);
+    QVERIFY2(m_qbsStdout.contains(("product1.qbsmetatestmodule.prop: /sysroot2")), m_qbsStdout);
 
-    QVERIFY2(m_qbsStdout.contains(("product2.qbsmetatestmodule.prop: sysroot1")), m_qbsStdout);
-    QVERIFY2(m_qbsStdout.contains(("product2.qbsmetatestmodule.prop: sysroot2")), m_qbsStdout);
+    QVERIFY2(m_qbsStdout.contains(("product2.qbsmetatestmodule.prop: /sysroot1")), m_qbsStdout);
+    QVERIFY2(m_qbsStdout.contains(("product2.qbsmetatestmodule.prop: /sysroot2")), m_qbsStdout);
 }
 
 // Tests whether it is possible to set qbsModuleProviders in Product and Project items
diff --git a/tests/auto/blackbox/tst_blackbox.h b/tests/auto/blackbox/tst_blackbox.h
index 8f9bfc984..f4e001360 100644
--- a/tests/auto/blackbox/tst_blackbox.h
+++ b/tests/auto/blackbox/tst_blackbox.h
@@ -90,6 +90,8 @@ private slots:
     void cxxLanguageVersion_data();
     void conanfileProbe_data();
     void conanfileProbe();
+    void conflictingPropertyValues_data();
+    void conflictingPropertyValues();
     void cpuFeatures();
     void dependenciesProperty();
     void dependencyScanningLoop();
diff --git a/tests/auto/language/testdata/erroneous/module-depends-on-product.qbs b/tests/auto/language/testdata/module-depends-on-product.qbs
index a7db9e036..a7db9e036 100644
--- a/tests/auto/language/testdata/erroneous/module-depends-on-product.qbs
+++ b/tests/auto/language/testdata/module-depends-on-product.qbs
diff --git a/tests/auto/language/testdata/erroneous/modules/module-with-product-dependency/module-with-product-dependency.qbs b/tests/auto/language/testdata/modules/module-with-product-dependency/module-with-product-dependency.qbs
index 5781bd6de..5781bd6de 100644
--- a/tests/auto/language/testdata/erroneous/modules/module-with-product-dependency/module-with-product-dependency.qbs
+++ b/tests/auto/language/testdata/modules/module-with-product-dependency/module-with-product-dependency.qbs
diff --git a/tests/auto/language/tst_language.cpp b/tests/auto/language/tst_language.cpp
index 689b3bd7f..5ab65457f 100644
--- a/tests/auto/language/tst_language.cpp
+++ b/tests/auto/language/tst_language.cpp
@@ -899,8 +899,6 @@ void TestLanguage::erroneousFiles_data()
     QTest::newRow("conflicting-module-instances")
             << "There is more than one equally prioritized candidate for module "
                "'conflicting-instances'.";
-    QTest::newRow("module-depends-on-product")
-            << "module-with-product-dependency.qbs:2:5.*Modules cannot depend on products.";
     QTest::newRow("overwrite-inherited-readonly-property")
             << "overwrite-inherited-readonly-property.qbs"
                ":2:21.*Cannot set read-only property 'readOnlyString'.";
@@ -1929,7 +1927,7 @@ void TestLanguage::modulePropertiesInGroups()
         QCOMPARE(g2Gmod1List1, QStringList() << "gmod1_list1_proto" << "g2");
         const auto &g2Gmod1List2 = moduleProperty(g2Props, "gmod.gmod1", "gmod1_list2")
                 .toStringList();
-        QCOMPARE(g2Gmod1List2, QStringList() << "grouptest" << "g2" << "gmod1_list2_proto");
+        QCOMPARE(g2Gmod1List2, QStringList() << "grouptest" << "gmod1_string_proto" << "gmod1_list2_proto");
         const int g2P0 = moduleProperty(g2Props, "gmod.gmod1", "p0").toInt();
         QCOMPARE(g2P0, 6);
         const int g2DepProp = moduleProperty(g2Props, "gmod.gmod1", "depProp").toInt();
@@ -2095,7 +2093,25 @@ void TestLanguage::moduleScope()
         qDebug() << e.toString();
     }
     QCOMPARE(exceptionCaught, false);
+}
 
+void TestLanguage::moduleWithProductDependency()
+{
+    bool exceptionCaught = false;
+    try {
+        defaultParameters.setProjectFilePath(testProject("module-depends-on-product.qbs"));
+        TopLevelProjectPtr project = loader->loadProject(defaultParameters);
+        QVERIFY(project);
+        QHash<QString, ResolvedProductPtr> products = productsFromProject(project);
+        QCOMPARE(products.size(), 2);
+        ResolvedProductPtr product = products.value("p1");
+        QVERIFY(product);
+        QCOMPARE(int(product->dependencies.size()), 1);
+    } catch (const ErrorInfo &e) {
+        exceptionCaught = true;
+        qDebug() << e.toString();
+    }
+    QCOMPARE(exceptionCaught, false);
 }
 
 void TestLanguage::modules_data()
@@ -2821,8 +2837,8 @@ void TestLanguage::qbsPropertyConvenienceOverride()
         QCOMPARE(project->products.size(), size_t(1));
         QCOMPARE(project->products.front()->moduleProperties->qbsPropertyValue("installPrefix")
                  .toString(), QString("/opt"));
-    }
-    catch (const ErrorInfo &e) {
+    } catch (const ErrorInfo &e) {
+        exceptionCaught = true;
         qDebug() << e.toString();
     }
     QCOMPARE(exceptionCaught, false);
diff --git a/tests/auto/language/tst_language.h b/tests/auto/language/tst_language.h
index 4fe2752e4..3689e0c61 100644
--- a/tests/auto/language/tst_language.h
+++ b/tests/auto/language/tst_language.h
@@ -137,6 +137,7 @@ private slots:
     void modulePropertiesInGroups();
     void modulePropertyOverridesPerProduct();
     void moduleScope();
+    void moduleWithProductDependency();
     void modules_data();
     void modules();
     void multiplexedExports();