Heads up: major rebase -i -p rework coming up

Heads up: major rebase -i -p rework coming up

[Johannes Schindelin]

Hi all,

as probably everybody agrees, the code to preserve merges is a big mess 
right now.

Worse, the whole concept of "pick <merge-sha1>" just does not fly well.

So I started a _major_ cleanup, which happens to reduce the code very 
nicely so far.

It will take a few days to flesh out, I guess, but these are the major 
ideas of my work:

- pick $sha1

	will only work on non-merges in the future

- merge $sha1 [$sha1...] was $sha1 "Merge ..."

	will merge the given list of commits into the current HEAD, for 
	the user's reference and to keep up-to-date what was rewritten, 
	the original merge is shown after the keyword "was" (which is not 
	a valid SHA-1, luckily)

- goto $sha1

	will reset the HEAD to the given commit

- $sha1'

	for merge and goto, if a $sha1 ends in a single quote, the 
	rewritten commit is substituted (if there is one)

Example:

A - B - - - E 
  \       /
    C - D

could yield this TODO script:

	pick A
	pick C
	pick D
	goto A'
	pick B
	merge D' was E

This should lead to a much more intuitive user experience.

I am very sorry if somebody actually scripted rebase -i -p (by setting 
GIT_EDITOR with a script), but I am very certain that this cleanup is 
absolutely necessary to make rebase -i -p useful.

As always, I am thankful for suggestions to make this even more useful, or 
even easier to operate.

Ciao,
Dscho

Re: Heads up: major rebase -i -p rework coming up

[Junio C Hamano]

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

> 	pick A
> 	pick C
> 	pick D
> 	goto A'
> 	pick B
> 	merge D' was E
>
> This should lead to a much more intuitive user experience.
>
> I am very sorry if somebody actually scripted rebase -i -p (by setting 
> GIT_EDITOR with a script), but I am very certain that this cleanup is 
> absolutely necessary to make rebase -i -p useful.

Three questions.

- An obvious one first.  How does this relate to the sequencer project (that
  seems to have gone somewhat dark?)

- What's with the apostrophe?  I seem to remember that you argued it would
  be enough to make "A" stand for the original when it is used for the
  first time and the second and later use can stand for the result of the
  last use (e.g. the "goto A'" above can be simply spelled as "goto A"),
  when I suggested to use "mark" in a way similar to how fast-import
  language uses it during the sequencer discussion?

  I am not complaining; I am just being curious why the sudden change of
  heart.

- Why do you need "merge D' was E"?  Shouldn't "pick E" be able to notice
  that E is a merge and decompose it into "merge D' was E" internally?

  This one I am somewhat complaining, unless your answer is "because this
  way the user could drop some parents from the merge in the editor".

  And if your answer is that, then my next question will be "if that is
  the case, can the user be expected to easily find out which commit each
  parent SHA-1 refers to, without having more hint on the 'merge' insn
  line?"

Re: Heads up: major rebase -i -p rework coming up

[Johannes Schindelin]

Hi,

On Sat, 24 Jan 2009, Junio C Hamano wrote:

> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> 
> > 	pick A
> > 	pick C
> > 	pick D
> > 	goto A'
> > 	pick B
> > 	merge D' was E
> >
> > This should lead to a much more intuitive user experience.
> >
> > I am very sorry if somebody actually scripted rebase -i -p (by setting 
> > GIT_EDITOR with a script), but I am very certain that this cleanup is 
> > absolutely necessary to make rebase -i -p useful.
> 
> Three questions.
> 
> - An obvious one first.  How does this relate to the sequencer project 
>   (that seems to have gone somewhat dark?)

As far as I can see, Stephan can keep the "mark" command he cherishes so 
much, and we can still use thise syntax for rebase -i -p.

> - What's with the apostrophe?  I seem to remember that you argued it 
>   would be enough to make "A" stand for the original when it is used for 
>   the first time and the second and later use can stand for the result 
>   of the last use (e.g. the "goto A'" above can be simply spelled as 
>   "goto A"), when I suggested to use "mark" in a way similar to how 
>   fast-import language uses it during the sequencer discussion?
> 
>   I am not complaining; I am just being curious why the sudden change of 
>   heart.

Very easy explanation.  I got convinced by your arguments.  Even if I 
could imagine that I never use the thing without apostrophe, it is good to 
have an obvious indicator that this is not necessarily the original 
commit.

> - Why do you need "merge D' was E"?  Shouldn't "pick E" be able to 
>   notice that E is a merge and decompose it into "merge D' was E" 
>   internally?
> 
>   This one I am somewhat complaining, unless your answer is "because 
>   this way the user could drop some parents from the merge in the 
>   editor".

Not only that; the user could use this to fix mismerges, i.e. by replacing 
a SHA-1 with the SHA-1 (or indeed, a short name, unless it is "was") of 
the branch that she _actually_ wanted to merge with.

