Merge branch 'mz/cherry-pick-cmdline-order'

"git cherry-pick A C B" used to replay changes in A and then B and
then C if these three commits had committer timestamps in that
order, which is not what the user who said "A C B" naturally expects.

* mz/cherry-pick-cmdline-order:
cherry-pick/revert: respect order of revisions to pick
demonstrate broken 'git cherry-pick three one two'
teach log --no-walk=unsorted, which avoids sorting

diff --git a/Documentation/rev-list-options.txt b/Documentation/rev-list-options.txt
index def1340ac73cc3abab8084f14dfb8d54d2476177..5436eba6e761716cfeaee2683566137736497791 100644 (file)
--- a/Documentation/rev-list-options.txt
+++ b/Documentation/rev-list-options.txt
@@ -636,10 +636,14 @@ These options are mostly targeted for packing of git repositories.
        Only useful with '--objects'; print the object IDs that are not
        in packs.
 
---no-walk::
-
-       Only show the given revs, but do not traverse their ancestors.
-       This has no effect if a range is specified.
+--no-walk[=(sorted|unsorted)]::
+
+       Only show the given commits, but do not traverse their ancestors.
+       This has no effect if a range is specified. If the argument
+       "unsorted" is given, the commits are show in the order they were
+       given on the command line. Otherwise (if "sorted" or no argument
+       was given), the commits are show in reverse chronological order
+       by commit time.
 
 --do-walk::
 

diff --git a/builtin/log.c b/builtin/log.c
index e4e15dd55aef5b1cbcb8ddeae6432e6188ff8e4a..09cf43e6d40efc13f6b7bda2094fca5bdc1bbc87 100644 (file)
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -456,7 +456,7 @@ int cmd_show(int argc, const char **argv, const char *prefix)
        init_revisions(&rev, prefix);
        rev.diff = 1;
        rev.always_show_header = 1;
-       rev.no_walk = 1;
+       rev.no_walk = REVISION_WALK_NO_WALK_SORTED;
        rev.diffopt.stat_width = -1;    /* Scale to real terminal size */
 
        memset(&opt, 0, sizeof(opt));

diff --git a/builtin/revert.c b/builtin/revert.c
index 17dc8c8d1e837ceac780b0de8e3b3921e1329894..c5e36b94c080212ec9241146c20ef2e323663bea 100644 (file)
--- a/builtin/revert.c
+++ b/builtin/revert.c
@@ -195,7 +195,7 @@ static void parse_args(int argc, const char **argv, struct replay_opts *opts)
                struct setup_revision_opt s_r_opt;
                opts->revs = xmalloc(sizeof(*opts->revs));
                init_revisions(opts->revs, NULL);
-               opts->revs->no_walk = 1;
+               opts->revs->no_walk = REVISION_WALK_NO_WALK_UNSORTED;
                if (argc < 2)
                        usage_with_options(usage_str, options);
                memset(&s_r_opt, 0, sizeof(s_r_opt));

diff --git a/revision.c b/revision.c
index cbcae1086b346e2b956db7ef9b2729385eaa2ec9..dc3fecf903aa0ce4927b9ebe5b95f99cc9c1018d 100644 (file)
--- a/revision.c
+++ b/revision.c
@@ -1312,7 +1312,7 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg
            !strcmp(arg, "--no-walk") || !strcmp(arg, "--do-walk") ||
            !strcmp(arg, "--bisect") || !prefixcmp(arg, "--glob=") ||
            !prefixcmp(arg, "--branches=") || !prefixcmp(arg, "--tags=") ||
-           !prefixcmp(arg, "--remotes="))
+           !prefixcmp(arg, "--remotes=") || !prefixcmp(arg, "--no-walk="))
        {
                unkv[(*unkc)++] = arg;
                return 1;
@@ -1707,7 +1707,18 @@ static int handle_revision_pseudo_opt(const char *submodule,
        } else if (!strcmp(arg, "--not")) {
                *flags ^= UNINTERESTING;
        } else if (!strcmp(arg, "--no-walk")) {
-               revs->no_walk = 1;
+               revs->no_walk = REVISION_WALK_NO_WALK_SORTED;
+       } else if (!prefixcmp(arg, "--no-walk=")) {
+               /*
+                * Detached form ("--no-walk X" as opposed to "--no-walk=X")
+                * not allowed, since the argument is optional.
+                */
+               if (!strcmp(arg + 10, "sorted"))
+                       revs->no_walk = REVISION_WALK_NO_WALK_SORTED;
+               else if (!strcmp(arg + 10, "unsorted"))
+                       revs->no_walk = REVISION_WALK_NO_WALK_UNSORTED;
+               else
+                       return error("invalid argument to --no-walk");
        } else if (!strcmp(arg, "--do-walk")) {
                revs->no_walk = 0;
        } else {
@@ -2129,10 +2140,11 @@ int prepare_revision_walk(struct rev_info *revs)
                }
                e++;
        }
