fix look-up of scalar properties

The property map of a product used to contain all modules of the
dependency tree as direct children. The same was true for the
product's Item. This setup was done in
ModuleLoader::createAdditionalModuleInstancesInProduct.
The whole approach led to problems when retrieving module values
from modules deeper in the dependency hierarchy. Basic idea of this
fix is to make ProjectResolver::resolveModule recursively walk the
module dependency tree instead of trying to gather information on the
root level.

Change-Id: Icf1325272f90d7924437f41ca6f403d7264ce84c
Task-number: QBS-726
Task-number: QBS-706
Reviewed-by: Christian Kandeler <christian.kandeler@theqtcompany.com>

6 files changed, 59 insertions, 37 deletions

diff --git a/src/lib/corelib/language/language.cpp b/src/lib/corelib/language/language.cpp
index 5a4bd4a5a..e90b31907 100644
--- a/src/lib/corelib/language/language.cpp
+++ b/src/lib/corelib/language/language.cpp
@@ -548,6 +548,29 @@ enum EnvType
     BuildEnv, RunEnv
 };
 
+static bool findModuleMapRecursively_impl(const QVariantMap &cfg, const QString &moduleName,
+        QVariantMap *result)
+{
+    for (QVariantMap::const_iterator it = cfg.constBegin(); it != cfg.constEnd(); ++it) {
+        if (it.key() == moduleName) {
+            *result = it.value().toMap();
+            return true;
+        }
+        if (findModuleMapRecursively_impl(it.value().toMap().value("modules").toMap(), moduleName,
+                                          result)) {
+            return true;
+        }
+    }
+    return false;
+}
+
+static QVariantMap findModuleMapRecursively(const QVariantMap &cfg, const QString &moduleName)
+{
+    QVariantMap result;
+    findModuleMapRecursively_impl(cfg, moduleName, &result);
+    return result;
+}
+
 static QProcessEnvironment getProcessEnvironment(ScriptEngine *engine, EnvType envType,
                                                  const QList<ResolvedModuleConstPtr> &modules,
                                                  const PropertyMapConstPtr &productConfiguration,
@@ -627,7 +650,7 @@ static QProcessEnvironment getProcessEnvironment(ScriptEngine *engine, EnvType e
         }
 
         // expose the module's properties
-        QVariantMap moduleCfg = productModules.value(module->name).toMap();
+        QVariantMap moduleCfg = findModuleMapRecursively(productModules, module->name);
         for (QVariantMap::const_iterator it = moduleCfg.constBegin(); it != moduleCfg.constEnd(); ++it)
             scope.setProperty(it.key(), engine->toScriptValue(it.value()));
 
diff --git a/src/lib/corelib/language/moduleloader.cpp b/src/lib/corelib/language/moduleloader.cpp
index b17900b26..bb321b441 100644
--- a/src/lib/corelib/language/moduleloader.cpp
+++ b/src/lib/corelib/language/moduleloader.cpp
@@ -434,7 +434,6 @@ void ModuleLoader::handleProduct(ProjectContext *projectContext, Item *item)
     setScopeForDescendants(item, productContext.scope);
     resolveDependencies(&dependsContext, item);
     checkItemCondition(item);
-    createAdditionalModuleInstancesInProduct(&productContext);
 
     foreach (Item *child, item->children()) {
         if (child->typeName() == QLatin1String("Group"))
@@ -518,28 +517,6 @@ void ModuleLoader::handleSubProject(ModuleLoader::ProjectContext *projectContext
                   QSet<QString>(referencedFilePaths) << subProjectFilePath);
 }
 
-void ModuleLoader::createAdditionalModuleInstancesInProduct(ProductContext *productContext)
-{
-    Item::Modules modulesToCheck;
-    QSet<QStringList> modulesInProduct;
-    foreach (const Item::Module &module, productContext->item->modules()) {
-        modulesInProduct += module.name;
-        modulesToCheck += module.item->prototype()->modules();
-    }
-    while (!modulesToCheck.isEmpty()) {
-        Item::Module module = modulesToCheck.takeFirst();
-        if (modulesInProduct.contains(module.name))
-            continue;
-        modulesInProduct += module.name;
-        modulesToCheck += module.item->prototype()->modules();
-        Item *instance = Item::create(m_pool);
-        instantiateModule(productContext, productContext->item, instance, module.item->prototype(),
-                          module.name);
-        module.item = instance;
-        productContext->item->modules().append(module);
-    }
-}
-
 void ModuleLoader::handleGroup(ProductContext *productContext, Item *item)
 {
     checkCancelation();
@@ -1036,11 +1013,7 @@ void ModuleLoader::instantiateModule(ProductContext *productContext, Item *insta
             }
             QBS_CHECK(obj == depinst);
         }
-        depinst->setPrototype(m.item);
-        depinst->setFile(m.item->file());
-        depinst->setLocation(m.item->location());
-        depinst->setTypeName(m.item->typeName());
-        depinst->setScope(moduleInstance);
+        instantiateModule(productContext, moduleInstance, depinst, m.item, m.name);
         m.item = depinst;
         moduleInstance->modules() += m;
     }
diff --git a/src/lib/corelib/language/moduleloader.h b/src/lib/corelib/language/moduleloader.h
index c072fd59c..d36f4d568 100644
--- a/src/lib/corelib/language/moduleloader.h
+++ b/src/lib/corelib/language/moduleloader.h
@@ -165,7 +165,6 @@ private:
     void initProductProperties(const ProjectContext *project, Item *item);
     void handleSubProject(ProjectContext *projectContext, Item *item,
             const QSet<QString> &referencedFilePaths);
