{"thread":{"id":"31230","subject":"cherry-pick and 'log --no-walk' and ordering","startedAt":"2012-08-10T20:41:36Z","lastAt":"2012-08-30T21:02:05Z","messageCount":37,"participants":["Martin von Zweigbergk","Junio C Hamano","y@google.com","Dan Johnson"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"196833","messageId":"CAOeW2eE=VcUs1YcWqqEUc6vM6jW9JaXzE-_tVWy48VtPzm_+wA@mail.gmail.com","threadId":"31230","inReplyTo":null,"subject":"cherry-pick and 'log --no-walk' and ordering","fromName":"Martin von Zweigbergk","fromEmail":"martin.von.zweigbergk@gmail.com","sentAt":"2012-08-10T20:41:36Z","receivedAt":"2012-08-10T20:41:36Z","isPatch":false,"sender":{"key":"martinvonz@gmail.com","avatar":"https://avatars.githubusercontent.com/u/891642?v=4"},"body":"A while ago when I was looking at revision.c, I was surprised to see\nthat commits are sorted even when --no-walk is passed, but as 8e64006\n(Teach revision machinery about --no-walk, 2007-07-24) points out,\nthis can be useful for doing\n\n $ git log --abbrev-commit --pretty=oneline --decorate --all --no-walk\n\nand get the result sorted by date. However, it can also be useful\n_not_ to get a result sorted by date, e.g. when doing something like\n\" | git rev-list --oneline\n--no-walk --stdin\". Would a --no-sort option to rev-list be\nappreciated or are there better solutions?\n\nThere is also cherry-pick/revert, which I _think_ does not really want\nthe revisions sorted. cherry-pick implicitly reverses the order of the\nwalk, so 'git cherry-pick branch~2..branch' applies them in the right\norder (at least in the absence of clock skew). The documentation for\ncherry-pick suggests \"git rev-list --reverse master -- README | git\ncherry-pick -n --stdin\", which I think makes no sense -- this would\nreverse the output from rev-list only to have it reversed again in\ncherry-pick, if it wasn't for the sorting by date. I think the\n--reverse passed to rev-list might even break the cherry-pick if there\nare commits in non-increasing date order. This is also supported by\nthe fact that test still pass after applying the patch below. I think\nthe test cases make more sense after the patch.\n\nDo others agree with the analysis? I suppose it's too late to change\ncherry-pick to start differentiating between \"git cherry-pick commit1\ncommit2\" and \"git cherry-pick commit2 commit1\", but I think we should\nat least update the documentation as in the patch below (or maybe even\nwith a --topo-order passed to cherry-pick?). We could possibly change\ncherry-pick's ordering from the default ordering to topological\nordering.\n\n\n\nMartin\n\n\nSorry about the mangled whitespace below; just for reference, not\nintended to be applied.\n\ndiff --git a/Documentation/git-cherry-pick.txt\nb/Documentation/git-cherry-pick.txt\nindex 0e170a5..454e205 100644\n--- a/Documentation/git-cherry-pick.txt\n+++ b/Documentation/git-cherry-pick.txt\n@@ -181,7 +181,7 @@ EXAMPLES\n are in next but not HEAD to the current branch, creating a new\n commit for each new change.\n\n-`git rev-list --reverse master -- README | git cherry-pick -n --stdin`::\n+`git rev-list master -- README | git cherry-pick -n --stdin`::\n\n Apply the changes introduced by all commits on the master\n branch that touched README to the working tree and index,\ndiff --git a/t/t3508-cherry-pick-many-commits.sh\nb/t/t3508-cherry-pick-many-commits.sh\nindex 75f7ff4..020baaf 100755\n--- a/t/t3508-cherry-pick-many-commits.sh\n+++ b/t/t3508-cherry-pick-many-commits.sh\n@@ -164,7 +164,7 @@ test_expect_success 'cherry-pick --stdin works' '\n git checkout -f master &&\n git reset --hard first &&\n test_tick &&\n- git rev-list --reverse first..fourth | git cherry-pick --stdin &&\n+ git rev-list first..fourth | git cherry-pick --stdin &&\n git diff --quiet other &&\n git diff --quiet HEAD other &&\n check_head_differs_from fourth\ndiff --git a/t/t3510-cherry-pick-sequence.sh b/t/t3510-cherry-pick-sequence.sh\nindex f4e6450..9e28910 100755\n--- a/t/t3510-cherry-pick-sequence.sh\n+++ b/t/t3510-cherry-pick-sequence.sh\n@@ -400,7 +400,7 @@ test_expect_success '--continue of single-pick\nrespects -x' '\n\n test_expect_success '--continue respects -x in first commit in multi-pick' '\n pristine_detach initial &&\n- test_must_fail git cherry-pick -x picked anotherpick &&\n+ test_must_fail git cherry-pick -x anotherpick picked &&\n echo c >foo &&\n git add foo &&\n git cherry-pick --continue &&\n@@ -430,7 +430,7 @@ test_expect_success '--signoff is not\nautomatically propagated to resolved confl\n\n test_expect_success '--signoff dropped for implicit commit of\nresolution, multi-pick case' '\n pristine_detach initial &&\n- test_must_fail git cherry-pick -s picked anotherpick &&\n+ test_must_fail git cherry-pick -s anotherpick picked &&\n echo c >foo &&\n git add foo &&\n git cherry-pick --continue &&\n"},{"id":"196841","messageId":"7vfw7uig13.fsf@alter.siamese.dyndns.org","threadId":"31230","inReplyTo":"CAOeW2eE=VcUs1YcWqqEUc6vM6jW9JaXzE-_tVWy48VtPzm_+wA@mail.gmail.com","subject":"Re: cherry-pick and 'log --no-walk' and ordering","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-08-10T21:38:32Z","receivedAt":"2012-08-10T21:38:32Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Martin von Zweigbergk writes:\n\n> There is also cherry-pick/revert, which I _think_ does not really want\n> the revisions sorted.\n\nYes, I think sequencer.c::prepare_revs() is wrong to unconditoinally\ncall prepare_revision_walk().\n\nIt instead should first check the revs->pending.objects list to see\nif what was given by the caller is a mere collection of individual\nobjects or a range expression (i.e. check if any of them is marked\nwith UNINTERESTING), and refrain from going into the body of the\npreparation steps, which has to involve sorting.\n\nI think we had to fix a bug in \"git show\" coming from a similar root\ncause, but the bug manifested in the opposite direction.\n"},{"id":"196852","messageId":"CAOeW2eHz5un9cNoy-=7Y8=F_G6u-n8kk7kXGHQ+dKrHD8wW6BA@mail.gmail.com","threadId":"31230","inReplyTo":"7vfw7uig13.fsf@alter.siamese.dyndns.org","subject":"Re: cherry-pick and 'log --no-walk' and ordering","fromName":"Martin von Zweigbergk","fromEmail":"martin.von.zweigbergk@gmail.com","sentAt":"2012-08-11T05:34:32Z","receivedAt":"2012-08-11T05:34:32Z","isPatch":false,"sender":{"key":"martinvonz@gmail.com","avatar":"https://avatars.githubusercontent.com/u/891642?v=4"},"body":"On Fri, Aug 10, 2012 at 2:38 PM, Junio C Hamano wrote:\n> Martin von Zweigbergk writes:\n>\n>> There is also cherry-pick/revert, which I _think_ does not really want\n>> the revisions sorted.\n>\n> Yes, I think sequencer.c::prepare_revs() is wrong to unconditoinally\n> call prepare_revision_walk().\n>\n> It instead should first check the revs->pending.objects list to see\n> if what was given by the caller is a mere collection of individual\n> objects or a range expression (i.e. check if any of them is marked\n> with UNINTERESTING), and refrain from going into the body of the\n> preparation steps, which has to involve sorting.\n\nDo you mean \"has to involve sorting\" as in \"has to involve sorting in\norder not to break current users of e.g. 'git log --no-walk\n--branches'\" or \"revision walking inherently involves sorting\"? My\ncurrent working assumption is that it is the former.\n\nI will make rev_info.no_walk a tri-state {walk, no-walk-sorted,\nno-walk-unsorted}. The third state would be used from\ncherry-pick/revert (and maybe git-show, although it should make no\ndifference). I would also expose the third state to rev-list's command\nline, maybe as --no-walk=unsorted.\n\nActually, all but command-line parsing is done now and test seem fine,\nwith quite a small patch:\n$ git diff --stat\n builtin/log.c | 2 +-\n builtin/revert.c | 2 +-\n revision.c | 5 +++--\n revision.h | 6 +++++-\n 4 files changed, 10 insertions(+), 5 deletions(-)\n\nDid you see a problem with this approach, since you said that\nsequencer shouldn't unconditionally call prepare_revision_walk()? I\ncan see that git-show needs to go through revs->pending.objects\nbecause it handles tags and stuff, but cherry-pick/revert only seem to\nneed the revisions.\n\nMartin\n"},{"id":"196853","messageId":"7vpq6ygcy1.fsf@alter.siamese.dyndns.org","threadId":"31230","inReplyTo":"CAOeW2eHz5un9cNoy-=7Y8=F_G6u-n8kk7kXGHQ+dKrHD8wW6BA@mail.gmail.com","subject":"Re: cherry-pick and 'log --no-walk' and ordering","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-08-11T06:28:06Z","receivedAt":"2012-08-11T06:28:06Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Martin von Zweigbergk writes:\n\n> On Fri, Aug 10, 2012 at 2:38 PM, Junio C Hamano wrote:\n>> Martin von Zweigbergk writes:\n>>\n>>> There is also cherry-pick/revert, which I _think_ does not really want\n>>> the revisions sorted.\n>>\n>> Yes, I think sequencer.c::prepare_revs() is wrong to unconditoinally\n>> call prepare_revision_walk().\n>>\n>> It instead should first check the revs->pending.objects list to see\n>> if what was given by the caller is a mere collection of individual\n>> objects or a range expression (i.e. check if any of them is marked\n>> with UNINTERESTING), and refrain from going into the body of the\n>> preparation steps, which has to involve sorting.\n>\n> Do you mean \"has to involve sorting\" as in \"has to involve sorting in\n> order not to break current users of e.g. 'git log --no-walk\n> --branches'\" or \"revision walking inherently involves sorting\"?\n\nRange limited revision walking, e.g. \"git cherry-pick A..B D~4..D\",\nfundamentally implies sorting and you cannot assume B would appear\nbefore D only because B comes before D on the command line (B may\neven be inside D~4..D range in which case it would not even appear\nin the final output).\n\nAny caller that wants to retrieve the objects given from the command\nline in the order the user gave it, e.g. \"git cherry-pick A B C\",\nusing setup_revisions() and without walking the history, must look\nat revs->pending.objects without calling prepare_revision_walk().\n"},{"id":"196899","messageId":"50289e50.8458320a.7d31.3c46SMTPIN_ADDED@gmr-mx.google.com","threadId":"31230","inReplyTo":"7vpq6ygcy1.fsf@alter.siamese.dyndns.org","subject":"[PATCH 0/4] Re: cherry-pick and 'log --no-walk' and ordering","fromName":"","fromEmail":"y@google.com","sentAt":"2012-08-13T06:27:16Z","receivedAt":"2012-08-13T06:27:16Z","isPatch":true,"sender":{"key":"y@google.com","avatar":null},"body":"From: Martin von Zweigbergk \n\nThis series adds supports for 'git log --no-walk=unsorted', which\nshould be useful for the re-roll of my mz/rebase-range series. It also\naddresses the bug in cherry-pick/revert, which makes it sort revisions\nby date.\n\nOn Fri, Aug 10, 2012 at 11:28 PM, Junio C Hamano wrote:\n> Range limited revision walking, e.g. \"git cherry-pick A..B D~4..D\",\n> fundamentally implies sorting and you cannot assume B would appear\n> before D only because B comes before D on the command line (B may\n> even be inside D~4..D range in which case it would not even appear\n> in the final output).\n\nSorry, I probably wasn't clear; I mentioned \"revision walking\", but I\nonly meant the no-walk case. I hope the patches make sense.\n\n\nMartin von Zweigbergk (4):\n teach log --no-walk=unsorted, which avoids sorting\n revisions passed to cherry-pick should be in \"default\" order\n cherry-pick/revert: respect order of revisions to pick\n cherry-pick/revert: default to topological sorting\n\n Documentation/git-cherry-pick.txt | 2 +-\n builtin/log.c | 2 +-\n builtin/revert.c | 3 ++-\n revision.c | 18 +++++++++++++++---\n revision.h | 6 +++++-\n t/t3508-cherry-pick-many-commits.sh | 2 +-\n t/t3510-cherry-pick-sequence.sh | 4 ++--\n t/t4202-log.sh | 10 ++++++++++\n 8 files changed, 37 insertions(+), 10 deletions(-)\n\n-- \n1.7.11.1.104.ge7b44f1\n"},{"id":"196896","messageId":"50289e50.0aad320a.5916.39eaSMTPIN_ADDED@gmr-mx.google.com","threadId":"31230","inReplyTo":"1344839240-17402-1-git-send-email-y","subject":"[PATCH 1/4] teach log --no-walk=unsorted, which avoids sorting","fromName":"","fromEmail":"y@google.com","sentAt":"2012-08-13T06:27:17Z","receivedAt":"2012-08-13T06:27:17Z","isPatch":true,"sender":{"key":"y@google.com","avatar":null},"body":"From: Martin von Zweigbergk \n\nWhen 'git log' is passed the --no-walk option, no revision walk takes\nplace, naturally. Perhaps somewhat surprisingly, however, the provided\nrevisions still get sorted by commit date. So e.g 'git log --no-walk\nHEAD HEAD~1' and 'git log --no-walk HEAD~1 HEAD' give the same result\n(unless the two revisions share the commit date, in which case they\nwill retain the order given on the command line). As the commit that\nintroduced --no-walk (8e64006 (Teach revision machinery about\n--no-walk, 2007-07-24)) points out, the sorting is intentional, to\nallow things like\n\n git log --abbrev-commit --pretty=oneline --decorate --all --no-walk\n\nto show all refs in order by commit date.\n\nBut there are also other cases where the sorting is not wanted, such\nas\n\n |\n git log --oneline --no-walk --stdin\n\nTo accomodate both cases, leave the decision of whether or not to sort\nup to the caller, by allowing --no-walk={sorted,unsorted}, defaulting\nto 'sorted'.\n\nSigned-off-by: Martin von Zweigbergk \n---\n builtin/log.c | 2 +-\n builtin/revert.c | 2 +-\n revision.c | 18 +++++++++++++++---\n revision.h | 6 +++++-\n t/t4202-log.sh | 10 ++++++++++\n 5 files changed, 32 insertions(+), 6 deletions(-)\n\ndiff --git a/builtin/log.c b/builtin/log.c\nindex ecc2793..20838b1 100644\n--- a/builtin/log.c\n+++ b/builtin/log.c\n@@ -456,7 +456,7 @@ int cmd_show(int argc, const char **argv, const char *prefix)\n \tinit_revisions(&rev, prefix);\n \trev.diff = 1;\n \trev.always_show_header = 1;\n-\trev.no_walk = 1;\n+\trev.no_walk = REVISION_WALK_NO_WALK_SORTED;\n \trev.diffopt.stat_width = -1; \t/* Scale to real terminal size */\n \n \tmemset(&opt, 0, sizeof(opt));\ndiff --git a/builtin/revert.c b/builtin/revert.c\nindex 82d1bf8..42ce399 100644\n--- a/builtin/revert.c\n+++ b/builtin/revert.c\n@@ -193,7 +193,7 @@ static void parse_args(int argc, const char **argv, struct replay_opts *opts)\n \t\tstruct setup_revision_opt s_r_opt;\n \t\topts->revs = xmalloc(sizeof(*opts->revs));\n \t\tinit_revisions(opts->revs, NULL);\n-\t\topts->revs->no_walk = 1;\n+\t\topts->revs->no_walk = REVISION_WALK_NO_WALK_SORTED;\n \t\tif (argc < 2)\n \t\t\tusage_with_options(usage_str, options);\n \t\tmemset(&s_r_opt, 0, sizeof(s_r_opt));\ndiff --git a/revision.c b/revision.c\nindex 9e8f47a..2faf675 100644\n--- a/revision.c\n+++ b/revision.c\n@@ -1298,7 +1298,7 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg\n \t !strcmp(arg, \"--no-walk\") || !strcmp(arg, \"--do-walk\") ||\n \t !strcmp(arg, \"--bisect\") || !prefixcmp(arg, \"--glob=\") ||\n \t !prefixcmp(arg, \"--branches=\") || !prefixcmp(arg, \"--tags=\") ||\n-\t !prefixcmp(arg, \"--remotes=\"))\n+\t !prefixcmp(arg, \"--remotes=\") || !prefixcmp(arg, \"--no-walk=\"))\n \t{\n \t\tunkv[(*unkc)++] = arg;\n \t\treturn 1;\n@@ -1693,7 +1693,18 @@ static int handle_revision_pseudo_opt(const char *submodule,\n \t} else if (!strcmp(arg, \"--not\")) {\n \t\t*flags ^= UNINTERESTING;\n \t} else if (!strcmp(arg, \"--no-walk\")) {\n-\t\trevs->no_walk = 1;\n+\t\trevs->no_walk = REVISION_WALK_NO_WALK_SORTED;\n+\t} else if (!prefixcmp(arg, \"--no-walk=\")) {\n+\t\t/*\n+\t\t * Detached form (\"--no-walk X\" as opposed to \"--no-walk=X\")\n+\t\t * not allowed, since the argument is optional.\n+\t\t */\n+\t\tif (!strcmp(arg + 10, \"sorted\"))\n+\t\t\trevs->no_walk = REVISION_WALK_NO_WALK_SORTED;\n+\t\telse if (!strcmp(arg + 10, \"unsorted\"))\n+\t\t\trevs->no_walk = REVISION_WALK_NO_WALK_UNSORTED;\n+\t\telse\n+\t\t\treturn error(\"invalid argument to --no-walk\");\n \t} else if (!strcmp(arg, \"--do-walk\")) {\n \t\trevs->no_walk = 0;\n \t} else {\n@@ -2116,10 +2127,11 @@ int prepare_revision_walk(struct rev_info *revs)\n \t\t}\n \t\te++;\n \t}\n-\tcommit_list_sort_by_date(&revs->commits);\n \tif (!revs->leak_pending)\n \t\tfree(list);\n \n+\tif (revs->no_walk != REVISION_WALK_NO_WALK_UNSORTED)\n+\t\tcommit_list_sort_by_date(&revs->commits);\n \tif (revs->no_walk)\n \t\treturn 0;\n \tif (revs->limited)\ndiff --git a/revision.h b/revision.h\nindex cb5ab35..a95bd0b 100644\n--- a/revision.h\n+++ b/revision.h\n@@ -41,6 +41,10 @@ struct rev_cmdline_info {\n \t} *rev;\n };\n \n+#define REVISION_WALK_WALK 0\n+#define REVISION_WALK_NO_WALK_SORTED 1\n+#define REVISION_WALK_NO_WALK_UNSORTED 2\n+\n struct rev_info {\n \t/* Starting list */\n \tstruct commit_list *commits;\n@@ -62,7 +66,7 @@ struct rev_info {\n \t/* Traversal flags */\n \tunsigned int\tdense:1,\n \t\t\tprune:1,\n-\t\t\tno_walk:1,\n+\t\t\tno_walk:2,\n \t\t\tshow_all:1,\n \t\t\tremove_empty_trees:1,\n \t\t\tsimplify_history:1,\ndiff --git a/t/t4202-log.sh b/t/t4202-log.sh\nindex 71be59d..bd83355 100755\n--- a/t/t4202-log.sh\n+++ b/t/t4202-log.sh\n@@ -178,11 +178,21 @@ test_expect_success 'git log --no-walk sorts by commit time' '\n \ttest_cmp expect actual\n '\n \n+test_expect_success 'git log --no-walk=sorted sorts by commit time' '\n+\tgit log --no-walk=sorted --oneline 5d31159 804a787 394ef78 > actual &&\n+\ttest_cmp expect actual\n+'\n+\n cat > expect << EOF\n 5d31159 fourth\n 804a787 sixth\n 394ef78 fifth\n EOF\n+test_expect_success 'git log --no-walk=unsorted leaves list of commits as given' '\n+\tgit log --no-walk=unsorted --oneline 5d31159 804a787 394ef78 > actual &&\n+\ttest_cmp expect actual\n+'\n+\n test_expect_success 'git show leaves list of commits as given' '\n \tgit show --oneline -s 5d31159 804a787 394ef78 > actual &&\n \ttest_cmp expect actual\n-- \n1.7.11.1.104.ge7b44f1\n"},{"id":"196895","messageId":"50289e50.a19f320a.5d99.3fdfSMTPIN_ADDED@gmr-mx.google.com","threadId":"31230","inReplyTo":"1344839240-17402-1-git-send-email-y","subject":"[PATCH 2/4] revisions passed to cherry-pick should be in \"default\" order","fromName":"","fromEmail":"y@google.com","sentAt":"2012-08-13T06:27:18Z","receivedAt":"2012-08-13T06:27:18Z","isPatch":true,"sender":{"key":"y@google.com","avatar":null},"body":"From: Martin von Zweigbergk \n\n'git cherry-pick' internally sets the --reverse option while walking\nrevisions, so that 'git cherry-pick branch@{u}..branch' will apply the\nrevisions starting at the oldest one. If no uninteresing revisions are\ngiven, --no-walk is implied. Still, the documentation for 'git\ncherry-pick --stdin' uses the following example:\n\n git rev-list --reverse master -- README | git cherry-pick -n --stdin\n\nThe above would seem to reverse the revisions in the output (which it\ndoes), and then pipe them to 'git cherry-pick', which would reverse\nthem again and apply them in the wrong order. The same problem occurs\nwhen supplying revisions explicitly on the command line instead of\nsending them to stdin.\n\nBecause of the sorting-by-date that is done by the revision walker\n(even with the implied --no-walk), the ordering in the output from\n'git rev-list' in the example above is effectively ignored, and the\nabove actually works most of the time. However, if revisions share a\ncommit date (as can easily happen as a result of rebasing), they do\nget applied out-of-order.\n\nUpdate the documentation not to suggest reversing the input to 'git\ncherry-pick'. Also update test cases where the inputs are reversed.\n\nSigned-off-by: Martin von Zweigbergk \n---\n Documentation/git-cherry-pick.txt | 2 +-\n t/t3508-cherry-pick-many-commits.sh | 2 +-\n t/t3510-cherry-pick-sequence.sh | 4 ++--\n 3 files changed, 4 insertions(+), 4 deletions(-)\n\ndiff --git a/Documentation/git-cherry-pick.txt b/Documentation/git-cherry-pick.txt\nindex 0e170a5..454e205 100644\n--- a/Documentation/git-cherry-pick.txt\n+++ b/Documentation/git-cherry-pick.txt\n@@ -181,7 +181,7 @@ EXAMPLES\n \tare in next but not HEAD to the current branch, creating a new\n \tcommit for each new change.\n \n-`git rev-list --reverse master -- README | git cherry-pick -n --stdin`::\n+`git rev-list master -- README | git cherry-pick -n --stdin`::\n \n \tApply the changes introduced by all commits on the master\n \tbranch that touched README to the working tree and index,\ndiff --git a/t/t3508-cherry-pick-many-commits.sh b/t/t3508-cherry-pick-many-commits.sh\nindex 75f7ff4..020baaf 100755\n--- a/t/t3508-cherry-pick-many-commits.sh\n+++ b/t/t3508-cherry-pick-many-commits.sh\n@@ -164,7 +164,7 @@ test_expect_success 'cherry-pick --stdin works' '\n \tgit checkout -f master &&\n \tgit reset --hard first &&\n \ttest_tick &&\n-\tgit rev-list --reverse first..fourth | git cherry-pick --stdin &&\n+\tgit rev-list first..fourth | git cherry-pick --stdin &&\n \tgit diff --quiet other &&\n \tgit diff --quiet HEAD other &&\n \tcheck_head_differs_from fourth\ndiff --git a/t/t3510-cherry-pick-sequence.sh b/t/t3510-cherry-pick-sequence.sh\nindex f4e6450..9e28910 100755\n--- a/t/t3510-cherry-pick-sequence.sh\n+++ b/t/t3510-cherry-pick-sequence.sh\n@@ -400,7 +400,7 @@ test_expect_success '--continue of single-pick respects -x' '\n \n test_expect_success '--continue respects -x in first commit in multi-pick' '\n \tpristine_detach initial &&\n-\ttest_must_fail git cherry-pick -x picked anotherpick &&\n+\ttest_must_fail git cherry-pick -x anotherpick picked &&\n \techo c >foo &&\n \tgit add foo &&\n \tgit cherry-pick --continue &&\n@@ -430,7 +430,7 @@ test_expect_success '--signoff is not automatically propagated to resolved confl\n \n test_expect_success '--signoff dropped for implicit commit of resolution, multi-pick case' '\n \tpristine_detach initial &&\n-\ttest_must_fail git cherry-pick -s picked anotherpick &&\n+\ttest_must_fail git cherry-pick -s anotherpick picked &&\n \techo c >foo &&\n \tgit add foo &&\n \tgit cherry-pick --continue &&\n-- \n1.7.11.1.104.ge7b44f1\n"},{"id":"196898","messageId":"50289e51.29d0320a.65ff.2c47SMTPIN_ADDED@gmr-mx.google.com","threadId":"31230","inReplyTo":"1344839240-17402-1-git-send-email-y","subject":"[PATCH 3/4] cherry-pick/revert: respect order of revisions to pick","fromName":"","fromEmail":"y@google.com","sentAt":"2012-08-13T06:27:19Z","receivedAt":"2012-08-13T06:27:19Z","isPatch":true,"sender":{"key":"y@google.com","avatar":null},"body":"From: Martin von Zweigbergk \n\n'git cherry-pick A B' implicitly sends --no-walk=sorted to the\nrevision walker, which means that the older of A and B will be applied\nfirst, which is most likely surprising to most. Fix this by instead\nsending --no-walk=unsorted to the revision walker.\n\nSigned-off-by: Martin von Zweigbergk \n---\n\nThis has actually been reported before, in\nhttp://thread.gmane.org/gmane.comp.version-control.git/164794/focus=164807,\nwhere I apparently replied myself. Incidentally, it seems like the\nunrelated bug in 'git show' I reported in that thread is the one that\nJunio mentioned got fixed recently.\n\n builtin/revert.c | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/builtin/revert.c b/builtin/revert.c\nindex 42ce399..98ad641 100644\n--- a/builtin/revert.c\n+++ b/builtin/revert.c\n@@ -193,7 +193,7 @@ static void parse_args(int argc, const char **argv, struct replay_opts *opts)\n \t\tstruct setup_revision_opt s_r_opt;\n \t\topts->revs = xmalloc(sizeof(*opts->revs));\n \t\tinit_revisions(opts->revs, NULL);\n-\t\topts->revs->no_walk = REVISION_WALK_NO_WALK_SORTED;\n+\t\topts->revs->no_walk = REVISION_WALK_NO_WALK_UNSORTED;\n \t\tif (argc < 2)\n \t\t\tusage_with_options(usage_str, options);\n \t\tmemset(&s_r_opt, 0, sizeof(s_r_opt));\n-- \n1.7.11.1.104.ge7b44f1\n"},{"id":"196897","messageId":"50289e51.29d0320a.65ff.2c48SMTPIN_ADDED@gmr-mx.google.com","threadId":"31230","inReplyTo":"1344839240-17402-1-git-send-email-y","subject":"[PATCH 4/4] cherry-pick/revert: default to topological sorting","fromName":"","fromEmail":"y@google.com","sentAt":"2012-08-13T06:27:20Z","receivedAt":"2012-08-13T06:27:20Z","isPatch":true,"sender":{"key":"y@google.com","avatar":null},"body":"From: Martin von Zweigbergk \n\nWhen 'git cherry-pick' and 'git revert' are used with ranges such as\n'git cherry-pick A..B', the order of the commits to pick are\ndetermined by the default date-based sorting. If a commit has a commit\ndate before the commit date of its parent, it will therfore be applied\nbefore its parent. In the context of cherry-pick/revert, this is most\nlikely not what the user expected, so let's enable topological sorting\nby default.\n\nSigned-off-by: Martin von Zweigbergk \n---\n builtin/revert.c | 1 +\n 1 file changed, 1 insertion(+)\n\ndiff --git a/builtin/revert.c b/builtin/revert.c\nindex 98ad641..6880ce5 100644\n--- a/builtin/revert.c\n+++ b/builtin/revert.c\n@@ -194,6 +194,7 @@ static void parse_args(int argc, const char **argv, struct replay_opts *opts)\n \t\topts->revs = xmalloc(sizeof(*opts->revs));\n \t\tinit_revisions(opts->revs, NULL);\n \t\topts->revs->no_walk = REVISION_WALK_NO_WALK_UNSORTED;\n+\t\topts->revs->topo_order = 1;\n \t\tif (argc < 2)\n \t\t\tusage_with_options(usage_str, options);\n \t\tmemset(&s_r_opt, 0, sizeof(s_r_opt));\n-- \n1.7.11.1.104.ge7b44f1\n"},{"id":"196902","messageId":"7vhas7fefs.fsf@alter.siamese.dyndns.org","threadId":"31230","inReplyTo":"50289e50.8458320a.7d31.3c46SMTPIN_ADDED@gmr-mx.google.com","subject":"Re: [PATCH 0/4] Re: cherry-pick and 'log --no-walk' and ordering","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-08-13T07:17:59Z","receivedAt":"2012-08-13T07:17:59Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"y@google.com writes:\n\n[Administrivia: I somehow doubt y@google.com would reach you, and\nfutzed with the To: line above]\n\n> From: Martin von Zweigbergk \n>\n> This series adds supports for 'git log --no-walk=unsorted', which\n> should be useful for the re-roll of my mz/rebase-range series. It also\n> addresses the bug in cherry-pick/revert, which makes it sort revisions\n> by date.\n>\n> On Fri, Aug 10, 2012 at 11:28 PM, Junio C Hamano wrote:\n>> Range limited revision walking, e.g. \"git cherry-pick A..B D~4..D\",\n>> fundamentally implies sorting and you cannot assume B would appear\n>> before D only because B comes before D on the command line (B may\n>> even be inside D~4..D range in which case it would not even appear\n>> in the final output).\n>\n> Sorry, I probably wasn't clear; I mentioned \"revision walking\", but I\n> only meant the no-walk case. I hope the patches make sense.\n\nI actually think --no-walk, especially when given any negative\nrevision, that sorts is fundamentally a flawed concept (it led to\nthe inconsistency that made \"git show A..B C\" vs \"git show C A..B\"\nbehave differently, which we had to fix recently).\n\nWould anything break if we take your patch, but without two\npossibilities to revs->no_walk option (i.e. we never sort under\nno_walk)? That is, the core of your change would become something\nlike this:\n\n revision.c | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/revision.c b/revision.c\nindex 9e8f47a..589d17f 100644\n--- a/revision.c\n+++ b/revision.c\n@@ -2116,12 +2116,12 @@ int prepare_revision_walk(struct rev_info *revs)\n \t\t}\n \t\te++;\n \t}\n-\tcommit_list_sort_by_date(&revs->commits);\n \tif (!revs->leak_pending)\n \t\tfree(list);\n \n \tif (revs->no_walk)\n \t\treturn 0;\n+\tcommit_list_sort_by_date(&revs->commits);\n \tif (revs->limited)\n \t\tif (limit_list(revs) < 0)\n \t\t\treturn -1;\n"},{"id":"196903","messageId":"7vd32vfe24.fsf@alter.siamese.dyndns.org","threadId":"31230","inReplyTo":"7vhas7fefs.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 0/4] Re: cherry-pick and 'log --no-walk' and ordering","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-08-13T07:26:11Z","receivedAt":"2012-08-13T07:26:11Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> Would anything break if we take your patch, but without two\n> possibilities to revs->no_walk option (i.e. we never sort under\n> no_walk)?\n\nBy the way, by \"would anything break\", I do not just mean if our\nexisting tests trigger failures from \"test_expect_success\"; I\nsuspect some do assume the sorting behaviour. I am wondering if the\nsorting makes sense in the real users; in other words, if the\nfailing tests, if any, are expecting sensible and useful behaviour.\n\nAfter all, the sorting by the commit timestamp is made solely to\noptimize the limit_list() which wants to traverse commits ancestry\nnear the tip of the history, and sorting by the commit timestamp is\ndone because it is usually a good and quick approximation for\ntopological sorting.\n"},{"id":"196914","messageId":"CAOeW2eHprw73+zqVbJRird1eE7ayU_KjCUSoieYsGi1rbL5QBQ@mail.gmail.com","threadId":"31230","inReplyTo":"7vhas7fefs.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 0/4] Re: cherry-pick and 'log --no-walk' and ordering","fromName":"Martin von Zweigbergk","fromEmail":"martin.von.zweigbergk@gmail.com","sentAt":"2012-08-13T16:09:20Z","receivedAt":"2012-08-13T16:09:20Z","isPatch":true,"sender":{"key":"martinvonz@gmail.com","avatar":"https://avatars.githubusercontent.com/u/891642?v=4"},"body":"On Mon, Aug 13, 2012 at 12:17 AM, Junio C Hamano wrote:\n> y@google.com writes:\n>\n> [Administrivia: I somehow doubt y@google.com would reach you, and\n> futzed with the To: line above]\n\n:-( Sorry, sendemail.from now set. (I apparently answered \"y\" instead\nof just to accept the default.)\n\n> I actually think --no-walk, especially when given any negative\n> revision, that sorts is fundamentally a flawed concept (it led to\n> the inconsistency that made \"git show A..B C\" vs \"git show C A..B\"\n> behave differently, which we had to fix recently).\n\nI completely agree.\n\n> Would anything break if we take your patch, but without two\n> possibilities to revs->no_walk option (i.e. we never sort under\n> no_walk)? That is, the core of your change would become something\n> like this:\n\nI also thought the sorting was just a bug. From what I understand by\nlooking how the code has evolved, the sorting in the no-walk case was\nnot intentional, but more of a consequence of the implementation. That\npatch you suggested was my first attempt and led me to find the broken\ncherry-pick test cases that I then fixed in patch 2/4. But, it clearly\nwould break the test case in t4202 called 'git log --no-walk \nsorts by commit time'. So I started digging from there and found e.g.\n\nhttp://thread.gmane.org/gmane.comp.version-control.git/123205/focus=123216\n\nFor convenience, I have pasted the commit message of the commit\nmentioned in that thread at the end of this email. So we would be\nbreaking at least Johannes's use case if we changed it. I would think\nalmost everyone who doesn't already know would expect \"git rev-list A\nB\" to list them in that order, so is a migration desired? Or just\nchange the default for --no-walk from \"sorted\" to \"unsorted\" in git\n2.0?\n\nBy the way, git-log's documentation says \"By default, the commits are\nshown in reverse chronological order.\", which to some degree is in\nsupport of the current behavior.\n\n\ncommit 8e64006eee9c82eba513b98306c179c9e2385e4e\nAuthor: Johannes Schindelin \nDate: Tue Jul 24 00:38:40 2007 +0100\n\n Teach revision machinery about --no-walk\n\n The flag \"no_walk\" is present in struct rev_info since a long time, but\n so far has been in use exclusively by \"git show\".\n\n With this flag, you can see all your refs, ordered by date of the last\n commit:\n\n $ git log --abbrev-commit --pretty=oneline --decorate --all --no-walk\n\n which is extremely helpful if you have to juggle with a lot topic\n branches, and do not remember in which one you introduced that uber\n debug option, or simply want to get an overview what is cooking.\n\n (Note that the \"git log\" invocation above does not output the same as\n\n $ git show --abbrev-commit --pretty=oneline --decorate --all --quiet\n\n since \"git show\" keeps the alphabetic order that \"--all\" returns the\n refs in, even if the option \"--date-order\" was passed.)\n\n For good measure, this also adds the \"--do-walk\" option which overrides\n \"--no-walk\".\n\n Signed-off-by: Johannes Schindelin \n Signed-off-by: Junio C Hamano \n"},{"id":"196917","messageId":"7vzk5yen99.fsf@alter.siamese.dyndns.org","threadId":"31230","inReplyTo":"CAOeW2eHprw73+zqVbJRird1eE7ayU_KjCUSoieYsGi1rbL5QBQ@mail.gmail.com","subject":"Re: [PATCH 0/4] Re: cherry-pick and 'log --no-walk' and ordering","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-08-13T17:05:06Z","receivedAt":"2012-08-13T17:05:06Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Martin von Zweigbergk writes:\n\n> I also thought the sorting was just a bug. From what I understand by\n> looking how the code has evolved, the sorting in the no-walk case was\n> not intentional, but more of a consequence of the implementation. That\n> patch you suggested was my first attempt and led me to find the broken\n> cherry-pick test cases that I then fixed in patch 2/4. But, it clearly\n> would break the test case in t4202 called 'git log --no-walk \n> sorts by commit time'. So I started digging from there and found e.g.\n>\n> http://thread.gmane.org/gmane.comp.version-control.git/123205/focus=123216\n>\n> For convenience, I have pasted the commit message of the commit\n> mentioned in that thread at the end of this email. So we would be\n> breaking at least Johannes's use case if we changed it.\n\nOk. Having a way to conveniently sort based on committer date is\nindeed handy, and losing it would be a regression.\n\nNot that the accident that supports only on committer date is a\nnicely designed feature. The user may want to sort on author date\ninstead, but there is no way to do so with --no-walk. So in that\nsense, Johannes's use case happens to work by accident.\n\n> ... so is a migration desired? Or just\n> change the default for --no-walk from \"sorted\" to \"unsorted\" in git\n> 2.0?\n\nI think the proper support for Johannes's case should give users\nmore control on what to sort on, and that switch should not be tied\nto \"--no-walk\". After all, being able to sort commits in the result\nof limit_list() with various criteria would equally useful as being\nable to sort commits listed on the command line with --no-walk.\nThink about what \"git shortlog A..B\" does, for example. It is like\nfirst enumerating commits within the given range, and sorting the\nresult using author as the primary and then timestamp as the\nsecondary sort column.\n\nSo let's not even think about migration, and go in the direction of\ngiving \"--no-walk\" two flavours, for now. Either it keeps the order\ncommits were given from the command line, or it does the default\nsort using the timestamp. We can later add the --sort-on option that\nwould work with or without --no-walk for people who want output that\nis differently sorted, but that is outside the scope of your series.\n\n> By the way, git-log's documentation says \"By default, the commits are\n> shown in reverse chronological order.\", which to some degree is in\n> support of the current behavior.\n\nThat is talking about the presentation order of the result of\nlimit_list(), predates --no-walk, and was not adjusted to the new\nworld order when --no-walk was introduced, so I would not take it as\na supporting argument.\n\nBut not regressing the current \"you can see them sorted on the\ncommit timestamp (this is merely an accident and not a designed\nfeature, so you cannot choose to sort on other things)\" behaviour is\na reason enough not to disable sorting for the plain \"--no-walk\"\noption.\n\nThanks.\n"},{"id":"196925","messageId":"CAOeW2eF67Tj0Mq+g+-3UFyh_Xvt=ZcKDc9LjCKGwu9y2G39NBQ@mail.gmail.com","threadId":"31230","inReplyTo":"7vzk5yen99.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 0/4] Re: cherry-pick and 'log --no-walk' and ordering","fromName":"Martin von Zweigbergk","fromEmail":"martin.von.zweigbergk@gmail.com","sentAt":"2012-08-13T18:28:30Z","receivedAt":"2012-08-13T18:28:30Z","isPatch":true,"sender":{"key":"martinvonz@gmail.com","avatar":"https://avatars.githubusercontent.com/u/891642?v=4"},"body":"On Mon, Aug 13, 2012 at 10:05 AM, Junio C Hamano wrote:\n> Martin von Zweigbergk writes:\n>>\n>> ... so is a migration desired? Or just\n>> change the default for --no-walk from \"sorted\" to \"unsorted\" in git\n>> 2.0?\n>\n> I think the proper support for Johannes's case should give users\n> more control on what to sort on, and that switch should not be tied\n> to \"--no-walk\". After all, being able to sort commits in the result\n> of limit_list() with various criteria would equally useful as being\n> able to sort commits listed on the command line with --no-walk.\n> Think about what \"git shortlog A..B\" does, for example. It is like\n> first enumerating commits within the given range, and sorting the\n> result using author as the primary and then timestamp as the\n> secondary sort column.\n>\n> So let's not even think about migration, and go in the direction of\n> giving \"--no-walk\" two flavours, for now. Either it keeps the order\n> commits were given from the command line, or it does the default\n> sort using the timestamp. We can later add the --sort-on option that\n> would work with or without --no-walk for people who want output that\n> is differently sorted, but that is outside the scope of your series.\n\nMakes sense. The shortlog example is a good example of sorting that\ncompletely reorders the commit graph sometimes even making sense for\nranges. Thanks!\n"},{"id":"196931","messageId":"7vtxw6d0ct.fsf@alter.siamese.dyndns.org","threadId":"31230","inReplyTo":"50289e50.a19f320a.5d99.3fdfSMTPIN_ADDED@gmr-mx.google.com","subject":"Re: [PATCH 2/4] revisions passed to cherry-pick should be in \"default\" order","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-08-13T20:05:06Z","receivedAt":"2012-08-13T20:05:06Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"y@google.com writes:\n\n> From: Martin von Zweigbergk \n>\n> 'git cherry-pick' internally sets the --reverse option while walking\n> revisions, so that 'git cherry-pick branch@{u}..branch' will apply the\n> revisions starting at the oldest one. If no uninteresing revisions are\n> given, --no-walk is implied. Still, the documentation for 'git\n> cherry-pick --stdin' uses the following example:\n>\n> git rev-list --reverse master -- README | git cherry-pick -n --stdin\n>\n> The above would seem to reverse the revisions in the output (which it\n> does), and then pipe them to 'git cherry-pick', which would reverse\n> them again and apply them in the wrong order.\n\nI think we have cleared this confusion up in the previous\ndiscussion. It it sequencer's bug that reorders the commits when\nthe caller (\"rev-list --reverse\" in this case) gives list of\nindividual commits to replay.\n\nSo I think we are all OK with chucking this patch. Am I mistaken?\n"},{"id":"196932","messageId":"CAOeW2eENsnrPqBL795FcwMgHURS6YsPBW6FYvb=DwD-UtgPZ5g@mail.gmail.com","threadId":"31230","inReplyTo":"50289e50.a19f320a.5d99.3fdfSMTPIN_ADDED@gmr-mx.google.com","subject":"Re: [PATCH 2/4] revisions passed to cherry-pick should be in \"default\" order","fromName":"Martin von Zweigbergk","fromEmail":"martin.von.zweigbergk@gmail.com","sentAt":"2012-08-13T20:10:24Z","receivedAt":"2012-08-13T20:10:24Z","isPatch":true,"sender":{"key":"martinvonz@gmail.com","avatar":"https://avatars.githubusercontent.com/u/891642?v=4"},"body":"On Sun, Aug 12, 2012 at 11:27 PM, wrote:\n> From: Martin von Zweigbergk \n>\n> 'git cherry-pick' internally sets the --reverse option while walking\n> revisions, so that 'git cherry-pick branch@{u}..branch' will apply the\n> revisions starting at the oldest one.\n\nBy the way, I can see the usefulness of --reverse when giving a range,\nbut I think it's a little confusing when not giving a range. So \"git\ncherry-pick A B\" will apply B first, then A. I thought I'd mention\nthat explicitly in case it wasn't clear.\n"},{"id":"196933","messageId":"7vpq6uczis.fsf@alter.siamese.dyndns.org","threadId":"31230","inReplyTo":"50289e51.29d0320a.65ff.2c48SMTPIN_ADDED@gmr-mx.google.com","subject":"Re: [PATCH 4/4] cherry-pick/revert: default to topological sorting","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-08-13T20:23:07Z","receivedAt":"2012-08-13T20:23:07Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"y@google.com writes:\n\n> From: Martin von Zweigbergk \n>\n> When 'git cherry-pick' and 'git revert' are used with ranges such as\n> 'git cherry-pick A..B', the order of the commits to pick are\n> determined by the default date-based sorting. If a commit has a commit\n> date before the commit date of its parent, it will therfore be applied\n> before its parent.\n\nIs that what --topo-order really means?\n\nI just tried this:\n\n\t$ git checkout v1.7.12-rc2\n\t$ GIT_COMMITTER_DATE='@0 +0000' git commit --allow-empty -m old\n $ git log --pretty=fuller -2\n\nand (obviously) the result shows the \"old\" one and then the v1.7.12-rc2.\n\nThe point of --topo-order is to deal with merges more sensibly, I\nthink, e.g. with a history with this shape with timestamps,\n\n ---1----2----4----7\n \\ \\\n 3----5----6----8--- \n\n\"git log\" may show \"8 7 6 5 4 3 2 1\", while \"git log --topo-order\"\nwould give you \"8 6 5 3 7 4 2 1\".\n\nAnd indeed in the context of cherry-pick and revert, topo-order is a\nmore sensible option.\n\nSo there is nothing wrong in the patch, but the above explanation of\nyours is flawed.\n\n> In the context of cherry-pick/revert, this is most\n> likely not what the user expected, so let's enable topological sorting\n> by default.\n>\n> Signed-off-by: Martin von Zweigbergk \n> ---\n> builtin/revert.c | 1 +\n> 1 file changed, 1 insertion(+)\n>\n> diff --git a/builtin/revert.c b/builtin/revert.c\n> index 98ad641..6880ce5 100644\n> --- a/builtin/revert.c\n> +++ b/builtin/revert.c\n> @@ -194,6 +194,7 @@ static void parse_args(int argc, const char **argv, struct replay_opts *opts)\n> \t\topts->revs = xmalloc(sizeof(*opts->revs));\n> \t\tinit_revisions(opts->revs, NULL);\n> \t\topts->revs->no_walk = REVISION_WALK_NO_WALK_UNSORTED;\n> +\t\topts->revs->topo_order = 1;\n> \t\tif (argc < 2)\n> \t\t\tusage_with_options(usage_str, options);\n> \t\tmemset(&s_r_opt, 0, sizeof(s_r_opt));\n"},{"id":"196934","messageId":"CAOeW2eEbe9_m_QSbsJUbWPhf6G17X3vqbh__TCefrB0G2VKXdw@mail.gmail.com","threadId":"31230","inReplyTo":"7vtxw6d0ct.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 2/4] revisions passed to cherry-pick should be in \"default\" order","fromName":"Martin von Zweigbergk","fromEmail":"martin.von.zweigbergk@gmail.com","sentAt":"2012-08-13T20:50:35Z","receivedAt":"2012-08-13T20:50:35Z","isPatch":true,"sender":{"key":"martinvonz@gmail.com","avatar":"https://avatars.githubusercontent.com/u/891642?v=4"},"body":"On Mon, Aug 13, 2012 at 1:05 PM, Junio C Hamano wrote:\n> y@google.com writes:\n>\n>> From: Martin von Zweigbergk \n>>\n>> 'git cherry-pick' internally sets the --reverse option while walking\n>> revisions, so that 'git cherry-pick branch@{u}..branch' will apply the\n>> revisions starting at the oldest one. If no uninteresing revisions are\n>> given, --no-walk is implied. Still, the documentation for 'git\n>> cherry-pick --stdin' uses the following example:\n>>\n>> git rev-list --reverse master -- README | git cherry-pick -n --stdin\n>>\n>> The above would seem to reverse the revisions in the output (which it\n>> does), and then pipe them to 'git cherry-pick', which would reverse\n>> them again and apply them in the wrong order.\n>\n> I think we have cleared this confusion up in the previous\n> discussion. It it sequencer's bug that reorders the commits when\n> the caller (\"rev-list --reverse\" in this case) gives list of\n> individual commits to replay.\n>\n> So I think we are all OK with chucking this patch. Am I mistaken?\n\nI can't really say. I suppose the current patch is smaller (it can't\nreally get smaller than one line), but iterating over the arguments\nthe sequencer level might be more correct. Would the result be\ndifferent in some cases? I would be happy to add a test case at least,\nalthough I'm not sure when I would have time to implement it in\nsequencer.\n\nTo connect to the other mail I sent on this thread (in parallel with\nyours), do you think \"git cherrry-pick HEAD HEAD~1\" should apply the\ncommits in the same order as \"git cherry-pick HEAD~2..HEAD\" (which\nwould give the same result if passed to 'rev-list --no-walk' for a\nlinear history) or in the order specified on the command line? I\ncouldn't find any conclusive evidence of what was intended in either\nlog messages or test cases.\n"},{"id":"196935","messageId":"7vlihicy57.fsf@alter.siamese.dyndns.org","threadId":"31230","inReplyTo":"CAOeW2eENsnrPqBL795FcwMgHURS6YsPBW6FYvb=DwD-UtgPZ5g@mail.gmail.com","subject":"Re: [PATCH 2/4] revisions passed to cherry-pick should be in \"default\" order","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-08-13T20:52:52Z","receivedAt":"2012-08-13T20:52:52Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Martin von Zweigbergk writes:\n\n> By the way, I can see the usefulness of --reverse when giving a range,\n> but I think it's a little confusing when not giving a range.\n\n\"git rev-list --reverse --root v1.0.0\" is a way to say \"give me a\nlist of commits to be replayed in sequence\" without having a bottom,\nno?\n\nAh, you mean when we do _not_ walk.\n\nYeah, that is why I said that when we do not walk, we should not\neven call into prepare_revision_walk() in the first place in my\nearlier message. We should take the commits as given from the\nrevs->pending.objects list instead.\n\nWith your \"no_walk = NO_WALK_UNSORTED\", calling prepare_revision_walk()\nwould amont to the same thing, as you would not sort the commits and\nuse them as given by the user.\n\n> So \"git cherry-pick A B\" will apply B first, then A.\n\nI am confused a bit. Are you describing a buggy behaviour in the\ncurrent codebase, or are you saying we should fix it to behave that\nway?\n"},{"id":"196937","messageId":"7vehnacxkf.fsf@alter.siamese.dyndns.org","threadId":"31230","inReplyTo":"CAOeW2eEbe9_m_QSbsJUbWPhf6G17X3vqbh__TCefrB0G2VKXdw@mail.gmail.com","subject":"Re: [PATCH 2/4] revisions passed to cherry-pick should be in \"default\" order","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-08-13T21:05:20Z","receivedAt":"2012-08-13T21:05:20Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Martin von Zweigbergk writes:\n\n> To connect to the other mail I sent on this thread (in parallel with\n> yours), do you think \"git cherrry-pick HEAD HEAD~1\" should apply the\n> commits in the same order as \"git cherry-pick HEAD~2..HEAD\" (which\n> would give the same result if passed to 'rev-list --no-walk' for a\n> linear history) or in the order specified on the command line?\n\nDefinitely the latter; I do not think of any semi-reasonable excuse\nto do otherwise.\n\n> I couldn't find any conclusive evidence of what was intended in\n> either log messages or test cases.\n\nDo not take the \"multi-commit handling\" that was bolted on to\ncherry-pick and revert long after these commands with a single\ncommit form were polished and have become stable too seriously and\nits behaviour cast in stone. There is no reason to believe the\nbolted-on part was designed with sufficient thoughts behind it, nor\nwas implemented with the same competency as the code before it was\nintroduced. I recall myself applying these patches after only\ncursory review, saying \"Meh, I wouldn't do multiple commits anyway,\nand bugs found by people can be fixed later\" ;-).\n\nIt is OK to consider its doneness as \"the developers declared\nsuccess based on their limited testing; it internally still sorts,\nbut sorting a range by timestamp happens to yield the correct result\nmost of the time, and this bug was not found until much later. There\ncertainly are other bugs, at both implementation and design level,\nyet to be discovered.\" phase of its lifecycle.\n"},{"id":"196942","messageId":"7v1ujacwcd.fsf@alter.siamese.dyndns.org","threadId":"31230","inReplyTo":"CAOeW2eF67Tj0Mq+g+-3UFyh_Xvt=ZcKDc9LjCKGwu9y2G39NBQ@mail.gmail.com","subject":"Re: [PATCH 0/4] Re: cherry-pick and 'log --no-walk' and ordering","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-08-13T21:31:46Z","receivedAt":"2012-08-13T21:31:46Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Martin von Zweigbergk writes:\n\n> Makes sense. The shortlog example is a good example of sorting that\n> completely reorders the commit graph sometimes even making sense for\n> ranges. Thanks!\n\nBy the way, does this topic relate to the long stalled \"rebase\"\ntopic from you, and if so how?\n"},{"id":"196943","messageId":"7vwr12bgwp.fsf@alter.siamese.dyndns.org","threadId":"31230","inReplyTo":"7vpq6uczis.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 4/4] cherry-pick/revert: default to topological sorting","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-08-13T21:50:30Z","receivedAt":"2012-08-13T21:50:30Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> y@google.com writes:\n>\n>> From: Martin von Zweigbergk \n>>\n>> When 'git cherry-pick' and 'git revert' are used with ranges such as\n>> 'git cherry-pick A..B', the order of the commits to pick are\n>> determined by the default date-based sorting. If a commit has a commit\n>> date before the commit date of its parent, it will therfore be applied\n>> before its parent.\n>\n> Is that what --topo-order really means?\n\nAnd it turns out that the documentation is crappy. Perhaps\nsomething like this, but an illustration may not hurt.\n\n Documentation/rev-list-options.txt | 7 ++++---\n 1 file changed, 4 insertions(+), 3 deletions(-)\n\ndiff --git a/Documentation/rev-list-options.txt b/Documentation/rev-list-options.txt\nindex d9b2b5b..c147117 100644\n--- a/Documentation/rev-list-options.txt\n+++ b/Documentation/rev-list-options.txt\n@@ -579,9 +579,10 @@ Commit Ordering\n By default, the commits are shown in reverse chronological order.\n \n --topo-order::\n-\n-\tThis option makes them appear in topological order (i.e.\n-\tdescendant commits are shown before their parents).\n+\tThis option makes them appear in topological order. Even\n+\twithout this option, descendant commits are shown before\n+\ttheir parents, but this tries to avoid showing commits on\n+\tmultiple lines of history intermixed.\n \n --date-order::\n \n"},{"id":"196944","messageId":"CAOeW2eEbN-n72cq4Ywt=o6uVFkBWB5L6jAn9Qx_FBwyZLJdjMQ@mail.gmail.com","threadId":"31230","inReplyTo":"7v1ujacwcd.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 0/4] Re: cherry-pick and 'log --no-walk' and ordering","fromName":"Martin von Zweigbergk","fromEmail":"martin.von.zweigbergk@gmail.com","sentAt":"2012-08-13T22:01:39Z","receivedAt":"2012-08-13T22:01:39Z","isPatch":true,"sender":{"key":"martinvonz@gmail.com","avatar":"https://avatars.githubusercontent.com/u/891642?v=4"},"body":"On Mon, Aug 13, 2012 at 2:31 PM, Junio C Hamano wrote:\n> Martin von Zweigbergk writes:\n>\n>> Makes sense. The shortlog example is a good example of sorting that\n>> completely reorders the commit graph sometimes even making sense for\n>> ranges. Thanks!\n>\n> By the way, does this topic relate to the long stalled \"rebase\"\n> topic from you, and if so how?\n\nYes, but only through the first patch in the series. Unless I'm\nmistaken, I would can get a list of revisions to rebase using\ngit-patch-id, but to convert that into a instruction list with running\ngit-log on each commit, I planned to use 'git rev-list --format=...\n--no-walk=unsorted --stdin', which of course doesn't exist before\npatch 1/4.\n\nThe rest of the current series is a little fuzzy to me, especially the\nconfusion about reversing or not. Feel free to split out patch 1 into\na separate topic if you like, or however you would handle that.\n"},{"id":"197032","messageId":"CAOeW2eH--Y_gq4jBBhd5EQRw+uuaNWrMT-Sua7CeJO-N9KHCLg@mail.gmail.com","threadId":"31230","inReplyTo":"7vehnacxkf.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 2/4] revisions passed to cherry-pick should be in \"default\" order","fromName":"Martin von Zweigbergk","fromEmail":"martin.von.zweigbergk@gmail.com","sentAt":"2012-08-15T06:05:21Z","receivedAt":"2012-08-15T06:05:21Z","isPatch":true,"sender":{"key":"martinvonz@gmail.com","avatar":"https://avatars.githubusercontent.com/u/891642?v=4"},"body":"On Mon, Aug 13, 2012 at 2:05 PM, Junio C Hamano wrote:\n> Martin von Zweigbergk writes:\n>\n>> To connect to the other mail I sent on this thread (in parallel with\n>> yours), do you think \"git cherrry-pick HEAD HEAD~1\" should apply the\n>> commits in the same order as \"git cherry-pick HEAD~2..HEAD\" (which\n>> would give the same result if passed to 'rev-list --no-walk' for a\n>> linear history) or in the order specified on the command line?\n>\n> Definitely the latter; I do not think of any semi-reasonable excuse\n> to do otherwise.\n\nIndeed. My patches tried to fix the wrong problem.\n\nSorry I'm slow, but I think I'm finally starting to understand\nwhat you've been saying all along about the bug being in\nsequencer. I'll try to recapitulate a bit for my own and maybe\nothers' understanding. For simplicity, let's assume a linear\nhistory with unique timestamps, but not necessarily increasing\nwith each commit.\n\nCurrently:\n\n 1) 'git cherry-pick A..C' picks the commits order in\n reverse \"default\" order\n\n 2) 'git cherry-pick B C' picks the commits in chronological\n order\n\n 3) 'git rev-list --reverse A..C | git cherry-pick --stdin'\n behaves just like 'git cherry-pick B C' and therefore picks\n the commits in chronological order\n\nIn cases 2) and 3), even though cherry-pick tells the revision\nwalker not to walk, it still sorts the commits in reverse\nchronological order. But cherry-pick also tells the revision\nwalker explicitly to reverse the list, so in the end, the order\nis chronological.\n\nIn case 2), however, the first ordering make no difference in\nthis \"limited\" case (IIUC). So the \"default\" ordering (which\nwould be C, then B in this case, regardless of timestamps), gets\nreversed and B gets applied first, followed by C.\n\nSo all of the above case give the right result in the end as long\nas the timestamps are chronological, and case 1) gives the right\nresult regardless. The other two cases only works in most cases\nbecause the unexpcted sorting when no-walk is in effect\ncounteracts the final reversal.\n\nWhen I noticed that the order of inputs to cases 2) and 3) above\nwas ignored, and thinking that 'git rev-list A..C | git\ncherry-pick --stdin' should mimic 'git cherry-pick A..C', I\nincorrectly thought that the error was the use of --reverse to\n'git rev-list' as well as the sorting done in the no-walk case. I\nthink completely ignored case 2) at this point.\n\nI now think I understand that the sorting done in the no-walk\ncase is indeed incorrect, but that the --reverse passed to\nrev-list is correct. Instead, the final reversal, which is\ncurrently unconditional, should not be done in the no-walk case.\n\nIIUC, this could be implemented by making cherry-pick iterate\nover rev_info.pending.objects just like 'git show' does when not\nwalking.\n\nJunio, I think it makes sense to just drop this whole series for\nnow. I'll probably include patch 1/4 in my stalled rebase-range\nseries instead. If I understood you correctly, you didn't have\nany objections to that patch.\n"},{"id":"197044","messageId":"7vk3x06ppi.fsf@alter.siamese.dyndns.org","threadId":"31230","inReplyTo":"CAOeW2eH--Y_gq4jBBhd5EQRw+uuaNWrMT-Sua7CeJO-N9KHCLg@mail.gmail.com","subject":"Re: [PATCH 2/4] revisions passed to cherry-pick should be in \"default\" order","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-08-15T17:16:09Z","receivedAt":"2012-08-15T17:16:09Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Martin von Zweigbergk writes:\n\n> So all of the above case give the right result in the end as long\n> as the timestamps are chronological, and case 1) gives the right\n> result regardless. The other two cases only works in most cases\n> because the unexpcted sorting when no-walk is in effect\n> counteracts the final reversal.\n\nIn short, if you have three commits in a row, A--B--C, with\ntimestamps that are not skewed, and want to replay changes of B and\nthen C in that order, all three you listed ends up doing the right\nthing. But if you want to apply the change C and then B:\n\n - \"git cherry-pick A..C\" is obviously not a way to do so, so we\n won't discuss it further.\n\n - \"git cherry-pick C B\" is the most natural way the user would\n want to express this request, but because of the sorting\n (i.e. commit_list_sort_by_date() in prepare_revision_walk(),\n combined with ->reverse in sequencer.c::prepare_revs()), it\n applies B and then C. That is the real bug.\n\n Feeding the revs to \"git cherry-pick --stdin\" in the order the\n user wishes them to be applied has the same issue.\n\n> IIUC, this could be implemented by making cherry-pick iterate\n> over rev_info.pending.objects just like 'git show' does when not\n> walking.\n\nYes, that was exactly why I said sequencer.c::prepare_revs() is\nwrong to call prepare_revision_walk() unconditionally, even when\nthere is no revision walking involved.\n\nI actually think your approach to place the \"do not sort when we are\nnot walking\" logic in prepare_revision_walk() makes more sense.\n\"show\" has to look at pending.objects[] because it needs to show\nobjects other than commits (e.g. \"git show :foo\"), so there won't be\nany change in its implementation with your change. It will have to\nlook at pending.objects[] itself.\n\nBut \"cherry-pick\" and sequencer-derived commands only deal with\ncommits. It would be far less error prone to let them call\nget_revision() repeatedly like all other revision enumerating\ncommands do, than to have them go over the pending.objects[] list,\ndereferencing tags and using only commits. The resulting callers\nwould be more readable, too, I would think.\n"},{"id":"197050","messageId":"CAOeW2eFK+cKt9Tnh5oe74dU+f8rOOTaWk3KvE2rtUpgcOeDD7g@mail.gmail.com","threadId":"31230","inReplyTo":"7vk3x06ppi.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 2/4] revisions passed to cherry-pick should be in \"default\" order","fromName":"Martin von Zweigbergk","fromEmail":"martin.von.zweigbergk@gmail.com","sentAt":"2012-08-15T18:22:18Z","receivedAt":"2012-08-15T18:22:18Z","isPatch":true,"sender":{"key":"martinvonz@gmail.com","avatar":"https://avatars.githubusercontent.com/u/891642?v=4"},"body":"On Wed, Aug 15, 2012 at 10:16 AM, Junio C Hamano wrote:\n> Martin von Zweigbergk writes:\n>\n>> So all of the above case give the right result in the end as long\n>> as the timestamps are chronological, and case 1) gives the right\n>> result regardless. The other two cases only works in most cases\n>> because the unexpcted sorting when no-walk is in effect\n>> counteracts the final reversal.\n>\n> In short, if you have three commits in a row, A--B--C, with\n> timestamps that are not skewed, and want to replay changes of B and\n> then C in that order, all three you listed ends up doing the right\n> thing. But if you want to apply the change C and then B:\n>\n> - \"git cherry-pick A..C\" is obviously not a way to do so, so we\n> won't discuss it further.\n>\n> - \"git cherry-pick C B\" is the most natural way the user would\n> want to express this request, but because of the sorting\n> (i.e. commit_list_sort_by_date() in prepare_revision_walk(),\n> combined with ->reverse in sequencer.c::prepare_revs()), it\n> applies B and then C. That is the real bug.\n>\n> Feeding the revs to \"git cherry-pick --stdin\" in the order the\n> user wishes them to be applied has the same issue.\n\nExactly.\n\n> I actually think your approach to place the \"do not sort when we are\n> not walking\" logic in prepare_revision_walk() makes more sense.\n> \"show\" has to look at pending.objects[] because it needs to show\n> objects other than commits (e.g. \"git show :foo\"), so there won't be\n> any change in its implementation with your change. It will have to\n> look at pending.objects[] itself.\n\nYes, I noticed that's why \"show\" has to do it that way.\n\n> But \"cherry-pick\" and sequencer-derived commands only deal with\n> commits. It would be far less error prone to let them call\n> get_revision() repeatedly like all other revision enumerating\n> commands do, than to have them go over the pending.objects[] list,\n> dereferencing tags and using only commits. The resulting callers\n> would be more readable, too, I would think.\n\nMakes sense, I'll try to implement it that way. I was afraid that\nwe would need to call prepare_revision_walk() once first and then\nif we afterwards find out that we should not walk, we would need\nto call it again without the reverse option. But after looking at\nhow rev_info.reverse is used, it seem like it's only used in\nget_revision(), so we can leave it either on or off during the\nprepare_revision_walk() and the and set appropriately before\ncalling get_revision(), like so:\n\n init_revisions(&revs);\n revs.no_walk = REVISION_WALK_NO_WALK_UNSORTED;\n setup_revisions(...);\n prepare_revision_walk(&revs);\n revs.reverse = !revs.no_walk;\n // iterate over revisions\n"},{"id":"197053","messageId":"7vr4r857au.fsf@alter.siamese.dyndns.org","threadId":"31230","inReplyTo":"CAOeW2eFK+cKt9Tnh5oe74dU+f8rOOTaWk3KvE2rtUpgcOeDD7g@mail.gmail.com","subject":"Re: [PATCH 2/4] revisions passed to cherry-pick should be in \"default\" order","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-08-15T18:39:05Z","receivedAt":"2012-08-15T18:39:05Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Martin von Zweigbergk writes:\n\n> Makes sense, I'll try to implement it that way. I was afraid that\n> we would need to call prepare_revision_walk() once first and then\n> if we afterwards find out that we should not walk, we would need\n> to call it again without the reverse option.\n\n> But after looking at\n> how rev_info.reverse is used, it seem like it's only used in\n> get_revision(), so we can leave it either on or off during the\n> prepare_revision_walk() and the and set appropriately before\n> calling get_revision(), like so:\n>\n> init_revisions(&revs);\n> revs.no_walk = REVISION_WALK_NO_WALK_UNSORTED;\n> setup_revisions(...);\n> prepare_revision_walk(&revs);\n> revs.reverse = !revs.no_walk;\n\nSorry, but I do not understand why you frutz with \"reverse\" after\nprepare, and not before.\n\nI think you can just set no_walk and let setup_revisions() turn it\noff upon seeing a range (this happens in add_pending_object()).\nAfter setup_revisions() returns, if no_walk is still set, you only\ngot individual refs without ranges, so no reversing required.\n\nYou also need to be careful about \"revert\" that shares the code;\nwhen reverting range A..C in your example, you want to undo C and\nthen B, and you do not want to reverse them.\n"},{"id":"197058","messageId":"CAOeW2eGcVQ74WLOOHWvKao9WXfWnJpOhQwE8Jxip_E4SzkFjyA@mail.gmail.com","threadId":"31230","inReplyTo":"7vr4r857au.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 2/4] revisions passed to cherry-pick should be in \"default\" order","fromName":"Martin von Zweigbergk","fromEmail":"martin.von.zweigbergk@gmail.com","sentAt":"2012-08-15T20:50:01Z","receivedAt":"2012-08-15T20:50:01Z","isPatch":true,"sender":{"key":"martinvonz@gmail.com","avatar":"https://avatars.githubusercontent.com/u/891642?v=4"},"body":"On Wed, Aug 15, 2012 at 11:39 AM, Junio C Hamano wrote:\n> Martin von Zweigbergk writes:\n>\n>> Makes sense, I'll try to implement it that way. I was afraid that\n>> we would need to call prepare_revision_walk() once first and then\n>> if we afterwards find out that we should not walk, we would need\n>> to call it again without the reverse option.\n>\n>> But after looking at\n>> how rev_info.reverse is used, it seem like it's only used in\n>> get_revision(), so we can leave it either on or off during the\n>> prepare_revision_walk() and the and set appropriately before\n>> calling get_revision(), like so:\n>>\n>> init_revisions(&revs);\n>> revs.no_walk = REVISION_WALK_NO_WALK_UNSORTED;\n>> setup_revisions(...);\n>> prepare_revision_walk(&revs);\n>> revs.reverse = !revs.no_walk;\n>\n> Sorry, but I do not understand why you frutz with \"reverse\" after\n> prepare, and not before.\n>\n> I think you can just set no_walk and let setup_revisions() turn it\n> off upon seeing a range (this happens in add_pending_object()).\n\nAh, of course. For some reason I thought that was called from\nprepare_revision_walk()\n\n> After setup_revisions() returns, if no_walk is still set, you only\n> got individual refs without ranges, so no reversing required.\n\nYes, it's in the other case (e.g. 'git cherry-pick A..C', when\nno_walk is not set), that we need to set reverse before walking.\n\n> You also need to be careful about \"revert\" that shares the code;\n> when reverting range A..C in your example, you want to undo C and\n> then B, and you do not want to reverse them.\n\nYep. It looks like this, so should be safe. But thanks for the reminder.\n\n if (opts->action != REPLAY_REVERT)\n opts->revs->reverse ^= 1;\n"},{"id":"198026","messageId":"1346220956-25034-1-git-send-email-martinvonz@gmail.com","threadId":"31230","inReplyTo":"50289e50.8458320a.7d31.3c46SMTPIN_ADDED@gmr-mx.google.com","subject":"[PATCH v2 0/3] revision (no-)walking in order","fromName":"Martin von Zweigbergk","fromEmail":"martinvonz@gmail.com","sentAt":"2012-08-29T06:15:53Z","receivedAt":"2012-08-29T06:15:53Z","isPatch":true,"sender":{"key":"martinvonz@gmail.com","avatar":"https://avatars.githubusercontent.com/u/891642?v=4"},"body":"I'm still working on a re-roll of my rebase-range series, but I think\nthese three are quite unrelated and shouldn't be held up by that other\nseries.\n\nJunio, thanks for all the help with explaining revision walking. It\nwas a little blurry for a long time, but at least I feel more\ncomfortable with these few patches now.\n\nBtw, the rebase-range series seems to need (or be greatly simplified),\nalthough I'm not 100% sure yet, by teaching patch-id --keep-empty,\nwhich would be its first command line option. Let me know if you\n(plural) sees a problem with that.\n\nBtw2, I'm migrating my email to martinvonz@gmail.com (not y@google.com\n;-) which saves a few keystrokes and matches some of my other\naccounts, so these patches will be the first ones from the new\naddress.\n\nMartin von Zweigbergk (3):\n teach log --no-walk=unsorted, which avoids sorting\n demonstrate broken 'git cherry-pick three one two'\n cherry-pick/revert: respect order of revisions to pick\n\n Documentation/rev-list-options.txt | 12 ++++++++----\n builtin/log.c | 2 +-\n builtin/revert.c | 2 +-\n revision.c | 18 +++++++++++++++---\n revision.h | 6 +++++-\n sequencer.c | 4 +++-\n t/t3508-cherry-pick-many-commits.sh | 15 +++++++++++++++\n t/t4202-log.sh | 10 ++++++++++\n 8 files changed, 58 insertions(+), 11 deletions(-)\n\n-- \n1.7.11.1.104.ge7b44f1\n"},{"id":"198029","messageId":"1346220956-25034-2-git-send-email-martinvonz@gmail.com","threadId":"31230","inReplyTo":"1346220956-25034-1-git-send-email-martinvonz@gmail.com","subject":"[PATCH v2 1/3] teach log --no-walk=unsorted, which avoids sorting","fromName":"Martin von Zweigbergk","fromEmail":"martinvonz@gmail.com","sentAt":"2012-08-29T06:15:54Z","receivedAt":"2012-08-29T06:15:54Z","isPatch":true,"sender":{"key":"martinvonz@gmail.com","avatar":"https://avatars.githubusercontent.com/u/891642?v=4"},"body":"When 'git log' is passed the --no-walk option, no revision walk takes\nplace, naturally. Perhaps somewhat surprisingly, however, the provided\nrevisions still get sorted by commit date. So e.g 'git log --no-walk\nHEAD HEAD~1' and 'git log --no-walk HEAD~1 HEAD' give the same result\n(unless the two revisions share the commit date, in which case they\nwill retain the order given on the command line). As the commit that\nintroduced --no-walk (8e64006 (Teach revision machinery about\n--no-walk, 2007-07-24)) points out, the sorting is intentional, to\nallow things like\n\n git log --abbrev-commit --pretty=oneline --decorate --all --no-walk\n\nto show all refs in order by commit date.\n\nBut there are also other cases where the sorting is not wanted, such\nas\n\n |\n git log --oneline --no-walk --stdin\n\nTo accomodate both cases, leave the decision of whether or not to sort\nup to the caller, by allowing --no-walk={sorted,unsorted}, defaulting\nto 'sorted' for backward-compatibility reasons.\n\nSigned-off-by: Martin von Zweigbergk \n---\n Documentation/rev-list-options.txt | 12 ++++++++----\n builtin/log.c | 2 +-\n builtin/revert.c | 2 +-\n revision.c | 18 +++++++++++++++---\n revision.h | 6 +++++-\n t/t4202-log.sh | 10 ++++++++++\n 6 files changed, 40 insertions(+), 10 deletions(-)\n\ndiff --git a/Documentation/rev-list-options.txt b/Documentation/rev-list-options.txt\nindex def1340..5436eba 100644\n--- a/Documentation/rev-list-options.txt\n+++ b/Documentation/rev-list-options.txt\n@@ -636,10 +636,14 @@ These options are mostly targeted for packing of git repositories.\n \tOnly useful with '--objects'; print the object IDs that are not\n \tin packs.\n \n---no-walk::\n-\n-\tOnly show the given revs, but do not traverse their ancestors.\n-\tThis has no effect if a range is specified.\n+--no-walk[=(sorted|unsorted)]::\n+\n+\tOnly show the given commits, but do not traverse their ancestors.\n+\tThis has no effect if a range is specified. If the argument\n+\t\"unsorted\" is given, the commits are show in the order they were\n+\tgiven on the command line. Otherwise (if \"sorted\" or no argument\n+\twas given), the commits are show in reverse chronological order\n+\tby commit time.\n \n --do-walk::\n \ndiff --git a/builtin/log.c b/builtin/log.c\nindex ecc2793..20838b1 100644\n--- a/builtin/log.c\n+++ b/builtin/log.c\n@@ -456,7 +456,7 @@ int cmd_show(int argc, const char **argv, const char *prefix)\n \tinit_revisions(&rev, prefix);\n \trev.diff = 1;\n \trev.always_show_header = 1;\n-\trev.no_walk = 1;\n+\trev.no_walk = REVISION_WALK_NO_WALK_SORTED;\n \trev.diffopt.stat_width = -1; \t/* Scale to real terminal size */\n \n \tmemset(&opt, 0, sizeof(opt));\ndiff --git a/builtin/revert.c b/builtin/revert.c\nindex 82d1bf8..42ce399 100644\n--- a/builtin/revert.c\n+++ b/builtin/revert.c\n@@ -193,7 +193,7 @@ static void parse_args(int argc, const char **argv, struct replay_opts *opts)\n \t\tstruct setup_revision_opt s_r_opt;\n \t\topts->revs = xmalloc(sizeof(*opts->revs));\n \t\tinit_revisions(opts->revs, NULL);\n-\t\topts->revs->no_walk = 1;\n+\t\topts->revs->no_walk = REVISION_WALK_NO_WALK_SORTED;\n \t\tif (argc < 2)\n \t\t\tusage_with_options(usage_str, options);\n \t\tmemset(&s_r_opt, 0, sizeof(s_r_opt));\ndiff --git a/revision.c b/revision.c\nindex 442a945..66ba2e6 100644\n--- a/revision.c\n+++ b/revision.c\n@@ -1300,7 +1300,7 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg\n \t !strcmp(arg, \"--no-walk\") || !strcmp(arg, \"--do-walk\") ||\n \t !strcmp(arg, \"--bisect\") || !prefixcmp(arg, \"--glob=\") ||\n \t !prefixcmp(arg, \"--branches=\") || !prefixcmp(arg, \"--tags=\") ||\n-\t !prefixcmp(arg, \"--remotes=\"))\n+\t !prefixcmp(arg, \"--remotes=\") || !prefixcmp(arg, \"--no-walk=\"))\n \t{\n \t\tunkv[(*unkc)++] = arg;\n \t\treturn 1;\n@@ -1695,7 +1695,18 @@ static int handle_revision_pseudo_opt(const char *submodule,\n \t} else if (!strcmp(arg, \"--not\")) {\n \t\t*flags ^= UNINTERESTING;\n \t} else if (!strcmp(arg, \"--no-walk\")) {\n-\t\trevs->no_walk = 1;\n+\t\trevs->no_walk = REVISION_WALK_NO_WALK_SORTED;\n+\t} else if (!prefixcmp(arg, \"--no-walk=\")) {\n+\t\t/*\n+\t\t * Detached form (\"--no-walk X\" as opposed to \"--no-walk=X\")\n+\t\t * not allowed, since the argument is optional.\n+\t\t */\n+\t\tif (!strcmp(arg + 10, \"sorted\"))\n+\t\t\trevs->no_walk = REVISION_WALK_NO_WALK_SORTED;\n+\t\telse if (!strcmp(arg + 10, \"unsorted\"))\n+\t\t\trevs->no_walk = REVISION_WALK_NO_WALK_UNSORTED;\n+\t\telse\n+\t\t\treturn error(\"invalid argument to --no-walk\");\n \t} else if (!strcmp(arg, \"--do-walk\")) {\n \t\trevs->no_walk = 0;\n \t} else {\n@@ -2117,10 +2128,11 @@ int prepare_revision_walk(struct rev_info *revs)\n \t\t}\n \t\te++;\n \t}\n-\tcommit_list_sort_by_date(&revs->commits);\n \tif (!revs->leak_pending)\n \t\tfree(list);\n \n+\tif (revs->no_walk != REVISION_WALK_NO_WALK_UNSORTED)\n+\t\tcommit_list_sort_by_date(&revs->commits);\n \tif (revs->no_walk)\n \t\treturn 0;\n \tif (revs->limited)\ndiff --git a/revision.h b/revision.h\nindex cb5ab35..a95bd0b 100644\n--- a/revision.h\n+++ b/revision.h\n@@ -41,6 +41,10 @@ struct rev_cmdline_info {\n \t} *rev;\n };\n \n+#define REVISION_WALK_WALK 0\n+#define REVISION_WALK_NO_WALK_SORTED 1\n+#define REVISION_WALK_NO_WALK_UNSORTED 2\n+\n struct rev_info {\n \t/* Starting list */\n \tstruct commit_list *commits;\n@@ -62,7 +66,7 @@ struct rev_info {\n \t/* Traversal flags */\n \tunsigned int\tdense:1,\n \t\t\tprune:1,\n-\t\t\tno_walk:1,\n+\t\t\tno_walk:2,\n \t\t\tshow_all:1,\n \t\t\tremove_empty_trees:1,\n \t\t\tsimplify_history:1,\ndiff --git a/t/t4202-log.sh b/t/t4202-log.sh\nindex 71be59d..bd83355 100755\n--- a/t/t4202-log.sh\n+++ b/t/t4202-log.sh\n@@ -178,11 +178,21 @@ test_expect_success 'git log --no-walk sorts by commit time' '\n \ttest_cmp expect actual\n '\n \n+test_expect_success 'git log --no-walk=sorted sorts by commit time' '\n+\tgit log --no-walk=sorted --oneline 5d31159 804a787 394ef78 > actual &&\n+\ttest_cmp expect actual\n+'\n+\n cat > expect << EOF\n 5d31159 fourth\n 804a787 sixth\n 394ef78 fifth\n EOF\n+test_expect_success 'git log --no-walk=unsorted leaves list of commits as given' '\n+\tgit log --no-walk=unsorted --oneline 5d31159 804a787 394ef78 > actual &&\n+\ttest_cmp expect actual\n+'\n+\n test_expect_success 'git show leaves list of commits as given' '\n \tgit show --oneline -s 5d31159 804a787 394ef78 > actual &&\n \ttest_cmp expect actual\n-- \n1.7.11.1.104.ge7b44f1\n"},{"id":"198027","messageId":"1346220956-25034-3-git-send-email-martinvonz@gmail.com","threadId":"31230","inReplyTo":"1346220956-25034-1-git-send-email-martinvonz@gmail.com","subject":"[PATCH v2 2/3] demonstrate broken 'git cherry-pick three one two'","fromName":"Martin von Zweigbergk","fromEmail":"martinvonz@gmail.com","sentAt":"2012-08-29T06:15:55Z","receivedAt":"2012-08-29T06:15:55Z","isPatch":true,"sender":{"key":"martinvonz@gmail.com","avatar":"https://avatars.githubusercontent.com/u/891642?v=4"},"body":"Cherry-picking commits out of order (w.r.t. commit time stamp) doesn't\ncurrently work. Add a test case to demonstrate it.\n\nSigned-off-by: Martin von Zweigbergk \n---\n t/t3508-cherry-pick-many-commits.sh | 15 +++++++++++++++\n 1 file changed, 15 insertions(+)\n\ndiff --git a/t/t3508-cherry-pick-many-commits.sh b/t/t3508-cherry-pick-many-commits.sh\nindex 75f7ff4..fff20c3 100755\n--- a/t/t3508-cherry-pick-many-commits.sh\n+++ b/t/t3508-cherry-pick-many-commits.sh\n@@ -44,6 +44,21 @@ test_expect_success 'cherry-pick first..fourth works' '\n \tcheck_head_differs_from fourth\n '\n \n+test_expect_failure 'cherry-pick three one two works' '\n+\tgit checkout -f first &&\n+\ttest_commit one &&\n+\ttest_commit two &&\n+\ttest_commit three &&\n+\tgit checkout -f master &&\n+\tgit reset --hard first &&\n+\tgit cherry-pick three one two &&\n+\tgit diff --quiet three &&\n+\tgit diff --quiet HEAD three &&\n+\ttest \"$(git log --reverse --format=%s first..)\" == \"three\n+one\n+two\"\n+'\n+\n test_expect_success 'output to keep user entertained during multi-pick' '\n \tcat <<-\\EOF >expected &&\n \t[master OBJID] second\n-- \n1.7.11.1.104.ge7b44f1\n"},{"id":"198028","messageId":"1346220956-25034-4-git-send-email-martinvonz@gmail.com","threadId":"31230","inReplyTo":"1346220956-25034-1-git-send-email-martinvonz@gmail.com","subject":"[PATCH v2 3/3] cherry-pick/revert: respect order of revisions to pick","fromName":"Martin von Zweigbergk","fromEmail":"martinvonz@gmail.com","sentAt":"2012-08-29T06:15:56Z","receivedAt":"2012-08-29T06:15:56Z","isPatch":true,"sender":{"key":"martinvonz@gmail.com","avatar":"https://avatars.githubusercontent.com/u/891642?v=4"},"body":"When giving multiple individual revisions to cherry-pick or revert, as\nin 'git cherry-pick A B' or 'git revert B A', one would expect them to\nbe picked/reverted in the order given on the command line. They are\ninstead ordered by their commit timestamp -- in chronological order\nfor \"cherry-pick\" and in reverse chronological order for\n\"revert\". This matches the order in which one would usually give them\non the command line, making this bug somewhat hard to notice. Still,\nit has been reported at least once before [1].\n\nIt seems like the chronological sorting happened by accident because\nthe revision walker has traditionally always sorted commits in reverse\nchronological order when rev_info.no_walk was enabled. In the case of\n'git revert B A' where B is newer than A, this sorting is a no-op. For\n'git cherry-pick A B', the sorting would reverse the arguments, but\nbecause the sequencer also flips the rev_info.reverse flag when\npicking (as opposed to reverting), the end result is a chronological\norder. The rev_info.reverse flag was probably flipped so that the\nrevision walker emits B before C in 'git cherry-pick A..C'; that it\nhappened to effectively undo the unexpected sorting done when not\nwalking, was probably a coincidence that allowed this bug to happen at\nall.\n\nFix the bug by telling the revision walker not to sort the commits\nwhen not walking. The only case we want to reverse the order is now\nwhen cherry-picking and walking revisions (rev_info.no_walk = 0).\n\n [1] http://thread.gmane.org/gmane.comp.version-control.git/164794\n\nSigned-off-by: Martin von Zweigbergk \n---\n builtin/revert.c | 2 +-\n sequencer.c | 4 +++-\n t/t3508-cherry-pick-many-commits.sh | 2 +-\n 3 files changed, 5 insertions(+), 3 deletions(-)\n\ndiff --git a/builtin/revert.c b/builtin/revert.c\nindex 42ce399..98ad641 100644\n--- a/builtin/revert.c\n+++ b/builtin/revert.c\n@@ -193,7 +193,7 @@ static void parse_args(int argc, const char **argv, struct replay_opts *opts)\n \t\tstruct setup_revision_opt s_r_opt;\n \t\topts->revs = xmalloc(sizeof(*opts->revs));\n \t\tinit_revisions(opts->revs, NULL);\n-\t\topts->revs->no_walk = REVISION_WALK_NO_WALK_SORTED;\n+\t\topts->revs->no_walk = REVISION_WALK_NO_WALK_UNSORTED;\n \t\tif (argc < 2)\n \t\t\tusage_with_options(usage_str, options);\n \t\tmemset(&s_r_opt, 0, sizeof(s_r_opt));\ndiff --git a/sequencer.c b/sequencer.c\nindex bf078f2..9f32104 100644\n--- a/sequencer.c\n+++ b/sequencer.c\n@@ -543,7 +543,9 @@ static int do_pick_commit(struct commit *commit, struct replay_opts *opts)\n \n static void prepare_revs(struct replay_opts *opts)\n {\n-\tif (opts->action != REPLAY_REVERT)\n+\t// picking (but not reverting) ranges (but not individual revisions)\n+\t// should be done in reverse\n+\tif (opts->action == REPLAY_PICK && !opts->revs->no_walk)\n \t\topts->revs->reverse ^= 1;\n \n \tif (prepare_revision_walk(opts->revs))\ndiff --git a/t/t3508-cherry-pick-many-commits.sh b/t/t3508-cherry-pick-many-commits.sh\nindex fff20c3..04b5ad4 100755\n--- a/t/t3508-cherry-pick-many-commits.sh\n+++ b/t/t3508-cherry-pick-many-commits.sh\n@@ -44,7 +44,7 @@ test_expect_success 'cherry-pick first..fourth works' '\n \tcheck_head_differs_from fourth\n '\n \n-test_expect_failure 'cherry-pick three one two works' '\n+test_expect_success 'cherry-pick three one two works' '\n \tgit checkout -f first &&\n \ttest_commit one &&\n \ttest_commit two &&\n-- \n1.7.11.1.104.ge7b44f1\n"},{"id":"198032","messageId":"7vd32axkiv.fsf@alter.siamese.dyndns.org","threadId":"31230","inReplyTo":"1346220956-25034-1-git-send-email-martinvonz@gmail.com","subject":"Re: [PATCH v2 0/3] revision (no-)walking in order","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-08-29T06:46:48Z","receivedAt":"2012-08-29T06:46:48Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Martin von Zweigbergk writes:\n\n> Btw2, I'm migrating my email to martinvonz@gmail.com (not y@google.com\n> ;-) which saves a few keystrokes and matches some of my other\n> accounts, so these patches will be the first ones from the new\n> address.\n\nPlease send in something like this, then.\n\n .mailmap | 1 +\n 1 file changed, 1 insertion(+)\n\ndiff --git i/.mailmap w/.mailmap\nindex 6303782..2650f9e 100644\n--- i/.mailmap\n+++ w/.mailmap\n@@ -43,6 +43,7 @@ Lars Doelle \n Li Hong \n Lukas Sandström \n Martin Langhoff \n+Martin von Zweigbergk \n Michael Coleman \n Michael J Gruber \n Michael W. Olson \n"},{"id":"198045","messageId":"1346257259-14691-1-git-send-email-martinvonz@gmail.com","threadId":"31230","inReplyTo":"7vd32axkiv.fsf@alter.siamese.dyndns.org","subject":"[PATCH] Martin von Zweigbergk has a new e-mail address","fromName":"Martin von Zweigbergk","fromEmail":"martinvonz@gmail.com","sentAt":"2012-08-29T16:20:59Z","receivedAt":"2012-08-29T16:20:59Z","isPatch":true,"sender":{"key":"martinvonz@gmail.com","avatar":"https://avatars.githubusercontent.com/u/891642?v=4"},"body":"Signed-off-by: Martin von Zweigbergk \n---\n .mailmap | 1 +\n 1 file changed, 1 insertion(+)\n\ndiff --git a/.mailmap b/.mailmap\nindex 6303782..2650f9e 100644\n--- a/.mailmap\n+++ b/.mailmap\n@@ -43,6 +43,7 @@ Lars Doelle \n Li Hong \n Lukas Sandström \n Martin Langhoff \n+Martin von Zweigbergk \n Michael Coleman \n Michael J Gruber \n Michael W. Olson \n-- \n1.7.11.1.104.ge7b44f1\n"},{"id":"198050","messageId":"CAPBPrnstykXxTEH56YFDqU2XW+o8WRKBN6=QOTpLJ1jRU7DXCA@mail.gmail.com","threadId":"31230","inReplyTo":"1346220956-25034-2-git-send-email-martinvonz@gmail.com","subject":"Re: [PATCH v2 1/3] teach log --no-walk=unsorted, which avoids sorting","fromName":"Dan Johnson","fromEmail":"computerdruid@gmail.com","sentAt":"2012-08-29T17:34:08Z","receivedAt":"2012-08-29T17:34:08Z","isPatch":true,"sender":{"key":"computerdruid@gmail.com","avatar":"https://avatars.githubusercontent.com/u/34696?v=4"},"body":"On Wed, Aug 29, 2012 at 2:15 AM, Martin von Zweigbergk\n wrote:\n> When 'git log' is passed the --no-walk option, no revision walk takes\n> place, naturally. Perhaps somewhat surprisingly, however, the provided\n> revisions still get sorted by commit date. So e.g 'git log --no-walk\n> HEAD HEAD~1' and 'git log --no-walk HEAD~1 HEAD' give the same result\n> (unless the two revisions share the commit date, in which case they\n> will retain the order given on the command line). As the commit that\n> introduced --no-walk (8e64006 (Teach revision machinery about\n> --no-walk, 2007-07-24)) points out, the sorting is intentional, to\n> allow things like\n>\n> git log --abbrev-commit --pretty=oneline --decorate --all --no-walk\n>\n> to show all refs in order by commit date.\n>\n> But there are also other cases where the sorting is not wanted, such\n> as\n>\n> |\n> git log --oneline --no-walk --stdin\n>\n> To accomodate both cases, leave the decision of whether or not to sort\n> up to the caller, by allowing --no-walk={sorted,unsorted}, defaulting\n> to 'sorted' for backward-compatibility reasons.\n>\n> Signed-off-by: Martin von Zweigbergk \n> ---\n\nPerhaps I am missing something from an earlier discussion, but it is\nnot obvious to me why this is an option to the no-walk behavior and\nnot something like --sorted/--unsorted as a separate option.\n\nIn other words, I don't understand why you always want to sort if you\nare doing revision walking.\n\nThanks for any explanation,\n-Dan\n"},{"id":"198051","messageId":"7vharlwq5l.fsf@alter.siamese.dyndns.org","threadId":"31230","inReplyTo":"CAPBPrnstykXxTEH56YFDqU2XW+o8WRKBN6=QOTpLJ1jRU7DXCA@mail.gmail.com","subject":"Re: [PATCH v2 1/3] teach log --no-walk=unsorted, which avoids sorting","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-08-29T17:42:46Z","receivedAt":"2012-08-29T17:42:46Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Dan Johnson writes:\n\n> Perhaps I am missing something from an earlier discussion, but it is\n\n http://thread.gmane.org/gmane.comp.version-control.git/203259/focus=203344\n\n> not obvious to me why this is an option to the no-walk behavior and\n> not something like --sorted/--unsorted as a separate option.\n>\n> In other words, I don't understand why you always want to sort if you\n> are doing revision walking.\n\nWhen you have more than one starting points to dig the history from\n(e.g. \"git log foo bar baz\"), you would want to start digging from\nthe newer ones, as that would help you find the fork points of the\nbranches involved more efficiently. But you need to follow the\nprevious discussion if you want to understand implications around\nsorting.\n"},{"id":"198125","messageId":"7vehmoqek2.fsf@alter.siamese.dyndns.org","threadId":"31230","inReplyTo":"1346220956-25034-3-git-send-email-martinvonz@gmail.com","subject":"Re: [PATCH v2 2/3] demonstrate broken 'git cherry-pick three one two'","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-08-30T21:02:05Z","receivedAt":"2012-08-30T21:02:05Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Martin von Zweigbergk writes:\n\n> Cherry-picking commits out of order (w.r.t. commit time stamp) doesn't\n> currently work. Add a test case to demonstrate it.\n>\n> Signed-off-by: Martin von Zweigbergk \n> ---\n> t/t3508-cherry-pick-many-commits.sh | 15 +++++++++++++++\n> 1 file changed, 15 insertions(+)\n>\n> diff --git a/t/t3508-cherry-pick-many-commits.sh b/t/t3508-cherry-pick-many-commits.sh\n> index 75f7ff4..fff20c3 100755\n> --- a/t/t3508-cherry-pick-many-commits.sh\n> +++ b/t/t3508-cherry-pick-many-commits.sh\n> @@ -44,6 +44,21 @@ test_expect_success 'cherry-pick first..fourth works' '\n> \tcheck_head_differs_from fourth\n> '\n> \n> +test_expect_failure 'cherry-pick three one two works' '\n> +\tgit checkout -f first &&\n> +\ttest_commit one &&\n> +\ttest_commit two &&\n> +\ttest_commit three &&\n> +\tgit checkout -f master &&\n> +\tgit reset --hard first &&\n> +\tgit cherry-pick three one two &&\n> +\tgit diff --quiet three &&\n> +\tgit diff --quiet HEAD three &&\n> +\ttest \"$(git log --reverse --format=%s first..)\" == \"three\n> +one\n> +two\"\n> +'\n\n\"test $A == $B\" is not POSIX. I'll drop '=' when queuing, so no\nneed to resend.\n\nThanks.\n\n> +\n> test_expect_success 'output to keep user entertained during multi-pick' '\n> \tcat <<-\\EOF >expected &&\n> \t[master OBJID] second\n"}]}