Thanks, this is long needed. I regret that I haven't had time in several months to continue work on the real fix. The combination "rebase -i -p" does work correctly in one common use case: where you only use "edit" and similar commands, and do not reorder the commits. I used it regularly for this purpose before I even noticed that it fails when you do reorder commits. Not sure if it's worth trying to make that distinction clear, though. I suggest s/instruction sheet/todo list/, as the latter terminology seems more natural to me, and also appears already elsewhere in the "git rebase" documentation. Cheers, Greg On Mon, May 31, 2010 at 9:43 PM, Jonathan Nieder <jrnieder@xxxxxxxxx> wrote: > 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. > > Reported-by: Jiří Paleček <jpalecek@xxxxxx> > Cc: Johannes Schindelin <Johannes.Schindelin@xxxxxx> > Cc: Thomas Rast <trast@xxxxxxxxxxxxxxx> > Signed-off-by: Jonathan Nieder <jrnieder@xxxxxxxxx> > --- > Pointers towards a real fix: > > . http://thread.gmane.org/gmane.comp.version-control.git/79424/focus=79504 > . http://thread.gmane.org/gmane.comp.version-control.git/77965/focus=80835 > . http://thread.gmane.org/gmane.comp.version-control.git/131921/focus=132679 > . http://repo.or.cz/w/git/price.git > . http://repo.or.cz/w/git/dscho.git/shortlog/refs/heads/rebase-i-p > > Thanks to all who have worked on this before. > > Thoughts? > > Documentation/git-rebase.txt | 26 ++++++++++++++++++++++++++ > t/t3404-rebase-interactive.sh | 6 ++++++ > 2 files changed, 32 insertions(+), 0 deletions(-) > > diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt > index 5863dec..330e9ea 100644 > --- a/Documentation/git-rebase.txt > +++ b/Documentation/git-rebase.txt > @@ -306,6 +306,11 @@ link:howto/revert-a-faulty-merge.txt[revert-a-faulty-merge How-To] for details). > -p:: > --preserve-merges:: > Instead of ignoring merges, try to recreate them. > ++ > +This uses the `--interactive` machinery internally, but combining it > +with the `--interactive` option explicitly is generally not a good > +idea (see BUGS below). > + > > --root:: > Rebase all commits reachable from <branch>, instead of > @@ -607,6 +612,27 @@ The ripple effect of a "hard case" recovery is especially bad: > case" recovery too! > > > +BUGS > +---- > +The instruction sheet presented by `--preserve-merges --interactive` > +does not represent the topology of the revision graph. Attempts to > +reorder it tend to produce counterintuitive results. > + > +For example, an attempt to rearrange > +------------ > +1 --- 2 --- 3 --- 4 --- 5 > +------------ > +to > +------------ > +1 --- 2 --- 4 --- 3 --- 5 > +------------ > +by moving the "pick 4" line will result in the following history: > +------------ > + 3 > + / > +1 --- 2 --- 4 --- 5 > +------------ > + > Authors > ------ > Written by Junio C Hamano <gitster@xxxxxxxxx> and > diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh > index f20ea38..6668907 100755 > --- a/t/t3404-rebase-interactive.sh > +++ b/t/t3404-rebase-interactive.sh > @@ -181,6 +181,12 @@ test_expect_success '-p handles "no changes" gracefully' ' > test $HEAD = $(git rev-parse HEAD) > ' > > +test_expect_failure 'exchange two commits with -p' ' > + FAKE_LINES="2 1" git rebase -i -p HEAD~2 && > + test H = $(git cat-file commit HEAD^ | sed -ne \$p) && > + test G = $(git cat-file commit HEAD | sed -ne \$p) > +' > + > test_expect_success 'preserve merges with -p' ' > git checkout -b to-be-preserved master^ && > : > unrelated-file && > -- > 1.7.1 > > -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html