From f8610c412ad84f89d3a6fbcc9271ae081e517620 Mon Sep 17 00:00:00 2001
From: Jonathan Lebon <jonathan@jlebon.com>
Date: Tue, 22 May 2018 15:20:33 -0400
Subject: [PATCH] daemon/upgrader: Consistently remove transient state

With the new support for pinning deployments, we need to also update
rpm-ostree to clean up the transient state as is now done in the ostree
sysroot upgrader.

This addresses that issue as well as tries to be a little cleaner in how
we clean up other transient state. Notably, we add a new helper function
to `RpmOstreeOrigin` to do this for us and use it in the upgrader. In
other cases, we do want this transient information since it allows us to
describe the deployment.

Closes: https://github.com/ostreedev/ostree/issues/1595
---
 src/daemon/rpmostree-sysroot-upgrader.c |  6 +-----
 src/libpriv/rpmostree-origin.c          | 19 ++++++++++++++++---
 src/libpriv/rpmostree-origin.h          |  3 +++
 tests/vmcheck/test-misc-2.sh            | 18 +++++++++++++++++-
 4 files changed, 37 insertions(+), 9 deletions(-)

diff --git a/src/daemon/rpmostree-sysroot-upgrader.c b/src/daemon/rpmostree-sysroot-upgrader.c
index 721b5d7716..fc404067b5 100644
--- a/src/daemon/rpmostree-sysroot-upgrader.c
+++ b/src/daemon/rpmostree-sysroot-upgrader.c
@@ -108,6 +108,7 @@ parse_origin_deployment (RpmOstreeSysrootUpgrader *self,
   self->origin = rpmostree_origin_parse_deployment (deployment, error);
   if (!self->origin)
     return FALSE;
+  rpmostree_origin_remove_transient_state (self->origin);
 
   if (rpmostree_origin_get_unconfigured_state (self->origin) &&
       !(self->flags & RPMOSTREE_SYSROOT_UPGRADER_FLAGS_IGNORE_UNCONFIGURED))
@@ -119,11 +120,6 @@ parse_origin_deployment (RpmOstreeSysrootUpgrader *self,
       return FALSE;
     }
 
-  /* A bit hacky; here we clean out the live state which is deployment specific.
-   * We don't expect users of the upgrader to want the live state.
-   */
-  rpmostree_origin_set_live_state (self->origin, NULL, NULL);
-
   return TRUE;
 }
 
diff --git a/src/libpriv/rpmostree-origin.c b/src/libpriv/rpmostree-origin.c
index 16c298b862..6c8a5005e6 100644
--- a/src/libpriv/rpmostree-origin.c
+++ b/src/libpriv/rpmostree-origin.c
@@ -111,9 +111,6 @@ rpmostree_origin_parse_keyfile (GKeyFile         *origin,
   ret->cached_overrides_remove =
     g_hash_table_new_full (g_str_hash, g_str_equal, g_free, NULL);
 
-  /* NOTE hack here - see https://github.com/ostreedev/ostree/pull/343 */
-  g_key_file_remove_key (ret->kf, "origin", "unlocked", NULL);
-
   ret->cached_unconfigured_state = g_key_file_get_string (ret->kf, "origin", "unconfigured-state", NULL);
 
   g_autofree char *refspec = g_key_file_get_string (ret->kf, "origin", "refspec", NULL);
@@ -177,6 +174,22 @@ rpmostree_origin_dup (RpmOstreeOrigin *origin)
   return ret;
 }
 
+/* This is useful if the origin is meant to be used to generate a *new* deployment, as
+ * opposed to simply gathering information about an existing one. In such cases, there are
+ * some things that we do not generally want to apply to a new deployment. */
+void
+rpmostree_origin_remove_transient_state (RpmOstreeOrigin *origin)
+{
+  /* first libostree-known things */
+  ostree_deployment_origin_remove_transient_state (origin->kf);
+
+  /* this is already covered by the above, but the below also updates the cached value */
+  rpmostree_origin_set_override_commit (origin, NULL, NULL);
+
+  /* then rpm-ostree specific things */
+  rpmostree_origin_set_live_state (origin, NULL, NULL);
+}
+
 const char *
 rpmostree_origin_get_refspec (RpmOstreeOrigin *origin)
 {
diff --git a/src/libpriv/rpmostree-origin.h b/src/libpriv/rpmostree-origin.h
index 2ad99e630e..c64d140f3d 100644
--- a/src/libpriv/rpmostree-origin.h
+++ b/src/libpriv/rpmostree-origin.h
@@ -52,6 +52,9 @@ rpmostree_origin_parse_deployment (OstreeDeployment *deployment,
 RpmOstreeOrigin *
 rpmostree_origin_dup (RpmOstreeOrigin *origin);
 
+void
+rpmostree_origin_remove_transient_state (RpmOstreeOrigin *origin);
+
 const char *
 rpmostree_origin_get_refspec (RpmOstreeOrigin *origin);
 
diff --git a/tests/vmcheck/test-misc-2.sh b/tests/vmcheck/test-misc-2.sh
index 1bdf77dae0..d8c3ffc64a 100755
--- a/tests/vmcheck/test-misc-2.sh
+++ b/tests/vmcheck/test-misc-2.sh
@@ -59,13 +59,29 @@ vm_rpmostree status > status.txt
 assert_not_file_has_content status.txt "Pinned: yes"
 echo "ok pinning"
 
+# trying to clean up a pinned pending deployment should be a no-op
 vm_cmd ostree admin pin 0
+vm_assert_status_jq ".deployments|length == 2" \
+                    ".deployments[0][\"pinned\"] == true"
 vm_rpmostree cleanup -p
 vm_assert_status_jq ".deployments|length == 2"
+echo "ok pinned pending"
+
+vm_build_rpm bar
+vm_rpmostree install bar
+vm_assert_status_jq ".deployments|length == 3"
+# but that new one shouldn't be pinned
+vm_assert_status_jq ".deployments[0][\"pinned\"] == false"
+vm_rpmostree cleanup -p
+vm_assert_status_jq ".deployments|length == 2"
+echo "ok pinning not carried over"
+
+# and now check that we can unpin and cleanup
 vm_cmd ostree admin pin -u 0
+vm_assert_status_jq ".deployments[0][\"pinned\"] == false"
 vm_rpmostree cleanup -p
 vm_assert_status_jq ".deployments|length == 1"
-echo "ok pinned retained"
+echo "ok unpin"
 
 # https://github.com/ostreedev/ostree/pull/1055
 vm_cmd ostree commit -b vmcheck --tree=ref=vmcheck --timestamp=\"October 25 1985\"