>   And if your answer is that, then my next question will be "if that is 
>   the case, can the user be expected to easily find out which commit 
>   each parent SHA-1 refers to, without having more hint on the 'merge' 
>   insn line?"

Nope.

In most cases, however, that should be plenty enough:

	merge 9383af1' was f39d50a Merge branch 'mh/unify-color' into next

The user does not have to guess much what 9383af1 might refer to.

In case of octopodes, or when the commit message was changed, the user has 
to open another command line and look for herself, though.

Ciao,
Dscho

Re: Heads up: major rebase -i -p rework coming up

[Johannes Schindelin]

Hi,

On Sat, 24 Jan 2009, Johannes Schindelin wrote:

> > - Why do you need "merge D' was E"?  Shouldn't "pick E" be able to 
> >   notice that E is a merge and decompose it into "merge D' was E"  
> >   internally?
> > 
> >   This one I am somewhat complaining, unless your answer is "because 
> >   this way the user could drop some parents from the merge in the 
> >   editor".
> 
> Not only that; the user could use this to fix mismerges, i.e. by 
> replacing a SHA-1 with the SHA-1 (or indeed, a short name, unless it is 
> "was") of the branch that she _actually_ wanted to merge with.
> 
> >   And if your answer is that, then my next question will be "if that 
> >   is the case, can the user be expected to easily find out which 
> >   commit each parent SHA-1 refers to, without having more hint on the 
> >   'merge' insn line?"
> 
> Nope.
> 
> In most cases, however, that should be plenty enough:
> 
> 	merge 9383af1' was f39d50a Merge branch 'mh/unify-color' into next
> 
> The user does not have to guess much what 9383af1 might refer to.

Heh, I think it is much easier than I thought:  How about this?

 	merge 9383af1' was f39d50a Merge branch 'mh/unify-color' into next
	#   \ 9383af1 Revert previous two commits

Obviously, for octopodes, there would be multiple "#   \ <SHA-1> <oneline>" 
lines...

Ciao,
Dscho

Re: Heads up: major rebase -i -p rework coming up

[Junio C Hamano]

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

>> - What's with the apostrophe?  I seem to remember that you argued it 
>>   would be enough to make "A" stand for the original when it is used for 
>>   the first time and the second and later use can stand for the result 
>>   of the last use (e.g. the "goto A'" above can be simply spelled as 
>>   "goto A"), when I suggested to use "mark" in a way similar to how 
>>   fast-import language uses it during the sequencer discussion?
>> 
>>   I am not complaining; I am just being curious why the sudden change of 
>>   heart.
>
> Very easy explanation.  I got convinced by your arguments.  Even if I 
> could imagine that I never use the thing without apostrophe, it is good to 
> have an obvious indicator that this is not necessarily the original 
> commit.

Now that does not make much sense to me.

The reason I suggested that 'mark' would give a cleaner semantics was
because in your earlier design "A" could either stand for the original or
it could stand for the result of an operation that used "A", and there
could be more than one operation that uses "A".  Explicitly naming each
result with a mark would give us an unambiguous way to distinguish them.

I however do not think you would ever use A twice in the context of
"rebase -i/-p".  Cherry-picking the same commit twice to create two copies
of them will not happen in that context.

While trying to recreate something like this on top of a commit "o", you
would have to talk about "A" multiple times,...

          B---M
         /   / \
 ---o---A---C   \
     \   \       \
      D---N-------O

... but even in such a picture, after one "pick A", you would always want
to refer to the result of the pick, and never the original A.

    pick A
    goto A'^
    pick D
    merge A' was N
    goto A'
    pick B
    goto A'
    pick C
    merge B' was M
    merge N' was O

So I am inclined to think that "first use refers to the original, second
and thereafter will refer to the result of the first use" would be a good
enough semantics for "rebase -i/-p", and you do not need "A" vs "A'" for
this.

By the way, I think this example shows that your "goto" might need a way
to refer to the "onto" commit in some way (I just used "A'^" there).

On the other hand, if you are aiming to allow users to create (by editing
the insn file) an arbitrarily different structure like this, starting from
the same topology:

  ---o---B---C---A
      \           \
       A---D-------O

that is, rebasing the upper line of development into one linear sequence
with different patch order, while rebasing the lower line into another
linear sequence by rebasing D on top of A, you would need to be able to
refer to the two different results of "using A", and your "A'" notation
would not help.

    pick B
    pick C
    pick A
    goto B'^
    pick A
    pick D
    merge A' was O

The last "merge A' was O" is done while on the result of applying D on top
of the result of applying A on the lower line, and wants to call the tip
of the upper line by referring it as "the result of applying A". 

But there are two results from applying A, and I do not think you can
avoid 'mark', even though you for some reason seem to hate it.

If this kind of transformation is outside the scope of your redesign
(which I think is a sensible design decision), I do not see why you would
need "A vs A'".

You either need the full power of 'mark', or "A is original until it is
used, and then the one and only one result once it is used,"; nothing in
between like "A vs A'" would make much sense.

