From dc4aa346a360d4fd30496f701d61a438039ec8af Mon Sep 17 00:00:00 2001
From: Jonathan Lebon <jonathan@jlebon.com>
Date: Tue, 24 Apr 2018 12:51:38 -0400
Subject: [PATCH] lib/deploy: Also compare deployment csum versions

When comparing deployments to determine whether we need a new
bootversion, we should also check whether the commit "version" metadata
is the same. Otherwise, we may end up with the a bootconfig whose
`title` includes a version that doesn't match the one from the
deployment checksum.

Closes: https://github.com/projectatomic/rpm-ostree/issues/1343

Closes: #1556
Approved by: cgwalters
---
 src/libostree/ostree-sysroot-deploy.c | 28 +++++++++++++++++++++++++--
 tests/libtest.sh                      |  2 +-
 tests/test-admin-deploy-2.sh          | 15 +++++++++++++-
 3 files changed, 41 insertions(+), 4 deletions(-)

diff --git a/src/libostree/ostree-sysroot-deploy.c b/src/libostree/ostree-sysroot-deploy.c
index 6e5d19c0..a3f00b4b 100644
--- a/src/libostree/ostree-sysroot-deploy.c
+++ b/src/libostree/ostree-sysroot-deploy.c
@@ -1907,6 +1907,23 @@ get_deployment_nonostree_kargs (OstreeDeployment *deployment)
   return _ostree_kernel_args_to_string (kargs);
 }
 
+static char*
+get_deployment_ostree_version (OstreeRepo       *repo,
+                               OstreeDeployment *deployment)
+{
+  const char *csum = ostree_deployment_get_csum (deployment);
+
+  g_autofree char *version = NULL;
+  g_autoptr(GVariant) variant = NULL;
+  if (ostree_repo_load_variant (repo, OSTREE_OBJECT_TYPE_COMMIT, csum, &variant, NULL))
+    {
+      g_autoptr(GVariant) metadata = g_variant_get_child_value (variant, 0);
+      g_variant_lookup (metadata, OSTREE_COMMIT_META_KEY_VERSION, "s", &version);
+    }
+
+  return g_steal_pointer (&version);
+}
+
 /* OSTree implements a special optimization where we want to avoid touching
  * the bootloader configuration if the kernel layout hasn't changed.  This is
  * handled by the ostree= kernel argument referring to a "bootlink".  But
@@ -1917,7 +1934,8 @@ get_deployment_nonostree_kargs (OstreeDeployment *deployment)
  * bootloader perspective.
  */
 static gboolean
-deployment_bootconfigs_equal (OstreeDeployment *a,
+deployment_bootconfigs_equal (OstreeRepo       *repo,
+                              OstreeDeployment *a,
                               OstreeDeployment *b)
 {
   /* same kernel & initramfs? */
@@ -1932,6 +1950,11 @@ deployment_bootconfigs_equal (OstreeDeployment *a,
   if (strcmp (a_boot_options_without_ostree, b_boot_options_without_ostree) != 0)
     return FALSE;
 
+  /* same ostree version? this is just for the menutitle, we won't have to cp the kernel */
+  g_autofree char *a_version = get_deployment_ostree_version (repo, a);
+  g_autofree char *b_version = get_deployment_ostree_version (repo, b);
+  if (g_strcmp0 (a_version, b_version) != 0)
+    return FALSE;
 
   return TRUE;
 }
@@ -2191,13 +2214,14 @@ ostree_sysroot_write_deployments_with_options (OstreeSysroot     *self,
   else
     {
       gboolean is_noop = TRUE;
+      OstreeRepo *repo = ostree_sysroot_repo (self);
       for (guint i = 0; i < new_deployments->len; i++)
         {
           OstreeDeployment *cur_deploy = self->deployments->pdata[i];
           if (ostree_deployment_is_staged (cur_deploy))
             continue;
           OstreeDeployment *new_deploy = new_deployments->pdata[i];
-          if (!deployment_bootconfigs_equal (cur_deploy, new_deploy))
+          if (!deployment_bootconfigs_equal (repo, cur_deploy, new_deploy))
             {
               requires_new_bootversion = TRUE;
               is_noop = FALSE;
diff --git a/tests/libtest.sh b/tests/libtest.sh
index 586bf9ef..f6b6eb2f 100755
--- a/tests/libtest.sh
+++ b/tests/libtest.sh
@@ -491,7 +491,7 @@ os_repository_new_commit ()
 
     echo "content iteration ${content_iteration}" > usr/bin/content-iteration
 
-    version=$(date "+%Y%m%d.${content_iteration}")
+    export version=$(date "+%Y%m%d.${content_iteration}")
 
     ${CMD_PREFIX} ostree --repo=${test_tmpdir}/testos-repo commit  --add-metadata-string "version=${version}" -b $branch -s "Build"
     cd ${test_tmpdir}
diff --git a/tests/test-admin-deploy-2.sh b/tests/test-admin-deploy-2.sh
index eab0a3d3..7e69ec88 100755
--- a/tests/test-admin-deploy-2.sh
+++ b/tests/test-admin-deploy-2.sh
@@ -26,7 +26,7 @@ set -euo pipefail
 # Exports OSTREE_SYSROOT so --sysroot not needed.
 setup_os_repository "archive" "syslinux"
 
-echo "1..6"
+echo "1..7"
 
 ${CMD_PREFIX} ostree --repo=sysroot/ostree/repo pull-local --remote=testos testos-repo testos/buildmaster/x86_64-runtime
 rev=$(${CMD_PREFIX} ostree --repo=sysroot/ostree/repo rev-parse testos/buildmaster/x86_64-runtime)
@@ -64,6 +64,19 @@ assert_file_has_content sysroot/ostree/deploy/testos/deploy/${newrev}.0/etc/os-r
 
 echo "ok manual cleanup"
 
+# Commit + upgrade twice, so that we'll rotate out the original deployment
+os_repository_new_commit "1"
+${CMD_PREFIX} ostree admin upgrade --os=testos
+oldversion=${version}
+# another commit with *same* bootcsum but *new* content
+os_repository_new_commit "1" "2"
+newversion=${version}
+assert_file_has_content sysroot/boot/loader/entries/ostree-testos-0.conf ${oldversion}
+${CMD_PREFIX} ostree admin upgrade --os=testos
+assert_file_has_content sysroot/boot/loader/entries/ostree-testos-0.conf ${newversion}
+
+echo "ok new version same bootcsum"
+
 assert_n_pinned() {
     local n=$1
     ${CMD_PREFIX} ostree admin status > status.txt