-    void createAdditionalModuleInstancesInProduct(ProductContext *productContext);
     void handleGroup(ProductContext *productContext, Item *group);
     void deferExportItem(ProductContext *productContext, Item *item);
     void mergeExportItems(ProductContext *productContext);
diff --git a/src/lib/corelib/language/projectresolver.cpp b/src/lib/corelib/language/projectresolver.cpp
index 14eacd2ea..0bf8b5ed3 100644
--- a/src/lib/corelib/language/projectresolver.cpp
+++ b/src/lib/corelib/language/projectresolver.cpp
@@ -50,6 +50,7 @@
 
 #include <QFileInfo>
 #include <QDir>
+#include <QQueue>
 #include <QSet>
 
 #include <algorithm>
@@ -363,9 +364,7 @@ void ProjectResolver::resolveProduct(Item *item, ProjectContext *projectContext)
     foreach (Item *child, subItems)
         callItemFunction(mapping, child, projectContext);
 
-    foreach (const Item::Module &module, item->modules())
-        resolveModule(module.name, module.item, projectContext);
-
+    resolveModules(item, projectContext);
     product->fileTags += productContext.additionalFileTags;
 
     foreach (const ResolvedTransformerPtr &transformer, product->transformers) {
@@ -384,12 +383,33 @@ void ProjectResolver::resolveProduct(Item *item, ProjectContext *projectContext)
         m_progressObserver->incrementProgressValue();
 }
 
+void ProjectResolver::resolveModules(const Item *item, ProjectContext *projectContext)
+{
+    // Breadth first search needed here, because the product might set properties on the cpp module,
+    // whose children must be evaluated in that context then.
+    QQueue<Item::Module> modules;
+    foreach (const Item::Module &m, item->modules())
+        modules.enqueue(m);
+    QSet<QStringList> seen;
+    while (!modules.isEmpty()) {
+        const Item::Module m = modules.takeFirst();
+        if (seen.contains(m.name))
+            continue;
+        seen.insert(m.name);
+        resolveModule(m.name, m.item, projectContext);
+        foreach (const Item::Module &childModule, m.item->modules())
+            modules.enqueue(childModule);
+    }
+}
+
 void ProjectResolver::resolveModule(const QStringList &moduleName, Item *item,
                                     ProjectContext *projectContext)
 {
     checkCancelation();
     if (!m_evaluator->boolValue(item, QLatin1String("present")))
         return;
+
+    ModuleContext * const oldModuleContext = m_moduleContext;
     ModuleContext moduleContext;
     moduleContext.module = ResolvedModule::create();
     m_moduleContext = &moduleContext;
@@ -420,7 +440,7 @@ void ProjectResolver::resolveModule(const QStringList &moduleName, Item *item,
     foreach (Item *child, item->children())
         callItemFunction(mapping, child, projectContext);
 
-    m_moduleContext = 0;
+    m_moduleContext = oldModuleContext;
 }
 
 SourceArtifactPtr ProjectResolver::createSourceArtifact(const ResolvedProductConstPtr &rproduct,
@@ -1063,11 +1083,17 @@ void ProjectResolver::evaluateModuleValues(Item *item, QVariantMap *modulesMap,
         bool lookupPrototype) const
 {
     checkCancelation();
+    QSet<QStringList> seenModuleNames;
+
     for (Item::Modules::const_iterator it = item->modules().constBegin();
          it != item->modules().constEnd(); ++it)
     {
-        QVariantMap depmods;
         const Item::Module &module = *it;
+        if (seenModuleNames.contains(module.name))
+            continue;
+        seenModuleNames << module.name;
+
+        QVariantMap depmods;
         evaluateModuleValues(module.item, &depmods, lookupPrototype);
         QVariantMap dep = evaluateProperties(module.item, lookupPrototype);
         dep.insert(QLatin1String("modules"), depmods);
diff --git a/src/lib/corelib/language/projectresolver.h b/src/lib/corelib/language/projectresolver.h
index d90c0507c..85edf1d07 100644
--- a/src/lib/corelib/language/projectresolver.h
+++ b/src/lib/corelib/language/projectresolver.h
@@ -104,6 +104,7 @@ private:
     void resolveProject(Item *item, ProjectContext *projectContext);
     void resolveSubProject(Item *item, ProjectContext *projectContext);
     void resolveProduct(Item *item, ProjectContext *projectContext);
+    void resolveModules(const Item *item, ProjectContext *projectContext);
     void resolveModule(const QStringList &moduleName, Item *item, ProjectContext *projectContext);
     void resolveGroup(Item *item, ProjectContext *projectContext);
     void resolveRule(Item *item, ProjectContext *projectContext);
diff --git a/src/lib/corelib/language/tst_language.cpp b/src/lib/corelib/language/tst_language.cpp
index d50592e03..9e314e543 100644
--- a/src/lib/corelib/language/tst_language.cpp
+++ b/src/lib/corelib/language/tst_language.cpp
@@ -873,10 +873,10 @@ void TestLanguage::moduleProperties_data()
     QTest::newRow("init") << QString() << QStringList();
     QTest::newRow("merge_lists")
             << "defines"
-            << (QStringList() << "THE_PRODUCT" << "QT_CORE" << "QT_GUI" << "QT_NETWORK");
+            << (QStringList() << "THE_PRODUCT" << "QT_GUI" << "QT_CORE" << "QT_NETWORK");
     QTest::newRow("merge_lists_and_values")
             << "defines"
-            << (QStringList() << "THE_PRODUCT" << "QT_CORE" << "QT_GUI" << "QT_NETWORK");
+            << (QStringList() << "THE_PRODUCT" << "QT_GUI" << "QT_CORE" << "QT_NETWORK");
     QTest::newRow("merge_lists_with_duplicates")
             << "cxxFlags"
             << (QStringList() << "-foo" << "BAR" << "-foo" << "BAZ");