Re: Heads up: major rebase -i -p rework coming up

[Johannes Schindelin]

Hi,

On Sat, 24 Jan 2009, Junio C Hamano wrote:

> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> 
> >> - What's with the apostrophe?  I seem to remember that you argued it 
> >>   would be enough to make "A" stand for the original when it is used 
> >>   for the first time and the second and later use can stand for the 
> >>   result of the last use (e.g. the "goto A'" above can be simply 
> >>   spelled as "goto A"), when I suggested to use "mark" in a way 
> >>   similar to how fast-import language uses it during the sequencer 
> >>   discussion?
> >> 
> >>   I am not complaining; I am just being curious why the sudden change 
> >>   of heart.
> >
> > Very easy explanation.  I got convinced by your arguments.  Even if I 
> > could imagine that I never use the thing without apostrophe, it is 
> > good to have an obvious indicator that this is not necessarily the 
> > original commit.
> 
> Now that does not make much sense to me.
> 
> The reason I suggested that 'mark' would give a cleaner semantics was 
> because in your earlier design "A" could either stand for the original 
> or it could stand for the result of an operation that used "A", and 
> there could be more than one operation that uses "A".  Explicitly naming 
> each result with a mark would give us an unambiguous way to distinguish 
> them.

But that is not what rebase -i is about!  Either you rewrite a commit, or 
you don't.  You don't rewrite it multiple times _and_ reference all of the 
intermediate steps!

Should you suggest that this is a sane worflow, you would really ask for 
trouble.

As it is, "mark" is useless.  It would give one and the same thing two 
names, one short SHA-1, and one numeric value, and the relationship 
between the two -- even if they mean the same! -- would be completely 
arbitrary.

> I however do not think you would ever use A twice in the context of 
> "rebase -i/-p".

Exactly!

> Cherry-picking the same commit twice to create two copies of them will 
> not happen in that context.

Not exactly true, as you could split a patch by

	edit abcdefg Patch to be split
	edit abcdefg Patch to be split

