Limitiations of git rebase --preserve-merges --interactive

Limitiations of git rebase --preserve-merges --interactive

[Stefan Haller]

The BUGS section of the git-rebase manpage says that editing or
rewording commits "should work fine", but attempts to reorder commits
usually don't do what you want.

I'd like to know more about what does or doesn't work. For example, will
squashing commits work? (I.e. using the "fixup" or "squash" commands in
the todo sheet.)

Will dropping commits work?

Does it make sense to insert "exec" commands, or will they run at
arbitrary times?


-- 
Stefan Haller
Berlin, Germany
http://www.haller-berlin.de/

Re: Limitiations of git rebase --preserve-merges --interactive

[Anatoly Borodin]

Hi Stefan,

this section was added to the manual in the commit
cddb42d2c58a9de9b2b5ef68817778e7afaace3e by "Jonathan Nieder"
<jrnieder@gmail.com> 6 years ago. Maybe he remembers better?

-- 
Mit freundlichen Grüßen,
Anatoly Borodin

Re: Limitiations of git rebase --preserve-merges --interactive

[Kevin Daudt]

On Thu, Sep 22, 2016 at 07:33:11PM +0000, Anatoly Borodin wrote:
> Hi Stefan,
> 
> this section was added to the manual in the commit
> cddb42d2c58a9de9b2b5ef68817778e7afaace3e by "Jonathan Nieder"
> <jrnieder@gmail.com> 6 years ago. Maybe he remembers better?
> 

Just to make it clear, this section explicitly talks about 'bugs' with
preserve-merges and interactive rebase.  Without the --preserve-merges
option, those operations works as expected.

The reason, as that section explains, is that it's not possible to store
the merge structure in the flat todo list. I assume this means git
internally remembers where the merge commit was, and then restores it
while rebasing.

Changing the order, or dropping commits might then give unexpected
results.

Re: Limitiations of git rebase --preserve-merges --interactive

[Stefan Beller]

On Thu, Sep 22, 2016 at 12:48 PM, Kevin Daudt <me@ikke.info> wrote:
> On Thu, Sep 22, 2016 at 07:33:11PM +0000, Anatoly Borodin wrote:
>> Hi Stefan,
>>
>> this section was added to the manual in the commit
>> cddb42d2c58a9de9b2b5ef68817778e7afaace3e by "Jonathan Nieder"
>> <jrnieder@gmail.com> 6 years ago. Maybe he remembers better?
>>
>
> Just to make it clear, this section explicitly talks about 'bugs' with
> preserve-merges and interactive rebase.  Without the --preserve-merges
> option, those operations works as expected.
>
> The reason, as that section explains, is that it's not possible to store
> the merge structure in the flat todo list. I assume this means git
> internally remembers where the merge commit was, and then restores it
> while rebasing.
>
> Changing the order, or dropping commits might then give unexpected
> results.
>

The commit message may help as well:

    rebase -i -p: document shortcomings

    The rebase --preserve-merges facility presents a list of commits
    in its instruction sheet and uses a separate table to keep
    track of their parents.  Unfortunately, in practice this means
    that with -p after most attempts to rearrange patches, some
    commits have the "wrong" parent and the resulting history is
    rarely what the caller expected.

    Yes, it would be nice to fix that.  But first, add a warning to the
    manual to help the uninitiated understand what is going on.

Re: Limitiations of git rebase --preserve-merges --interactive

[Stefan Haller]

Stefan Beller <sbeller@google.com> wrote:

> On Thu, Sep 22, 2016 at 12:48 PM, Kevin Daudt <me@ikke.info> wrote:
> > On Thu, Sep 22, 2016 at 07:33:11PM +0000, Anatoly Borodin wrote:
> >> Hi Stefan,
> >>
> >> this section was added to the manual in the commit
> >> cddb42d2c58a9de9b2b5ef68817778e7afaace3e by "Jonathan Nieder"
> >> <jrnieder@gmail.com> 6 years ago. Maybe he remembers better?
> >>
> >
> > Just to make it clear, this section explicitly talks about 'bugs' with
> > preserve-merges and interactive rebase.  Without the --preserve-merges
> > option, those operations works as expected.
> >
> > The reason, as that section explains, is that it's not possible to store
> > the merge structure in the flat todo list. I assume this means git
> > internally remembers where the merge commit was, and then restores it
> > while rebasing.
> >
> > Changing the order, or dropping commits might then give unexpected
> > results.
> >
> 
> The commit message may help as well:
> 
>     rebase -i -p: document shortcomings
> 
>     The rebase --preserve-merges facility presents a list of commits
>     in its instruction sheet and uses a separate table to keep
>     track of their parents.  Unfortunately, in practice this means
>     that with -p after most attempts to rearrange patches, some
>     commits have the "wrong" parent and the resulting history is
>     rarely what the caller expected.
> 
>     Yes, it would be nice to fix that.  But first, add a warning to the
>     manual to help the uninitiated understand what is going on.

