| Message ID | 20181211161139.31686-1-newren@gmail.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | Reimplement rebase --merge via interactive machinery | expand |
On Tue, Dec 11, 2018 at 8:11 AM Elijah Newren <newren@gmail.com> wrote: > > This series continues the work of making rebase more self-consistent > by removing inconsistencies between different backends. In > particular, this series focuses on making the merge machinery behave > like the interactive machinery (though a few differences between the am > and interactive backends are also fixed along the way), and ultimately > removes the merge backend in favor of reimplementing the relevant > options on top of the interactive machinery. Friendly ping...let me know if you want me to simply resend v4. > Differences since v3 (full range-diff below): > - Fixed the redundant "fatal: error:" error message prefixes, as pointed > out by Duy > - Rebased on 2.20.0 > > Elijah Newren (8): > rebase: make builtin and legacy script error messages the same > rebase: fix incompatible options error message > t5407: add a test demonstrating how interactive handles --skip > differently > am, rebase--merge: do not overlook --skip'ed commits with post-rewrite > git-rebase, sequencer: extend --quiet option for the interactive > machinery > git-legacy-rebase: simplify unnecessary triply-nested if > rebase: define linearization ordering and enforce it > rebase: Implement --merge via the interactive machinery > ...
Elijah Newren <newren@gmail.com> writes: > On Tue, Dec 11, 2018 at 8:11 AM Elijah Newren <newren@gmail.com> wrote: >> >> This series continues the work of making rebase more self-consistent >> by removing inconsistencies between different backends. In >> particular, this series focuses on making the merge machinery behave >> like the interactive machinery (though a few differences between the am >> and interactive backends are also fixed along the way), and ultimately >> removes the merge backend in favor of reimplementing the relevant >> options on top of the interactive machinery. > > Friendly ping...let me know if you want me to simply resend v4. > If you have anything newer than 90673135 ("rebase: Implement --merge via the interactive machinery", 2018-12-11), then yeah, I haven't seen it. Thanks. P.S. even if that one is latest, I would need to downcase Implement before it hits 'next' ;-) >> Differences since v3 (full range-diff below): >> - Fixed the redundant "fatal: error:" error message prefixes, as pointed >> out by Duy >> - Rebased on 2.20.0 >> >> Elijah Newren (8): >> rebase: make builtin and legacy script error messages the same >> rebase: fix incompatible options error message >> t5407: add a test demonstrating how interactive handles --skip >> differently >> am, rebase--merge: do not overlook --skip'ed commits with post-rewrite >> git-rebase, sequencer: extend --quiet option for the interactive >> machinery >> git-legacy-rebase: simplify unnecessary triply-nested if >> rebase: define linearization ordering and enforce it >> rebase: Implement --merge via the interactive machinery >> > ...
Junio C Hamano <gitster@pobox.com> writes: > Elijah Newren <newren@gmail.com> writes: > >> On Tue, Dec 11, 2018 at 8:11 AM Elijah Newren <newren@gmail.com> wrote: >>> >>> This series continues the work of making rebase more self-consistent >>> by removing inconsistencies between different backends. In >>> particular, this series focuses on making the merge machinery behave >>> like the interactive machinery (though a few differences between the am >>> and interactive backends are also fixed along the way), and ultimately >>> removes the merge backend in favor of reimplementing the relevant >>> options on top of the interactive machinery. >> >> Friendly ping...let me know if you want me to simply resend v4. >> > > If you have anything newer than 90673135 ("rebase: Implement --merge > via the interactive machinery", 2018-12-11), then yeah, I haven't > seen it. > > Thanks. > > P.S. even if that one is latest, I would need to downcase Implement > before it hits 'next' ;-) Ah, one thing I forgot to mention. Some of the tests updated in this series are unhappy with Dscho's "drive 'am' directly from the built-in code, bypassing git-rebase--am.sh scriptlet" topic.
Hi, On Mon, Jan 7, 2019 at 12:11 PM Junio C Hamano <gitster@pobox.com> wrote: > > Junio C Hamano <gitster@pobox.com> writes: > > > Elijah Newren <newren@gmail.com> writes: > > > >> On Tue, Dec 11, 2018 at 8:11 AM Elijah Newren <newren@gmail.com> wrote: > >>> > >>> This series continues the work of making rebase more self-consistent > >>> by removing inconsistencies between different backends. In > >>> particular, this series focuses on making the merge machinery behave > >>> like the interactive machinery (though a few differences between the am > >>> and interactive backends are also fixed along the way), and ultimately > >>> removes the merge backend in favor of reimplementing the relevant > >>> options on top of the interactive machinery. > >> > >> Friendly ping...let me know if you want me to simply resend v4. > >> > > > > If you have anything newer than 90673135 ("rebase: Implement --merge > > via the interactive machinery", 2018-12-11), then yeah, I haven't > > seen it. > > > > Thanks. > > > > P.S. even if that one is latest, I would need to downcase Implement > > before it hits 'next' ;-) > > Ah, one thing I forgot to mention. Some of the tests updated in > this series are unhappy with Dscho's "drive 'am' directly from the > built-in code, bypassing git-rebase--am.sh scriptlet" topic. 2018-12-11 is the newest (and is almost the same as the version from mid November); it's just been waiting for review. I'll fix up the casing of 'Implement' along with any other feedback, if any...maybe including rebasing on Dscho's series depending on how he wants to take it. Dscho: Looks like our series conflicts slightly. Would you like me to rebase mine on top of yours and squash the following change into commit c91c944a068e ("rebase: define linearization ordering and enforce it", 2018-12-11), or do you want to rebase your series on mine and either make a new commit out of this change or squash it in somewhere? diff --git a/builtin/rebase.c b/builtin/rebase.c index 0317280f83..54023547ff 100644 --- a/builtin/rebase.c +++ b/builtin/rebase.c @@ -578,7 +578,8 @@ static int run_am(struct rebase_options *opts) argv_array_pushl(&format_patch.args, "format-patch", "-k", "--stdout", "--full-index", "--cherry-pick", "--right-only", "--src-prefix=a/", "--dst-prefix=b/", "--no-renames", - "--no-cover-letter", "--pretty=mboxrd", NULL); + "--no-cover-letter", "--pretty=mboxrd", + "--topo-order", NULL); if (opts->git_format_patch_opt.len) argv_array_split(&format_patch.args, opts->git_format_patch_opt.buf); Elijah
Hi Junio, A small update... On Mon, Jan 7, 2019 at 12:39 PM Elijah Newren <newren@gmail.com> wrote: > On Mon, Jan 7, 2019 at 12:11 PM Junio C Hamano <gitster@pobox.com> wrote: > > Junio C Hamano <gitster@pobox.com> writes: > > > > > Elijah Newren <newren@gmail.com> writes: > > > > > >> On Tue, Dec 11, 2018 at 8:11 AM Elijah Newren <newren@gmail.com> wrote: > > >>> > > >>> This series continues the work of making rebase more self-consistent > > >>> by removing inconsistencies between different backends. In .... > > > P.S. even if that one is latest, I would need to downcase Implement > > > before it hits 'next' ;-) ... > > Ah, one thing I forgot to mention. Some of the tests updated in > > this series are unhappy with Dscho's "drive 'am' directly from the > > built-in code, bypassing git-rebase--am.sh scriptlet" topic. It looks like you've already done the downcasing in pu; thanks. Also, Dscho told me in private email that: "FWIW I am fine with your patches going in first, and I would rebase mine on top. It will take me probably until next week to get to my patch series and Junio's comments, anyway." So, I'm guessing that means he'll apply my patch below to his series. As such, other than maybe wait for folks to review my updated series, I'm not aware of any further changes I need to make to the en/rebase-merge-on-sequencer topic. > Dscho: Looks like our series conflicts slightly. Would you like me to > rebase mine on top of yours and squash the following change into > commit c91c944a068e ("rebase: define linearization ordering and > enforce it", 2018-12-11), or do you want to rebase your series on mine > and either make a new commit out of this change or squash it in > somewhere? > > diff --git a/builtin/rebase.c b/builtin/rebase.c > index 0317280f83..54023547ff 100644 > --- a/builtin/rebase.c > +++ b/builtin/rebase.c > @@ -578,7 +578,8 @@ static int run_am(struct rebase_options *opts) > argv_array_pushl(&format_patch.args, "format-patch", "-k", "--stdout", > "--full-index", "--cherry-pick", "--right-only", > "--src-prefix=a/", "--dst-prefix=b/", "--no-renames", > - "--no-cover-letter", "--pretty=mboxrd", NULL); > + "--no-cover-letter", "--pretty=mboxrd", > + "--topo-order", NULL); > if (opts->git_format_patch_opt.len) > argv_array_split(&format_patch.args, > opts->git_format_patch_opt.buf); > > > Elijah
Hi Elijah, On Mon, 7 Jan 2019, Elijah Newren wrote: > On Mon, Jan 7, 2019 at 12:11 PM Junio C Hamano <gitster@pobox.com> wrote: > > > > Junio C Hamano <gitster@pobox.com> writes: > > > > > Elijah Newren <newren@gmail.com> writes: > > > > > >> On Tue, Dec 11, 2018 at 8:11 AM Elijah Newren <newren@gmail.com> wrote: > > >>> > > >>> This series continues the work of making rebase more self-consistent > > >>> by removing inconsistencies between different backends. In > > >>> particular, this series focuses on making the merge machinery behave > > >>> like the interactive machinery (though a few differences between the am > > >>> and interactive backends are also fixed along the way), and ultimately > > >>> removes the merge backend in favor of reimplementing the relevant > > >>> options on top of the interactive machinery. > > >> > > >> Friendly ping...let me know if you want me to simply resend v4. > > >> > > > > > > If you have anything newer than 90673135 ("rebase: Implement --merge > > > via the interactive machinery", 2018-12-11), then yeah, I haven't > > > seen it. > > > > > > Thanks. > > > > > > P.S. even if that one is latest, I would need to downcase Implement > > > before it hits 'next' ;-) > > > > Ah, one thing I forgot to mention. Some of the tests updated in > > this series are unhappy with Dscho's "drive 'am' directly from the > > built-in code, bypassing git-rebase--am.sh scriptlet" topic. > > 2018-12-11 is the newest (and is almost the same as the version from > mid November); it's just been waiting for review. I'll fix up the > casing of 'Implement' along with any other feedback, if any...maybe > including rebasing on Dscho's series depending on how he wants to take > it. > > > Dscho: Looks like our series conflicts slightly. Would you like me to > rebase mine on top of yours and squash the following change into > commit c91c944a068e ("rebase: define linearization ordering and > enforce it", 2018-12-11), or do you want to rebase your series on mine > and either make a new commit out of this change or squash it in > somewhere? > > diff --git a/builtin/rebase.c b/builtin/rebase.c > index 0317280f83..54023547ff 100644 > --- a/builtin/rebase.c > +++ b/builtin/rebase.c > @@ -578,7 +578,8 @@ static int run_am(struct rebase_options *opts) > argv_array_pushl(&format_patch.args, "format-patch", "-k", "--stdout", > "--full-index", "--cherry-pick", "--right-only", > "--src-prefix=a/", "--dst-prefix=b/", "--no-renames", > - "--no-cover-letter", "--pretty=mboxrd", NULL); > + "--no-cover-letter", "--pretty=mboxrd", > + "--topo-order", NULL); > if (opts->git_format_patch_opt.len) > argv_array_split(&format_patch.args, > opts->git_format_patch_opt.buf); I can easily squash that in. Thank you! Dscho > > > Elijah > >
Hi Elijah, On Fri, 18 Jan 2019, Johannes Schindelin wrote: > On Mon, 7 Jan 2019, Elijah Newren wrote: > > > Dscho: Looks like our series conflicts slightly. Would you like me to > > rebase mine on top of yours and squash the following change into > > commit c91c944a068e ("rebase: define linearization ordering and > > enforce it", 2018-12-11), or do you want to rebase your series on mine > > and either make a new commit out of this change or squash it in > > somewhere? > > > > diff --git a/builtin/rebase.c b/builtin/rebase.c > > index 0317280f83..54023547ff 100644 > > --- a/builtin/rebase.c > > +++ b/builtin/rebase.c > > @@ -578,7 +578,8 @@ static int run_am(struct rebase_options *opts) > > argv_array_pushl(&format_patch.args, "format-patch", "-k", "--stdout", > > "--full-index", "--cherry-pick", "--right-only", > > "--src-prefix=a/", "--dst-prefix=b/", "--no-renames", > > - "--no-cover-letter", "--pretty=mboxrd", NULL); > > + "--no-cover-letter", "--pretty=mboxrd", > > + "--topo-order", NULL); > > if (opts->git_format_patch_opt.len) > > argv_array_split(&format_patch.args, > > opts->git_format_patch_opt.buf); > > I can easily squash that in. Actually, I take that back. This is tested-for in the regression test suite, and I need to keep the built-in and the scripted rebase in sync for that test suite to pass, so that single-liner would incur more changes than I am comfortable with adopting into the builtin-rebase--am patch series... I am about to submit a new iteration of my patch series, would it be too much trouble for you to rebase on top? If it would be, let me know, then I will rebase on top of yours. Ciao, Dscho
Johannes Schindelin <Johannes.Schindelin@gmx.de> writes: > I am about to submit a new iteration of my patch series, would it be too > much trouble for you to rebase on top? If it would be, let me know, then I > will rebase on top of yours. Or both of you keep the topics as-is and self-consistent, and let the rerere machinery to squash it in when the two topics gets merged.
On Fri, Jan 18, 2019 at 9:55 AM Junio C Hamano <gitster@pobox.com> wrote: > > Johannes Schindelin <Johannes.Schindelin@gmx.de> writes: > > > I am about to submit a new iteration of my patch series, would it be too > > much trouble for you to rebase on top? If it would be, let me know, then I > > will rebase on top of yours. > > Or both of you keep the topics as-is and self-consistent, and let > the rerere machinery to squash it in when the two topics gets > merged. Yeah, I see that commit 8d808e12a2cc ("Merge branch 'js/rebase-am' into jch", 2019-01-17) has my fix squashed in; thanks Junio. I'm happy to do whatever is needed with en/rebase-merge-on-sequencer that makes it easiest for everyone else. I'm assuming right now that there's nothing I should do to improve it, but if anyone wants me to rebase it or fix something up, just let me know.
Hi Junio, On Fri, 18 Jan 2019, Junio C Hamano wrote: > Johannes Schindelin <Johannes.Schindelin@gmx.de> writes: > > > I am about to submit a new iteration of my patch series, would it be too > > much trouble for you to rebase on top? If it would be, let me know, then I > > will rebase on top of yours. > > Or both of you keep the topics as-is and self-consistent, and let > the rerere machinery to squash it in when the two topics gets > merged. With all the experience I have with rerere, I don't trust it. Ciao, Johannes
Johannes Schindelin <Johannes.Schindelin@gmx.de> writes: > Hi Junio, > > On Fri, 18 Jan 2019, Junio C Hamano wrote: > >> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes: >> >> > I am about to submit a new iteration of my patch series, would it be too >> > much trouble for you to rebase on top? If it would be, let me know, then I >> > will rebase on top of yours. >> >> Or both of you keep the topics as-is and self-consistent, and let >> the rerere machinery to squash it in when the two topics gets >> merged. > > With all the experience I have with rerere, I don't trust it. FWIW, I trust it once I got right resolution better than randomly rebased resubmission that needs to be re-reviewed afresh.
Hi Elijah, On Tue, 11 Dec 2018, Elijah Newren wrote: > Differences since v3 (full range-diff below): > - Fixed the redundant "fatal: error:" error message prefixes, as pointed > out by Duy > - Rebased on 2.20.0 - Fixed the "comptable" tyop This, and the range-diff, look reasonable to me. Thanks, Dscho > Elijah Newren (8): > rebase: make builtin and legacy script error messages the same > rebase: fix incompatible options error message > t5407: add a test demonstrating how interactive handles --skip > differently > am, rebase--merge: do not overlook --skip'ed commits with post-rewrite > git-rebase, sequencer: extend --quiet option for the interactive > machinery > git-legacy-rebase: simplify unnecessary triply-nested if > rebase: define linearization ordering and enforce it > rebase: Implement --merge via the interactive machinery > > .gitignore | 1 - > Documentation/git-rebase.txt | 17 +--- > Makefile | 1 - > builtin/am.c | 9 ++ > builtin/rebase.c | 30 ++---- > git-legacy-rebase.sh | 65 ++++++------ > git-rebase--am.sh | 2 +- > git-rebase--common.sh | 2 +- > git-rebase--merge.sh | 164 ------------------------------ > sequencer.c | 23 +++-- > sequencer.h | 1 + > t/t3406-rebase-message.sh | 7 +- > t/t3420-rebase-autostash.sh | 78 ++------------ > t/t3421-rebase-topology-linear.sh | 10 +- > t/t3425-rebase-topology-merges.sh | 15 ++- > t/t5407-post-rewrite-hook.sh | 34 +++++++ > t/t9903-bash-prompt.sh | 2 +- > 17 files changed, 121 insertions(+), 340 deletions(-) > delete mode 100644 git-rebase--merge.sh > > Range-diff: > -: ---------- > 1: 2e8b1bcb8b rebase: make builtin and legacy script error messages the same > 1: 2f4bdd1980 ! 2: eba87828c6 rebase: fix incompatible options error message > @@ -9,12 +9,12 @@ > understood by separate backends were used: > > $ git rebase --keep --ignore-whitespace > - fatal: error: cannot combine interactive options (--interactive, --exec, > + fatal: cannot combine interactive options (--interactive, --exec, > --rebase-merges, --preserve-merges, --keep-empty, --root + --onto) with > am options (.git/rebase-apply/applying) > > $ git rebase --merge --ignore-whitespace > - fatal: error: cannot combine merge options (--merge, --strategy, > + fatal: cannot combine merge options (--merge, --strategy, > --strategy-option) with am options (.git/rebase-apply/applying) > > Note that in both cases, the list of "am options" is > @@ -33,18 +33,17 @@ > break; > > if (is_interactive(&options) && i >= 0) > -- die(_("error: cannot combine interactive options " > +- die(_("cannot combine interactive options " > - "(--interactive, --exec, --rebase-merges, " > - "--preserve-merges, --keep-empty, --root + " > - "--onto) with am options (%s)"), buf.buf); > -+ die(_("error: cannot combine am options " > ++ die(_("cannot combine am options " > + "with interactive options")); > if (options.type == REBASE_MERGE && i >= 0) > -- die(_("error: cannot combine merge options (--merge, " > +- die(_("cannot combine merge options (--merge, " > - "--strategy, --strategy-option) with am options " > - "(%s)"), buf.buf); > -+ die(_("error: cannot combine am options " > -+ "with merge options ")); > ++ die(_("cannot combine am options with merge options ")); > } > > if (options.signoff) { > @@ -56,15 +55,15 @@ > then > if test -n "$incompatible_opts" > then > -- die "$(gettext "error: cannot combine interactive options (--interactive, --exec, --rebase-merges, --preserve-merges, --keep-empty, --root + --onto) with am options ($incompatible_opts)")" > -+ die "$(gettext "error: cannot combine am options with interactive options")" > +- die "$(gettext "fatal: cannot combine interactive options (--interactive, --exec, --rebase-merges, --preserve-merges, --keep-empty, --root + --onto) with am options ($incompatible_opts)")" > ++ die "$(gettext "fatal: cannot combine am options with interactive options")" > fi > fi > if test -n "$do_merge"; then > if test -n "$incompatible_opts" > then > -- die "$(gettext "error: cannot combine merge options (--merge, --strategy, --strategy-option) with am options ($incompatible_opts)")" > -+ die "$(gettext "error: cannot combine am options with merge options")" > +- die "$(gettext "fatal: cannot combine merge options (--merge, --strategy, --strategy-option) with am options ($incompatible_opts)")" > ++ die "$(gettext "fatal: cannot combine am options with merge options")" > fi > fi > fi > 2: cc33a8ccc1 = 3: 15d929edb2 t5407: add a test demonstrating how interactive handles --skip differently > 3: f5838ef763 = 4: c9d6d5141e am, rebase--merge: do not overlook --skip'ed commits with post-rewrite > 4: 50dc863d9f = 5: 0b19ad8e2d git-rebase, sequencer: extend --quiet option for the interactive machinery > 5: 35cf552f27 ! 6: 5ded8654ec git-legacy-rebase: simplify unnecessary triply-nested if > @@ -18,7 +18,7 @@ > moving the innermost conditional to the outside, allowing us to remove > the test on git_am_opt entirely and giving us the following form: > > - if incomptable_opts: > + if incompatible_opts: > if interactive: > show_error_about_interactive_and_am_incompatibilities > if rebase-merge: > @@ -44,18 +44,18 @@ > then > - if test -n "$incompatible_opts" > - then > -- die "$(gettext "error: cannot combine am options with interactive options")" > +- die "$(gettext "fatal: cannot combine am options with interactive options")" > - fi > -+ die "$(gettext "error: cannot combine am options with interactive options")" > ++ die "$(gettext "fatal: cannot combine am options with interactive options")" > fi > - if test -n "$do_merge"; then > - if test -n "$incompatible_opts" > - then > -- die "$(gettext "error: cannot combine am options with merge options")" > +- die "$(gettext "fatal: cannot combine am options with merge options")" > - fi > + if test -n "$do_merge" > + then > -+ die "$(gettext "error: cannot combine am options with merge options")" > ++ die "$(gettext "fatal: cannot combine am options with merge options")" > fi > fi > > 6: 2a3d8ff1c1 = 7: bb8e5a4527 rebase: define linearization ordering and enforce it > 7: 58371d377a ! 8: 5de428d695 rebase: Implement --merge via the interactive machinery > @@ -142,14 +142,15 @@ > imply_interactive(&options, "--root without --onto"); > > @@ > + break; > > if (is_interactive(&options) && i >= 0) > - die(_("error: cannot combine am options " > +- die(_("cannot combine am options " > - "with interactive options")); > - if (options.type == REBASE_MERGE && i >= 0) > -- die(_("error: cannot combine am options " > -- "with merge options ")); > -+ "with either interactive or merge options")); > +- die(_("cannot combine am options with merge options ")); > ++ die(_("cannot combine am options with either " > ++ "interactive or merge options")); > } > > if (options.signoff) { > @@ -205,13 +206,13 @@ > then > - if test -n "$interactive_rebase" > - then > -- die "$(gettext "error: cannot combine am options with interactive options")" > +- die "$(gettext "fatal: cannot combine am options with interactive options")" > - fi > - if test -n "$do_merge" > + if test -n "$actually_interactive" || test "$do_merge" > then > -- die "$(gettext "error: cannot combine am options with merge options")" > -+ die "$(gettext "error: cannot combine am options with either interactive or merge options")" > +- die "$(gettext "fatal: cannot combine am options with merge options")" > ++ die "$(gettext "fatal: cannot combine am options with either interactive or merge options")" > fi > fi > > @@ -225,7 +226,7 @@ > # linear history? > ! (git rev-list --parents "$onto".."$orig_head" | sane_grep " .* ") > /dev/null > @@ > - GIT_PAGER='' git diff --stat --summary "$mb" "$onto" > + GIT_PAGER='' git diff --stat --summary "$mb_tree" "$onto" > fi > > +if test -z "$actually_interactive" && test "$mb" = "$orig_head" > -- > 2.20.0.8.g5de428d695 > >
Hi Elijah, On Mon, 21 Jan 2019, Johannes Schindelin wrote: > On Tue, 11 Dec 2018, Elijah Newren wrote: > > > Differences since v3 (full range-diff below): > > - Fixed the redundant "fatal: error:" error message prefixes, as pointed > > out by Duy > > - Rebased on 2.20.0 > > - Fixed the "comptable" tyop > > This, and the range-diff, look reasonable to me. And now that I looked through all 8 patches, those look reasonable to me, too. I think this is ready for `next`. Ciao, Dscho > > Thanks, > Dscho > > > Elijah Newren (8): > > rebase: make builtin and legacy script error messages the same > > rebase: fix incompatible options error message > > t5407: add a test demonstrating how interactive handles --skip > > differently > > am, rebase--merge: do not overlook --skip'ed commits with post-rewrite > > git-rebase, sequencer: extend --quiet option for the interactive > > machinery > > git-legacy-rebase: simplify unnecessary triply-nested if > > rebase: define linearization ordering and enforce it > > rebase: Implement --merge via the interactive machinery > > > > .gitignore | 1 - > > Documentation/git-rebase.txt | 17 +--- > > Makefile | 1 - > > builtin/am.c | 9 ++ > > builtin/rebase.c | 30 ++---- > > git-legacy-rebase.sh | 65 ++++++------ > > git-rebase--am.sh | 2 +- > > git-rebase--common.sh | 2 +- > > git-rebase--merge.sh | 164 ------------------------------ > > sequencer.c | 23 +++-- > > sequencer.h | 1 + > > t/t3406-rebase-message.sh | 7 +- > > t/t3420-rebase-autostash.sh | 78 ++------------ > > t/t3421-rebase-topology-linear.sh | 10 +- > > t/t3425-rebase-topology-merges.sh | 15 ++- > > t/t5407-post-rewrite-hook.sh | 34 +++++++ > > t/t9903-bash-prompt.sh | 2 +- > > 17 files changed, 121 insertions(+), 340 deletions(-) > > delete mode 100644 git-rebase--merge.sh > > > > Range-diff: > > -: ---------- > 1: 2e8b1bcb8b rebase: make builtin and legacy script error messages the same > > 1: 2f4bdd1980 ! 2: eba87828c6 rebase: fix incompatible options error message > > @@ -9,12 +9,12 @@ > > understood by separate backends were used: > > > > $ git rebase --keep --ignore-whitespace > > - fatal: error: cannot combine interactive options (--interactive, --exec, > > + fatal: cannot combine interactive options (--interactive, --exec, > > --rebase-merges, --preserve-merges, --keep-empty, --root + --onto) with > > am options (.git/rebase-apply/applying) > > > > $ git rebase --merge --ignore-whitespace > > - fatal: error: cannot combine merge options (--merge, --strategy, > > + fatal: cannot combine merge options (--merge, --strategy, > > --strategy-option) with am options (.git/rebase-apply/applying) > > > > Note that in both cases, the list of "am options" is > > @@ -33,18 +33,17 @@ > > break; > > > > if (is_interactive(&options) && i >= 0) > > -- die(_("error: cannot combine interactive options " > > +- die(_("cannot combine interactive options " > > - "(--interactive, --exec, --rebase-merges, " > > - "--preserve-merges, --keep-empty, --root + " > > - "--onto) with am options (%s)"), buf.buf); > > -+ die(_("error: cannot combine am options " > > ++ die(_("cannot combine am options " > > + "with interactive options")); > > if (options.type == REBASE_MERGE && i >= 0) > > -- die(_("error: cannot combine merge options (--merge, " > > +- die(_("cannot combine merge options (--merge, " > > - "--strategy, --strategy-option) with am options " > > - "(%s)"), buf.buf); > > -+ die(_("error: cannot combine am options " > > -+ "with merge options ")); > > ++ die(_("cannot combine am options with merge options ")); > > } > > > > if (options.signoff) { > > @@ -56,15 +55,15 @@ > > then > > if test -n "$incompatible_opts" > > then > > -- die "$(gettext "error: cannot combine interactive options (--interactive, --exec, --rebase-merges, --preserve-merges, --keep-empty, --root + --onto) with am options ($incompatible_opts)")" > > -+ die "$(gettext "error: cannot combine am options with interactive options")" > > +- die "$(gettext "fatal: cannot combine interactive options (--interactive, --exec, --rebase-merges, --preserve-merges, --keep-empty, --root + --onto) with am options ($incompatible_opts)")" > > ++ die "$(gettext "fatal: cannot combine am options with interactive options")" > > fi > > fi > > if test -n "$do_merge"; then > > if test -n "$incompatible_opts" > > then > > -- die "$(gettext "error: cannot combine merge options (--merge, --strategy, --strategy-option) with am options ($incompatible_opts)")" > > -+ die "$(gettext "error: cannot combine am options with merge options")" > > +- die "$(gettext "fatal: cannot combine merge options (--merge, --strategy, --strategy-option) with am options ($incompatible_opts)")" > > ++ die "$(gettext "fatal: cannot combine am options with merge options")" > > fi > > fi > > fi > > 2: cc33a8ccc1 = 3: 15d929edb2 t5407: add a test demonstrating how interactive handles --skip differently > > 3: f5838ef763 = 4: c9d6d5141e am, rebase--merge: do not overlook --skip'ed commits with post-rewrite > > 4: 50dc863d9f = 5: 0b19ad8e2d git-rebase, sequencer: extend --quiet option for the interactive machinery > > 5: 35cf552f27 ! 6: 5ded8654ec git-legacy-rebase: simplify unnecessary triply-nested if > > @@ -18,7 +18,7 @@ > > moving the innermost conditional to the outside, allowing us to remove > > the test on git_am_opt entirely and giving us the following form: > > > > - if incomptable_opts: > > + if incompatible_opts: > > if interactive: > > show_error_about_interactive_and_am_incompatibilities > > if rebase-merge: > > @@ -44,18 +44,18 @@ > > then > > - if test -n "$incompatible_opts" > > - then > > -- die "$(gettext "error: cannot combine am options with interactive options")" > > +- die "$(gettext "fatal: cannot combine am options with interactive options")" > > - fi > > -+ die "$(gettext "error: cannot combine am options with interactive options")" > > ++ die "$(gettext "fatal: cannot combine am options with interactive options")" > > fi > > - if test -n "$do_merge"; then > > - if test -n "$incompatible_opts" > > - then > > -- die "$(gettext "error: cannot combine am options with merge options")" > > +- die "$(gettext "fatal: cannot combine am options with merge options")" > > - fi > > + if test -n "$do_merge" > > + then > > -+ die "$(gettext "error: cannot combine am options with merge options")" > > ++ die "$(gettext "fatal: cannot combine am options with merge options")" > > fi > > fi > > > > 6: 2a3d8ff1c1 = 7: bb8e5a4527 rebase: define linearization ordering and enforce it > > 7: 58371d377a ! 8: 5de428d695 rebase: Implement --merge via the interactive machinery > > @@ -142,14 +142,15 @@ > > imply_interactive(&options, "--root without --onto"); > > > > @@ > > + break; > > > > if (is_interactive(&options) && i >= 0) > > - die(_("error: cannot combine am options " > > +- die(_("cannot combine am options " > > - "with interactive options")); > > - if (options.type == REBASE_MERGE && i >= 0) > > -- die(_("error: cannot combine am options " > > -- "with merge options ")); > > -+ "with either interactive or merge options")); > > +- die(_("cannot combine am options with merge options ")); > > ++ die(_("cannot combine am options with either " > > ++ "interactive or merge options")); > > } > > > > if (options.signoff) { > > @@ -205,13 +206,13 @@ > > then > > - if test -n "$interactive_rebase" > > - then > > -- die "$(gettext "error: cannot combine am options with interactive options")" > > +- die "$(gettext "fatal: cannot combine am options with interactive options")" > > - fi > > - if test -n "$do_merge" > > + if test -n "$actually_interactive" || test "$do_merge" > > then > > -- die "$(gettext "error: cannot combine am options with merge options")" > > -+ die "$(gettext "error: cannot combine am options with either interactive or merge options")" > > +- die "$(gettext "fatal: cannot combine am options with merge options")" > > ++ die "$(gettext "fatal: cannot combine am options with either interactive or merge options")" > > fi > > fi > > > > @@ -225,7 +226,7 @@ > > # linear history? > > ! (git rev-list --parents "$onto".."$orig_head" | sane_grep " .* ") > /dev/null > > @@ > > - GIT_PAGER='' git diff --stat --summary "$mb" "$onto" > > + GIT_PAGER='' git diff --stat --summary "$mb_tree" "$onto" > > fi > > > > +if test -z "$actually_interactive" && test "$mb" = "$orig_head" > > -- > > 2.20.0.8.g5de428d695 > > > > >
Hi Junio, On Fri, 18 Jan 2019, Junio C Hamano wrote: > Johannes Schindelin <Johannes.Schindelin@gmx.de> writes: > > > On Fri, 18 Jan 2019, Junio C Hamano wrote: > > > >> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes: > >> > >> > I am about to submit a new iteration of my patch series, would it > >> > be too much trouble for you to rebase on top? If it would be, let > >> > me know, then I will rebase on top of yours. > >> > >> Or both of you keep the topics as-is and self-consistent, and let the > >> rerere machinery to squash it in when the two topics gets merged. > > > > With all the experience I have with rerere, I don't trust it. > > FWIW, I trust it once I got right resolution better than randomly > rebased resubmission that needs to be re-reviewed afresh. Once again, our experiences teach us diametrically opposed things. I definitely trust my rebased version more than your rerere-affected resolution. Ciao, Dscho
Hi Dscho, On Mon, Jan 21, 2019 at 1:01 PM Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote: > > Hi Elijah, > > On Mon, 21 Jan 2019, Johannes Schindelin wrote: > > > On Tue, 11 Dec 2018, Elijah Newren wrote: > > > > > Differences since v3 (full range-diff below): > > > - Fixed the redundant "fatal: error:" error message prefixes, as pointed > > > out by Duy > > > - Rebased on 2.20.0 > > > > - Fixed the "comptable" tyop > > > > This, and the range-diff, look reasonable to me. > > And now that I looked through all 8 patches, those look reasonable to me, > too. I think this is ready for `next`. > > Ciao, > Dscho Thanks for reviewing it!
This series continues the work of making rebase more self-consistent by removing inconsistencies between different backends. In particular, this series focuses on making the merge machinery behave like the interactive machinery (though a few differences between the am and interactive backends are also fixed along the way), and ultimately removes the merge backend in favor of reimplementing the relevant options on top of the interactive machinery. Differences since v3 (full range-diff below): - Fixed the redundant "fatal: error:" error message prefixes, as pointed out by Duy - Rebased on 2.20.0 Elijah Newren (8): rebase: make builtin and legacy script error messages the same rebase: fix incompatible options error message t5407: add a test demonstrating how interactive handles --skip differently am, rebase--merge: do not overlook --skip'ed commits with post-rewrite git-rebase, sequencer: extend --quiet option for the interactive machinery git-legacy-rebase: simplify unnecessary triply-nested if rebase: define linearization ordering and enforce it rebase: Implement --merge via the interactive machinery .gitignore | 1 - Documentation/git-rebase.txt | 17 +--- Makefile | 1 - builtin/am.c | 9 ++ builtin/rebase.c | 30 ++---- git-legacy-rebase.sh | 65 ++++++------ git-rebase--am.sh | 2 +- git-rebase--common.sh | 2 +- git-rebase--merge.sh | 164 ------------------------------ sequencer.c | 23 +++-- sequencer.h | 1 + t/t3406-rebase-message.sh | 7 +- t/t3420-rebase-autostash.sh | 78 ++------------ t/t3421-rebase-topology-linear.sh | 10 +- t/t3425-rebase-topology-merges.sh | 15 ++- t/t5407-post-rewrite-hook.sh | 34 +++++++ t/t9903-bash-prompt.sh | 2 +- 17 files changed, 121 insertions(+), 340 deletions(-) delete mode 100644 git-rebase--merge.sh Range-diff: -: ---------- > 1: 2e8b1bcb8b rebase: make builtin and legacy script error messages the same 1: 2f4bdd1980 ! 2: eba87828c6 rebase: fix incompatible options error message @@ -9,12 +9,12 @@ understood by separate backends were used: $ git rebase --keep --ignore-whitespace - fatal: error: cannot combine interactive options (--interactive, --exec, + fatal: cannot combine interactive options (--interactive, --exec, --rebase-merges, --preserve-merges, --keep-empty, --root + --onto) with am options (.git/rebase-apply/applying) $ git rebase --merge --ignore-whitespace - fatal: error: cannot combine merge options (--merge, --strategy, + fatal: cannot combine merge options (--merge, --strategy, --strategy-option) with am options (.git/rebase-apply/applying) Note that in both cases, the list of "am options" is @@ -33,18 +33,17 @@ break; if (is_interactive(&options) && i >= 0) -- die(_("error: cannot combine interactive options " +- die(_("cannot combine interactive options " - "(--interactive, --exec, --rebase-merges, " - "--preserve-merges, --keep-empty, --root + " - "--onto) with am options (%s)"), buf.buf); -+ die(_("error: cannot combine am options " ++ die(_("cannot combine am options " + "with interactive options")); if (options.type == REBASE_MERGE && i >= 0) -- die(_("error: cannot combine merge options (--merge, " +- die(_("cannot combine merge options (--merge, " - "--strategy, --strategy-option) with am options " - "(%s)"), buf.buf); -+ die(_("error: cannot combine am options " -+ "with merge options ")); ++ die(_("cannot combine am options with merge options ")); } if (options.signoff) { @@ -56,15 +55,15 @@ then if test -n "$incompatible_opts" then -- die "$(gettext "error: cannot combine interactive options (--interactive, --exec, --rebase-merges, --preserve-merges, --keep-empty, --root + --onto) with am options ($incompatible_opts)")" -+ die "$(gettext "error: cannot combine am options with interactive options")" +- die "$(gettext "fatal: cannot combine interactive options (--interactive, --exec, --rebase-merges, --preserve-merges, --keep-empty, --root + --onto) with am options ($incompatible_opts)")" ++ die "$(gettext "fatal: cannot combine am options with interactive options")" fi fi if test -n "$do_merge"; then if test -n "$incompatible_opts" then -- die "$(gettext "error: cannot combine merge options (--merge, --strategy, --strategy-option) with am options ($incompatible_opts)")" -+ die "$(gettext "error: cannot combine am options with merge options")" +- die "$(gettext "fatal: cannot combine merge options (--merge, --strategy, --strategy-option) with am options ($incompatible_opts)")" ++ die "$(gettext "fatal: cannot combine am options with merge options")" fi fi fi 2: cc33a8ccc1 = 3: 15d929edb2 t5407: add a test demonstrating how interactive handles --skip differently 3: f5838ef763 = 4: c9d6d5141e am, rebase--merge: do not overlook --skip'ed commits with post-rewrite 4: 50dc863d9f = 5: 0b19ad8e2d git-rebase, sequencer: extend --quiet option for the interactive machinery 5: 35cf552f27 ! 6: 5ded8654ec git-legacy-rebase: simplify unnecessary triply-nested if @@ -18,7 +18,7 @@ moving the innermost conditional to the outside, allowing us to remove the test on git_am_opt entirely and giving us the following form: - if incomptable_opts: + if incompatible_opts: if interactive: show_error_about_interactive_and_am_incompatibilities if rebase-merge: @@ -44,18 +44,18 @@ then - if test -n "$incompatible_opts" - then -- die "$(gettext "error: cannot combine am options with interactive options")" +- die "$(gettext "fatal: cannot combine am options with interactive options")" - fi -+ die "$(gettext "error: cannot combine am options with interactive options")" ++ die "$(gettext "fatal: cannot combine am options with interactive options")" fi - if test -n "$do_merge"; then - if test -n "$incompatible_opts" - then -- die "$(gettext "error: cannot combine am options with merge options")" +- die "$(gettext "fatal: cannot combine am options with merge options")" - fi + if test -n "$do_merge" + then -+ die "$(gettext "error: cannot combine am options with merge options")" ++ die "$(gettext "fatal: cannot combine am options with merge options")" fi fi 6: 2a3d8ff1c1 = 7: bb8e5a4527 rebase: define linearization ordering and enforce it 7: 58371d377a ! 8: 5de428d695 rebase: Implement --merge via the interactive machinery @@ -142,14 +142,15 @@ imply_interactive(&options, "--root without --onto"); @@ + break; if (is_interactive(&options) && i >= 0) - die(_("error: cannot combine am options " +- die(_("cannot combine am options " - "with interactive options")); - if (options.type == REBASE_MERGE && i >= 0) -- die(_("error: cannot combine am options " -- "with merge options ")); -+ "with either interactive or merge options")); +- die(_("cannot combine am options with merge options ")); ++ die(_("cannot combine am options with either " ++ "interactive or merge options")); } if (options.signoff) { @@ -205,13 +206,13 @@ then - if test -n "$interactive_rebase" - then -- die "$(gettext "error: cannot combine am options with interactive options")" +- die "$(gettext "fatal: cannot combine am options with interactive options")" - fi - if test -n "$do_merge" + if test -n "$actually_interactive" || test "$do_merge" then -- die "$(gettext "error: cannot combine am options with merge options")" -+ die "$(gettext "error: cannot combine am options with either interactive or merge options")" +- die "$(gettext "fatal: cannot combine am options with merge options")" ++ die "$(gettext "fatal: cannot combine am options with either interactive or merge options")" fi fi @@ -225,7 +226,7 @@ # linear history? ! (git rev-list --parents "$onto".."$orig_head" | sane_grep " .* ") > /dev/null @@ - GIT_PAGER='' git diff --stat --summary "$mb" "$onto" + GIT_PAGER='' git diff --stat --summary "$mb_tree" "$onto" fi +if test -z "$actually_interactive" && test "$mb" = "$orig_head"