And removing half of the patch in the first edit (e.g. by "git reset 
HEAD^ && git add -e" or something similar).

> While trying to recreate something like this on top of a commit "o", you
> would have to talk about "A" multiple times,...
> 
>           B---M
>          /   / \
>  ---o---A---C   \
>      \   \       \
>       D---N-------O
> 
> ... but even in such a picture, after one "pick A", you would always want
> to refer to the result of the pick, and never the original A.
> 
>     pick A
>     goto A'^

... or goto $ONTO...

>     pick D
>     merge A' was N
>     goto A'
>     pick B
>     goto A'
>     pick C
>     merge B' was M
>     merge N' was O
> 
> So I am inclined to think that "first use refers to the original, second
> and thereafter will refer to the result of the first use" would be a good
> enough semantics for "rebase -i/-p", and you do not need "A" vs "A'" for
> this.
> 
> By the way, I think this example shows that your "goto" might need a way
> to refer to the "onto" commit in some way (I just used "A'^" there).

It will use $ONTO.

> On the other hand, if you are aiming to allow users to create (by editing
> the insn file) an arbitrarily different structure like this, starting from
> the same topology:
> 
>   ---o---B---C---A
>       \           \
>        A---D-------O
> 
> that is, rebasing the upper line of development into one linear sequence
> with different patch order, while rebasing the lower line into another
> linear sequence by rebasing D on top of A, you would need to be able to
> refer to the two different results of "using A", and your "A'" notation
> would not help.
> 
>     pick B
>     pick C
>     pick A
>     goto B'^
>     pick A
>     pick D
>     merge A' was O
> 
> The last "merge A' was O" is done while on the result of applying D on top
> of the result of applying A on the lower line, and wants to call the tip
> of the upper line by referring it as "the result of applying A". 
> 
> But there are two results from applying A, and I do not think you can
> avoid 'mark', even though you for some reason seem to hate it.

You can, by doing the sane thing and first apply one strand of the two 
branches, then going back and applying the other strand.  You would not 
even once need "goto A'".

> If this kind of transformation is outside the scope of your redesign 
> (which I think is a sensible design decision), I do not see why you 
> would need "A vs A'".
> 
> You either need the full power of 'mark', or "A is original until it is 
> used, and then the one and only one result once it is used,"; nothing in 
> between like "A vs A'" would make much sense.

Your example seems a little bit constructed to the purpose of showing the 
shortcomings of the A' notation.

But it has a shortcoming in and of its own: if you want to apply A for 
both branches, it would make a metric ton more sense to apply A before 
branching:

   ---o---A---B---C
           \       \
            D-------O

Besides, if you would concoct a real obscure situation where you really 
needed to apply one and the same patch twice, _and_ refer to both "A'" 
(something like

   ---o---B---A---C----H
       \       \ /    /
        \       E    /
         \          /
           D---A---F
                \ /
                 G

Then you could still do the part B...C first (with the first version of 
A'), then D...F (with the second version of A') and be done with it.  
Unless you would want anything like


 C---A---B
        /
    ---A

which is ugly beyond belief IMO, but in which case you could _still_ do it 
with an "edit C; merge A' was B" where you just git cherry-pick A.

So it is possible, even if it needs trickery, which is okay IMHO as this 
is not the common case.  And I want to optimize for the common case.

Ciao,
Dscho

Re: Heads up: major rebase -i -p rework coming up

[Marc Branchaud]

I'm sorry, but I just don't understand the purpose of 'was E' (or 
whatever syntax) in the merge command.  Why is there a need to refer to 
E at all?  The only reason I can think of is to replicate E's commit 
message.  Am I missing something?

Come to think of it, what if the user wants to edit a merge commit's 
message?  Should there be an 'editmerge' command?

		M.


Johannes Schindelin wrote:
>
>> - Why do you need "merge D' was E"?  Shouldn't "pick E" be able to 
>>   notice that E is a merge and decompose it into "merge D' was E" 
>>   internally?
>>
>>   This one I am somewhat complaining, unless your answer is "because 
>>   this way the user could drop some parents from the merge in the 
>>   editor".
> 
> Not only that; the user could use this to fix mismerges, i.e. by replacing 
> a SHA-1 with the SHA-1 (or indeed, a short name, unless it is "was") of 
> the branch that she _actually_ wanted to merge with.
> 
>>   And if your answer is that, then my next question will be "if that is 
>>   the case, can the user be expected to easily find out which commit 
>>   each parent SHA-1 refers to, without having more hint on the 'merge' 
>>   insn line?"
> 
> Nope.
> 
> In most cases, however, that should be plenty enough:
> 
> 	merge 9383af1' was f39d50a Merge branch 'mh/unify-color' into next
> 
> The user does not have to guess much what 9383af1 might refer to.

Re: Heads up: major rebase -i -p rework coming up

[Thomas Rast]

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

Johannes Schindelin wrote:
> Worse, the whole concept of "pick <merge-sha1>" just does not fly well.
[...]
> - merge $sha1 [$sha1...] was $sha1 "Merge ..."
> 
> 	will merge the given list of commits into the current HEAD, for 
> 	the user's reference and to keep up-to-date what was rewritten, 
> 	the original merge is shown after the keyword "was" (which is not 
> 	a valid SHA-1, luckily)

I really like the underlying idea.  I'm not even sure if the current
semantics are well-defined in all cases; an explicit merge command at
least makes it very clear what is going on.

However, I think the syntax as proposed above is a bit confusing in
the usual two-parent merge.  I couldn't tell whether

  merge A was B

was intended to be read as "the merge of A into the current branch" or
"the merge with sha1 A" right away, and I doubt I'll be able to tell
without looking in the (rare) cases I have to invoke rebase -i -p.

I can't really come up with a better replacement for 'was', so how
about

  merge A  # was B "Merge..."

which would make it more clear that the "was B..." has no effect
whatsoever on the merge's semantics.

> A - B - - - E 
>   \       /
>     C - D
> 
> could yield this TODO script:
> 
> 	pick A
> 	pick C
> 	pick D
> 	goto A'
> 	pick B
> 	merge D' was E

I kind of wonder if it would be possible to decorate the TODO with
'git log --graph' output, to make it easier to follow the history as
it is built.  Perhaps something like

  *   pick A
  |\
  * | pick B
      goto A'
  | * pick C
  | * pick D
  |/
      goto B'
  *   merge D'  # was E

Well, maybe it's not such a good idea after all.

-- 
Thomas Rast
trast@{inf,student}.ethz.ch

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

Re: Heads up: major rebase -i -p rework coming up

[Johannes Schindelin]

Hi,

On Sat, 24 Jan 2009, Thomas Rast wrote:

> Johannes Schindelin wrote:
> > Worse, the whole concept of "pick <merge-sha1>" just does not fly well.
> [...]
> > - merge $sha1 [$sha1...] was $sha1 "Merge ..."
> > 
> > 	will merge the given list of commits into the current HEAD, for 
> > 	the user's reference and to keep up-to-date what was rewritten, the 
> > 	original merge is shown after the keyword "was" (which is not a valid 
> > 	SHA-1, luckily)
> 
> I really like the underlying idea.  I'm not even sure if the current 
> semantics are well-defined in all cases; an explicit merge command at 
> least makes it very clear what is going on.
> 
> However, I think the syntax as proposed above is a bit confusing in the 
> usual two-parent merge.  I couldn't tell whether
> 
>   merge A was B
> 
> was intended to be read as "the merge of A into the current branch" or 
> "the merge with sha1 A" right away, and I doubt I'll be able to tell 
> without looking in the (rare) cases I have to invoke rebase -i -p.
> 
> I can't really come up with a better replacement for 'was', so how about
> 
>   merge A  # was B "Merge..."
> 
> which would make it more clear that the "was B..." has no effect 
> whatsoever on the merge's semantics.

Hmm.  You're right, that is not really intuitive.  How about

	merge (B) A # Merge...

instead?

> > A - B - - - E 
> >   \       /
> >     C - D
> > 
> > could yield this TODO script:
> > 
> > 	pick A
> > 	pick C
> > 	pick D
> > 	goto A'
> > 	pick B
> > 	merge D' was E
> 
> I kind of wonder if it would be possible to decorate the TODO with
> 'git log --graph' output, to make it easier to follow the history as
> it is built.

I wondered about that, too, and abandoned it as my common operation is cut 
& past lines around.  The result would look _utterly_ confusing.

Maybe I should have mentioned that to spare you the brain cycles thinking 
about --graph...

Ciao,
Dscho

Re: Heads up: major rebase -i -p rework coming up

[Johannes Schindelin]

Hi,

On Sun, 25 Jan 2009, Johannes Schindelin wrote:

> On Sat, 24 Jan 2009, Thomas Rast wrote:
> 
> > Johannes Schindelin wrote:
> > > Worse, the whole concept of "pick <merge-sha1>" just does not fly well.
> > [...]
> > > - merge $sha1 [$sha1...] was $sha1 "Merge ..."
> > > 
> > > 	will merge the given list of commits into the current HEAD, for 
> > > 	the user's reference and to keep up-to-date what was rewritten, the 
> > > 	original merge is shown after the keyword "was" (which is not a 
> > > 	valid SHA-1, luckily)
> > 
> > I really like the underlying idea.  I'm not even sure if the current 
> > semantics are well-defined in all cases; an explicit merge command at 
> > least makes it very clear what is going on.
> > 
> > However, I think the syntax as proposed above is a bit confusing in 
> > the usual two-parent merge.  I couldn't tell whether
> > 
> >   merge A was B
> > 
> > was intended to be read as "the merge of A into the current branch" or 
> > "the merge with sha1 A" right away, and I doubt I'll be able to tell 
> > without looking in the (rare) cases I have to invoke rebase -i -p.
> > 
> > I can't really come up with a better replacement for 'was', so how 
> > about
> > 
> >   merge A # was B "Merge..."
> > 
> > which would make it more clear that the "was B..." has no effect 
> > whatsoever on the merge's semantics.
> 
> Hmm.  You're right, that is not really intuitive.  How about
> 
> 	merge (B) A # Merge...
> 
> instead?

Or even better:

	merge B parent A' # Merge...

?

Ciao,
Dscho

Re: Heads up: major rebase -i -p rework coming up

[Jakub Narebski]

Johannes Schindelin wrote:

>> Hmm.  You're right, that is not really intuitive.  How about
>> 
>>       merge (B) A # Merge...
>> 
>> instead?
> 
> Or even better:
> 
>         merge B parent A' # Merge...

merge B with A' # Merge... 

;-)

-- 
Jakub Narebski
Warsaw, Poland
ShadeHawk on #git

Re: Heads up: major rebase -i -p rework coming up

[Johannes Schindelin]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 1040 bytes --]