Thanks, but all of this still talks about the issues in very generic
terms ("most attempts to rearrange patches"). I'm interested in more
details as to exactly what kind of attempts do or don't work. In
particular, I'm interested in fixup/squash commands (without reordering
anything else), or dropping (non-merge) commits.

I could of course experiment with these and try to find out myself, but
I was hoping someone would just know the answer off the top of their
head, saving me some time.


-- 
Stefan Haller
Berlin, Germany
http://www.haller-berlin.de/

Re: Limitiations of git rebase --preserve-merges --interactive

[Johannes Schindelin]

Hi Stefan,

On Fri, 23 Sep 2016, Stefan Haller wrote:

> Stefan Beller <sbeller@google.com> wrote:
> 
> > On Thu, Sep 22, 2016 at 12:48 PM, Kevin Daudt <me@ikke.info> wrote:
> > > On Thu, Sep 22, 2016 at 07:33:11PM +0000, Anatoly Borodin wrote:
> > >> Hi Stefan,
> > >>
> > >> this section was added to the manual in the commit
> > >> cddb42d2c58a9de9b2b5ef68817778e7afaace3e by "Jonathan Nieder"
> > >> <jrnieder@gmail.com> 6 years ago. Maybe he remembers better?
> > >>
> > >
> > > Just to make it clear, this section explicitly talks about 'bugs' with
> > > preserve-merges and interactive rebase.  Without the --preserve-merges
> > > option, those operations works as expected.
> > >
> > > The reason, as that section explains, is that it's not possible to store
> > > the merge structure in the flat todo list. I assume this means git
> > > internally remembers where the merge commit was, and then restores it
> > > while rebasing.
> > >
> > > Changing the order, or dropping commits might then give unexpected
> > > results.
> > >
> > 
> > The commit message may help as well:
> > 
> >     rebase -i -p: document shortcomings
> > 
> >     The rebase --preserve-merges facility presents a list of commits
> >     in its instruction sheet and uses a separate table to keep
> >     track of their parents.  Unfortunately, in practice this means
> >     that with -p after most attempts to rearrange patches, some
> >     commits have the "wrong" parent and the resulting history is
> >     rarely what the caller expected.
> > 
> >     Yes, it would be nice to fix that.  But first, add a warning to the
> >     manual to help the uninitiated understand what is going on.
> 
> Thanks, but all of this still talks about the issues in very generic
> terms ("most attempts to rearrange patches"). I'm interested in more
> details as to exactly what kind of attempts do or don't work. In
> particular, I'm interested in fixup/squash commands (without reordering
> anything else), or dropping (non-merge) commits.
> 
> I could of course experiment with these and try to find out myself, but
> I was hoping someone would just know the answer off the top of their
> head, saving me some time.

The fundamental problem here is the underlying design of bolting on the
"recreate a merge" functionality onto the "pick" command.

That is, if you try to rebase non-linear commit history, it will still
generate a linear list of "pick <commit-name>" lines, as if it were
linear, except that it will include the merge commits, too.

It then will try to guess what you want to do by recording which commit
was rewritten as which commit. And when it encounters a "pick" with a
merge commit, it will try to merge the *rewritten* commit.

In other words, the design does not allow for changing the tip of any
merged branch. Not reordering, not dropping.

And I do not think that there is a way to fix that design. That is why I
came up with the Git garden shears (see the link I sent elsewhere in this
thread).

Ciao,
Johannes

Re: Limitiations of git rebase --preserve-merges --interactive

[Stefan Beller]

