Fix restoring product build data.

- The main problem was that the leaf rule/root rule connections were
  done before all product build data was restored, leading to the
  "enabled == true <=> build data != null" invariant being broken,
  which resulted in a crash (only triggerable with release builds).
- In addition, the FindRootRules visitor connected too many rules, as
  it descended into child rules.

Change-Id: I2fd575ed6c41ad29cc20ea651f4388880ef86634
Reviewed-by: Joerg Bornemann <joerg.bornemann@digia.com>

2 files changed, 98 insertions, 97 deletions

diff --git a/src/lib/corelib/buildgraph/projectbuilddata.cpp b/src/lib/corelib/buildgraph/projectbuilddata.cpp
index 96d0e925c..cea83b3cb 100644
--- a/src/lib/corelib/buildgraph/projectbuilddata.cpp
+++ b/src/lib/corelib/buildgraph/projectbuilddata.cpp
@@ -49,6 +49,89 @@
 namespace qbs {
 namespace Internal {
 
+static QSet<ResolvedProductPtr> findDependentProducts(const ResolvedProductPtr &product)
+{
+    QSet<ResolvedProductPtr> result;
+    foreach (const ResolvedProductPtr &parent, product->topLevelProject()->allProducts()) {
+        if (parent->dependencies.contains(product))
+            result += parent;
+    }
+    return result;
+}
+
+class FindLeafRules : public BuildGraphVisitor
+{
+public:
+    FindLeafRules()
+    {
+    }
+
+    const QSet<RuleNode *> &apply(const ResolvedProductPtr &product)
+    {
+        m_result.clear();
+        m_product = product;
+        QBS_CHECK(product->buildData);
+        foreach (BuildGraphNode *n, product->buildData->nodes)
+            n->accept(this);
+        return m_result;
+    }
+
+private:
+    virtual bool visit(Artifact *)
+    {
+        return false;
+    }
+
+    virtual bool visit(RuleNode *node)
+    {
+        if (!hasChildRuleInThisProduct(node))
+            m_result << node;
+        return false;
+    }
+
+    bool hasChildRuleInThisProduct(const RuleNode *node) const
+    {
+        foreach (BuildGraphNode *c, node->children) {
+            if (c->product == m_product && c->type() == BuildGraphNode::RuleNodeType)
+                return true;
+        }
+        return false;
+    }
+
+    ResolvedProductPtr m_product;
+    QSet<RuleNode *> m_result;
+};
+
+class FindRootRules : public BuildGraphVisitor
+{
+public:
+    FindRootRules()
+    {
+    }
+
+    const QList<RuleNode *> &apply(const ResolvedProductPtr &product)
+    {
+        m_result.clear();
+        foreach (BuildGraphNode *n, product->buildData->roots)
+            n->accept(this);
+        return m_result;
+    }
+
+private:
+    bool visit(Artifact *)
+    {
+        return false;
+    }
+
+    bool visit(RuleNode *node)
+    {
+        m_result << node;
+        return false;
+    }
+
+    QList<RuleNode *> m_result;
+};
+
 ProjectBuildData::ProjectBuildData(const ProjectBuildData *other)
     : isDirty(true), m_doCleanupInDestructor(true)
 {
@@ -249,104 +332,23 @@ void BuildDataResolver::resolveProductBuildDataForExistingProject(const TopLevel
     m_project = project;
     foreach (const ResolvedProductPtr &product, freshProducts) {
         if (product->enabled)
-            resolveProductBuildDataForExistingProject(product);
+            resolveProductBuildData(product);
     }
-}
 
-static QSet<ResolvedProductPtr> findDependentProducts(const ResolvedProductPtr &product)
-{
-    QSet<ResolvedProductPtr> result;
-    foreach (const ResolvedProductPtr &parent, product->topLevelProject()->allProducts()) {
-        if (parent->dependencies.contains(product))
-            result += parent;
-    }
-    return result;
-}
-
-class FindLeafRules : public BuildGraphVisitor
-{
-public:
-    FindLeafRules()
-    {
-    }
-
-    const QSet<RuleNode *> &apply(const ResolvedProductPtr &product)
-    {
-        m_result.clear();
-        m_product = product;
-        foreach (BuildGraphNode *n, product->buildData->nodes)
-            n->accept(this);
-        return m_result;
-    }
-
-private:
-    virtual bool visit(Artifact *)
-    {
-        return false;
-    }
-
-    virtual bool visit(RuleNode *node)
-    {
-        if (!hasChildRuleInThisProduct(node))
-            m_result << node;
-        return false;
-    }
-
-    bool hasChildRuleInThisProduct(const RuleNode *node) const
-    {
-        foreach (BuildGraphNode *c, node->children) {
-            if (c->product == m_product && c->type() == BuildGraphNode::RuleNodeType)
-                return true;
-        }
-        return false;
-    }
-
-    ResolvedProductPtr m_product;
-    QSet<RuleNode *> m_result;
-};
-
-class FindRootRules : public BuildGraphVisitor
-{
-public:
-    FindRootRules()
-    {
-    }
-
-    const QList<RuleNode *> &apply(const ResolvedProductPtr &product)
-    {
-        m_result.clear();
-        foreach (BuildGraphNode *n, product->buildData->roots)
-            n->accept(this);
-        return m_result;
-    }
-
-private:
-    virtual bool visit(Artifact *)
-    {
-        return true;
-    }
-
-    virtual bool visit(RuleNode *node)
-    {
-        m_result << node;
-        return true;
-    }
-
-    QList<RuleNode *> m_result;
-};
-
-
-void BuildDataResolver::resolveProductBuildDataForExistingProject(const ResolvedProductPtr &product)
-{
-    resolveProductBuildData(product);
-
-    // Connect the leaf rules of all dependent products to the root rules of this product.
-    const QList<RuleNode *> rootRules = FindRootRules().apply(product);
-    QSet<ResolvedProductPtr> dependents = findDependentProducts(product);
-    foreach (const ResolvedProductPtr &dependentProduct, dependents) {
-        foreach (RuleNode *leaf, FindLeafRules().apply(dependentProduct)) {
-            foreach (RuleNode *root, rootRules) {
-                loggedConnect(leaf, root, m_logger);
+    // Connect the leaf rules of all dependent products to the root rules of the dependency.
+    foreach (const ResolvedProductPtr &product, freshProducts) {
+        if (!product->enabled)
+            continue;
+        QBS_CHECK(product->buildData);
+        const QList<RuleNode *> rootRules = FindRootRules().apply(product);
+        QSet<ResolvedProductPtr> dependents = findDependentProducts(product);
+        foreach (const ResolvedProductPtr &dependentProduct, dependents) {
+            if (!dependentProduct->enabled)
+                continue;
+            foreach (RuleNode *leaf, FindLeafRules().apply(dependentProduct)) {
+                foreach (RuleNode *root, rootRules) {
+                    loggedConnect(leaf, root, m_logger);
+                }
             }
         }
     }
diff --git a/src/lib/corelib/buildgraph/projectbuilddata.h b/src/lib/corelib/buildgraph/projectbuilddata.h
index afbd363d5..eecc39f27 100644
--- a/src/lib/corelib/buildgraph/projectbuilddata.h
+++ b/src/lib/corelib/buildgraph/projectbuilddata.h
@@ -96,7 +96,6 @@ public:
             const QList<ResolvedProductPtr> &freshProducts);
 
 private:
-    void resolveProductBuildDataForExistingProject(const ResolvedProductPtr &product);
     void resolveProductBuildData(const ResolvedProductPtr &product);
     RulesEvaluationContextPtr evalContext() const;
     ScriptEngine *engine() const;