Hi,

[please do not forget to Cc: me; today is a slow day, so I did not miss 
 your mail, but that is definitely not true on other days.]

On Sun, 25 Jan 2009, Jakub Narebski wrote:

> Johannes Schindelin wrote:
> 
> >> Hmm.  You're right, that is not really intuitive.  How about
> >> 
> >>       merge (B) A # Merge...
> >> 
> >> instead?
> > 
> > Or even better:
> > 
> >         merge B parent A' # Merge...
> 
> merge B with A' # Merge... 

No, that does not catch the meaning.

B is the _original_ merge commit.  So it actually knows what parents it 
has, but we want to give the user the freedom to change those parents.

The first parent is easy: this will be HEAD at that stage.

The other parents will be relatively easy: just replace A' by something 
else.

_However_ now that the merge commit B will be _redone_, we _still_ want to 
be able to refer to it later in the rebase script.  Therefore, rebase has 
to know that we _redid_ B at this stage.

Another idea:

	merge B Merge bla/blub
	parent A' bla/blub

Hmm?

Ciao,
Dscho

Re: Heads up: major rebase -i -p rework coming up

[Sverre Rabbelier]

Heya,

On Sun, Jan 25, 2009 at 15:17, Johannes Schindelin
<Johannes.Schindelin@gmx.de> wrote:
>> >>       merge (B) A # Merge...
>> >         merge B parent A' # Merge...
>> merge B with A' # Merge...
>        merge B Merge bla/blub
>        parent A' bla/blub

Oh goody, more painting! I was wondering when the next pick-a-word
contest would be!

-- 
Cheers,

Sverre Rabbelier

Re: Heads up: major rebase -i -p rework coming up

[Johannes Schindelin]

Hi,

On Sun, 25 Jan 2009, Sverre Rabbelier wrote:

