git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH v2 2/2] rebase: omit patch-identical commits with --fork-point

From
Junio C Hamano <gitster@pobox.com>
Date
Jul 16, 2014, 20:26 UTC
Message-ID
<xmqqa989rqx3.fsf@gitster.dls.corp.google.com>
In-Reply-To
<374b26180807c67f7bd152ce5a2f52e34397e3a6.1405538598.git.john@keeping.me.uk>
John Keeping <john@keeping.me.uk> writes:
Show 52 quoted lines
> 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@tedfelix.com>
> Signed-off-by: John Keeping <john@keeping.me.uk>
> ---
>
> Change from v1:
>     - add a test case
Show 21 quoted lines
> 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.

Show 9 quoted lines
> +	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 &&
Previous: John KeepingNext: John Keeping
Message 12 of 15 in “[BUG] rebase no longer omits local commits”
  1. Ted FelixJul 3, 2014
  2. John KeepingJul 3, 2014
  3. John KeepingJul 3, 2014
  4. Junio C HamanoJul 7, 2014
  5. John KeepingJul 7, 2014
  6. 1/2 rebase--am: use --cherry-pick instead of --ignore-if-in-upstreamJohn Keeping, Jul 15, 2014
  7. 2/2 rebase: omit patch-identical commits with --fork-pointJohn Keeping, Jul 15, 2014
  8. Ted FelixJul 15, 2014
  9. Junio C HamanoJul 15, 2014
  10. 1/2 rebase--am: use --cherry-pick instead of --ignore-if-in-upstreamJohn Keeping, Jul 16, 2014
  11. 2/2 rebase: omit patch-identical commits with --fork-pointJohn Keeping, Jul 16, 2014
  12. Junio C HamanoJul 16, 2014
  13. John KeepingJul 16, 2014
  14. Ted FelixJul 16, 2014
  15. John KeepingJul 17, 2014

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.