git@vger.kernel.org mailing list mirror (one of many)
 help / Atom feed
* [PATCH 0/8] rebase -i: offer to recreate merge commits
@ 2018-01-18 15:35 Johannes Schindelin
  2018-01-18 16:49 ` Jacob Keller
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Johannes Schindelin @ 2018-01-18 15:35 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Jacob Keller

Once upon a time, I dreamt of an interactive rebase that would not
flatten branch structure, but instead recreate the commit topology
faithfully.

My original attempt was --preserve-merges, but that design was so
limited that I did not even enable it in interactive mode.

Subsequently, it *was* enabled in interactive mode, with the predictable
consequences: as the --preserve-merges design does not allow for
specifying the parents of merge commits explicitly, all the new commits'
parents are defined *implicitly* by the previous commit history, and
hence it is *not possible to even reorder commits*.

This design flaw cannot be fixed. Not without a complete re-design, at
least. This patch series offers such a re-design.

Think of --recreate-merges as "--preserve-merges done right". It
introduces new verbs for the todo list, `label`, `reset` and `merge`.
For a commit topology like this:

            A - B - C
              \   /
                D

the generated todo list would look like this:

            # branch D
            pick 0123 A
            label branch-point
            pick 1234 D
            label D

            reset branch-point
            pick 2345 B
            merge 3456 D C

There are more patches in the pipeline, based on this patch series, but
left for later in the interest of reviewable patch series: one mini
series to use the sequencer even for `git rebase -i --root`, and another
one to add support for octopus merges to --recreate-merges.


Johannes Schindelin (8):
  sequencer: introduce new commands to reset the revision
  sequencer: introduce the `merge` command
  sequencer: fast-forward merge commits, if possible
  rebase-helper --make-script: introduce a flag to recreate merges
  rebase: introduce the --recreate-merges option
  sequencer: handle autosquash and post-rewrite for merge commands
  pull: accept --rebase=recreate to recreate the branch topology
  rebase -i: introduce --recreate-merges=no-rebase-cousins

 Documentation/config.txt               |   8 +
 Documentation/git-pull.txt             |   5 +-
 Documentation/git-rebase.txt           |  13 +-
 builtin/pull.c                         |  14 +-
 builtin/rebase--helper.c               |  13 +-
 builtin/remote.c                       |   2 +
 contrib/completion/git-completion.bash |   4 +-
 git-rebase--interactive.sh             |   6 +
 git-rebase.sh                          |  16 +
 refs.c                                 |   3 +-
 sequencer.c                            | 697 ++++++++++++++++++++++++++++++++-
 sequencer.h                            |   9 +
 t/t3430-rebase-recreate-merges.sh      | 208 ++++++++++
 13 files changed, 977 insertions(+), 21 deletions(-)
 create mode 100755 t/t3430-rebase-recreate-merges.sh


base-commit: 2512f15446149235156528dafbe75930c712b29e
Published-As: https://github.com/dscho/git/releases/tag/recreate-merges-v1
Fetch-It-Via: git fetch https://github.com/dscho/git recreate-merges-v1
-- 
2.15.1.windows.2.1430.ga56c4f9e2a9


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 0/8] rebase -i: offer to recreate merge commits
  2018-01-18 15:35 [PATCH 0/8] rebase -i: offer to recreate merge commits Johannes Schindelin
@ 2018-01-18 16:49 ` Jacob Keller
  2018-01-19 20:25 ` Junio C Hamano
  2018-01-23 20:29 ` Junio C Hamano
  2 siblings, 0 replies; 6+ messages in thread
From: Jacob Keller @ 2018-01-18 16:49 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: Git mailing list, Junio C Hamano