> On Sun, Jan 25, 2009 at 15:17, Johannes Schindelin
> <Johannes.Schindelin@gmx.de> wrote:
> >> >>       merge (B) A # Merge...
> >> >         merge B parent A' # Merge...
> >> merge B with A' # Merge...
> >        merge B Merge bla/blub
> >        parent A' bla/blub
> 
> Oh goody, more painting! I was wondering when the next pick-a-word
> contest would be!

In this case, it is not about painting.  I will gladly ignore all those 
who think they must submit a new word for this action.

The thing is: I want the command to be intuitive, so I need a syntax where 
even the most idiotic dullard will understand what are the parents, and 
why we need the original merge's commit name, too.

So maybe I answered my question myself:

	merge parents $sha1 [$sha1...] original $sha1 $msg

Ciao,
Dscho

Re: Heads up: major rebase -i -p rework coming up

[Junio C Hamano]

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

> So maybe I answered my question myself:
>
> 	merge parents $sha1 [$sha1...] original $sha1 $msg

When you are reparenting, how would original commit get in the picture?
You wouldn't want the resulting merge to claim it merged X (which would be
what's in original's commit log) when in fact it now merged Y because the
user reparented it, would you?

Re: Heads up: major rebase -i -p rework coming up

[Johannes Schindelin]

Hi,

On Sun, 25 Jan 2009, Junio C Hamano wrote:

> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> 
> > So maybe I answered my question myself:
> >
> > 	merge parents $sha1 [$sha1...] original $sha1 $msg
> 
> When you are reparenting, how would original commit get in the picture?
> You wouldn't want the resulting merge to claim it merged X (which would be
> what's in original's commit log) when in fact it now merged Y because the
> user reparented it, would you?

Oh yes, I would!  Example:


	A - B - C
	  /
	X - Y

If I merged X into B by accident, but actually meant to merge Y, then C' 
should still come after B', no?

Ciao,
Dscho

Re: Heads up: major rebase -i -p rework coming up

[Jakub Narebski]

On Sun, 25 Jan 2009, Junio C Hamano wrote:
> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> 
> > So maybe I answered my question myself:
> >
> > 	merge parents $sha1 [$sha1...] original $sha1 $msg
> 
> When you are reparenting, how would original commit get in the picture?
> You wouldn't want the resulting merge to claim it merged X (which would be
> what's in original's commit log) when in fact it now merged Y because the
> user reparented it, would you?

Well, the subject part of merge (with merged branches names) shouldn't,
I guess, change. The summary (shortlog) part might, or perhaps even
should following rewrite (if it was present here).

But there is one issue I am wondering about: could we pick up _merge_
_resolution_? So if you have evil merge, and the change is for example
splitting commits without visible final changes, or just changing some
commit message before merge, it would get recreated without problems?

-- 
Jakub Narebski
Poland

Re: Heads up: major rebase -i -p rework coming up

[Johannes Schindelin]

Hi,

On Sun, 25 Jan 2009, Jakub Narebski wrote:

> On Sun, 25 Jan 2009, Junio C Hamano wrote:
> > Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> > 
> > > So maybe I answered my question myself:
> > >
> > > 	merge parents $sha1 [$sha1...] original $sha1 $msg
> > 
> > When you are reparenting, how would original commit get in the 
> > picture? You wouldn't want the resulting merge to claim it merged X 
> > (which would be what's in original's commit log) when in fact it now 
> > merged Y because the user reparented it, would you?
> 
> Well, the subject part of merge (with merged branches names) shouldn't, 
> I guess, change. The summary (shortlog) part might, or perhaps even 
> should following rewrite (if it was present here).
> 
> But there is one issue I am wondering about: could we pick up _merge_ 
> _resolution_? So if you have evil merge, and the change is for example 
> splitting commits without visible final changes, or just changing some 
> commit message before merge, it would get recreated without problems?

Nanako had a script at some stage; I would prefer an subcommand to "git 
rerere" which reconstructs the whole merge in-memory, and then records the 
conflict's resolution.

However, I really think you are getting ahead of yourself.  That is by no 
means something we want to have in rebase -p.  And even then, it would 
have to be non-automatic, i.e the user has to check the resolution.

We _know_ that git rerere does a fine job most of the time, almost all of 
the exceptions to be found when working with rebase -i extensively, as you 
are prone to take different decisions during development.

Ciao,
Dscho

Re: Heads up: major rebase -i -p rework coming up

[Nanako Shiraishi]

Quoting Johannes Schindelin <Johannes.Schindelin@gmx.de>:

> Nanako had a script at some stage; I would prefer an subcommand to "git 
> rerere" which reconstructs the whole merge in-memory, and then records the 
> conflict's resolution.

I'm sorry but I'm not sure what script of mine you are referring to.

-- 
Nanako Shiraishi
http://ivory.ap.teacup.com/nanako3/

Re: Heads up: major rebase -i -p rework coming up

[Johannes Schindelin]

Hi,

On Tue, 3 Feb 2009, Nanako Shiraishi wrote:

