John Keeping <john@xxxxxxxxxxxxx> writes: > When the `--fork-point` argument was added to `git rebase`, we changed > the value of $upstream to be the fork point instead of the point from > which we want to rebase. When $orig_head..$upstream is empty this does > not change the behaviour, but when there are new changes in the upstream > we are no longer checking if any of them are patch-identical with > changes in $upstream..$orig_head. > > Fix this by introducing a new variable to hold the fork point and using > this to restrict the range as an extra (negative) revision argument so > that the set of desired revisions becomes (in fork-point mode): > > git rev-list --cherry-pick --right-only \ > $upstream...$orig_head ^$fork_point > > This allows us to correctly handle the scenario where we have the > following topology: > > C --- D --- E <- dev > / > B <- master@{1} > / > o --- B' --- C* --- D* <- master > > where: > - B' is a fixed-up version of B that is not patch-identical with B; > - C* and D* are patch-identical to C and D respectively and conflict > textually if applied in the wrong order; > - E depends textually on D. > > The correct result of `git rebase master dev` is that B is identified as > the fork-point of dev and master, so that C, D, E are the commits that > need to be replayed onto master; but C and D are patch-identical with C* > and D* and so can be dropped, so that the end result is: > > o --- B' --- C* --- D* --- E <- dev > > If the fork-point is not identified, then picking B onto a branch > containing B' results in a conflict and if the patch-identical commits > are not correctly identified then picking C onto a branch containing D > (or equivalently D*) results in a conflict. > > This change allows us to handle both of these cases, where previously we > either identified the fork-point (with `--fork-point`) but not the > patch-identical commits *or* (with `--no-fork-point`) identified the > patch-identical commits but not the fact that master had been rewritten. > > Reported-by: Ted Felix <ted@xxxxxxxxxxxx> > Signed-off-by: John Keeping <john@xxxxxxxxxxxxx> > --- > > Change from v1: > - add a test case > diff --git a/t/t3400-rebase.sh b/t/t3400-rebase.sh > index 80e0a95..47b5682 100755 > --- a/t/t3400-rebase.sh > +++ b/t/t3400-rebase.sh > @@ -169,6 +169,29 @@ test_expect_success 'default to common base in @{upstream}s reflog if no upstrea > test_cmp expect actual > ' > > +test_expect_success 'cherry-picked commits and fork-point work together' ' > + git checkout default-base && > + echo Amended >A && > + git commit -a --no-edit --amend && > + test_commit B B && > + test_commit new_B B "New B" && > + test_commit C C && > + git checkout default && > + git reset --hard default-base@{4} && > + test_commit D D && > + git cherry-pick -2 default-base^ && > + test_commit final_B B "Final B" && > + git rebase && mental note: this rebases default (i.e. the current branch) on default-base; it depends on branch.default.{remote,merge} being set by the previous test piece. > + echo Amended >expect && > + test_cmp A expect && > + echo "Final B" >expect && > + test_cmp B expect && > + echo C >expect && > + test_cmp C expect && > + echo D >expect && > + test_cmp D expect > +' Thanks. Do these labels on the commits have any relation to the topology illustrated in the log message? It looks like the above produces this: a'---D---B'--new_B'---final_B (default) / o----a---B---new_B---C (default-base) \ D''---final_B'' where 'a' is "Modify A." from the original set-up and B and new_B are the cherry-picks to be filtered out during the rebase. Am I reading the test correctly? > test_expect_success 'rebase -q is quiet' ' > git checkout -b quiet topic && > git rebase -q master >output.out 2>&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