llvm_tools: add clean_up_old_llvm_patches script

BUG=b:332589934
TEST=Ran the script; it uploaded a CL correctly: crrev.com/c/5421089

Change-Id: Ib7f18f5d55102c3eeceb6db942832063e9883d41
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/third_party/toolchain-utils/+/5420775
Tested-by: George Burgess <gbiv@chromium.org>
Reviewed-by: Jordan Abrahams-Whitehead <ajordanr@google.com>
Commit-Queue: George Burgess <gbiv@chromium.org>

4 files changed, 256 insertions, 86 deletions

diff --git a/llvm_tools/clean_up_old_llvm_patches.py b/llvm_tools/clean_up_old_llvm_patches.py
new file mode 100755
index 00000000..fef4cf00
--- /dev/null
+++ b/llvm_tools/clean_up_old_llvm_patches.py
@@ -0,0 +1,183 @@
+#!/usr/bin/env python3
+# Copyright 2024 The ChromiumOS Authors
+# Use of this source code is governed by a BSD-style license that can be
+# found in the LICENSE file.
+
+"""Removes all LLVM patches before a certain point."""
+
+import argparse
+import logging
+from pathlib import Path
+import subprocess
+import sys
+import textwrap
+from typing import List, Optional
+
+from cros_utils import git_utils
+import patch_utils
+
+
+# The chromiumos-overlay packages to GC patches in.
+PACKAGES_TO_COLLECT = patch_utils.CHROMEOS_PATCHES_JSON_PACKAGES
+
+# Folks who should be on the R-line of any CLs that get uploaded.
+CL_REVIEWERS = (git_utils.REVIEWER_DETECTIVE,)
+
+# Folks who should be on the CC-line of any CLs that get uploaded.
+CL_CC = ("gbiv@google.com",)
+
+
+def maybe_autodetect_cros_overlay(my_dir: Path) -> Optional[Path]:
+    third_party = my_dir.parent.parent
+    cros_overlay = third_party / "chromiumos-overlay"
+    if cros_overlay.exists():
+        return cros_overlay
+    return None
+
+
+def remove_old_patches(cros_overlay: Path, min_revision: int) -> bool:
+    """Removes patches in cros_overlay. Returns whether changes were made."""
+    patches_removed = 0
+    for package in PACKAGES_TO_COLLECT:
+        logging.info("GC'ing patches from %s...", package)
+        patches_json = cros_overlay / package / "files/PATCHES.json"
+        removed_patch_files = patch_utils.remove_old_patches(
+            min_revision, patches_json
+        )
+        if not removed_patch_files:
+            logging.info("No patches removed from %s", patches_json)
+            continue
+
+        patches_removed += len(removed_patch_files)
+        for patch in removed_patch_files:
+            logging.info("Removing %s...", patch)
+            patch.unlink()
+    return patches_removed != 0
+
+
+def commit_changes(cros_overlay: Path, min_rev: int):
+    commit_msg = textwrap.dedent(
+        f"""
+        llvm: remove old patches
+
+        These patches stopped applying before r{min_rev}, so should no longer
+        be needed.
+
+        BUG=b:332601837
+        TEST=CQ
+        """
+    )
+
+    subprocess.run(
+        ["git", "commit", "--quiet", "-a", "-m", commit_msg],
+        cwd=cros_overlay,
+        check=True,
+        stdin=subprocess.DEVNULL,
+    )
+
+
+def upload_changes(cros_overlay: Path) -> None:
+    cl_ids = git_utils.upload_to_gerrit(
+        cros_overlay,
+        remote="cros",
+        branch="main",
+        reviewers=CL_REVIEWERS,
+        cc=CL_CC,
+    )
+
+    if len(cl_ids) > 1:
+        raise ValueError(f"Unexpected: wanted just one CL upload; got {cl_ids}")
+
+    cl_id = cl_ids[0]
+    logging.info("Uploaded CL http://crrev.com/c/%s successfully.", cl_id)
+    git_utils.try_set_autosubmit_labels(cros_overlay, cl_id)
+
+
+def get_opts(my_dir: Path, argv: List[str]) -> argparse.Namespace:
+    """Returns options for the script."""
+
+    parser = argparse.ArgumentParser(
+        description=__doc__,
+        formatter_class=argparse.RawDescriptionHelpFormatter,
+    )
+    parser.add_argument(
+        "--chromiumos-overlay",
+        type=Path,
+        help="""
+        Path to chromiumos-overlay. Will autodetect if none is specified. If
+        autodetection fails and none is specified, this script will fail.
+        """,
+    )
+    parser.add_argument(
+        "--revision",
+        type=int,
+        help="""
+        Revision to delete before (exclusive). All patches that stopped
+        applying before this will be removed. Phrased as an int, e.g.,
+        `--revision=1234`.
+        """,
+    )
+    parser.add_argument(
+        "--commit",
+        action="store_true",
+        help="Commit changes after making them.",
+    )
+    parser.add_argument(
+        "--upload-with-autoreview",
+        action="store_true",
+        help="""
+        Upload changes after committing them. Implies --commit. Also adds
+        default reviewers, and starts CQ+1 (among other convenience features).
+        """,
+    )
+    opts = parser.parse_args(argv)
+
+    if not opts.chromiumos_overlay:
+        maybe_overlay = maybe_autodetect_cros_overlay(my_dir)
+        if not maybe_overlay:
+            parser.error(
+                "Failed to autodetect --chromiumos-overlay; please pass a value"
+            )
+        opts.chromiumos_overlay = maybe_overlay
+    return opts
+
+
+def main(argv: List[str]) -> None:
+    logging.basicConfig(
+        format=">> %(asctime)s: %(levelname)s: %(filename)s:%(lineno)d: "
+        "%(message)s",
+        level=logging.INFO,
+    )
+
+    my_dir = Path(__file__).resolve().parent
+    opts = get_opts(my_dir, argv)
+
+    cros_overlay = opts.chromiumos_overlay
+    upload = opts.upload_with_autoreview
+    commit = opts.commit or upload
+    revision = opts.revision
+
+    made_changes = remove_old_patches(cros_overlay, revision)
+    if not made_changes:
+        logging.info("No changes made; exiting.")
+        return
+
+    if not commit:
+        logging.info(
+            "Changes were made, but --commit wasn't specified. My job is done."
+        )
+        return
+
+    logging.info("Committing changes...")
+    commit_changes(cros_overlay, revision)
+    if not upload:
+        logging.info("Change with removed patches has been committed locally.")
+        return
+
+    logging.info("Uploading changes...")
+    upload_changes(cros_overlay)
+    logging.info("Change sent for review.")
+
+
+if __name__ == "__main__":
+    main(sys.argv[1:])
diff --git a/llvm_tools/patch_utils.py b/llvm_tools/patch_utils.py
index b5383ac9..521ff6c6 100644
--- a/llvm_tools/patch_utils.py
+++ b/llvm_tools/patch_utils.py
@@ -34,6 +34,15 @@ HUNK_HEADER_RE = re.compile(r"^@@\s+-(\d+),(\d+)\s+\+(\d+),(\d+)\s+@@")
 HUNK_END_RE = re.compile(r"^--\s*$")
 PATCH_SUBFILE_HEADER_RE = re.compile(r"^\+\+\+ [ab]/(.*)$")
 