On Fri, Sep 23, 2016 at 2:13 PM, Johannes Schindelin
<Johannes.Schindelin@gmx.de> wrote:
> Hi Stefan,
>
> On Fri, 23 Sep 2016, Stefan Haller wrote:
>
>> Stefan Beller <sbeller@google.com> wrote:
>>
>> > On Thu, Sep 22, 2016 at 12:48 PM, Kevin Daudt <me@ikke.info> wrote:
>> > > On Thu, Sep 22, 2016 at 07:33:11PM +0000, Anatoly Borodin wrote:
>> > >> Hi Stefan,
>> > >>
>> > >> this section was added to the manual in the commit
>> > >> cddb42d2c58a9de9b2b5ef68817778e7afaace3e by "Jonathan Nieder"
>> > >> <jrnieder@gmail.com> 6 years ago. Maybe he remembers better?
>> > >>
>> > >
>> > > Just to make it clear, this section explicitly talks about 'bugs' with
>> > > preserve-merges and interactive rebase.  Without the --preserve-merges
>> > > option, those operations works as expected.
>> > >
>> > > The reason, as that section explains, is that it's not possible to store
>> > > the merge structure in the flat todo list. I assume this means git
>> > > internally remembers where the merge commit was, and then restores it
>> > > while rebasing.
>> > >
>> > > Changing the order, or dropping commits might then give unexpected
>> > > results.
>> > >
>> >
>> > The commit message may help as well:
>> >
>> >     rebase -i -p: document shortcomings
>> >
>> >     The rebase --preserve-merges facility presents a list of commits
>> >     in its instruction sheet and uses a separate table to keep
>> >     track of their parents.  Unfortunately, in practice this means
>> >     that with -p after most attempts to rearrange patches, some
>> >     commits have the "wrong" parent and the resulting history is
>> >     rarely what the caller expected.
>> >
>> >     Yes, it would be nice to fix that.  But first, add a warning to the
>> >     manual to help the uninitiated understand what is going on.
>>
>> Thanks, but all of this still talks about the issues in very generic
>> terms ("most attempts to rearrange patches"). I'm interested in more
>> details as to exactly what kind of attempts do or don't work. In
>> particular, I'm interested in fixup/squash commands (without reordering
>> anything else), or dropping (non-merge) commits.
>>
>> I could of course experiment with these and try to find out myself, but
>> I was hoping someone would just know the answer off the top of their
>> head, saving me some time.
>
> The fundamental problem here is the underlying design of bolting on the
> "recreate a merge" functionality onto the "pick" command.
>
> That is, if you try to rebase non-linear commit history, it will still
> generate a linear list of "pick <commit-name>" lines, as if it were
> linear, except that it will include the merge commits, too.

Which on a more fundamental design level would be ok.
(C.f. your shell history is a linear list of git commands, but it
deals just fine
with non linear DAGSs)

>
> It then will try to guess what you want to do by recording which commit
> was rewritten as which commit. And when it encounters a "pick" with a
> merge commit, it will try to merge the *rewritten* commit.

Instead of guessing we'd need to differentiate between "pick" and "pickmerge",
whereas the later describes creating commits with more than one parent (i.e.
the prior pick line).

I could imagine the "pickmerge" to list all additional parents (The
first parent being
the previously picked commit) via symbolic naming:

    pick 1234affe implement foo
    pickmerge 3456feed origin/js/new-feature-1 # Merge origin/js/new-feature-1
    pick 45678ead implement feature-2

The "pickmerge" would have first the merge tips, and then the old
subject line after
a # character.

>
> In other words, the design does not allow for changing the tip of any
> merged branch. Not reordering, not dropping.

I see how the current design is problematic as there is no argument
possible that
allows the user to correct the wrong guess.

>
> And I do not think that there is a way to fix that design. That is why I
> came up with the Git garden shears (see the link I sent elsewhere in this
> thread).

I'll look into that.

Thanks,
Stefan

Re: Limitiations of git rebase --preserve-merges --interactive

[Anatoly Borodin]

Hi Kevin,

Kevin Daudt <me@ikke.info> wrote:
> Changing the order, or dropping commits might then give unexpected
> results.

The question that Stefan has is rather "what is *supposed* to work /
give *expected* results?". Some stuff can be found in the tests
(t/t*rebase*preserve*), but maybe there is more?

-- 
Mit freundlichen Grüßen,
Anatoly Borodin

Re: Limitiations of git rebase --preserve-merges --interactive

[Anatoly Borodin]

Hi Stefan,

I've also done some archaeology and found that the original version of
the merge preserving code was written by Johannes Schindelin
<Johannes.Schindelin@gmx.de>, see e.g.
f09c9b8c5ff9d8a15499b09ccd6c3e7b3c76af77

There were also some big discussion threads in 2007-2008 regarding a
better mechanism to "mark" or "tag" the heads of rebased branches, but I
haven't seen consensus and development in that direction after that.

PS There are also some pieces of "what should work" in these tests:

t/t3409-rebase-preserve-merges.sh*
t/t3410-rebase-preserve-dropped-merges.sh*
t/t3411-rebase-preserve-around-merges.sh*
t/t3414-rebase-preserve-onto.sh*


-- 
Mit freundlichen Grüßen,
Anatoly Borodin

Re: Limitiations of git rebase --preserve-merges --interactive

[Stefan Beller]

