# [PATCH] rebase -m: skip cherry-picked commits more reliably 3 messages from 2010-11-22 to 2010-11-24. Participants: Knut Franke, Martin von Zweigbergk. Thread: https://gitlist.dev/t/25813 ## Knut Franke, 2010-11-22 23:09 Subject: [PATCH] rebase -m: skip cherry-picked commits more reliably Message-ID: <1290467372-6487-1-git-send-email-Knut.Franke@gmx.de> URL: https://gitlist.dev/e/1290467372-6487-1-git-send-email-Knut.Franke%40gmx.de ``` 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. 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 --- 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 ``` ## Martin von Zweigbergk, 2010-11-23 21:03 Subject: Re: [PATCH] rebase -m: skip cherry-picked commits more reliably Message-ID: URL: https://gitlist.dev/e/alpine.DEB.1.10.1011232156510.17721%40debian In-Reply-To: <1290467372-6487-1-git-send-email-Knut.Franke@gmx.de> ``` 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 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 and . 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 > --- > 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 > > ``` ## Martin von Zweigbergk, 2010-11-24 06:53 Subject: Re: [PATCH] rebase -m: skip cherry-picked commits more reliably Message-ID: URL: https://gitlist.dev/e/alpine.DEB.1.10.1011240746030.17721%40debian In-Reply-To: ``` On Tue, 23 Nov 2010, Martin von Zweigbergk wrote: > 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?) To try to answer my own question, would the lines prefixed with a plus in the output of 'git cherry $onto $branch $(git merge-base $upstream $branch)' work? /Martin ```