From 787c880b64be98457ed46dbcc7dbdf40776e40e7 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Thu, 20 Jul 2017 21:55:22 -0400 Subject: [PATCH] bin/rebase: Add -b and -m options The rebase command syntax has confused people a lot. Let's follow git here and add a `-b/--branch` option and encourage people to use that. The case of switching remotes is `-m/--remote`; it's definitely unfortunate that `-r` is already taken for `--reboot`. One thing I'm a little bit unhappy about is how we're doing logic on the client side here. Changing the DBus API for this would also be awkward though. Closes: https://github.com/projectatomic/rpm-ostree/issues/886 Closes: #890 Approved by: jlebon --- man/rpm-ostree.xml | 22 ++++++++++++++----- src/app/rpmostree-builtin-rebase.c | 35 ++++++++++++++++++++++++------ tests/check/test-basic.sh | 14 +++++++++++- 3 files changed, 58 insertions(+), 13 deletions(-) diff --git a/man/rpm-ostree.xml b/man/rpm-ostree.xml index d904eddd..afa68d62 100644 --- a/man/rpm-ostree.xml +++ b/man/rpm-ostree.xml @@ -247,12 +247,24 @@ Boston, MA 02111-1307, USA. The full syntax is rebase REMOTENAME:BRANCHNAME. - Specifying just BRANCHNAME will reuse the same - remote. You may also omit one of REMOTENAME or - BRANCHNAME (keeping the colon). In the former - case, the branch refers to a local branch; in the latter case, the - same branch will be used on a different remote. + Alternatively, you can use the --branch or + --remote options mentioned below. With the + argument syntax, specifying just BRANCHNAME will + reuse the same remote. You may also omit one of + REMOTENAME or BRANCHNAME + (keeping the colon). In the former case, the branch refers to a + local branch; in the latter case, the same branch will be used on a + different remote. + + --branch or -b to + to pick a branch name. + + + --remote or -m to + to pick a remote name. + + diff --git a/src/app/rpmostree-builtin-rebase.c b/src/app/rpmostree-builtin-rebase.c index 6bc1f273..07f2c6a7 100644 --- a/src/app/rpmostree-builtin-rebase.c +++ b/src/app/rpmostree-builtin-rebase.c @@ -33,9 +33,13 @@ static char *opt_osname; static gboolean opt_reboot; static gboolean opt_skip_purge; +static char * opt_branch; +static char * opt_remote; static GOptionEntry option_entries[] = { { "os", 0, 0, G_OPTION_ARG_STRING, &opt_osname, "Operate on provided OSNAME", "OSNAME" }, + { "branch", 'b', 0, G_OPTION_ARG_STRING, &opt_branch, "Rebase to branch BRANCH; use --remote to change remote as well", "BRANCH" }, + { "remote", 'm', 0, G_OPTION_ARG_STRING, &opt_remote, "Rebase to current branch name using REMOTE; may also be combined with --branch", "REMOTE" }, { "reboot", 'r', 0, G_OPTION_ARG_NONE, &opt_reboot, "Initiate a reboot after rebase is finished", NULL }, { "skip-purge", 0, 0, G_OPTION_ARG_NONE, &opt_skip_purge, "Keep previous refspec after rebase", NULL }, { NULL } @@ -49,6 +53,7 @@ rpmostree_builtin_rebase (int argc, GError **error) { const char *new_provided_refspec; + g_autofree char *new_refspec_owned = NULL; const char *revision = NULL; /* forced blank for now */ @@ -74,21 +79,37 @@ rpmostree_builtin_rebase (int argc, error)) return EXIT_FAILURE; - if (argc < 2 || argc > 3) + if (argc > 3) { - rpmostree_usage_error (context, "Too few or too many arguments", error); + rpmostree_usage_error (context, "Too many arguments", error); return EXIT_FAILURE; } - new_provided_refspec = argv[1]; - - if (argc == 3) - revision = argv[2]; - if (!rpmostree_load_os_proxy (sysroot_proxy, opt_osname, cancellable, &os_proxy, error)) return EXIT_FAILURE; + if (argc < 2 && !(opt_branch || opt_remote)) + { + return rpmostree_usage_error (context, "Must specify refspec, or -b branch or -r remote", error), EXIT_FAILURE; + } + else if (argc >= 2) + { + new_provided_refspec = argv[1]; + if (argc == 3) + revision = argv[2]; + } + else + { + if (opt_remote) + { + new_provided_refspec = new_refspec_owned = + g_strconcat (opt_remote, ":", opt_branch ?: "", NULL); + } + else + new_provided_refspec = opt_branch; + } + g_autoptr(GVariant) options = rpmostree_get_options_variant (opt_reboot, TRUE, /* allow-downgrade */ diff --git a/tests/check/test-basic.sh b/tests/check/test-basic.sh index cfa7af01..e80aaf9c 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..23" +echo "1..24" setup_os_repository "archive-z2" "syslinux" @@ -157,6 +157,18 @@ rpm-ostree rebase --os=testos another-branch assert_status_jq '.deployments[0].origin == "another-branch"' echo "ok rebase from local branch to local 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" \ + --tree=ref=testos/buildmaster/x86_64-runtime +rpm-ostree rebase --skip-purge --os=testos -b testos/stable/x86_64-runtime +assert_status_jq '.deployments[0].origin == "testos:testos/stable/x86_64-runtime"' +ostree --repo=${test_tmpdir}/sysroot/ostree/repo commit -b localbranch --tree=ref=testos:testos/stable/x86_64-runtime +rpm-ostree rebase --skip-purge --os=testos -m '' -b localbranch +assert_status_jq '.deployments[0].origin == "localbranch"' +echo "ok rebase new syntax" + +rpm-ostree rebase --os=testos :another-branch originpath=$(ostree admin --sysroot=sysroot --print-current-dir).origin echo "unconfigured-state=Access to TestOS requires ONE BILLION DOLLARS" >> ${originpath} rpm-ostree reload