diff --git a/src/app/rpmostree-builtin-deploy.c b/src/app/rpmostree-builtin-deploy.c index 403de88841..3e76b9d521 100644 --- a/src/app/rpmostree-builtin-deploy.c +++ b/src/app/rpmostree-builtin-deploy.c @@ -52,7 +52,6 @@ rpmostree_builtin_deploy (int argc, g_autoptr(GOptionContext) context = NULL; glnx_unref_object RPMOSTreeOS *os_proxy = NULL; glnx_unref_object RPMOSTreeSysroot *sysroot_proxy = NULL; - g_autoptr(GVariant) new_default_deployment = NULL; g_autofree char *transaction_address = NULL; const char * const packages[] = { NULL }; const char *revision; @@ -93,6 +92,8 @@ rpmostree_builtin_deploy (int argc, cancellable, &os_proxy, error)) return EXIT_FAILURE; + g_autoptr(GVariant) previous_deployment = rpmostree_os_dup_default_deployment (os_proxy); + if (opt_preview) { if (!rpmostree_os_call_download_deploy_rpm_diff_sync (os_proxy, @@ -105,8 +106,6 @@ rpmostree_builtin_deploy (int argc, } else { - rpmostree_monitor_default_deployment_change (os_proxy, &new_default_deployment); - g_autoptr(GVariant) options = rpmostree_get_options_variant (opt_reboot, TRUE, /* allow-downgrade */ @@ -174,7 +173,7 @@ rpmostree_builtin_deploy (int argc, } else if (!opt_reboot) { - if (new_default_deployment == NULL) + if (!rpmostree_has_new_default_deployment (os_proxy, previous_deployment)) return RPM_OSTREE_EXIT_UNCHANGED; /* do diff without dbus: https://github.com/projectatomic/rpm-ostree/pull/116 */ diff --git a/src/app/rpmostree-builtin-upgrade.c b/src/app/rpmostree-builtin-upgrade.c index b4c28edcfb..50cb882fa0 100644 --- a/src/app/rpmostree-builtin-upgrade.c +++ b/src/app/rpmostree-builtin-upgrade.c @@ -60,7 +60,6 @@ rpmostree_builtin_upgrade (int argc, g_autoptr(GOptionContext) context = g_option_context_new (""); glnx_unref_object RPMOSTreeOS *os_proxy = NULL; glnx_unref_object RPMOSTreeSysroot *sysroot_proxy = NULL; - g_autoptr(GVariant) new_default_deployment = NULL; g_autofree char *transaction_address = NULL; _cleanup_peer_ GPid peer_pid = 0; const char *const *install_pkgs = NULL; @@ -107,6 +106,8 @@ rpmostree_builtin_upgrade (int argc, cancellable, &os_proxy, error)) return EXIT_FAILURE; + g_autoptr(GVariant) previous_deployment = rpmostree_os_dup_default_deployment (os_proxy); + if (opt_preview || opt_check) { if (!rpmostree_os_call_download_update_rpm_diff_sync (os_proxy, @@ -117,8 +118,6 @@ rpmostree_builtin_upgrade (int argc, } else { - rpmostree_monitor_default_deployment_change (os_proxy, &new_default_deployment); - g_autoptr(GVariant) options = rpmostree_get_options_variant (opt_reboot, opt_allow_downgrade, @@ -184,7 +183,7 @@ rpmostree_builtin_upgrade (int argc, } else if (!opt_reboot) { - if (new_default_deployment == NULL) + if (!rpmostree_has_new_default_deployment (os_proxy, previous_deployment)) { if (opt_upgrade_unchanged_exit_77) return RPM_OSTREE_EXIT_UNCHANGED; diff --git a/src/app/rpmostree-libbuiltin.c b/src/app/rpmostree-libbuiltin.c index e5c9f998f8..edd7d69c2a 100644 --- a/src/app/rpmostree-libbuiltin.c +++ b/src/app/rpmostree-libbuiltin.c @@ -42,21 +42,28 @@ rpmostree_usage_error (GOptionContext *context, (void) glnx_throw (error, "usage error: %s", message); } -static void -default_deployment_change_cb (GObject *object, - GParamSpec *pspec, - GVariant **value) +static const char* +get_id_from_deployment_variant (GVariant *deployment) { - g_object_get (object, pspec->name, value, NULL); + g_autoptr(GVariantDict) dict = g_variant_dict_new (deployment); + const char *id; + g_assert (g_variant_dict_lookup (dict, "id", "&s", &id)); + return id; } -void -rpmostree_monitor_default_deployment_change (RPMOSTreeOS *os_proxy, - GVariant **deployment) +gboolean +rpmostree_has_new_default_deployment (RPMOSTreeOS *os_proxy, + GVariant *previous_deployment) { - /* This will set the GVariant if the default deployment changes. */ - g_signal_connect (os_proxy, "notify::default-deployment", - G_CALLBACK (default_deployment_change_cb), deployment); + g_autoptr(GVariant) new_deployment = rpmostree_os_dup_default_deployment (os_proxy); + + /* trivial case */ + if (g_variant_equal (previous_deployment, new_deployment)) + return FALSE; + + const char *previous_id = get_id_from_deployment_variant (previous_deployment); + const char *new_id = get_id_from_deployment_variant (new_deployment); + return !g_str_equal (previous_id, new_id); } /* Print the diff between the booted and pending deployments */ diff --git a/src/app/rpmostree-libbuiltin.h b/src/app/rpmostree-libbuiltin.h index 3c004c8cae..b96528aeac 100644 --- a/src/app/rpmostree-libbuiltin.h +++ b/src/app/rpmostree-libbuiltin.h @@ -31,6 +31,10 @@ rpmostree_usage_error (GOptionContext *context, const char *message, GError **error); +gboolean +rpmostree_has_new_default_deployment (RPMOSTreeOS *os_proxy, + GVariant *previous_deployment); + gboolean rpmostree_print_treepkg_diff (OstreeSysroot *sysroot, GCancellable *cancellable, @@ -41,8 +45,4 @@ rpmostree_print_treepkg_diff_from_sysroot_path (const gchar *sysroot_path, GCancellable *cancellable, GError **error); -void -rpmostree_monitor_default_deployment_change (RPMOSTreeOS *os_proxy, - GVariant **deployment); - G_END_DECLS 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/src/daemon/rpmostreed-transaction-types.c b/src/daemon/rpmostreed-transaction-types.c index 5773fc2bfa..e75215e9df 100644 --- a/src/daemon/rpmostreed-transaction-types.c +++ b/src/daemon/rpmostreed-transaction-types.c @@ -814,7 +814,23 @@ deploy_transaction_execute (RpmostreedTransaction *transaction, &base_changed, cancellable, error)) return FALSE; - changed = changed || base_changed; + if (base_changed) + changed = TRUE; + else + { + /* If we're on a live deployment, then allow redeploying a clean version of the + * same base commit. This is useful if e.g. the pushed rollback was cleaned up. */ + + OstreeDeployment *deployment = + rpmostree_sysroot_upgrader_get_merge_deployment (upgrader); + + gboolean is_live; + if (!rpmostree_syscore_deployment_is_live (sysroot, deployment, &is_live, error)) + return FALSE; + + if (is_live) + changed = TRUE; + } } /* let's figure out if those new overrides are valid and if so, canonicalize 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" \ diff --git a/tests/vmcheck/test-basic.sh b/tests/vmcheck/test-basic.sh index bba7d4acd8..23be115ff2 100755 --- a/tests/vmcheck/test-basic.sh +++ b/tests/vmcheck/test-basic.sh @@ -118,3 +118,21 @@ fi assert_not_file_has_content err.txt "Writing rpmdb" assert_file_has_content err.txt "File exists" echo "ok detecting file name conflicts before writing rpmdb" + +# check that the way we detect deployment changes is not dependent on pending-* +# https://github.com/projectatomic/rpm-ostree/issues/981 +vm_rpmostree cleanup -rp +csum=$(vm_cmd ostree commit -b vmcheck --tree=ref=vmcheck) +# restart to make daemon see the pending checksum +vm_cmd systemctl restart rpm-ostreed +vm_assert_status_jq '.deployments[0]["pending-base-checksum"]' +# hard reset to booted csum (simulates what deploy does to remote refspecs) +vm_cmd ostree reset vmcheck $(vm_get_booted_csum) +rc=0 +vm_rpmostree deploy $(vm_get_booted_csum) > out.txt || rc=$? +if [ $rc != 77 ]; then + assert_not_reached "trying to re-deploy same commit didn't exit 77" +fi +assert_file_has_content out.txt 'No change.' +vm_assert_status_jq '.deployments[0]["pending-base-checksum"]|not' +echo "ok changes to deployment variant don't affect deploy" diff --git a/tests/vmcheck/test-livefs.sh b/tests/vmcheck/test-livefs.sh index ef961f7f23..e5948f3657 100755 --- a/tests/vmcheck/test-livefs.sh +++ b/tests/vmcheck/test-livefs.sh @@ -110,6 +110,12 @@ vm_cmd cat /etc/test-livefs-with-etc.conf > test-livefs-with-etc.conf assert_file_has_content test-livefs-with-etc.conf "custom" echo "ok livefs preserved modified config" +vm_rpmostree cleanup -p +# make sure there's no layering going on somehow +vm_assert_status_jq '.deployments[0]["base-checksum"]|not' +vm_rpmostree deploy $(vm_get_booted_deployment_info checksum) +echo "ok livefs redeploy booted commit" + reset vm_rpmostree install /tmp/vmcheck/yumrepo/packages/x86_64/foo-1.0-1.x86_64.rpm vm_rpmostree ex livefs