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>

4 files changed, 52 insertions, 0 deletions

diff --git a/src/lib/tools/setupprojectparameters.cpp b/src/lib/tools/setupprojectparameters.cpp
index ab8d58399..3741124c6 100644
--- a/src/lib/tools/setupprojectparameters.cpp
+++ b/src/lib/tools/setupprojectparameters.cpp
@@ -62,6 +62,7 @@ public:
     QVariantMap buildConfiguration;
     mutable QVariantMap overriddenValuesTree;
     mutable QVariantMap buildConfigurationTree;
+    mutable QVariantMap finalBuildConfigtree;
     bool ignoreDifferentProjectFilePath;
     bool dryRun;
     bool logElapsedTime;
@@ -183,6 +184,7 @@ void SetupProjectParameters::setOverriddenValues(const QVariantMap &values)
     }
     d->overriddenValues = values;
     d->overriddenValuesTree.clear();
+    d->finalBuildConfigtree.clear();
 }
 
 static void provideValuesTree(const QVariantMap &values, QVariantMap *valueTree)
@@ -231,6 +233,7 @@ void SetupProjectParameters::setBuildConfiguration(const QVariantMap &buildConfi
     }
     d->buildConfiguration = buildConfiguration;
     d->buildConfigurationTree.clear();
+    d->finalBuildConfigtree.clear();
 }
 
 /*!
@@ -309,6 +312,22 @@ ErrorInfo SetupProjectParameters::expandBuildConfiguration(Settings *settings)
 }
 
 /*!
+ * \brief Returns the build configuration in tree form, with overridden values taken into account.
+ */
+QVariantMap SetupProjectParameters::finalBuildConfigurationTree() const
+{
+    if (d->finalBuildConfigtree.isEmpty()) {
+        QVariantMap finalMap = d->buildConfiguration;
+        for (QVariantMap::ConstIterator it = d->overriddenValues.constBegin();
+             it != d->overriddenValues.constEnd(); ++it) {
+            finalMap.insert(it.key(), it.value());
+        }
+        provideValuesTree(finalMap, &d->finalBuildConfigtree);
+    }
+    return d->finalBuildConfigtree;
+}
+
+/*!
  * \variable SetupProjectParameters::ignoreDifferentProjectFilePath
  * \brief Returns true iff the saved build graph should be used even if its path to the
  *        project file is different from \c SetupProjectParameters::projectFilePath()
diff --git a/src/lib/tools/setupprojectparameters.h b/src/lib/tools/setupprojectparameters.h
index 77e09ef9a..b3df17d39 100644
--- a/src/lib/tools/setupprojectparameters.h
+++ b/src/lib/tools/setupprojectparameters.h
@@ -74,6 +74,8 @@ public:
     QVariantMap buildConfigurationTree() const;
     ErrorInfo expandBuildConfiguration(Settings *settings);
 
+    QVariantMap finalBuildConfigurationTree() const;
+
     bool ignoreDifferentProjectFilePath() const;
     void setIgnoreDifferentProjectFilePath(bool doIgnore);
 
diff --git a/src/lib/tools/tst_tools.cpp b/src/lib/tools/tst_tools.cpp
index 68789e29e..7d4c61794 100644
--- a/src/lib/tools/tst_tools.cpp
+++ b/src/lib/tools/tst_tools.cpp
@@ -34,6 +34,8 @@
 #include <tools/hostosinfo.h>
 #include <tools/profile.h>
 #include <tools/settings.h>
+#include <tools/setupprojectparameters.h>
+
 #include <QFileInfo>
 #include <QTemporaryFile>
 #include <QTest>
@@ -144,5 +146,33 @@ void TestTools::testProfiles()
     QVERIFY(exceptionCaught);
 }
 
+void TestTools::testBuildConfigMerging()
+{
+    QVariantMap buildConfigMap;
+    buildConfigMap.insert(QLatin1String("topLevelKey"), QLatin1String("topLevelValue"));
+    buildConfigMap.insert(QLatin1String("qbs.toolchain"), QLatin1String("gcc"));
+    buildConfigMap.insert(QLatin1String("qbs.architecture"),
+                          QLatin1String("Jean-Claude Pillemann"));
+    buildConfigMap.insert(QLatin1String("cpp.treatWarningsAsErrors"), true);
+    QVariantMap overrideMap;
+    overrideMap.insert(QLatin1String("qbs.toolchain"), QLatin1String("clang"));
+    SetupProjectParameters params;
+    params.setBuildConfiguration(buildConfigMap);
+    params.setOverriddenValues(overrideMap);
+    const QVariantMap finalMap = params.finalBuildConfigurationTree();
+    QCOMPARE(finalMap.count(), 3);
+    QCOMPARE(finalMap.value(QLatin1String("topLevelKey")).toString(),
+             QString::fromLatin1("topLevelValue"));
+    const QVariantMap finalQbsMap = finalMap.value(QLatin1String("qbs")).toMap();
+    QCOMPARE(finalQbsMap.count(), 2);
+    QCOMPARE(finalQbsMap.value(QLatin1String("toolchain")).toString(),
+             QString::fromLatin1("clang"));
+    QCOMPARE(finalQbsMap.value(QLatin1String("architecture")).toString(),
+             QString::fromLatin1("Jean-Claude Pillemann"));
+    const QVariantMap finalCppMap = finalMap.value(QLatin1String("cpp")).toMap();
+    QCOMPARE(finalCppMap.count(), 1);
+    QCOMPARE(finalCppMap.value(QLatin1String("treatWarningsAsErrors")).toBool(), true);
+}
+
 } // namespace Internal
 } // namespace qbs
diff --git a/src/lib/tools/tst_tools.h b/src/lib/tools/tst_tools.h
index 9957c4f2f..a43f41272 100644
--- a/src/lib/tools/tst_tools.h
+++ b/src/lib/tools/tst_tools.h
@@ -46,6 +46,7 @@ private slots:
     void testFileInfo();
     void fileCaseCheck();
     void testProfiles();
+    void testBuildConfigMerging();
 
 private:
     Settings * const m_settings;