cherry-pick and 'log --no-walk' and ordering

From: Martin von Zweigbergk <martin.von.zweigbergk@gmail.com>
Date: Aug 10, 2012, 20:41 UTC
Message-ID: <CAOeW2eE=VcUs1YcWqqEUc6vM6jW9JaXzE-_tVWy48VtPzm_+wA@mail.gmail.com>

A while ago when I was looking at revision.c, I was surprised to see that commits are sorted even when --no-walk is passed, but as 8e64006 (Teach revision machinery about --no-walk, 2007-07-24) points out, this can be useful for doing

 $ git log --abbrev-commit --pretty=oneline --decorate --all --no-walk

and get the result sorted by date. However, it can also be useful _not_ to get a result sorted by date, e.g. when doing something like "<generate an ordered list of revisions> | git rev-list --oneline --no-walk --stdin". Would a --no-sort option to rev-list be appreciated or are there better solutions?

There is also cherry-pick/revert, which I _think_ does not really want the revisions sorted. cherry-pick implicitly reverses the order of the walk, so 'git cherry-pick branch~2..branch' applies them in the right order (at least in the absence of clock skew). The documentation for cherry-pick suggests "git rev-list --reverse master -- README | git cherry-pick -n --stdin", which I think makes no sense -- this would reverse the output from rev-list only to have it reversed again in cherry-pick, if it wasn't for the sorting by date. I think the --reverse passed to rev-list might even break the cherry-pick if there are commits in non-increasing date order. This is also supported by the fact that test still pass after applying the patch below. I think the test cases make more sense after the patch.

Do others agree with the analysis? I suppose it's too late to change cherry-pick to start differentiating between "git cherry-pick commit1 commit2" and "git cherry-pick commit2 commit1", but I think we should at least update the documentation as in the patch below (or maybe even with a --topo-order passed to cherry-pick?). We could possibly change cherry-pick's ordering from the default ordering to topological ordering.

Martin

Sorry about the mangled whitespace below; just for reference, not intended to be applied.

diff --git a/Documentation/git-cherry-pick.txt
b/Documentation/git-cherry-pick.txt
index 0e170a5..454e205 100644
--- a/Documentation/git-cherry-pick.txt
+++ b/Documentation/git-cherry-pick.txt
@@ -181,7 +181,7 @@ EXAMPLES
        are in next but not HEAD to the current branch, creating a new
        commit for each new change.

-`git rev-list --reverse master -- README | git cherry-pick -n --stdin`::
+`git rev-list master -- README | git cherry-pick -n --stdin`::

        Apply the changes introduced by all commits on the master
        branch that touched README to the working tree and index,
diff --git a/t/t3508-cherry-pick-many-commits.sh
b/t/t3508-cherry-pick-many-commits.sh
index 75f7ff4..020baaf 100755
--- a/t/t3508-cherry-pick-many-commits.sh
+++ b/t/t3508-cherry-pick-many-commits.sh
@@ -164,7 +164,7 @@ test_expect_success 'cherry-pick --stdin works' '
        git checkout -f master &&
        git reset --hard first &&
        test_tick &&
-       git rev-list --reverse first..fourth | git cherry-pick --stdin &&
+       git rev-list first..fourth | git cherry-pick --stdin &&
        git diff --quiet other &&
        git diff --quiet HEAD other &&
        check_head_differs_from fourth
diff --git a/t/t3510-cherry-pick-sequence.sh b/t/t3510-cherry-pick-sequence.sh
index f4e6450..9e28910 100755
--- a/t/t3510-cherry-pick-sequence.sh
+++ b/t/t3510-cherry-pick-sequence.sh
@@ -400,7 +400,7 @@ test_expect_success '--continue of single-pick
respects -x' '

 test_expect_success '--continue respects -x in first commit in multi-pick' '
        pristine_detach initial &&
-       test_must_fail git cherry-pick -x picked anotherpick &&
+       test_must_fail git cherry-pick -x anotherpick picked &&
        echo c >foo &&
        git add foo &&
        git cherry-pick --continue &&
@@ -430,7 +430,7 @@ test_expect_success '--signoff is not
automatically propagated to resolved confl

 test_expect_success '--signoff dropped for implicit commit of
resolution, multi-pick case' '
        pristine_detach initial &&
-       test_must_fail git cherry-pick -s picked anotherpick &&
+       test_must_fail git cherry-pick -s anotherpick picked &&
        echo c >foo &&
        git add foo &&
        git cherry-pick --continue &&