On Thu, Jan 18, 2018 at 7:35 AM, Johannes Schindelin
<johannes.schindelin@gmx.de> wrote:
> Once upon a time, I dreamt of an interactive rebase that would not
> flatten branch structure, but instead recreate the commit topology
> faithfully.
>
> My original attempt was --preserve-merges, but that design was so
> limited that I did not even enable it in interactive mode.
>
> Subsequently, it *was* enabled in interactive mode, with the predictable
> consequences: as the --preserve-merges design does not allow for
> specifying the parents of merge commits explicitly, all the new commits'
> parents are defined *implicitly* by the previous commit history, and
> hence it is *not possible to even reorder commits*.
>
> This design flaw cannot be fixed. Not without a complete re-design, at
> least. This patch series offers such a re-design.
>
> Think of --recreate-merges as "--preserve-merges done right". It
> introduces new verbs for the todo list, `label`, `reset` and `merge`.
> For a commit topology like this:
>
>             A - B - C
>               \   /
>                 D
>
> the generated todo list would look like this:
>
>             # branch D
>             pick 0123 A
>             label branch-point
>             pick 1234 D
>             label D
>
>             reset branch-point
>             pick 2345 B
>             merge 3456 D C
>
> There are more patches in the pipeline, based on this patch series, but
> left for later in the interest of reviewable patch series: one mini
> series to use the sequencer even for `git rebase -i --root`, and another
> one to add support for octopus merges to --recreate-merges.
>
>

I've been looking forward to seeing this hit the list! Overall I don't
think I have any major complaints, except to make sure the special
label "onto" is documented.

I think it's possible to "reword" or "edit" the merge commit by
inserting "x false" after the merge commnd to halt the rebase and let
the user perform commands to edit, so that should work well.

Thanks for taking the time to make this a reality!

Thanks,
Jake

> Johannes Schindelin (8):
>   sequencer: introduce new commands to reset the revision
>   sequencer: introduce the `merge` command
>   sequencer: fast-forward merge commits, if possible
>   rebase-helper --make-script: introduce a flag to recreate merges
>   rebase: introduce the --recreate-merges option
>   sequencer: handle autosquash and post-rewrite for merge commands
>   pull: accept --rebase=recreate to recreate the branch topology
>   rebase -i: introduce --recreate-merges=no-rebase-cousins
>
>  Documentation/config.txt               |   8 +
>  Documentation/git-pull.txt             |   5 +-
>  Documentation/git-rebase.txt           |  13 +-
>  builtin/pull.c                         |  14 +-
>  builtin/rebase--helper.c               |  13 +-
>  builtin/remote.c                       |   2 +
>  contrib/completion/git-completion.bash |   4 +-
>  git-rebase--interactive.sh             |   6 +
>  git-rebase.sh                          |  16 +
>  refs.c                                 |   3 +-
>  sequencer.c                            | 697 ++++++++++++++++++++++++++++++++-
>  sequencer.h                            |   9 +
>  t/t3430-rebase-recreate-merges.sh      | 208 ++++++++++
>  13 files changed, 977 insertions(+), 21 deletions(-)
>  create mode 100755 t/t3430-rebase-recreate-merges.sh
>
>
> base-commit: 2512f15446149235156528dafbe75930c712b29e
> Published-As: https://github.com/dscho/git/releases/tag/recreate-merges-v1
> Fetch-It-Via: git fetch https://github.com/dscho/git recreate-merges-v1
> --
> 2.15.1.windows.2.1430.ga56c4f9e2a9
>

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 0/8] rebase -i: offer to recreate merge commits
  2018-01-18 15:35 [PATCH 0/8] rebase -i: offer to recreate merge commits Johannes Schindelin
  2018-01-18 16:49 ` Jacob Keller
@ 2018-01-19 20:25 ` Junio C Hamano
  2018-01-29 21:53   ` Johannes Schindelin
  2018-01-23 20:29 ` Junio C Hamano
  2 siblings, 1 reply; 6+ messages in thread
From: Junio C Hamano @ 2018-01-19 20:25 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: git, Jacob Keller

Johannes Schindelin <johannes.schindelin@gmx.de> writes:

> Think of --recreate-merges as "--preserve-merges done right". It
> introduces new verbs for the todo list, `label`, `reset` and `merge`.
> For a commit topology like this:
>
>             A - B - C
>               \   /
>                 D
>
> the generated todo list would look like this:
>
>             # branch D
>             pick 0123 A
>             label branch-point
>             pick 1234 D
>             label D
>
>             reset branch-point
>             pick 2345 B
>             merge 3456 D C

Yup.  I've seen this design talked about on list in the past, and
I've always felt that this is "sequencer done right".

At the first glance, it may feel somewhat unsatisfying that "merge"
has to say effects of which commits should be reflected in the
result and which commot to take the log message from, i.e.
(recreated)D is merged to form the resulting tree, and 3456=C is
used for the log, to recreate C in the above example, while "pick"
always uses the same commit for both, i.e. recreated B inherits both
the changes and log message from the original B=2345 (or depending
on the readers' point of view, "merge" is allowed to use two
different commits, while "pick" is always limited to the same one).

