Re: BUG: git rebase -i -p silently looses commits

[Greg Price <price@ksplice.com>]

On Wed, Nov 11, 2009 at 12:32 PM, Johannes Schindelin
<Johannes.Schindelin@gmx.de> wrote:
> That is very interesting!
>
> However, for rebase-i-p to have a chance to be accepted, I think a few
> things are necessary still (this is all from memory, so please take
> everything with a grain of salt):

Great, this is helpful, and it overlaps with my existing to-do list.
I have a couple of questions.


> - reorder the series to have the -i fixes first, the new commands next,
>  and then the changes to the actual -p mode

This one will be easy when everything else is ready, I think.


> - rework the mark stuff so that 'todo' works properly, and then change the
>  system to use ':<name>' style bookmarks.

This is the biggest change I was going to suggest!  Glad we're on the
same page.  To be clear, what I want to do here is
 - add a 'mark' command
 - emit 'mark' commands in the TODO generation for the target of each
'goto', and use them.
Is that also what you had in mind?


> - fix that nasty bug which makes one revision not pass the tests (I forgot
>  which one, but it should be in the TODOs)

Hmm.  I see one TODO comment in your patches, and it doesn't sound
like this.  Is there a TODO somewhere else that I'm missing?
Alternatively, I can always end up just running the tests on all the
revisions and find out.


> - add proper handling for the case when a patch has been applied in
>  upstream already, but was not correctly identified as that by
>  --cherry-pick (well, this TODO is actually not really related to rebase
>  -i -p, but something I deeply care about)

Hmm.  I'll have to think about what the behavior could be here.
Unless you've already worked out a behavior you would like to see?
For context, I think the issue you're referring to is that sometimes
the patch-id changed, so that --cherry-pick doesn't identify the
patch; and then some later upstream patch has touched the same code
again, so that there's a conflict when we try to apply the older
patch.  I would also like to see this fixed, but I don't see offhand
what the right behavior would be.

The "read my mind" behavior might be something like, somewhere between
the merge-base and the upstream there is a commit after which this one
would apply as no changes, so let's say that commit already applied
this patch.  But that could be the wrong thing if e.g. a patch was
applied and later reverted.  And I don't know offhand how to implement
it efficiently.

Anyway, I think you're right that this improvement is orthogonal to
rebase -i -p.


> Unfortunately, I am getting more and more deprived of Git time budget
> these days, so that I cannot seem to find a few hours to at least restart
> my efforts.

Understood.  I may have some time to work on this soon, we'll see.  I
think the priorities will be to
 - add "mark" as you say
 - add the "pause" command, to make it possible to amend a merge
 - write tests
 - fix a couple of bugs, track down the one you mentioned
 - write documentation

At that point, and with the reordering you suggested, I think it will
be ready to submit for inclusion.

Further comments, and bug reports from anyone else using the
development version, are welcome.

Thanks,
Greg