| Message ID | f802e5442013613221a4efd8ef1fecce0f3a9914.1553354374.git.liu.denton@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | rebase: learn --keep-base | expand |
On Sat, Mar 23, 2019 at 11:25 AM Denton Liu <liu.denton@gmail.com> wrote:> > [...] > Since rebasing onto the merge base of the branch and the upstream is > such a common case, introduce the --keep-base option as a shortcut. > [...] > Signed-off-by: Denton Liu <liu.denton@gmail.com> > --- > diff --git a/builtin/rebase.c b/builtin/rebase.c > @@ -1541,10 +1551,19 @@ int cmd_rebase(int argc, const char **argv, const char *prefix) > /* Make sure the branch to rebase onto is valid. */ > - if (!options.onto_name) > + if (keep_base) { > + strbuf_reset(&buf); > + strbuf_addstr(&buf, options.upstream_name); > + strbuf_addstr(&buf, "..."); > + options.onto_name = xstrdup(buf.buf); > + } else if (!options.onto_name) > options.onto_name = options.upstream_name; > if (strstr(options.onto_name, "...")) { > if (get_oid_mb(options.onto_name, &merge_base) < 0) > + if (keep_base) > + die(_("'%s': need exactly one merge base with branch"), > + options.upstream_name); > + else Style: Indent with tabs, not spaces.
Denton Liu <liu.denton@gmail.com> writes: > A common scenario is if a user is working on a topic branch and they > wish to make some changes to intermediate commits or autosquashing, they > would run something such as > > git rebase -i --onto master... master > > in order to preserve the merge base. This prevents unnecessary commit > churning. > > Alternatively, a user wishing to test individual commits in a topic > branch without changing anything may run > > git rebase -x ./test.sh master... master > > Since rebasing onto the merge base of the branch and the upstream is > such a common case, introduce the --keep-base option as a shortcut. > > This allows us to rewrite the above as > > git rebase -i --keep-base master > > and > > git rebase -x ./test.sh --keep-base master > > respectively. I never use the "feature" myself, but I recall that when "git rebase" is run on a branch appropriately prepared, you do not even have to say <upstream> (iow, you type "git rebase<RET>" and rebase on top of @{upstream}). Can this new "--keep-base" feature mesh well with it? When the current branch has forked from origin/master, for example, it would be good if $ git rebase -i --same-base becomes a usable short-hand for $ git rebase -i --same-base origin/master aka $ git rebase -i --onto $(git merge-base HEAD origin/master) origin/master
Denton Liu <liu.denton@gmail.com> writes: > if (strstr(options.onto_name, "...")) { > if (get_oid_mb(options.onto_name, &merge_base) < 0) > + if (keep_base) > + die(_("'%s': need exactly one merge base with branch"), > + options.upstream_name); > + else > die(_("'%s': need exactly one merge base"), > options.onto_name); If you turn a single statement body into if/else, have {} around the body of the outer if, i.e. if (get_oid_mb(...) < 0) { if (keep_base) die(_("'%s': need exactly one merge base with branch"), options.upstream_name); else die(_("'%s': need exactly one merge base"), options.onto_name); } Otherwise -Werror=danglng-else will stop compilation. Thanks.
Hi Junio, On Sun, Mar 24, 2019 at 10:20:28PM +0900, Junio C Hamano wrote: > Denton Liu <liu.denton@gmail.com> writes: > > > A common scenario is if a user is working on a topic branch and they > > wish to make some changes to intermediate commits or autosquashing, they > > would run something such as > > > > git rebase -i --onto master... master > > > > in order to preserve the merge base. This prevents unnecessary commit > > churning. > > > > Alternatively, a user wishing to test individual commits in a topic > > branch without changing anything may run > > > > git rebase -x ./test.sh master... master > > > > Since rebasing onto the merge base of the branch and the upstream is > > such a common case, introduce the --keep-base option as a shortcut. > > > > This allows us to rewrite the above as > > > > git rebase -i --keep-base master > > > > and > > > > git rebase -x ./test.sh --keep-base master > > > > respectively. > > I never use the "feature" myself, but I recall that when "git > rebase" is run on a branch appropriately prepared, you do not even > have to say <upstream> (iow, you type "git rebase<RET>" and rebase > on top of @{upstream}). > > Can this new "--keep-base" feature mesh well with it? When the > current branch has forked from origin/master, for example, it would > be good if > > $ git rebase -i --same-base > > becomes a usable short-hand for > > $ git rebase -i --same-base origin/master By "--same-base", I am assuming you mistyped and meant to write "--keep-base"? If that's the case, I can make it a shorthand. Thanks, Denton > > aka > > $ git rebase -i --onto $(git merge-base HEAD origin/master) origin/master >
On Sun, Mar 24, 2019 at 05:06:18PM -0700, Denton Liu wrote: > Hi Junio, > > On Sun, Mar 24, 2019 at 10:20:28PM +0900, Junio C Hamano wrote: > > Denton Liu <liu.denton@gmail.com> writes: > > > > > A common scenario is if a user is working on a topic branch and they > > > wish to make some changes to intermediate commits or autosquashing, they > > > would run something such as > > > > > > git rebase -i --onto master... master > > > > > > in order to preserve the merge base. This prevents unnecessary commit > > > churning. > > > > > > Alternatively, a user wishing to test individual commits in a topic > > > branch without changing anything may run > > > > > > git rebase -x ./test.sh master... master > > > > > > Since rebasing onto the merge base of the branch and the upstream is > > > such a common case, introduce the --keep-base option as a shortcut. > > > > > > This allows us to rewrite the above as > > > > > > git rebase -i --keep-base master > > > > > > and > > > > > > git rebase -x ./test.sh --keep-base master > > > > > > respectively. > > > > I never use the "feature" myself, but I recall that when "git > > rebase" is run on a branch appropriately prepared, you do not even > > have to say <upstream> (iow, you type "git rebase<RET>" and rebase > > on top of @{upstream}). > > > > Can this new "--keep-base" feature mesh well with it? When the > > current branch has forked from origin/master, for example, it would > > be good if > > > > $ git rebase -i --same-base > > > > becomes a usable short-hand for > > > > $ git rebase -i --same-base origin/master > > By "--same-base", I am assuming you mistyped and meant to write > "--keep-base"? If that's the case, I can make it a shorthand. Sorry, I misunderstood your question. "--keep-base" already has the shorthand case handled by default. One caveat is that "--fork-point" is automatically implied if no upstream is supplied but this behaviour is the same for "--onto" or when no other options are supplied as well. I don't use the feature either so I'm not really sure if this behaviour would be the most sane thing to do. > Thanks, > > Denton > > > > > aka > > > > $ git rebase -i --onto $(git merge-base HEAD origin/master) origin/master > >
On Sat, Mar 23, 2019 at 08:25:28AM -0700, Denton Liu wrote: > A common scenario is if a user is working on a topic branch and they > wish to make some changes to intermediate commits or autosquashing, they Sorry, small typo here: s/autosquashing/autosquash/ -Denton > would run something such as > > git rebase -i --onto master... master > > in order to preserve the merge base. This prevents unnecessary commit > churning. > > Alternatively, a user wishing to test individual commits in a topic > branch without changing anything may run > > git rebase -x ./test.sh master... master > > Since rebasing onto the merge base of the branch and the upstream is > such a common case, introduce the --keep-base option as a shortcut. > > This allows us to rewrite the above as > > git rebase -i --keep-base master > > and > > git rebase -x ./test.sh --keep-base master > > respectively. > > Signed-off-by: Denton Liu <liu.denton@gmail.com> > --- > builtin/rebase.c | 25 ++++++++++++++++++++++--- > 1 file changed, 22 insertions(+), 3 deletions(-) > > diff --git a/builtin/rebase.c b/builtin/rebase.c > index 77deebc65c..fffee89064 100644 > --- a/builtin/rebase.c > +++ b/builtin/rebase.c > @@ -27,8 +27,8 @@ > #include "branch.h" > > static char const * const builtin_rebase_usage[] = { > - N_("git rebase [-i] [options] [--exec <cmd>] [--onto <newbase>] " > - "[<upstream>] [<branch>]"), > + N_("git rebase [-i] [options] [--exec <cmd>] " > + "[--onto <newbase> | --keep-base] [<upstream> [<branch>]]"), > N_("git rebase [-i] [options] [--exec <cmd>] [--onto <newbase>] " > "--root [<branch>]"), > N_("git rebase --continue | --abort | --skip | --edit-todo"), > @@ -1018,6 +1018,7 @@ int cmd_rebase(int argc, const char **argv, const char *prefix) > }; > const char *branch_name; > int ret, flags, total_argc, in_progress = 0; > + int keep_base = 0; > int ok_to_skip_pre_rebase = 0; > struct strbuf msg = STRBUF_INIT; > struct strbuf revisions = STRBUF_INIT; > @@ -1051,6 +1052,8 @@ int cmd_rebase(int argc, const char **argv, const char *prefix) > OPT_STRING(0, "onto", &options.onto_name, > N_("revision"), > N_("rebase onto given branch instead of upstream")), > + OPT_BOOL(0, "keep-base", &keep_base, > + N_("use the merge-base of upstream and branch as the current base")), > OPT_BOOL(0, "no-verify", &ok_to_skip_pre_rebase, > N_("allow pre-rebase hook to run")), > OPT_NEGBIT('q', "quiet", &options.flags, > @@ -1217,6 +1220,13 @@ int cmd_rebase(int argc, const char **argv, const char *prefix) > usage_with_options(builtin_rebase_usage, > builtin_rebase_options); > > + if (keep_base) { > + if (options.onto_name) > + die(_("cannot combine '--keep-base' with '--onto'")); > + if (options.root) > + die(_("cannot combine '--keep-base' with '--root'")); > + } > + > if (action != NO_ACTION && !in_progress) > die(_("No rebase in progress?")); > setenv(GIT_REFLOG_ACTION_ENVIRONMENT, "rebase", 0); > @@ -1541,10 +1551,19 @@ int cmd_rebase(int argc, const char **argv, const char *prefix) > } > > /* Make sure the branch to rebase onto is valid. */ > - if (!options.onto_name) > + if (keep_base) { > + strbuf_reset(&buf); > + strbuf_addstr(&buf, options.upstream_name); > + strbuf_addstr(&buf, "..."); > + options.onto_name = xstrdup(buf.buf); > + } else if (!options.onto_name) > options.onto_name = options.upstream_name; > if (strstr(options.onto_name, "...")) { > if (get_oid_mb(options.onto_name, &merge_base) < 0) > + if (keep_base) > + die(_("'%s': need exactly one merge base with branch"), > + options.upstream_name); > + else > die(_("'%s': need exactly one merge base"), > options.onto_name); > options.onto = lookup_commit_or_die(&merge_base, > -- > 2.21.0.512.g57bf1b23e1 >
Hi Denton, On Sat, 23 Mar 2019, Denton Liu wrote: > [...] > > This allows us to rewrite the above as > > git rebase -i --keep-base master > > and > > git rebase -x ./test.sh --keep-base master > > respectively. Just a quick note: this breaks t5407 because that test uses `git rebase --keep` and expects that abbreviated option to be expanded to `--keep-empty`, which is now no longer the only possible expansion. I just submitted a patch series to fix that, and other uses of abbreviated options in the test suite, in https://public-inbox.org/git/pull.167.git.gitgitgadget@gmail.com/T/#t Ciao, Johannes P.S.: Did you run the test suite before submitting your patches?
Hi Johannes, On Mon, Mar 25, 2019 at 07:50:38PM +0100, Johannes Schindelin wrote: > Hi Denton, > > On Sat, 23 Mar 2019, Denton Liu wrote: > > > [...] > > > > This allows us to rewrite the above as > > > > git rebase -i --keep-base master > > > > and > > > > git rebase -x ./test.sh --keep-base master > > > > respectively. > > Just a quick note: this breaks t5407 because that test uses `git rebase > --keep` and expects that abbreviated option to be expanded to > `--keep-empty`, which is now no longer the only possible expansion. > > I just submitted a patch series to fix that, and other uses of abbreviated > options in the test suite, in > https://public-inbox.org/git/pull.167.git.gitgitgadget@gmail.com/T/#t Thanks for catching this. I replied with a (tiny) review. > > Ciao, > Johannes > > P.S.: Did you run the test suite before submitting your patches? Usually I'm more diligent about running tests but I wrote this patchset in the back of a car when I was running low on batteries. I only ran the rebase-related tests but I guess that wasn't enough. My mistake, though. I'll be sure to _always_ run tests in the future. Thanks, Denton
Hi Denton, On Mon, 25 Mar 2019, Denton Liu wrote: > On Mon, Mar 25, 2019 at 07:50:38PM +0100, Johannes Schindelin wrote: > > > P.S.: Did you run the test suite before submitting your patches? > > Usually I'm more diligent about running tests but I wrote this patchset > in the back of a car when I was running low on batteries. I only ran the > rebase-related tests but I guess that wasn't enough. I totally understand that kind of scenario. > My mistake, though. I'll be sure to _always_ run tests in the future. Please do not feel bad. I did work pretty hard on making the Azure Pipelines support as fast as it is for the very purpose of helping exactly situations like yours: to hand off the testing to the cloud when batteries run low (or when you want to use your laptop for something else than running Git's test suite). And of course to verify that you don't break things on platforms other than the one you happen to develop on. And to check the documentation. And to run some static analyses. :-) Maybe you want to open PRs at https://github.com/git/git or at https://github.com/gitgitgadget/git to benefit from that kind of automatic testing, benefitting from my work (which would make me feel real good about spending that amount of time on it). Ciao, Johannes
Denton Liu <liu.denton@gmail.com> writes: >> > I never use the "feature" myself, but I recall that when "git >> > rebase" is run on a branch appropriately prepared, you do not even >> > have to say <upstream> (iow, you type "git rebase<RET>" and rebase >> > on top of @{upstream}). >> > >> > Can this new "--keep-base" feature mesh well with it? When the >> > current branch has forked from origin/master, for example, it would >> > be good if >> > >> > $ git rebase -i --same-base >> > >> > becomes a usable short-hand for >> > >> > $ git rebase -i --same-base origin/master >> >> By "--same-base", I am assuming you mistyped and meant to write >> "--keep-base"? If that's the case, I can make it a shorthand. > > Sorry, I misunderstood your question. "--keep-base" already has the > shorthand case handled by default. I actually think you understood _my_ question perfectly well, but misremembered what your implementation already supported ;-) If the new option works well with "the branch knows who its upstream is" feature already, so that the user does not have to type origin/master in the above example without doing anything special on your implementation's side, that is a great news.
diff --git a/builtin/rebase.c b/builtin/rebase.c index 77deebc65c..fffee89064 100644 --- a/builtin/rebase.c +++ b/builtin/rebase.c @@ -27,8 +27,8 @@ #include "branch.h" static char const * const builtin_rebase_usage[] = { - N_("git rebase [-i] [options] [--exec <cmd>] [--onto <newbase>] " - "[<upstream>] [<branch>]"), + N_("git rebase [-i] [options] [--exec <cmd>] " + "[--onto <newbase> | --keep-base] [<upstream> [<branch>]]"), N_("git rebase [-i] [options] [--exec <cmd>] [--onto <newbase>] " "--root [<branch>]"), N_("git rebase --continue | --abort | --skip | --edit-todo"), @@ -1018,6 +1018,7 @@ int cmd_rebase(int argc, const char **argv, const char *prefix) }; const char *branch_name; int ret, flags, total_argc, in_progress = 0; + int keep_base = 0; int ok_to_skip_pre_rebase = 0; struct strbuf msg = STRBUF_INIT; struct strbuf revisions = STRBUF_INIT; @@ -1051,6 +1052,8 @@ int cmd_rebase(int argc, const char **argv, const char *prefix) OPT_STRING(0, "onto", &options.onto_name, N_("revision"), N_("rebase onto given branch instead of upstream")), + OPT_BOOL(0, "keep-base", &keep_base, + N_("use the merge-base of upstream and branch as the current base")), OPT_BOOL(0, "no-verify", &ok_to_skip_pre_rebase, N_("allow pre-rebase hook to run")), OPT_NEGBIT('q', "quiet", &options.flags, @@ -1217,6 +1220,13 @@ int cmd_rebase(int argc, const char **argv, const char *prefix) usage_with_options(builtin_rebase_usage, builtin_rebase_options); + if (keep_base) { + if (options.onto_name) + die(_("cannot combine '--keep-base' with '--onto'")); + if (options.root) + die(_("cannot combine '--keep-base' with '--root'")); + } + if (action != NO_ACTION && !in_progress) die(_("No rebase in progress?")); setenv(GIT_REFLOG_ACTION_ENVIRONMENT, "rebase", 0); @@ -1541,10 +1551,19 @@ int cmd_rebase(int argc, const char **argv, const char *prefix) } /* Make sure the branch to rebase onto is valid. */ - if (!options.onto_name) + if (keep_base) { + strbuf_reset(&buf); + strbuf_addstr(&buf, options.upstream_name); + strbuf_addstr(&buf, "..."); + options.onto_name = xstrdup(buf.buf); + } else if (!options.onto_name) options.onto_name = options.upstream_name; if (strstr(options.onto_name, "...")) { if (get_oid_mb(options.onto_name, &merge_base) < 0) + if (keep_base) + die(_("'%s': need exactly one merge base with branch"), + options.upstream_name); + else die(_("'%s': need exactly one merge base"), options.onto_name); options.onto = lookup_commit_or_die(&merge_base,
A common scenario is if a user is working on a topic branch and they wish to make some changes to intermediate commits or autosquashing, they would run something such as git rebase -i --onto master... master in order to preserve the merge base. This prevents unnecessary commit churning. Alternatively, a user wishing to test individual commits in a topic branch without changing anything may run git rebase -x ./test.sh master... master Since rebasing onto the merge base of the branch and the upstream is such a common case, introduce the --keep-base option as a shortcut. This allows us to rewrite the above as git rebase -i --keep-base master and git rebase -x ./test.sh --keep-base master respectively. Signed-off-by: Denton Liu <liu.denton@gmail.com> --- builtin/rebase.c | 25 ++++++++++++++++++++++--- 1 file changed, 22 insertions(+), 3 deletions(-)