Re: Git garden shears, was Re: [PATCH 13/22] sequencer: remember the onelines when parsing the todo file

[Johannes Schindelin <Johannes.Schindelin@gmx.de>]

[-- Attachment #1: Type: text/plain, Size: 8819 bytes --]

Hi Kuba,

On Wed, 21 Sep 2016, Jakub Narębski wrote:

> W dniu 11.09.2016 o 10:33, Johannes Schindelin napisał: 
> > On Fri, 9 Sep 2016, Jakub Narębski wrote:
> [...]
> 
> >> When preserving merges, there are (as far as I understand it), two
> >> problems:
> >>  - what it means to preserve changes (which change to pick,
> >>    that is what is the mainline changes rebase is re-applying)
> >>  - what are parents of the merge commit (at least one parent
> >>    would be usually rewritten)
> >>
> >> Maybe the internal (and perhaps also user-visible) representation
> >> of merge in instruction sheet could use the notation of filter-branch,
> >> that is 'map(<sha-1>)'... it could also imply the mainline.
> >>
> >> That is the instruction in the internal instruction sheet could
> >> look like this:
> >>
> >>   merge -m 1 map(2fd4e1c6...) da39a3ee... \t Merge 'foo' into master  
> >>
> >>
> >> Note that it has nothing to do with this series!
> > 
> > Right. But I did solve that already. In the Git garden shears [*1*]
> > (essentially my New And Improved attempt at recreating branch structures
> > while rebasing), I generate and process scripts like this:
> > 
> > 	mark onto
> > 
> > 	# Branch: super-cool-feature
> > 	rewind onto
> > 	pick 00001 feature
> > 	pick 00002 documentation
> > 	mark super-cool-feature
> > 
> > 	# Branch: typo-fix
> > 	rewind onto
> > 	pick 0000a fix a tyop
> 
> There probably should be there
> 
>  	mark typo-fix

Correct. Sorry for the omission.

> > 	rewind onto
> > 	merge -C cafebabe super-cool-feature
> > 	merge -C babecafe typo-fix
> > 
> > 	cleanup super-cool-feature typo-fix
> > 
> > Of course this will change a little, still, once I get around to implement
> > this on top of the rebase--helper.
> 
> Do I understand it correctly that it is user-visible instruction sheet, and
> not the internal instruction sheet for sequencer?  This looks very nice
> and is well readable.

It is intended as that.

Currently I need a little trickery to make this work, as rebase -i does
not understand rewind nor merge. The trick is to re-use the shears.sh
script as editor that then populates the edit script, calls the real
editor, and then installs a temporary alias that gets called for all
custom commands via exec (turning e.g. "rewind abc" into "exec git .r
rewind abc").

The temporary alias point back to the shears script, too, of course.

In the end, I hope to teach the sequencer a variant of this dialect, as
well as the trick to generate such edit scripts.

> I guess that it needs to be pre-populated by Git based on topology of the
> branch being rebased.

Yes. The shears.sh script is in charge of that, and it has to perform a
couple of Git calls to do so.

> As I see, there are three basic topologies of non-linear branch to be
> rebased; all else is combination of thereof, or derivative:
> 
> 1. Merge commit without branching point, that is we need to go
>    from the following situation
> 
>    *---*---*---#---o---o---o        <-- old base
>                \\
>                 \\=a===b===M===c    <-- branch being rebased
>                           /
>          ...---x---x---x-/          <-- side branch
> 
>   to the following:
> 
>    *---*---*---#---o---o---o        
>                             \
>                              \-a'--b'--M'--c' 
>                                       /
>          ...---x---x---x-------------/          

In other words: rebasing a merge commit merging non-rebased commits.

This is not yet supported by the shears script, as it would require logic
that is not only slow in shell script, but also convoluted. IOW this
feature waits for the sequencer to know how to run regular rebase -i
already.

> I think this case is the only one supported by `--preserve-merges`,
> but I may be mistaken - I never had the need to use this feature IRL.

No, -p would handle merges of non-rebased commits as well as merges of
to-be-rebased commits.

> 2. Branching point without accompanying merge commit, or in other words
>    rebasing many branches tied together; a shrub if you will.  That is,
>    we want to go from the following situation:
> 
>    *---*---*---#---o---o---o           <-- old base
>                \
>                 \--a---b---c           <-- branch being rebased
>                         \
>                          \-1           <-- dependent branch
> 
>    to the following one:
> 
>    *---*---*---#---o---o---o
>                             \
>                              \--a'--b'--c'
>                                      \
>                                       \-1'

This is outside the scope of rebase -i (and of the shears), as you are
talking about *parallel* rebases. I don't do that, nor does rebase -i,
rebase -p nor the shears.

> I don't think Git supports something like that out of the box, but it
> is not hard to create something like that "by hand". It is not much
> of a problem... unless you forget to rebase the second dependent branch.

The safer thing to do, of course, is to merge all those tips back into the
branch you then rebase in one go.

> 3. Branching point with merge point, that is subbranch created and
>    merged - an "eye" (it is not a loop in DAG):
> 
>    *---*---*---#---o---o---o             <-- old base
>                \
>                 \--a---b---c---M---d     <-- branch being rebased
>                     \         /
>                      \-1---2-/         [ <-- possibly a branch ]
> 
>    All edges are directed edges, with arrows pointing from right to
>    left; that is  *---*  is really  *<---*
> 
>    The expected result is:
> 
>    *---*---*---#---o---o---o
>                             \
>                              \--a'--b'--c'--M'--d'
>                                  \         /
>                                   \-1'--2'/
> 
> I guess that is the main purpose of your git-garden-shears script,
> isn't it?

Yes.

> > For example, I am not so hot about the "merge -C ..." syntax. I'll
> > probably split that into a "remerge <SHA-1> <mark>" and a new "merge
> > <mark>" command (the latter asking interactively for the merge commit
> > message).
> 
> There is also an additional complication in that merge commit message
> may be *partially* automatically generated. First there is the subject
> generated by 'git merge' ("Merge branch 'foo'") or 'git pull <URL>'.
> It might have been translated, or extended.  Second there is a place
> for branch cover letter. Third, subject to merge.log / merge.summary
> there is a shortlog.

True. The current shears will simply use the original commit message
verbatim. And I think it would be wise to keep this behaviour until the
sequencer knows how to recreate merges.

Once that happens, I cordially invite you to implement any merge commit
message munging mode your heart desires, on top of the straight port of
the shears.

> From those shortlog should be surely updated to correspond to the
> post-rebase state.  The first line could be used to pre-populate
> mark lines, but during merge it should be, I think, updated to the
> new name of internal branch if it was changed.

Surely. Or not so surely. Too unsurely, for sure, to be discussed without
a dedicated patch series submitted by a dedicated person requiring that
feature.

> As to 'merge -C <sha1> <marker>' vs 'remerge <sha1> <marker>',
> I don't have specified opinion.  It would be nice to have one
> character shortcuts for insn sheet instructions, to which
> 'm -C <sha1> <marker>' is more amendable... 

I am pretty certain that you will find yourself disagreeing with that
statement once you used the shears for a while.

I use it for over a year now, and I *never* had to type any "merge -C"
command (i.e. the command I would call "remerge"). The reason is that you
simply move those generated lines, and that there is no scenario I
encountered where you will want to insert a merge manually: you simply
merge all you want *before* or *after* rebasing.

As to "merge <marker>": those are quite common, in particular when
splitting patch series into parts.

> > No :1 or some such. That's machine readable. But it's utter nonsense for
> > user-facing UIs.
> 
> Of course. It's all right for machine-facing instructions, like the
> 'todo' file for the sequencer, or for git-fast-import stream...

The 'todo' file for the sequencer *should* be human-friendly. There is no
reason not to make it so, in particular when you can simply reuse the refs
machinery for marks, and when it facilitates debugging (and I can testify
to that: it does, oh yeah, it does...).

Ciao,
Dscho