On Tue, 23 Nov 2010, Knut Franke wrote:
> Unlike ordinary and interactive rebase, rebase -m halts with a conflict
> if two conflicting patches have been applied to both source and target
> branch (e.g. by cherry-picking). Fix this by using git rev-list
> --cherry-pick to generate the list of patches to apply.
I think the idea is sound, but...
Until I saw your patch I had not thought about the impact of the
--cherry-pick option (thanks!). I think there is a bug in the existing
rebase code related to it when rebase is used with the --onto option.
I will try to explain how I think it currently works.
Let's say we have the below history, where E is a cherry-pick of e.
.-c
/
a---b---d---e---f
\
.-g---E
If we now run 'git rebase --onto c f E', the revisions that will be
applied onto 'c' are given by 'git format-patch
--ignore-if-in-upstream f..E'. In this case that would be only 'g' and
NOT 'E'.
What I think we really want to do is to remove any patches in
upstream..branch that also exist in branch..onto. I don't know how to
do that efficiently, though. (And am I even right about it?)
I think this may be quite a serious bug, since it might go unnoticed
that a patch was dropped. If the <upstream> ref is later removed, the
patch might be completely gone from history without the user noticing.
If the above is correct, is it better to fix that problem first before
applying your patch? On the other hand, your patch does not really
make things worse (except for duplicating the buggy code)...
With the current implementation, my patch in
http://thread.gmane.org/gmane.comp.version-control.git/161381/ would
also make things worse when there are cherry-picks between <upstream>
and <branch>. The above should be fixed before I re-submit.
>
> Also adapt t3406 a) to catch this case and b) not to expect
> "Already applied" messages which can't be emitted easily if duplicates
> are removed already when storing the patches.
>
> Signed-off-by: Knut Franke <Knut.Franke@xxxxxx>
> ---
> git-rebase.sh | 29 +++++++++++++++++++----------
> t/t3406-rebase-message.sh | 14 +++++++-------
> 2 files changed, 26 insertions(+), 17 deletions(-)
>
> diff --git a/git-rebase.sh b/git-rebase.sh
> index 10a238a..aa42744 100755
> --- a/git-rebase.sh
> +++ b/git-rebase.sh
> @@ -552,15 +552,14 @@ then
> exit 0
> fi
>
> -if test -n "$rebase_root"
> -then
> - revisions="$onto..$orig_head"
> -else
> - revisions="$upstream..$orig_head"
> -fi
> -
> if test -z "$do_merge"
> then
> + if test -n "$rebase_root"
> + then
> + revisions="$onto..$orig_head"
> + else
> + revisions="$upstream..$orig_head"
> + fi
> git format-patch -k --stdout --full-index --ignore-if-in-upstream \
> --src-prefix=a/ --dst-prefix=b/ \
> --no-renames $root_flag "$revisions" |
> @@ -587,11 +586,21 @@ echo "$orig_head" > "$dotest/orig-head"
> echo "$head_name" > "$dotest/head-name"
> echo "$GIT_QUIET" > "$dotest/quiet"
>
> +if test -n "$rebase_root"
> +then
> + revisions="$onto...$orig_head"
> +else
> + revisions="$upstream...$orig_head"
> +fi
> +
> msgnum=0
> -for cmt in `git rev-list --reverse --no-merges "$revisions"`
> +for cmt in `git rev-list --reverse --no-merges --cherry-pick "$revisions"`
> do
> - msgnum=$(($msgnum + 1))
> - echo "$cmt" > "$dotest/cmt.$msgnum"
> + if test $(git merge-base "$cmt" "$orig_head") = "$cmt"
> + then
> + msgnum=$(($msgnum + 1))
> + echo "$cmt" > "$dotest/cmt.$msgnum"
> + fi
> done
>
> echo 1 >"$dotest/msgnum"
> diff --git a/t/t3406-rebase-message.sh b/t/t3406-rebase-message.sh
> index 85fc7c4..41cb039 100755
> --- a/t/t3406-rebase-message.sh
> +++ b/t/t3406-rebase-message.sh
> @@ -5,8 +5,10 @@ test_description='messages from rebase operation'
> . ./test-lib.sh
>
> quick_one () {
> - echo "$1" >"file$1" &&
> - git add "file$1" &&
> + fileno=$2
> + test -z "$fileno" && fileno=$1
> + echo "$1" >"file$fileno" &&
> + git add "file$fileno" &&
> test_tick &&
> git commit -m "$1"
> }
> @@ -16,21 +18,19 @@ test_expect_success setup '
> git branch topic &&
> quick_one X &&
> quick_one A &&
> - quick_one B &&
> + quick_one B A &&
> quick_one Y &&
>
> git checkout topic &&
> quick_one A &&
> - quick_one B &&
> + quick_one B A &&
> quick_one Z &&
> git tag start
>
> '
>
> cat >expect <<\EOF
> -Already applied: 0001 A
> -Already applied: 0002 B
> -Committed: 0003 Z
> +Committed: 0001 Z
> EOF
>
> test_expect_success 'rebase -m' '
> --
> 1.7.3.2
>
>
--
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