diff options
author | George Burgess IV <gbiv@google.com> | 2021-07-26 23:56:45 +0000 |
---|---|---|
committer | George Burgess IV <gbiv@google.com> | 2021-07-28 00:51:05 +0000 |
commit | ce2a5fa72be3fd1d606505f98d3831706c28cfa8 (patch) | |
tree | 7a0a6109d097177d8de8ac9184e76d6d67dc81d8 | |
parent | [DAGCombiner] Fold SETCC(FREEZE(x),const) to FREEZE(SETCC(x,const)) if SETCC ... (diff) | |
download | llvm-project-ce2a5fa72be3fd1d606505f98d3831706c28cfa8.tar.gz llvm-project-ce2a5fa72be3fd1d606505f98d3831706c28cfa8.tar.bz2 llvm-project-ce2a5fa72be3fd1d606505f98d3831706c28cfa8.zip |
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
-rwxr-xr-x | llvm/utils/revert_checker.py | 9 | ||||
-rwxr-xr-x | llvm/utils/revert_checker_test.py | 6 |
2 files changed, 11 insertions, 4 deletions
diff --git a/llvm/utils/revert_checker.py b/llvm/utils/revert_checker.py index c61e4a3f9778..deb49412b1ec 100755 --- a/llvm/utils/revert_checker.py +++ b/llvm/utils/revert_checker.py @@ -170,7 +170,10 @@ def _find_common_parent_commit(git_dir: str, ref_a: str, ref_b: str) -> str: def find_reverts(git_dir: str, across_ref: str, root: str) -> List[Revert]: - """Finds reverts across `across_ref` in `git_dir`, starting from `root`.""" + """Finds reverts across `across_ref` in `git_dir`, starting from `root`. + + These reverts are returned in order of oldest reverts first. + """ across_sha = _rev_parse(git_dir, across_ref) root_sha = _rev_parse(git_dir, root) @@ -217,6 +220,10 @@ def find_reverts(git_dir: str, across_ref: str, root: str) -> List[Revert]: logging.error("%s claims to revert %s -- which isn't a commit -- %s", sha, object_type, reverted_sha) + # Since `all_reverts` contains reverts in log order (e.g., newer comes before + # older), we need to reverse this to keep with our guarantee of older = + # earlier in the result. + all_reverts.reverse() return all_reverts diff --git a/llvm/utils/revert_checker_test.py b/llvm/utils/revert_checker_test.py index a908e6675986..6573c25c1a0b 100755 --- a/llvm/utils/revert_checker_test.py +++ b/llvm/utils/revert_checker_test.py @@ -106,11 +106,11 @@ class Test(unittest.TestCase): root='9f981e9adf9c8d29bb80306daf08d2770263ade6') self.assertEqual(reverts, [ revert_checker.Revert( - sha='9f981e9adf9c8d29bb80306daf08d2770263ade6', - reverted_sha='4060016fce3e6a0b926ee9fc59e440a612d3a2ec'), - revert_checker.Revert( sha='4e0fe038f438ae1679eae9e156e1f248595b2373', reverted_sha='65b21282c710afe9c275778820c6e3c1cf46734b'), + revert_checker.Revert( + sha='9f981e9adf9c8d29bb80306daf08d2770263ade6', + reverted_sha='4060016fce3e6a0b926ee9fc59e440a612d3a2ec'), ]) |