> Quoting Johannes Schindelin <Johannes.Schindelin@gmx.de>:
> 
> > Nanako had a script at some stage; I would prefer an subcommand to 
> > "git rerere" which reconstructs the whole merge in-memory, and then 
> > records the conflict's resolution.
> 
> I'm sorry but I'm not sure what script of mine you are referring to.

http://article.gmane.org/gmane.comp.version-control.git/96911

Ciao,
Dscho

Re: Heads up: major rebase -i -p rework coming up

[Jakub Narebski]

On Sun, 25 Jan 2009, Johannes Schindelin wrote:
> Hi,
> 
> [please do not forget to Cc: me; today is a slow day, so I did not miss 
>  your mail, but that is definitely not true on other days.]

This was spur of the moment idea, one that I wouldn't mind if you
would miss it.

But now that you have something interesting to say, I'll re-added
CC list for this thread.

> On Sun, 25 Jan 2009, Jakub Narebski wrote:
>> Johannes Schindelin wrote:
>> 
>>>> Hmm.  You're right, that is not really intuitive.  How about
>>>> 
>>>>       merge (B) A # Merge...
>>>> 
>>>> instead?
>>> 
>>> Or even better:
>>> 
>>>         merge B parent A' # Merge...
>> 
>> merge B with A' # Merge... 
> 
> No, that does not catch the meaning.

Errr... I didn't mean for 'with' to mean 'into'.
 
> B is the _original_ merge commit.  So it actually knows what parents it 
> has, but we want to give the user the freedom to change those parents.
> 
> The first parent is easy: this will be HEAD at that stage.
> 
> The other parents will be relatively easy: just replace A' by something 
> else.
> 
> _However_ now that the merge commit B will be _redone_, we _still_ want to 
> be able to refer to it later in the rebase script.  Therefore, rebase has 
> to know that we _redid_ B at this stage.
> 
> Another idea:
> 
> 	merge B Merge bla/blub
> 	parent A' bla/blub

It would be good idea... even better if 'p' shortcut was not taken
by 'pick'...

This is similar to your earlier idea:

        merge 9383af1' was f39d50a Merge branch 'mh/unify-color' into next
        #   \ 9383af1 Revert previous two commits


Or perhaps:

	merge A' D' into B Merge bla/blub

-- 
Jakub Narebski
Poland

Re: Heads up: major rebase -i -p rework coming up

[Björn Steinbrink]

On 2009.01.25 17:22:52 +0100, Jakub Narebski wrote:
> Or perhaps:
> 
> 	merge A' D' into B Merge bla/blub

That would be confusing. I'd read it as:

git checkout B
git merge A' D'

But the point of B is just to tell rebase that the original merge commit
B is replaced by this new merge commit, so that B' works later.

Björn

Re: Heads up: major rebase -i -p rework coming up

[Marc Branchaud]

Johannes Schindelin wrote:
> 
> - $sha1'
> 
> 	for merge and goto, if a $sha1 ends in a single quote, the 
> 	rewritten commit is substituted (if there is one)

I find this notation fairly unintuitive.  I'm more inclined to let users 
specify their own names for important parts of the rebase.

I guess that's what Junio's 'mark' command is for, but not having seen a 
proper explanation of 'mark', I suggest instead a more inline method: an 
'as' keyword.  Any command in a todo script can be followed (on the same 
line, before the SHA1 value) with 'as <name>' allowing <name> to appear 
later in the script to refer to the result of the earlier command.

So the script in the example becomes

	pick as start A
	pick C
	pick as bottom D
	goto start
	pick B
	merge bottom was E

I find that much easier to understand.  Especially when real SHA1 values 
are floating around everywhere, I think this notation will help users 
get things right.

This approach also allows a commit name "A" to consistently refer to the 
original commit, which I think also makes things easier for users.

		M.

Re: Heads up: major rebase -i -p rework coming up

[Stephen Haberman]

> I am very sorry if somebody actually scripted rebase -i -p (by setting
> GIT_EDITOR with a script), but I am very certain that this cleanup is
> absolutely necessary to make rebase -i -p useful.

I have scripted rebase-i-p, but with GIT_EDITOR=: [1]. I assume this
will still work and just accept the default script?

(Er, maybe I can just use rebase-p...I forget why [1] is using the
GIT_EDITOR=: with -i.)

My primary pain point with rebase-i-p has been rebasing a branch that
has merged in another branch that has a lot of commits on it. E.g.:

    a -- b -- c  origin/feature
      \
       d -- e    feature
           /
      ... g      origin/master

Where e is merging in, say, a latest release that had a few hundred
commits in the master branch. After resolving conflicts/etc. in e, I
want to rebase d..e from a to be on c.

The two problems have been:

