From 97aaf72c2369364fe27067aa85e0b6be02644981 Mon Sep 17 00:00:00 2001 From: Jonathan Lebon Date: Mon, 11 Sep 2017 20:00:40 +0000 Subject: [PATCH] daemon/upgrader: make use of override-commit-ids Currently, when setting the `override-commit` key in the origin, the upgrader pulls that commit checksum directly and then updates the refspec to point to it. This behaviour was inherited from its ostree version; at the time it was implemented, the pull code didn't support passing a specific commit for a given refspec. However, we now have the override-commit-ids option, which will make libostree update the ref for us. We change the code here to make use of it and simplify the function. This also fixes the corner case of local branches: we shouldn't change the ref if we're on a local branch. This is actually what drove me to this patch as I was debugging #981. (Aside: I'm still not convinced updating the refspec is always the correct thing to do even in the remote case, though it's a bit messy to disentangle). --- src/daemon/rpmostree-sysroot-upgrader.c | 74 ++++++++++--------------- tests/check/test-basic.sh | 10 +++- 2 files changed, 39 insertions(+), 45 deletions(-) diff --git a/src/daemon/rpmostree-sysroot-upgrader.c b/src/daemon/rpmostree-sysroot-upgrader.c index 91a5d44315..91a28b99ab 100644 --- a/src/daemon/rpmostree-sysroot-upgrader.c +++ b/src/daemon/rpmostree-sysroot-upgrader.c @@ -373,21 +373,18 @@ rpmostree_sysroot_upgrader_pull_base (RpmOstreeSysrootUpgrader *self, GCancellable *cancellable, GError **error) { + const char *refspec = rpmostree_origin_get_refspec (self->origin); + g_autofree char *origin_remote = NULL; g_autofree char *origin_ref = NULL; - if (!ostree_parse_refspec (rpmostree_origin_get_refspec (self->origin), - &origin_remote, &origin_ref, error)) + if (!ostree_parse_refspec (refspec, &origin_remote, &origin_ref, error)) return FALSE; - char *refs_to_fetch[] = { NULL, NULL }; - if (rpmostree_origin_get_override_commit (self->origin) != NULL) - refs_to_fetch[0] = (char*)rpmostree_origin_get_override_commit (self->origin); - else - refs_to_fetch[0] = origin_ref; - const gboolean allow_older = (self->flags & RPMOSTREE_SYSROOT_UPGRADER_FLAGS_ALLOW_OLDER) > 0; + const char *override_commit = rpmostree_origin_get_override_commit (self->origin); + g_assert (self->origin_merge_deployment); if (origin_remote) { @@ -403,7 +400,12 @@ rpmostree_sysroot_upgrader_pull_base (RpmOstreeSysrootUpgrader *self, g_variant_builder_add (optbuilder, "{s@v}", "timestamp-check", g_variant_new_variant (g_variant_new_boolean (TRUE))); g_variant_builder_add (optbuilder, "{s@v}", "refs", - g_variant_new_variant (g_variant_new_strv ((const char *const*) refs_to_fetch, -1))); + g_variant_new_variant (g_variant_new_strv ( + (const char *const *)&origin_ref, 1))); + if (override_commit) + g_variant_builder_add (optbuilder, "{s@v}", "override-commit-ids", + g_variant_new_variant (g_variant_new_strv ( + (const char *const *)&override_commit, 1))); g_autoptr(GVariant) opts = g_variant_ref_sink (g_variant_builder_end (optbuilder)); if (!ostree_repo_pull_with_options (self->repo, origin_remote, opts, progress, @@ -414,48 +416,32 @@ rpmostree_sysroot_upgrader_pull_base (RpmOstreeSysrootUpgrader *self, ostree_async_progress_finish (progress); } - g_autofree char *new_base_revision = NULL; - if (rpmostree_origin_get_override_commit (self->origin) != NULL) - { - if (!ostree_repo_set_ref_immediate (self->repo, - origin_remote, - origin_ref, - rpmostree_origin_get_override_commit (self->origin), - cancellable, - error)) - return FALSE; - - new_base_revision = g_strdup (rpmostree_origin_get_override_commit (self->origin)); - } + g_autofree char *new_base_rev = NULL; + if (override_commit) + new_base_rev = g_strdup (override_commit); else { - if (!ostree_repo_resolve_rev (self->repo, rpmostree_origin_get_refspec (self->origin), FALSE, - &new_base_revision, error)) + if (!ostree_repo_resolve_rev (self->repo, refspec, FALSE, &new_base_rev, error)) return FALSE; } - { - gboolean changed = FALSE; - - if (strcmp (new_base_revision, self->base_revision) != 0) - { - changed = TRUE; - - if (!allow_older) - { - if (!ostree_sysroot_upgrader_check_timestamps (self->repo, self->base_revision, - new_base_revision, - error)) - return FALSE; - } - - g_free (self->base_revision); - self->base_revision = g_steal_pointer (&new_base_revision); - } + gboolean changed = !g_str_equal (new_base_rev, self->base_revision); + if (changed) + { + /* check timestamps here too in case the commit was already pulled (or the + * refspec is local) */ + if (!allow_older) + { + if (!ostree_sysroot_upgrader_check_timestamps (self->repo, self->base_revision, + new_base_rev, error)) + return FALSE; + } - *out_changed = changed; - } + g_free (self->base_revision); + self->base_revision = g_steal_pointer (&new_base_rev); + } + *out_changed = changed; return TRUE; } diff --git a/tests/check/test-basic.sh b/tests/check/test-basic.sh index e80aaf9c6a..c168456887 100755 --- a/tests/check/test-basic.sh +++ b/tests/check/test-basic.sh @@ -24,7 +24,7 @@ export RPMOSTREE_SUPPRESS_REQUIRES_ROOT_CHECK=yes ensure_dbus -echo "1..24" +echo "1..25" setup_os_repository "archive-z2" "syslinux" @@ -157,6 +157,14 @@ rpm-ostree rebase --os=testos another-branch assert_status_jq '.deployments[0].origin == "another-branch"' echo "ok rebase from local branch to local branch" +csum1=$($OSTREE commit -b another-branch --tree=ref=$branch) +csum2=$($OSTREE commit -b another-branch --tree=ref=$branch) +rpm-ostree deploy --os=testos $csum1 +if [ "$($OSTREE rev-parse another-branch)" != $csum2 ]; then + assert_not_reached "deploying from local branch changes branch" +fi +echo "ok local deploy doesn't affect branch" + rpm-ostree rebase --skip-purge --os=testos testos:testos/buildmaster/x86_64-runtime assert_status_jq '.deployments[0].origin == "testos:testos/buildmaster/x86_64-runtime"' ostree --repo=${test_tmpdir}/testos-repo commit --add-metadata-string version=1.0.2 -b testos/stable/x86_64-runtime -s "Build" \