But I think this distinction is probably fundamental and I am not
opposed to it at all.  The result of "pick" has only one parent, and
the parent is determined only by the previous actions and not by
anything on the "pick" line in the todo list.  But the result of
"merge" has to record all the other parents, and only the first
parent is determined implicitly by the previous actions.  We need to
tell the "merge" command about "3456=C" in order to recreate the
effect of original merge commit (i.e. changes between B and C) as
well as its log message, and we also need to tell it about label "D"
that it is the "other parent" that need to be recorded.

Obviously "merge" command syntax should allow recreating an octopus,
so whenever I said "two" in the above, I meant "N".  The original
merge commit is needed so that the effect to replay (roughly: a
patch going to the original merge result from its first parent) can
be learned from the existing history, and all the other "N-1"
parents needs to be given (and they must have been already created
in the todo list) so that the resulting recreated merge can be
recorded with them as parents (in addition to the first parent that
is implicitly given as the result of all the previous steps).

One interesting (and probably useful) thing to notice is that if A
were not rebased in the above sample picture, and only B were the
one that was tweaked, then a recreated C may use the same original D
as its side parent, and the mechanism outlined above naturally can
support it by allowing an un-rewritten commit to be given as a side
parent when "merge" is redoing C.

I probably won't have time to actually look at the code for a few
days, but I am reasonably excited about the topic ;-)

Thanks.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 0/8] rebase -i: offer to recreate merge commits
  2018-01-18 15:35 [PATCH 0/8] rebase -i: offer to recreate merge commits Johannes Schindelin
  2018-01-18 16:49 ` Jacob Keller
  2018-01-19 20:25 ` Junio C Hamano
@ 2018-01-23 20:29 ` Junio C Hamano
  2018-01-29 22:53   ` Johannes Schindelin
  2 siblings, 1 reply; 6+ messages in thread
From: Junio C Hamano @ 2018-01-23 20:29 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: git, Jacob Keller

Johannes Schindelin <johannes.schindelin@gmx.de> writes:

> My original attempt was --preserve-merges, but that design was so
> limited that I did not even enable it in interactive mode.
> ...
> There are more patches in the pipeline, based on this patch series, but
> left for later in the interest of reviewable patch series: one mini
> series to use the sequencer even for `git rebase -i --root`, and another
> one to add support for octopus merges to --recreate-merges.

I left comments on a handful of them, but I do not think any of them
spotted a grave design issue to be a show stopper.  Overall, the
series was quite a pleasant read, even with those minor nits and
rooms for improvements.

Thanks.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 0/8] rebase -i: offer to recreate merge commits
  2018-01-19 20:25 ` Junio C Hamano
@ 2018-01-29 21:53   ` Johannes Schindelin
  0 siblings, 0 replies; 6+ messages in thread
From: Johannes Schindelin @ 2018-01-29 21:53 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Jacob Keller

Hi Junio,

On Fri, 19 Jan 2018, Junio C Hamano wrote:

> Johannes Schindelin <johannes.schindelin@gmx.de> writes:
> 
> > Think of --recreate-merges as "--preserve-merges done right". It
> > introduces new verbs for the todo list, `label`, `reset` and `merge`.
> > For a commit topology like this:
> >
> >             A - B - C
> >               \   /
> >                 D
> >
> > the generated todo list would look like this:
> >
> >             # branch D
> >             pick 0123 A
> >             label branch-point
> >             pick 1234 D
> >             label D
> >
> >             reset branch-point
> >             pick 2345 B
> >             merge 3456 D C
> 
> Yup.  I've seen this design talked about on list in the past, and
> I've always felt that this is "sequencer done right".
> 
> At the first glance, it may feel somewhat unsatisfying that "merge"
> has to say effects of which commits should be reflected in the
> result and which commot to take the log message from, i.e.
> (recreated)D is merged to form the resulting tree, and 3456=C is
> used for the log, to recreate C in the above example, while "pick"
> always uses the same commit for both, i.e. recreated B inherits both
> the changes and log message from the original B=2345 (or depending
> on the readers' point of view, "merge" is allowed to use two
> different commits, while "pick" is always limited to the same one).
> 
> But I think this distinction is probably fundamental and I am not
> opposed to it at all.  The result of "pick" has only one parent, and
> the parent is determined only by the previous actions and not by
> anything on the "pick" line in the todo list.  But the result of
> "merge" has to record all the other parents, and only the first
> parent is determined implicitly by the previous actions.  We need to
> tell the "merge" command about "3456=C" in order to recreate the
> effect of original merge commit (i.e. changes between B and C) as
> well as its log message, and we also need to tell it about label "D"
> that it is the "other parent" that need to be recorded.

