Martin von Zweigbergk <martin.von.zweigbe...@gmail.com> writes: > So all of the above case give the right result in the end as long > as the timestamps are chronological, and case 1) gives the right > result regardless. The other two cases only works in most cases > because the unexpcted sorting when no-walk is in effect > counteracts the final reversal.
In short, if you have three commits in a row, A--B--C, with timestamps that are not skewed, and want to replay changes of B and then C in that order, all three you listed ends up doing the right thing. But if you want to apply the change C and then B: - "git cherry-pick A..C" is obviously not a way to do so, so we won't discuss it further. - "git cherry-pick C B" is the most natural way the user would want to express this request, but because of the sorting (i.e. commit_list_sort_by_date() in prepare_revision_walk(), combined with ->reverse in sequencer.c::prepare_revs()), it applies B and then C. That is the real bug. Feeding the revs to "git cherry-pick --stdin" in the order the user wishes them to be applied has the same issue. > IIUC, this could be implemented by making cherry-pick iterate > over rev_info.pending.objects just like 'git show' does when not > walking. Yes, that was exactly why I said sequencer.c::prepare_revs() is wrong to call prepare_revision_walk() unconditionally, even when there is no revision walking involved. I actually think your approach to place the "do not sort when we are not walking" logic in prepare_revision_walk() makes more sense. "show" has to look at pending.objects because it needs to show objects other than commits (e.g. "git show :foo"), so there won't be any change in its implementation with your change. It will have to look at pending.objects itself. But "cherry-pick" and sequencer-derived commands only deal with commits. It would be far less error prone to let them call get_revision() repeatedly like all other revision enumerating commands do, than to have them go over the pending.objects list, dereferencing tags and using only commits. The resulting callers would be more readable, too, I would think. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html