-       commit_list_sort_by_date(&revs->commits);
        if (!revs->leak_pending)
                free(list);
 
+       if (revs->no_walk != REVISION_WALK_NO_WALK_UNSORTED)
+               commit_list_sort_by_date(&revs->commits);
        if (revs->no_walk)
                return 0;
        if (revs->limited)

diff --git a/revision.h b/revision.h
index cb5ab3513bc9982095e292119b9119d333556256..a95bd0b3f3026068046d469a74d1a2d85e50e314 100644 (file)
--- a/revision.h
+++ b/revision.h
@@ -41,6 +41,10 @@ struct rev_cmdline_info {
        } *rev;
 };
 
+#define REVISION_WALK_WALK 0
+#define REVISION_WALK_NO_WALK_SORTED 1
+#define REVISION_WALK_NO_WALK_UNSORTED 2
+
 struct rev_info {
        /* Starting list */
        struct commit_list *commits;
@@ -62,7 +66,7 @@ struct rev_info {
        /* Traversal flags */
        unsigned int    dense:1,
                        prune:1,
-                       no_walk:1,
+                       no_walk:2,
                        show_all:1,
                        remove_empty_trees:1,
                        simplify_history:1,

diff --git a/sequencer.c b/sequencer.c
index 1ea5293f6ed5395b95fbf63fc41aa28c33db0fa8..f86f116a15326f514fbe0405a9301f4ee2416344 100644 (file)
--- a/sequencer.c
+++ b/sequencer.c
@@ -546,7 +546,11 @@ static int do_pick_commit(struct commit *commit, struct replay_opts *opts)
 
 static void prepare_revs(struct replay_opts *opts)
 {
-       if (opts->action != REPLAY_REVERT)
+       /*
+        * picking (but not reverting) ranges (but not individual revisions)
+        * should be done in reverse
+        */
+       if (opts->action == REPLAY_PICK && !opts->revs->no_walk)
                opts->revs->reverse ^= 1;
 
        if (prepare_revision_walk(opts->revs))

diff --git a/t/t3508-cherry-pick-many-commits.sh b/t/t3508-cherry-pick-many-commits.sh
index 75f7ff4f2fe21e86e0a26fe5a6c2119bef38404c..340afc760de030879fff496dca3db8432a6b992b 100755 (executable)
--- a/t/t3508-cherry-pick-many-commits.sh
+++ b/t/t3508-cherry-pick-many-commits.sh
@@ -44,6 +44,21 @@ test_expect_success 'cherry-pick first..fourth works' '
        check_head_differs_from fourth
 '
 
+test_expect_success 'cherry-pick three one two works' '
+       git checkout -f first &&
+       test_commit one &&
+       test_commit two &&
+       test_commit three &&
+       git checkout -f master &&
+       git reset --hard first &&
+       git cherry-pick three one two &&
+       git diff --quiet three &&
+       git diff --quiet HEAD three &&
+       test "$(git log --reverse --format=%s first..)" = "three
+one
+two"
+'
+
 test_expect_success 'output to keep user entertained during multi-pick' '
        cat <<-\EOF >expected &&
        [master OBJID] second

diff --git a/t/t4202-log.sh b/t/t4202-log.sh
index 0baaad2a240d401a29b341e1bf01173842a2ab54..924ba536ca9718da0f4126d53c316a5f7a1926a5 100755 (executable)
--- a/t/t4202-log.sh
+++ b/t/t4202-log.sh
@@ -178,11 +178,21 @@ test_expect_success 'git log --no-walk <commits> sorts by commit time' '
        test_cmp expect actual
 '
 
+test_expect_success 'git log --no-walk=sorted <commits> sorts by commit time' '
+       git log --no-walk=sorted --oneline 5d31159 804a787 394ef78 > actual &&
+       test_cmp expect actual
+'
+
 cat > expect << EOF
 5d31159 fourth
 804a787 sixth
 394ef78 fifth
 EOF
+test_expect_success 'git log --no-walk=unsorted <commits> leaves list of commits as given' '
+       git log --no-walk=unsorted --oneline 5d31159 804a787 394ef78 > actual &&
+       test_cmp expect actual
+'
+
 test_expect_success 'git show <commits> leaves list of commits as given' '
        git show --oneline -s 5d31159 804a787 394ef78 > actual &&
        test_cmp expect actual