Yes, this was the hard lesson of the failed preserve-merges design.

> Obviously "merge" command syntax should allow recreating an octopus,
> so whenever I said "two" in the above, I meant "N".  The original
> merge commit is needed so that the effect to replay (roughly: a
> patch going to the original merge result from its first parent) can
> be learned from the existing history, and all the other "N-1"
> parents needs to be given (and they must have been already created
> in the todo list) so that the resulting recreated merge can be
> recorded with them as parents (in addition to the first parent that
> is implicitly given as the result of all the previous steps).

I have two more patch series lined up after this one, the first one
implements --root via the sequencer, and the second one indeed extends
`merge` to handle octopus commits.

> One interesting (and probably useful) thing to notice is that if A
> were not rebased in the above sample picture, and only B were the
> one that was tweaked, then a recreated C may use the same original D
> as its side parent, and the mechanism outlined above naturally can
> support it by allowing an un-rewritten commit to be given as a side
> parent when "merge" is redoing C.

I think that you will get a kick out of reading the commit message of the
last commit, as it does talk about the problematic C: it *would* be
rebased by default.

In the next iteration I will actually switch around the default from
rebase-cousins to no-rebase-cousins for that reason.

Ciao,
Dscho

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 0/8] rebase -i: offer to recreate merge commits
  2018-01-23 20:29 ` Junio C Hamano
@ 2018-01-29 22:53   ` Johannes Schindelin
  0 siblings, 0 replies; 6+ messages in thread
From: Johannes Schindelin @ 2018-01-29 22:53 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Jacob Keller

Hi Junio,

On Tue, 23 Jan 2018, Junio C Hamano wrote:

> Johannes Schindelin <johannes.schindelin@gmx.de> writes:
> 
> > My original attempt was --preserve-merges, but that design was so
> > limited that I did not even enable it in interactive mode.
> > ...
> > There are more patches in the pipeline, based on this patch series, but
> > left for later in the interest of reviewable patch series: one mini
> > series to use the sequencer even for `git rebase -i --root`, and another
> > one to add support for octopus merges to --recreate-merges.
> 
> I left comments on a handful of them, but I do not think any of them
> spotted a grave design issue to be a show stopper.  Overall, the
> series was quite a pleasant read, even with those minor nits and
> rooms for improvements.

I objected to a couple obviously problematic suggestions, and I
implemented all others (even those to which I did not respond
specifically).

Ciao,
Dscho

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, back to index

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-01-18 15:35 [PATCH 0/8] rebase -i: offer to recreate merge commits Johannes Schindelin
2018-01-18 16:49 ` Jacob Keller
2018-01-19 20:25 ` Junio C Hamano
2018-01-29 21:53   ` Johannes Schindelin
2018-01-23 20:29 ` Junio C Hamano
2018-01-29 22:53   ` Johannes Schindelin

git@vger.kernel.org mailing list mirror (one of many)

Archives are clonable:
	git clone --mirror https://public-inbox.org/git
	git clone --mirror http://ou63pmih66umazou.onion/git
	git clone --mirror http://czquwvybam4bgbro.onion/git
	git clone --mirror http://hjrcffqmbrq6wope.onion/git

Newsgroups are available over NNTP:
	nntp://news.public-inbox.org/inbox.comp.version-control.git
	nntp://ou63pmih66umazou.onion/inbox.comp.version-control.git
	nntp://czquwvybam4bgbro.onion/inbox.comp.version-control.git
	nntp://hjrcffqmbrq6wope.onion/inbox.comp.version-control.git
	nntp://news.gmane.org/gmane.comp.version-control.git

 note: .onion URLs require Tor: https://www.torproject.org/
       or Tor2web: https://www.tor2web.org/

AGPL code for this site: git clone https://public-inbox.org/ public-inbox