Even better handling of property changes when restoring a build graph.

In detail:
    - Fix the build config comparison function so that it not only
checks whether keys present in both maps are the same, but also whether
there are additional keys in one of the maps.
    - Do not abort loading a restored build graph when the comparison
fails; instead, handle that condition the same way we handle changes in
build system files, environment variables etc; that is, re-resolve the
project and check whether the build data needs to be set up from
scratch for any products.
    - Take overridden properties into account when comparing the
old and new property maps. This has no effect on the command line
client at the moment, because it does its own merging, but
other API clients will benefit.
A particularly nice "side effect" of this patch is that changing
properties on the command line now works as expected with regards to
change tracking.

Task-number: QBS-374
Change-Id: I55583fb7a2ae78b80de225b09aa559229d851339
Reviewed-by: Joerg Bornemann <joerg.bornemann@digia.com>

1 files changed, 18 insertions, 20 deletions

diff --git a/src/lib/buildgraph/buildgraphloader.cpp b/src/lib/buildgraph/buildgraphloader.cpp
index 38f852853..989c85d17 100644
--- a/src/lib/buildgraph/buildgraphloader.cpp
+++ b/src/lib/buildgraph/buildgraphloader.cpp
@@ -55,18 +55,19 @@ BuildGraphLoader::BuildGraphLoader(const QProcessEnvironment &env, const Logger
 {
 }
 
-static bool isConfigCompatible(const QVariantMap &userCfg, const QVariantMap &projectCfg)
+static bool isConfigCompatible(const QVariantMap &cfg1, const QVariantMap &cfg2)
 {
-    QVariantMap::const_iterator it = userCfg.begin();
-    for (; it != userCfg.end(); ++it) {
+    if (cfg1.count() != cfg2.count())
+        return false;
+    QVariantMap::const_iterator it = cfg1.begin();
+    for (; it != cfg1.end(); ++it) {
         if (it.value().type() == QVariant::Map) {
-            if (!isConfigCompatible(it.value().toMap(), projectCfg.value(it.key()).toMap()))
+            if (!isConfigCompatible(it.value().toMap(), cfg2.value(it.key()).toMap()))
                 return false;
         } else {
-            QVariant value = projectCfg.value(it.key());
-            if (!value.isNull() && value != it.value()) {
+            QVariant value = cfg2.value(it.key());
+            if (value != it.value())
                 return false;
-            }
         }
     }
     return true;
@@ -111,15 +112,6 @@ BuildGraphLoadResult BuildGraphLoader::load(const SetupProjectParameters &parame
         return m_result;
     }
 
-    if (!isConfigCompatible(parameters.buildConfigurationTree(), pool.headData().projectConfig)) {
-        const QString message = Tr::tr("Cannot use stored build graph at '%1':"
-                "Incompatible project configuration.").arg(buildGraphFilePath);
-        if (parameters.restoreBehavior() == SetupProjectParameters::RestoreOnly)
-            throw ErrorInfo(message);
-        m_logger.qbsInfo() << message;
-        return m_result;
-    }
-
     const TopLevelProjectPtr project = TopLevelProject::create();
 
     // TODO: Store some meta data that will enable us to show actual progress (e.g. number of products).
@@ -156,20 +148,26 @@ BuildGraphLoadResult BuildGraphLoader::load(const SetupProjectParameters &parame
         return m_result;
     QBS_CHECK(parameters.restoreBehavior() == SetupProjectParameters::RestoreAndTrackChanges);
 
-    trackProjectChanges(parameters, buildGraphFilePath, project);
+    trackProjectChanges(parameters, buildGraphFilePath, project, pool.headData().projectConfig);
     return m_result;
 }
 
 void BuildGraphLoader::trackProjectChanges(const SetupProjectParameters &parameters,
-        const QString &buildGraphFilePath, const TopLevelProjectPtr &restoredProject)
+        const QString &buildGraphFilePath, const TopLevelProjectPtr &restoredProject,
+        const QVariantMap &oldProjectConfig)
 {
     const FileTime buildGraphTimeStamp = FileInfo(buildGraphFilePath).lastModified();
     QSet<QString> buildSystemFiles = restoredProject->buildSystemFiles;
     QList<ResolvedProductPtr> allRestoredProducts = restoredProject->allProducts();
     QList<ResolvedProductPtr> changedProducts;
     QList<ResolvedProductPtr> productsWithChangedFiles;
-    bool reResolvingNecessary = hasProductFileChanged(allRestoredProducts, buildGraphTimeStamp,
-                                                      buildSystemFiles, productsWithChangedFiles);
+    bool reResolvingNecessary = false;
+    if (!isConfigCompatible(parameters.finalBuildConfigurationTree(), oldProjectConfig))
+        reResolvingNecessary = true;
+    if (hasProductFileChanged(allRestoredProducts, buildGraphTimeStamp,
+                              buildSystemFiles, productsWithChangedFiles)) {
+        reResolvingNecessary = true;
+    }
 
     // "External" changes, e.g. in the environment or in a JavaScript file,
     // can make the list of source files in a product change without the respective file