+CHROMEOS_PATCHES_JSON_PACKAGES = (
+    "dev-util/lldb-server",
+    "sys-devel/llvm",
+    "sys-libs/compiler-rt",
+    "sys-libs/libcxx",
+    "sys-libs/llvm-libunwind",
+    "sys-libs/scudo",
+)
+
 
 @dataclasses.dataclass
 class Hunk:
@@ -232,16 +241,6 @@ class PatchEntry:
             until_v = sys.maxsize
         return from_v <= svn_version < until_v
 
-    def is_old(self, svn_version: int) -> bool:
-        """Is this patch old compared to `svn_version`?"""
-        if not self.version_range:
-            return False
-        until_v = self.version_range.get("until")
-        # Sometimes the key is there, but it's set to None.
-        if until_v is None:
-            until_v = sys.maxsize
-        return svn_version >= until_v
-
     def apply(
         self,
         root_dir: Path,
@@ -297,6 +296,17 @@ class PatchEntry:
         return self.metadata.get("title", "")
 
 
+def patch_applies_after(
+    version_range: Optional[Dict[str, Optional[int]]], svn_version: int
+) -> bool:
+    """Does this patch apply after `svn_version`?"""
+    if not version_range:
+        return True
+    until = version_range.get("until")
+    before_svn_version = until is not None and svn_version > until
+    return not before_svn_version
+
+
 @dataclasses.dataclass(frozen=True)
 class PatchInfo:
     """Holds info for a round of patch applications."""
@@ -596,9 +606,7 @@ def update_version_ranges_with_entries(
     return modified_entries, applied_patches
 
 
-def remove_old_patches(
-    svn_version: int, llvm_src_dir: Path, patches_json_fp: Path
-) -> PatchInfo:
+def remove_old_patches(svn_version: int, patches_json: Path) -> List[Path]:
     """Remove patches that don't and will never apply for the future.
 
     Patches are determined to be "old" via the "is_old" method for
@@ -606,37 +614,27 @@ def remove_old_patches(
 
     Args:
         svn_version: LLVM SVN version.
-        llvm_src_dir: LLVM source directory.
-        patches_json_fp: Location to edit patches on.
+        patches_json: Location of PATCHES.json.
 
     Returns:
-        PatchInfo for modified patches.
+        A list of all patch paths removed from PATCHES.json.
     """
-    with patches_json_fp.open(encoding="utf-8") as f:
-        contents = f.read()
+    contents = patches_json.read_text(encoding="utf-8")
     indent_len = predict_indent(contents.splitlines())
-    patch_entries = json_str_to_patch_entries(
-        llvm_src_dir,
-        contents,
-    )
-    oldness = [(entry, entry.is_old(svn_version)) for entry in patch_entries]
-    filtered_entries = [entry.to_dict() for entry, old in oldness if not old]
-    with atomic_write_file.atomic_write(patches_json_fp, encoding="utf-8") as f:
-        _write_json_changes(filtered_entries, f, indent_len=indent_len)
-    removed_entries = [entry for entry, old in oldness if old]
-    plural_patches = "patch" if len(removed_entries) == 1 else "patches"
-    print(f"Removed {len(removed_entries)} old {plural_patches}:")
-    for r in removed_entries:
-        print(f"- {r.rel_patch_path}: {r.title()}")
 
-    return PatchInfo(
-        non_applicable_patches=[],
-        applied_patches=[],
-        failed_patches=[],
-        disabled_patches=[],
-        removed_patches=[p.rel_patch_path for p in removed_entries],
-        modified_metadata=str(patches_json_fp) if removed_entries else None,
-    )
+    still_new = []
+    removed_patches = []
+    patches_parent = patches_json.parent
+    for entry in json.loads(contents):
+        if patch_applies_after(entry.get("version_range"), svn_version):
+            still_new.append(entry)
+        else:
+            removed_patches.append(patches_parent / entry["rel_patch_path"])
+
+    with atomic_write_file.atomic_write(patches_json, encoding="utf-8") as f:
+        _write_json_changes(still_new, f, indent_len=indent_len)
+
+    return removed_patches
 
 
 def git_am(patch_path: Path) -> List[Union[str, Path]]:
diff --git a/llvm_tools/patch_utils_unittest.py b/llvm_tools/patch_utils_unittest.py
index 362a8dfd..117046c4 100755
--- a/llvm_tools/patch_utils_unittest.py
+++ b/llvm_tools/patch_utils_unittest.py
@@ -9,9 +9,9 @@ import copy
 import io
 import json
 from pathlib import Path
+import shutil
 import subprocess
 import tempfile
-from typing import Callable
 import unittest
 from unittest import mock
 
@@ -21,6 +21,11 @@ import patch_utils as pu
 class TestPatchUtils(unittest.TestCase):
     """Test the patch_utils."""
 
+    def make_tempdir(self) -> Path:
+        tmpdir = Path(tempfile.mkdtemp(prefix="patch_utils_unittest"))
+        self.addCleanup(shutil.rmtree, tmpdir)
+        return tmpdir
+
     def test_predict_indent(self):
         test_str1 = """
 a
@@ -311,49 +316,39 @@ Hunk #1 SUCCEEDED at 96 with fuzz 1.
             self.assertEqual(patches2[1].version_range, {"from": 0, "until": 2})
             self.assertEqual(patches2[2].version_range, {"from": 4, "until": 5})
 
-    @mock.patch("builtins.print")
-    def test_remove_old_patches(self, _):
-        """Can remove old patches from PATCHES.json."""
-        one_patch_dict = {
-            "metadata": {
-                "title": "[some label] hello world",
+    def test_remove_old_patches(self):
+        patches = [
+            {"rel_patch_path": "foo.patch"},
+            {
+                "rel_patch_path": "bar.patch",
+                "version_range": {
+                    "from": 1,
+                },
             },
-            "platforms": [
-                "chromiumos",
-            ],
-            "rel_patch_path": "x/y/z",
-            "version_range": {
-                "from": 4,
-                "until": 5,
+            {
+                "rel_patch_path": "baz.patch",
+                "version_range": {
+                    "until": 1,
+                },
             },
-        }
-        patches = [
-            one_patch_dict,
-            {**one_patch_dict, "version_range": {"until": None}},
-            {**one_patch_dict, "version_range": {"from": 100}},
-            {**one_patch_dict, "version_range": {"until": 8}},
-        ]
-        cases = [
-            (0, lambda x: self.assertEqual(len(x), 4)),
-            (6, lambda x: self.assertEqual(len(x), 3)),
-            (8, lambda x: self.assertEqual(len(x), 2)),
-            (1000, lambda x: self.assertEqual(len(x), 2)),
         ]
 
-        def _t(dirname: str, svn_version: int, assertion_f: Callable):
-            json_filepath = Path(dirname) / "PATCHES.json"
-            with json_filepath.open("w", encoding="utf-8") as f:
-                json.dump(patches, f)
-            pu.remove_old_patches(svn_version, Path(), json_filepath)
-            with json_filepath.open("r", encoding="utf-8") as f:
-                result = json.load(f)
-            assertion_f(result)
+        tempdir = self.make_tempdir()
+        patches_json = tempdir / "PATCHES.json"
+        with patches_json.open("w", encoding="utf-8") as f:
+            json.dump(patches, f)
 
-        with tempfile.TemporaryDirectory(
-            prefix="patch_utils_unittest"
-        ) as dirname:
-            for r, a in cases:
-                _t(dirname, r, a)
+        removed_paths = pu.remove_old_patches(
+            svn_version=10, patches_json=patches_json
+        )
+        self.assertEqual(removed_paths, [tempdir / "baz.patch"])
+        expected_patches = [
+            x for x in patches if x["rel_patch_path"] != "baz.patch"
+        ]
+        self.assertEqual(
+            json.loads(patches_json.read_text(encoding="utf-8")),
+            expected_patches,
+        )
 
     @staticmethod
     def _default_json_dict():
diff --git a/llvm_tools/update_chromeos_llvm_hash.py b/llvm_tools/update_chromeos_llvm_hash.py
index bbb5edb3..f6161773 100755
--- a/llvm_tools/update_chromeos_llvm_hash.py
+++ b/llvm_tools/update_chromeos_llvm_hash.py
@@ -29,14 +29,8 @@ import patch_utils
 import subprocess_helpers
 
 
-DEFAULT_PACKAGES = [
-    "dev-util/lldb-server",
-    "sys-devel/llvm",
-    "sys-libs/compiler-rt",
-    "sys-libs/libcxx",
-    "sys-libs/llvm-libunwind",
-    "sys-libs/scudo",
-]
+# Default list of packages to update.
+DEFAULT_PACKAGES = patch_utils.CHROMEOS_PATCHES_JSON_PACKAGES
 
 DEFAULT_MANIFEST_PACKAGES = ["sys-devel/llvm"]