On Thu, Sep 22, 2016 at 2:01 PM, Anatoly Borodin
<anatoly.borodin@gmail.com> wrote:
> Hi Stefan,
>
> I've also done some archaeology and found that the original version of
> the merge preserving code was written by Johannes Schindelin
> <Johannes.Schindelin@gmx.de>, see e.g.

I think it would be helpful if you'd cc those folks involved, not just
the mailing list.

Re: Limitiations of git rebase --preserve-merges --interactive

[Johannes Schindelin]

Hi,

On Thu, 22 Sep 2016, Stefan Beller wrote:

> On Thu, Sep 22, 2016 at 2:01 PM, Anatoly Borodin
> <anatoly.borodin@gmail.com> wrote:
> > Hi Stefan,
> >
> > I've also done some archaeology and found that the original version of
> > the merge preserving code was written by Johannes Schindelin
> > <Johannes.Schindelin@gmx.de>, see e.g.
> 
> I think it would be helpful if you'd cc those folks involved, not just
> the mailing list.

Indeed. It is quite tedious to re-Cc: people. Anatoly, please do not force
me to put in that work in the future.

Also: I mentioned recently that I am not happy with the original design,
either, and that I came up with a new design that I intend to port to the
rebase--helper, once it is included in an official Git version:

https://public-inbox.org/git/alpine.DEB.2.20.1609111027330.129229@virtualbox/

Ciao,
Johannes

Re: Limitiations of git rebase --preserve-merges --interactive

[Stefan Haller]

Anatoly Borodin <anatoly.borodin@gmail.com> wrote:

> PS There are also some pieces of "what should work" in these tests:
> 
> t/t3409-rebase-preserve-merges.sh*
> t/t3410-rebase-preserve-dropped-merges.sh*
> t/t3411-rebase-preserve-around-merges.sh*
> t/t3414-rebase-preserve-onto.sh*

Thanks, this is interesting; I'm having trouble understanding the tests
though. Some of them use rebase -p -i, but I don't understand why they
use -i, or why that even works in a test (i.e. why it doesn't open an
editor).

In one test I saw "GIT_EDITOR=: git rebase -i -p", which I guess means
"use the initially given todo sheet unchanged". I don't see any tests
that do an interactive rebase and actually change the todo list.


-- 
Stefan Haller
Berlin, Germany
http://www.haller-berlin.de/

Re: Limitiations of git rebase --preserve-merges --interactive

[Junio C Hamano]

lists@haller-berlin.de (Stefan Haller) writes:

> Thanks, this is interesting; I'm having trouble understanding the tests
> though. Some of them use rebase -p -i, but I don't understand why they
> use -i, or why that even works in a test (i.e. why it doesn't open an
> editor).

Upon starting up, tests dot-source t/test-lib.sh file and it
unsets most of GIT_* environment variables to obtain a stable
testing environment that is not affected by things that testers
may have in their environment.

There is EDITOR=: in t/test-lib.sh, which was added in 2006 before
GIT_EDITOR was invented.  That is the one in effect for git
subcommands that usually interacts with editors during the test,
unless specific tests further override it with test_set_editor
helper.

Re: Limitiations of git rebase --preserve-merges --interactive

[Stefan Haller]

Junio C Hamano <gitster@pobox.com> wrote:

> lists@haller-berlin.de (Stefan Haller) writes:
> 
> > Thanks, this is interesting; I'm having trouble understanding the tests
> > though. Some of them use rebase -p -i, but I don't understand why they
> > use -i, or why that even works in a test (i.e. why it doesn't open an
> > editor).
> 
> Upon starting up, tests dot-source t/test-lib.sh file and it
> unsets most of GIT_* environment variables to obtain a stable
> testing environment that is not affected by things that testers
> may have in their environment.
> 
> There is EDITOR=: in t/test-lib.sh, which was added in 2006 before
> GIT_EDITOR was invented.  That is the one in effect for git
> subcommands that usually interacts with editors during the test,
> unless specific tests further override it with test_set_editor
> helper.

Thanks for the explanation. So this explains why -i works at all here;
it doesn't explain why -i is used in these tests. Unless I'm missing
something, they should all work with just -p.

And I don't see any tests that do rebase -p -i and actually do something
interesting with the -i part. So my original question still remains. :-)


-- 
Stefan Haller
Berlin, Germany
http://www.haller-berlin.de/

Re: Limitiations of git rebase --preserve-merges --interactive

[Johannes Sixt]

Am 23.09.2016 um 17:50 schrieb Stefan Haller:
> And I don't see any tests that do rebase -p -i and actually do something
> interesting with the -i part. So my original question still remains. :-)

-i -p came first. -p without -i was bolted on later.

-- Hannes