From e10c97007f3752b9041b65ec18b89dfc9a9e9b44 Mon Sep 17 00:00:00 2001 From: Jonathan Lebon Date: Wed, 21 Dec 2016 10:40:42 -0500 Subject: [PATCH] rebase: add support for rebasing to a specific rev Expand the available options in the Rebase() D-Bus method to also have a "revision" key. Its value has the same semantics as the "revision" key in the Deploy() method (e.g. the "revision=" and "version=" prefixes are also supported). Also expand the rebase CLI to allow for specifying the revision as an additional argument. This allows users to rebase to a specific version or checksum, rather than only to the latest. Conceptually, this is the equivalent of doing a rebase followed by a deploy. I.e. we specify an override-commit in the origin and expect the same behaviours that apply after a deploy to also apply here. Closes: #212 Closes: #555 Approved by: cgwalters --- src/app/rpmostree-builtin-rebase.c | 17 +- src/daemon/org.projectatomic.rpmostree1.xml | 1 + src/daemon/rpmostreed-os.c | 5 + src/daemon/rpmostreed-transaction-types.c | 162 +++++++++++--------- src/daemon/rpmostreed-transaction-types.h | 1 + tests/check/test-basic.sh | 27 +++- tests/common/libtest.sh | 1 + 7 files changed, 135 insertions(+), 79 deletions(-) diff --git a/src/app/rpmostree-builtin-rebase.c b/src/app/rpmostree-builtin-rebase.c index 33d4499b..72f97811 100644 --- a/src/app/rpmostree-builtin-rebase.c +++ b/src/app/rpmostree-builtin-rebase.c @@ -42,7 +42,7 @@ static GOptionEntry option_entries[] = { }; static GVariant * -get_args_variant (void) +get_args_variant (const char *revision) { GVariantDict dict; @@ -50,6 +50,9 @@ get_args_variant (void) g_variant_dict_insert (&dict, "skip-purge", "b", opt_skip_purge); g_variant_dict_insert (&dict, "reboot", "b", opt_reboot); + if (revision != NULL) + g_variant_dict_insert (&dict, "revision", "s", revision); + return g_variant_dict_end (&dict); } @@ -61,11 +64,12 @@ rpmostree_builtin_rebase (int argc, { int exit_status = EXIT_FAILURE; const char *new_provided_refspec; + const char *revision = NULL; /* forced blank for now */ const char *packages[] = { NULL }; - g_autoptr(GOptionContext) context = g_option_context_new ("REFSPEC - Switch to a different tree"); + g_autoptr(GOptionContext) context = g_option_context_new ("REFSPEC [REVISION] - Switch to a different tree"); glnx_unref_object RPMOSTreeOS *os_proxy = NULL; glnx_unref_object RPMOSTreeSysroot *sysroot_proxy = NULL; g_autofree char *transaction_address = NULL; @@ -79,20 +83,23 @@ rpmostree_builtin_rebase (int argc, error)) goto out; - if (argc < 2) + if (argc < 2 || argc > 3) { - rpmostree_usage_error (context, "REFSPEC must be specified", error); + rpmostree_usage_error (context, "Too few or too many arguments", error); goto out; } new_provided_refspec = argv[1]; + if (argc == 3) + revision = argv[2]; + if (!rpmostree_load_os_proxy (sysroot_proxy, opt_osname, cancellable, &os_proxy, error)) goto out; if (!rpmostree_os_call_rebase_sync (os_proxy, - get_args_variant (), + get_args_variant (revision), new_provided_refspec, packages, &transaction_address, diff --git a/src/daemon/org.projectatomic.rpmostree1.xml b/src/daemon/org.projectatomic.rpmostree1.xml index 17b36f7d..064ca4c7 100644 --- a/src/daemon/org.projectatomic.rpmostree1.xml +++ b/src/daemon/org.projectatomic.rpmostree1.xml @@ -144,6 +144,7 @@ diff --git a/src/daemon/rpmostreed-os.c b/src/daemon/rpmostreed-os.c index d8dd8ffd..0db9ccf7 100644 --- a/src/daemon/rpmostreed-os.c +++ b/src/daemon/rpmostreed-os.c @@ -649,6 +649,7 @@ os_handle_rebase (RPMOSTreeOS *interface, GVariantDict options_dict; gboolean opt_skip_purge = FALSE; const char *osname; + const char *opt_revision = NULL; gboolean opt_reboot = FALSE; GError *local_error = NULL; @@ -683,6 +684,9 @@ os_handle_rebase (RPMOSTreeOS *interface, g_variant_dict_lookup (&options_dict, "reboot", "b", &opt_reboot); + g_variant_dict_lookup (&options_dict, + "revision", "&s", + &opt_revision); g_variant_dict_clear (&options_dict); @@ -690,6 +694,7 @@ os_handle_rebase (RPMOSTreeOS *interface, ot_sysroot, osname, arg_refspec, + opt_revision, opt_skip_purge, opt_reboot, cancellable, diff --git a/src/daemon/rpmostreed-transaction-types.c b/src/daemon/rpmostreed-transaction-types.c index 142fb8db..b2d03e32 100644 --- a/src/daemon/rpmostreed-transaction-types.c +++ b/src/daemon/rpmostreed-transaction-types.c @@ -71,6 +71,82 @@ out: return ret; } +static gboolean +apply_revision_override (RpmostreedTransaction *transaction, + OstreeRepo *repo, + OstreeAsyncProgress *progress, + RpmOstreeSysrootUpgrader *upgrader, + const char *revision, + GCancellable *cancellable, + GError **error) +{ + g_autoptr(GKeyFile) origin = NULL; + g_autofree char *checksum = NULL; + g_autofree char *version = NULL; + const char *refspec; + + origin = rpmostree_sysroot_upgrader_dup_origin (upgrader); + if (origin == NULL) + { + g_set_error_literal (error, G_IO_ERROR, G_IO_ERROR_FAILED, + "Booted deployment has no origin"); + return FALSE; + } + + if (!rpmostreed_parse_revision (revision, + &checksum, + &version, + error)) + return FALSE; + + refspec = rpmostree_sysroot_upgrader_get_refspec (upgrader); + if (refspec == NULL) + { + g_set_error_literal (error, G_IO_ERROR, G_IO_ERROR_FAILED, + "Could not find refspec for booted deployment"); + return FALSE; + } + + if (version != NULL) + { + rpmostreed_transaction_emit_message_printf (transaction, + "Resolving version '%s'", + version); + + if (!rpmostreed_repo_lookup_version (repo, refspec, version, progress, + cancellable, &checksum, error)) + return FALSE; + } + else + { + g_assert (checksum != NULL); + + rpmostreed_transaction_emit_message_printf (transaction, + "Validating checksum '%s'", + checksum); + + if (!rpmostreed_repo_lookup_checksum (repo, refspec, checksum, + progress, cancellable, error)) + return FALSE; + } + + g_key_file_set_string (origin, "origin", "override-commit", checksum); + + if (version != NULL) + { + g_autofree char *comment = NULL; + + /* Add a comment with the version, to be nice. */ + comment = g_strdup_printf ("Version %s [%.10s]", version, checksum); + g_key_file_set_comment (origin, "origin", "override-commit", comment, NULL); + } + + if (!rpmostree_sysroot_upgrader_set_origin (upgrader, origin, cancellable, error)) + return FALSE; + + return TRUE; +} + /* ============================= Package Diff ============================= */ typedef struct { @@ -682,6 +758,7 @@ typedef struct { RpmostreedTransaction parent; char *osname; char *refspec; + char *revision; gboolean skip_purge; gboolean reboot; } RebaseTransaction; @@ -739,7 +816,7 @@ rebase_transaction_execute (RpmostreedTransaction *transaction, flags |= RPMOSTREE_SYSROOT_UPGRADER_FLAGS_IGNORE_UNCONFIGURED; upgrader = rpmostree_sysroot_upgrader_new (sysroot, self->osname, flags, - cancellable, error); + cancellable, error); if (upgrader == NULL) goto out; @@ -755,6 +832,13 @@ rebase_transaction_execute (RpmostreedTransaction *transaction, rpmostreed_transaction_connect_download_progress (transaction, progress); rpmostreed_transaction_connect_signature_progress (transaction, repo); + if (self->revision) + { + if (!apply_revision_override (transaction, repo, progress, upgrader, + self->revision, cancellable, error)) + goto out; + } + if (!rpmostree_sysroot_upgrader_pull (upgrader, NULL, 0, progress, &changed, cancellable, error)) @@ -810,6 +894,7 @@ rpmostreed_transaction_new_rebase (GDBusMethodInvocation *invocation, OstreeSysroot *sysroot, const char *osname, const char *refspec, + const char *revision, gboolean skip_purge, gboolean reboot, GCancellable *cancellable, @@ -832,6 +917,7 @@ rpmostreed_transaction_new_rebase (GDBusMethodInvocation *invocation, { self->osname = g_strdup (osname); self->refspec = g_strdup (refspec); + self->revision = g_strdup (revision); self->skip_purge = skip_purge; self->reboot = reboot; } @@ -880,11 +966,6 @@ deploy_transaction_execute (RpmostreedTransaction *transaction, glnx_unref_object OstreeRepo *repo = NULL; glnx_unref_object OstreeAsyncProgress *progress = NULL; - g_autoptr(GKeyFile) origin = NULL; - g_autofree char *checksum = NULL; - g_autofree char *version = NULL; - const char *refspec = NULL; - gboolean changed = FALSE; gboolean ret = FALSE; @@ -903,77 +984,12 @@ deploy_transaction_execute (RpmostreedTransaction *transaction, if (!ostree_sysroot_get_repo (sysroot, &repo, cancellable, error)) goto out; - origin = rpmostree_sysroot_upgrader_dup_origin (upgrader); - if (origin == NULL) - { - g_set_error_literal (error, G_IO_ERROR, G_IO_ERROR_FAILED, - "Booted deployment has no origin"); - goto out; - } - progress = ostree_async_progress_new (); rpmostreed_transaction_connect_download_progress (transaction, progress); rpmostreed_transaction_connect_signature_progress (transaction, repo); - if (!rpmostreed_parse_revision (self->revision, - &checksum, - &version, - error)) - goto out; - - refspec = rpmostree_sysroot_upgrader_get_refspec (upgrader); - if (refspec == NULL) - { - g_set_error_literal (error, G_IO_ERROR, G_IO_ERROR_FAILED, - "Could not find refspec for booted deployment"); - goto out; - } - - if (version != NULL) - { - - rpmostreed_transaction_emit_message_printf (transaction, - "Resolving version '%s'", - version); - - if (!rpmostreed_repo_lookup_version (repo, - refspec, - version, - progress, - cancellable, - &checksum, - error)) - goto out; - } - else - { - g_assert (checksum != NULL); - - rpmostreed_transaction_emit_message_printf (transaction, - "Validating checksum '%s'", - checksum); - - if (!rpmostreed_repo_lookup_checksum (repo, - refspec, - checksum, - progress, - cancellable, - error)) - goto out; - } - - g_key_file_set_string (origin, "origin", "override-commit", checksum); - - if (version != NULL) - { - g_autofree char *comment = NULL; - - /* Add a comment with the version, to be nice. */ - comment = g_strdup_printf ("Version %s [%.10s]", version, checksum); - g_key_file_set_comment (origin, "origin", "override-commit", comment, NULL); - } - - if (!rpmostree_sysroot_upgrader_set_origin (upgrader, origin, cancellable, error)) + if (!apply_revision_override (transaction, repo, progress, upgrader, + self->revision, cancellable, error)) goto out; if (!rpmostree_sysroot_upgrader_pull (upgrader, NULL, 0, diff --git a/src/daemon/rpmostreed-transaction-types.h b/src/daemon/rpmostreed-transaction-types.h index 8c6895bd..31d5129e 100644 --- a/src/daemon/rpmostreed-transaction-types.h +++ b/src/daemon/rpmostreed-transaction-types.h @@ -59,6 +59,7 @@ RpmostreedTransaction * OstreeSysroot *sysroot, const char *osname, const char *refspec, + const char *revision, gboolean skip_purge, gboolean reboot, GCancellable *cancellable, diff --git a/tests/check/test-basic.sh b/tests/check/test-basic.sh index 7d9cefe6..2c6eebf2 100755 --- a/tests/check/test-basic.sh +++ b/tests/check/test-basic.sh @@ -23,7 +23,7 @@ set -e ensure_dbus -echo "1..10" +echo "1..13" setup_os_repository "archive-z2" "syslinux" @@ -109,6 +109,31 @@ fi assert_file_has_content OUTPUT-err 'Checksum .* not found in .*' echo "ok error on deploying commit on other branch" +# Make sure we're currently on otheros +rpm-ostree status | head --lines 5 | tee OUTPUT-status.txt +assert_file_has_content OUTPUT-status.txt otheros:testos/buildmaster/x86_64-runtime + +os_repository_new_commit 2 2 +rpm-ostree rebase --os=testos testos:testos/buildmaster/x86_64-runtime $(date "+%Y%m%d.2") +rpm-ostree status | head --lines 5 | tee OUTPUT-status.txt +assert_file_has_content OUTPUT-status.txt testos +assert_file_has_content OUTPUT-status.txt $(date "+%Y%m%d\.2") +echo "ok rebase onto other branch at specific version" + +branch=testos/buildmaster/x86_64-runtime +new_csum=$(ostree --repo=${test_tmpdir}/testos-repo commit -b $branch --tree=ref=$branch) +rpm-ostree rebase --os=testos otheros:testos/buildmaster/x86_64-runtime $new_csum +rpm-ostree status | head --lines 5 | tee OUTPUT-status.txt +assert_file_has_content OUTPUT-status.txt otheros +assert_file_has_content OUTPUT-status.txt $new_csum +echo "ok rebase onto other branch at specific checksum" + +if rpm-ostree rebase --os=testos testos:testos/buildmaster/x86_64-runtime $other_rev 2>OUTPUT-err; then + assert_not_reached "Rebasing onto out-of-branch commit unexpectedly succeeded." +fi +assert_file_has_content OUTPUT-err 'Checksum .* not found in .*' +echo "ok error on rebasing onto commit on other branch" + # Ensure it returns an error when passing a wrong option. rpm-ostree --help | awk '/^$/ {in_commands=0} {if(in_commands==1){print $0}} /^Builtin Commands:/ {in_commands=1}' > commands while read command; do diff --git a/tests/common/libtest.sh b/tests/common/libtest.sh index cbfd1a5c..04cd12e4 100644 --- a/tests/common/libtest.sh +++ b/tests/common/libtest.sh @@ -341,6 +341,7 @@ os_repository_new_commit () echo "content iteration ${content_iteration}" > usr/bin/content-iteration version=$(date "+%Y%m%d.${content_iteration}") + echo "version: $version" ostree --repo=${test_tmpdir}/testos-repo commit --add-metadata-string "version=${version}" -b testos/buildmaster/x86_64-runtime -s "Build" cd ${test_tmpdir}