From: Junio C Hamano <gitster@pobox•com>
To: John Keeping <john@keeping•me.uk>
Cc: git@vger•kernel.org, Ted Felix <ted@tedfelix•com>
Subject: Re: [PATCH v2 2/2] rebase: omit patch-identical commits with --fork-point
Date: Wed, 16 Jul 2014 13:26:32 -0700 [thread overview]
Message-ID: <xmqqa989rqx3.fsf@gitster.dls.corp.google.com> (raw)
In-Reply-To: <374b26180807c67f7bd152ce5a2f52e34397e3a6.1405538598.git.john@keeping.me.uk> (John Keeping's message of "Wed, 16 Jul 2014 20:23:49 +0100")
John Keeping <john@keeping•me.uk> 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@tedfelix•com>
> Signed-off-by: John Keeping <john@keeping•me.uk>
> ---
>
> 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 &&
next prev parent reply other threads:[~2014-07-16 20:26 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-07-03 15:14 [BUG] rebase no longer omits local commits Ted Felix
2014-07-03 19:09 ` John Keeping
2014-07-03 22:25 ` John Keeping
2014-07-07 17:56 ` Junio C Hamano
2014-07-07 21:14 ` John Keeping
2014-07-15 19:14 ` [PATCH 1/2] rebase--am: use --cherry-pick instead of --ignore-if-in-upstream John Keeping
2014-07-15 19:14 ` [PATCH 2/2] rebase: omit patch-identical commits with --fork-point John Keeping
2014-07-15 19:48 ` Ted Felix
2014-07-15 22:06 ` Junio C Hamano
2014-07-16 19:23 ` [PATCH v2 1/2] rebase--am: use --cherry-pick instead of --ignore-if-in-upstream John Keeping
2014-07-16 19:23 ` [PATCH v2 2/2] rebase: omit patch-identical commits with --fork-point John Keeping
2014-07-16 20:26 ` Junio C Hamano [this message]
2014-07-16 21:27 ` John Keeping
2014-07-16 21:36 ` Ted Felix
2014-07-17 9:36 ` John Keeping
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=xmqqa989rqx3.fsf@gitster.dls.corp.google.com \
--to=gitster@pobox$(echo .)com \
--cc=git@vger$(echo .)kernel.org \
--cc=john@keeping$(echo .)me.uk \
--cc=ted@tedfelix$(echo .)com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox