{"thread":{"id":"25813","subject":"[PATCH] rebase -m: skip cherry-picked commits more reliably","startedAt":"2010-11-22T23:09:32Z","lastAt":"2010-11-24T06:53:49Z","messageCount":3,"participants":["Knut Franke","Martin von Zweigbergk"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"156340","messageId":"1290467372-6487-1-git-send-email-Knut.Franke@gmx.de","threadId":"25813","inReplyTo":null,"subject":"[PATCH] rebase -m: skip cherry-picked commits more reliably","fromName":"Knut Franke","fromEmail":"knut.franke@gmx.de","sentAt":"2010-11-22T23:09:32Z","receivedAt":"2010-11-22T23:09:32Z","isPatch":true,"sender":{"key":"knut.franke@gmx.de","avatar":null},"body":"Unlike ordinary and interactive rebase, rebase -m halts with a conflict\nif two conflicting patches have been applied to both source and target\nbranch (e.g. by cherry-picking). Fix this by using git rev-list\n--cherry-pick to generate the list of patches to apply.\n\nAlso adapt t3406 a) to catch this case and b) not to expect\n\"Already applied\" messages which can't be emitted easily if duplicates\nare removed already when storing the patches.\n\nSigned-off-by: Knut Franke <Knut.Franke@gmx.de>\n---\n git-rebase.sh             |   29 +++++++++++++++++++----------\n t/t3406-rebase-message.sh |   14 +++++++-------\n 2 files changed, 26 insertions(+), 17 deletions(-)\n\ndiff --git a/git-rebase.sh b/git-rebase.sh\nindex 10a238a..aa42744 100755\n--- a/git-rebase.sh\n+++ b/git-rebase.sh\n@@ -552,15 +552,14 @@ then\n \texit 0\n fi\n \n-if test -n \"$rebase_root\"\n-then\n-\trevisions=\"$onto..$orig_head\"\n-else\n-\trevisions=\"$upstream..$orig_head\"\n-fi\n-\n if test -z \"$do_merge\"\n then\n+\tif test -n \"$rebase_root\"\n+\tthen\n+\t\trevisions=\"$onto..$orig_head\"\n+\telse\n+\t\trevisions=\"$upstream..$orig_head\"\n+\tfi\n \tgit format-patch -k --stdout --full-index --ignore-if-in-upstream \\\n \t\t--src-prefix=a/ --dst-prefix=b/ \\\n \t\t--no-renames $root_flag \"$revisions\" |\n@@ -587,11 +586,21 @@ echo \"$orig_head\" > \"$dotest/orig-head\"\n echo \"$head_name\" > \"$dotest/head-name\"\n echo \"$GIT_QUIET\" > \"$dotest/quiet\"\n \n+if test -n \"$rebase_root\"\n+then\n+\trevisions=\"$onto...$orig_head\"\n+else\n+\trevisions=\"$upstream...$orig_head\"\n+fi\n+\n msgnum=0\n-for cmt in `git rev-list --reverse --no-merges \"$revisions\"`\n+for cmt in `git rev-list --reverse --no-merges --cherry-pick \"$revisions\"`\n do\n-\tmsgnum=$(($msgnum + 1))\n-\techo \"$cmt\" > \"$dotest/cmt.$msgnum\"\n+\tif test $(git merge-base \"$cmt\" \"$orig_head\") = \"$cmt\"\n+\tthen\n+\t\tmsgnum=$(($msgnum + 1))\n+\t\techo \"$cmt\" > \"$dotest/cmt.$msgnum\"\n+\tfi\n done\n \n echo 1 >\"$dotest/msgnum\"\ndiff --git a/t/t3406-rebase-message.sh b/t/t3406-rebase-message.sh\nindex 85fc7c4..41cb039 100755\n--- a/t/t3406-rebase-message.sh\n+++ b/t/t3406-rebase-message.sh\n@@ -5,8 +5,10 @@ test_description='messages from rebase operation'\n . ./test-lib.sh\n \n quick_one () {\n-\techo \"$1\" >\"file$1\" &&\n-\tgit add \"file$1\" &&\n+\tfileno=$2\n+\ttest -z \"$fileno\" && fileno=$1\n+\techo \"$1\" >\"file$fileno\" &&\n+\tgit add \"file$fileno\" &&\n \ttest_tick &&\n \tgit commit -m \"$1\"\n }\n@@ -16,21 +18,19 @@ test_expect_success setup '\n \tgit branch topic &&\n \tquick_one X &&\n \tquick_one A &&\n-\tquick_one B &&\n+\tquick_one B A &&\n \tquick_one Y &&\n \n \tgit checkout topic &&\n \tquick_one A &&\n-\tquick_one B &&\n+\tquick_one B A &&\n \tquick_one Z &&\n \tgit tag start\n \n '\n \n cat >expect <<\\EOF\n-Already applied: 0001 A\n-Already applied: 0002 B\n-Committed: 0003 Z\n+Committed: 0001 Z\n EOF\n \n test_expect_success 'rebase -m' '\n-- \n1.7.3.2\n"},{"id":"156463","messageId":"alpine.DEB.1.10.1011232156510.17721@debian","threadId":"25813","inReplyTo":"1290467372-6487-1-git-send-email-Knut.Franke@gmx.de","subject":"Re: [PATCH] rebase -m: skip cherry-picked commits more reliably","fromName":"Martin von Zweigbergk","fromEmail":"martin.von.zweigbergk@gmail.com","sentAt":"2010-11-23T21:03:13Z","receivedAt":"2010-11-23T21:03:13Z","isPatch":true,"sender":{"key":"martinvonz@gmail.com","avatar":"https://avatars.githubusercontent.com/u/891642?v=4"},"body":"\n\nOn Tue, 23 Nov 2010, Knut Franke wrote:\n\n> Unlike ordinary and interactive rebase, rebase -m halts with a conflict\n> if two conflicting patches have been applied to both source and target\n> branch (e.g. by cherry-picking). Fix this by using git rev-list\n> --cherry-pick to generate the list of patches to apply.\n\nI think the idea is sound, but...\n\nUntil I saw your patch I had not thought about the impact of the\n--cherry-pick option (thanks!). I think there is a bug in the existing\nrebase code related to it when rebase is used with the --onto option.\n\nI will try to explain how I think it currently works.\n\nLet's say we have the below history, where E is a cherry-pick of e.\n\n      .-c\n     /\na---b---d---e---f\n         \\\n          .-g---E\n\n\nIf we now run 'git rebase --onto c f E', the revisions that will be\napplied onto 'c' are given by 'git format-patch\n--ignore-if-in-upstream f..E'. In this case that would be only 'g' and\nNOT 'E'.\n\nWhat I think we really want to do is to remove any patches in\nupstream..branch that also exist in branch..onto. I don't know how to\ndo that efficiently, though. (And am I even right about it?)\n\nI think this may be quite a serious bug, since it might go unnoticed\nthat a patch was dropped. If the <upstream> ref is later removed, the\npatch might be completely gone from history without the user noticing.\n\n\nIf the above is correct, is it better to fix that problem first before\napplying your patch? On the other hand, your patch does not really\nmake things worse (except for duplicating the buggy code)...\n\nWith the current implementation, my patch in\nhttp://thread.gmane.org/gmane.comp.version-control.git/161381/ would\nalso make things worse when there are cherry-picks between <upstream>\nand <branch>. The above should be fixed before I re-submit.\n\n\n> \n> Also adapt t3406 a) to catch this case and b) not to expect\n> \"Already applied\" messages which can't be emitted easily if duplicates\n> are removed already when storing the patches.\n> \n> Signed-off-by: Knut Franke <Knut.Franke@gmx.de>\n> ---\n>  git-rebase.sh             |   29 +++++++++++++++++++----------\n>  t/t3406-rebase-message.sh |   14 +++++++-------\n>  2 files changed, 26 insertions(+), 17 deletions(-)\n> \n> diff --git a/git-rebase.sh b/git-rebase.sh\n> index 10a238a..aa42744 100755\n> --- a/git-rebase.sh\n> +++ b/git-rebase.sh\n> @@ -552,15 +552,14 @@ then\n>  \texit 0\n>  fi\n>  \n> -if test -n \"$rebase_root\"\n> -then\n> -\trevisions=\"$onto..$orig_head\"\n> -else\n> -\trevisions=\"$upstream..$orig_head\"\n> -fi\n> -\n>  if test -z \"$do_merge\"\n>  then\n> +\tif test -n \"$rebase_root\"\n> +\tthen\n> +\t\trevisions=\"$onto..$orig_head\"\n> +\telse\n> +\t\trevisions=\"$upstream..$orig_head\"\n> +\tfi\n>  \tgit format-patch -k --stdout --full-index --ignore-if-in-upstream \\\n>  \t\t--src-prefix=a/ --dst-prefix=b/ \\\n>  \t\t--no-renames $root_flag \"$revisions\" |\n> @@ -587,11 +586,21 @@ echo \"$orig_head\" > \"$dotest/orig-head\"\n>  echo \"$head_name\" > \"$dotest/head-name\"\n>  echo \"$GIT_QUIET\" > \"$dotest/quiet\"\n>  \n> +if test -n \"$rebase_root\"\n> +then\n> +\trevisions=\"$onto...$orig_head\"\n> +else\n> +\trevisions=\"$upstream...$orig_head\"\n> +fi\n> +\n>  msgnum=0\n> -for cmt in `git rev-list --reverse --no-merges \"$revisions\"`\n> +for cmt in `git rev-list --reverse --no-merges --cherry-pick \"$revisions\"`\n>  do\n> -\tmsgnum=$(($msgnum + 1))\n> -\techo \"$cmt\" > \"$dotest/cmt.$msgnum\"\n> +\tif test $(git merge-base \"$cmt\" \"$orig_head\") = \"$cmt\"\n> +\tthen\n> +\t\tmsgnum=$(($msgnum + 1))\n> +\t\techo \"$cmt\" > \"$dotest/cmt.$msgnum\"\n> +\tfi\n>  done\n>  \n>  echo 1 >\"$dotest/msgnum\"\n> diff --git a/t/t3406-rebase-message.sh b/t/t3406-rebase-message.sh\n> index 85fc7c4..41cb039 100755\n> --- a/t/t3406-rebase-message.sh\n> +++ b/t/t3406-rebase-message.sh\n> @@ -5,8 +5,10 @@ test_description='messages from rebase operation'\n>  . ./test-lib.sh\n>  \n>  quick_one () {\n> -\techo \"$1\" >\"file$1\" &&\n> -\tgit add \"file$1\" &&\n> +\tfileno=$2\n> +\ttest -z \"$fileno\" && fileno=$1\n> +\techo \"$1\" >\"file$fileno\" &&\n> +\tgit add \"file$fileno\" &&\n>  \ttest_tick &&\n>  \tgit commit -m \"$1\"\n>  }\n> @@ -16,21 +18,19 @@ test_expect_success setup '\n>  \tgit branch topic &&\n>  \tquick_one X &&\n>  \tquick_one A &&\n> -\tquick_one B &&\n> +\tquick_one B A &&\n>  \tquick_one Y &&\n>  \n>  \tgit checkout topic &&\n>  \tquick_one A &&\n> -\tquick_one B &&\n> +\tquick_one B A &&\n>  \tquick_one Z &&\n>  \tgit tag start\n>  \n>  '\n>  \n>  cat >expect <<\\EOF\n> -Already applied: 0001 A\n> -Already applied: 0002 B\n> -Committed: 0003 Z\n> +Committed: 0001 Z\n>  EOF\n>  \n>  test_expect_success 'rebase -m' '\n> -- \n> 1.7.3.2\n> \n> \n"},{"id":"156480","messageId":"alpine.DEB.1.10.1011240746030.17721@debian","threadId":"25813","inReplyTo":"alpine.DEB.1.10.1011232156510.17721@debian","subject":"Re: [PATCH] rebase -m: skip cherry-picked commits more reliably","fromName":"Martin von Zweigbergk","fromEmail":"martin.von.zweigbergk@gmail.com","sentAt":"2010-11-24T06:53:49Z","receivedAt":"2010-11-24T06:53:49Z","isPatch":true,"sender":{"key":"martinvonz@gmail.com","avatar":"https://avatars.githubusercontent.com/u/891642?v=4"},"body":"\n\nOn Tue, 23 Nov 2010, Martin von Zweigbergk wrote:\n\n> What I think we really want to do is to remove any patches in\n> upstream..branch that also exist in branch..onto. I don't know how to\n> do that efficiently, though. (And am I even right about it?)\n\nTo try to answer my own question, would the lines prefixed with a plus\nin the output of 'git cherry $onto $branch $(git merge-base $upstream\n$branch)' work?\n\n/Martin\n"}]}