Skip to content

Commit ce2a5fa

Browse files
committed
llvm/utils: guarantee revert_checker's revert ordering
At the moment, the revert ordering from this tool is unspecified (though it happens to be in `git log` order, so newest reverts come first). From the standpoint of tooling and users, this seems to be the opposite of what we want by default: tools and users will generally try to apply these reverts as cherry-picks. If two reverts in the list are close enough to each other, if the reverts get applied out of order, we'll get a merge conflict. Rather than having `reverse`s for all tools (and mental reverses for manual users), just guarantee an oldest-first output ordering for this function. Differential Revision: https://reviews.llvm.org/D106838
1 parent 4f71f59 commit ce2a5fa

File tree

2 files changed

+11
-4
lines changed

2 files changed

+11
-4
lines changed

llvm/utils/revert_checker.py

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -170,7 +170,10 @@ def _find_common_parent_commit(git_dir: str, ref_a: str, ref_b: str) -> str:
170170

171171

172172
def find_reverts(git_dir: str, across_ref: str, root: str) -> List[Revert]:
173-
"""Finds reverts across `across_ref` in `git_dir`, starting from `root`."""
173+
"""Finds reverts across `across_ref` in `git_dir`, starting from `root`.
174+
175+
These reverts are returned in order of oldest reverts first.
176+
"""
174177
across_sha = _rev_parse(git_dir, across_ref)
175178
root_sha = _rev_parse(git_dir, root)
176179

@@ -217,6 +220,10 @@ def find_reverts(git_dir: str, across_ref: str, root: str) -> List[Revert]:
217220
logging.error("%s claims to revert %s -- which isn't a commit -- %s", sha,
218221
object_type, reverted_sha)
219222

223+
# Since `all_reverts` contains reverts in log order (e.g., newer comes before
224+
# older), we need to reverse this to keep with our guarantee of older =
225+
# earlier in the result.
226+
all_reverts.reverse()
220227
return all_reverts
221228

222229

llvm/utils/revert_checker_test.py

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -105,12 +105,12 @@ def test_known_reverts_across_arbitrary_llvm_rev(self) -> None:
105105
across_ref='c47f971694be0159ffddfee8a75ae515eba91439',
106106
root='9f981e9adf9c8d29bb80306daf08d2770263ade6')
107107
self.assertEqual(reverts, [
108-
revert_checker.Revert(
109-
sha='9f981e9adf9c8d29bb80306daf08d2770263ade6',
110-
reverted_sha='4060016fce3e6a0b926ee9fc59e440a612d3a2ec'),
111108
revert_checker.Revert(
112109
sha='4e0fe038f438ae1679eae9e156e1f248595b2373',
113110
reverted_sha='65b21282c710afe9c275778820c6e3c1cf46734b'),
111+
revert_checker.Revert(
112+
sha='9f981e9adf9c8d29bb80306daf08d2770263ade6',
113+
reverted_sha='4060016fce3e6a0b926ee9fc59e440a612d3a2ec'),
114114
])
115115

116116

0 commit comments

Comments
 (0)