1) `git pull` with rebase set uses rebase-i, with no -p, so all of the
   commits from the latest release branch that got merged in with e are
   flattened/duplicated. This is what [1] tries to fix. I've made
   noises about hacking the branch rebase flag but haven't followed
   through.

   I know this is a git pull issue, but I bring it up because, IIRC, the
   t3410 test case came from a scenario where I was rebasing a merge
   like e above and due to --cherry-pick dropping a commit (probably e
   itself, I'm not sure), rebase-i-p as it existed then broke and
   produced a noop. So I set off to get it to do "something" and ended
   up introducing the "DROPPED" directory.

2) With manual invocation of `rebase-i-p`, previously you'd get a
   laundry list of commits from the e merge that are new to the feature
   branch, but since g and its ancestors aren't changing, you don't need
   to consider them in the script and so its (potentially a lot of)
   noise. This is what the parent probing back port from git sequencer
   addressed.

So, I don't mean to rehash old complaints, as I'd love to see the
rebase-i-p code cleaned up by someone who can really refactor it vs. my
hack patches. But I wanted to emphasize the motivation for my hacks over
their implementation so that hopefully you can still address these use
cases in the new version.

Thanks,
Stephen

[1]: http://github.com/stephenh/git-central/blob/master/scripts/pull

Re: Heads up: major rebase -i -p rework coming up

[Johannes Schindelin]

Hi,

On Tue, 27 Jan 2009, Stephen Haberman wrote:

> > I am very sorry if somebody actually scripted rebase -i -p (by setting 
> > GIT_EDITOR with a script), but I am very certain that this cleanup is 
> > absolutely necessary to make rebase -i -p useful.
> 
> I have scripted rebase-i-p, but with GIT_EDITOR=: [1]. I assume this
> will still work and just accept the default script?

Yes, this will still work.  AFAICT this is actually how git 
--no-interactive -p is implemented...

> (Er, maybe I can just use rebase-p...I forget why [1] is using the
> GIT_EDITOR=: with -i.)

See above... :-)

> My primary pain point with rebase-i-p has been rebasing a branch that
> has merged in another branch that has a lot of commits on it. E.g.:
> 
>     a -- b -- c  origin/feature
>       \
>        d -- e    feature
>            /
>       ... g      origin/master
> 
> Where e is merging in, say, a latest release that had a few hundred
> commits in the master branch. After resolving conflicts/etc. in e, I
> want to rebase d..e from a to be on c.
> 
> The two problems have been:
> 
> 1) `git pull` with rebase set uses rebase-i, with no -p, so all of the
>    commits from the latest release branch that got merged in with e are
>    flattened/duplicated.

Maybe teach git pull about --rebase=preserve[-merges] and 
branch.<name>.rebase=preserve[-merges]?

> 2) With manual invocation of `rebase-i-p`, previously you'd get a
>    laundry list of commits from the e merge that are new to the feature
>    branch, but since g and its ancestors aren't changing, you don't need
>    to consider them in the script and so its (potentially a lot of)
>    noise. This is what the parent probing back port from git sequencer
>    addressed.

I always meant to handle that in the fast-forward handling of pick_one().

> So, I don't mean to rehash old complaints, as I'd love to see the 
> rebase-i-p code cleaned up by someone who can really refactor it vs. my 
> hack patches. But I wanted to emphasize the motivation for my hacks over 
> their implementation so that hopefully you can still address these use 
> cases in the new version.

Well, let's see how things turn out once I use the patches for my own 
work...

Thanks,
Dscho

Re: Heads up: major rebase -i -p rework coming up

[Nanako Shiraishi]

Quoting Stephen Haberman <stephen@exigencecorp.com>:

> My primary pain point with rebase-i-p has been rebasing a branch that
> has merged in another branch that has a lot of commits on it. E.g.:
>
>     a -- b -- c  origin/feature
>       \
>        d -- e    feature
>            /
>       ... g      origin/master
>
> Where e is merging in, say, a latest release that had a few hundred
> commits in the master branch. After resolving conflicts/etc. in e, I
> want to rebase d..e from a to be on c.

Sorry for asking a basic question, but if "feature" is a topic branch for advance the feature, why are you merging origin/master into it? Doesn't it blur the theme of the branch by including "development of the feature and all the random things that happened while it was being developed in other places"?

-- 
Nanako Shiraishi
http://ivory.ap.teacup.com/nanako3/

Re: Heads up: major rebase -i -p rework coming up

[Stephen Haberman]

> >     a -- b -- c  origin/feature
> >       \
> >        d -- e    feature
> >            /
> >       ... g      origin/master

> Sorry for asking a basic question, but if "feature" is a topic branch
> for advance the feature, why are you merging origin/master into it?
> Doesn't it blur the theme of the branch by including "development of
> the feature and all the random things that happened while it was being
> developed in other places"?

We merged origin/master because a release had just happened (e.g. master
moved from 1.0 -> 1.1), and when QA looks at origin/feature, they wanted
to see it integrated with the latest release (e.g. 1.1).

Now, granted, if feature was a private/unpublished branch, we would
rebase the entire thing (a/b/c) on top of master (g), but a/b/c has
already been published to our bug tracker, email lists, and other
developers who are collaborating on origin/feature, so between polluting
feature with a merge from master and changing the published hashes, we
chose the merge.

- Stephen