llvm_tools: update_chromeos_llvm_hash failure modes
Support 'disable_patches' and 'remove_patches'
failure mode options.
BUG=b:250648178
TEST=./patch_utils_unittest.py
./update_chromeos_llvm_hash_unittest.py
./patch_manager_unittest.py
./update_chromeos_llvm_hash [...] --failure_mode remove_patches
./update_chromeos_llvm_hash [...] --failure_mode disable_patches
Change-Id: I6269b2220cf05413c7776087030297773ab9a154
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/third_party/toolchain-utils/+/3935651
Reviewed-by: Adrian Dole <adriandole@google.com>
Auto-Submit: Adrian Dole <adriandole@google.com>
Tested-by: Adrian Dole <adriandole@google.com>
Reviewed-by: Denis Nikitin <denik@chromium.org>
Reviewed-by: Jordan Abrahams-Whitehead <ajordanr@google.com>
Commit-Queue: Adrian Dole <adriandole@google.com>
diff --git a/llvm_tools/patch_manager.py b/llvm_tools/patch_manager.py
index 11e8222..4d4e838 100755
--- a/llvm_tools/patch_manager.py
+++ b/llvm_tools/patch_manager.py
@@ -7,11 +7,10 @@
import argparse
import enum
-import json
import os
from pathlib import Path
import sys
-from typing import Any, Dict, IO, Iterable, List, Optional, Tuple
+from typing import Iterable, List, Optional, Tuple
from failure_modes import FailureModes
import get_llvm_hash
@@ -97,13 +96,6 @@
return version
-def _WriteJsonChanges(patches: List[Dict[str, Any]], file_io: IO[str]):
- """Write JSON changes to file, does not acquire new file lock."""
- json.dump(patches, file_io, indent=4, separators=(",", ": "))
- # Need to add a newline as json.dump omits it.
- file_io.write("\n")
-
-
def GetCommitHashesForBisection(src_path, good_svn_version, bad_svn_version):
"""Gets the good and bad commit hashes required by `git bisect start`."""
@@ -114,105 +106,6 @@
return good_commit_hash, bad_commit_hash
-def RemoveOldPatches(
- svn_version: int, llvm_src_dir: Path, patches_json_fp: 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
- each patch entry.
-
- Args:
- svn_version: LLVM SVN version.
- llvm_src_dir: LLVM source directory.
- patches_json_fp: Location to edit patches on.
- """
- with patches_json_fp.open(encoding="utf-8") as f:
- patches_list = json.load(f)
- patch_entries = (
- patch_utils.PatchEntry.from_dict(llvm_src_dir, elem)
- for elem in patches_list
- )
- 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 patch_utils.atomic_write(patches_json_fp, encoding="utf-8") as f:
- _WriteJsonChanges(filtered_entries, f)
- 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()}")
-
-
-def UpdateVersionRanges(
- svn_version: int, llvm_src_dir: Path, patches_json_fp: Path
-):
- """Reduce the version ranges of failing patches.
-
- Patches which fail to apply will have their 'version_range.until'
- field reduced to the passed in svn_version.
-
- Modifies the contents of patches_json_fp.
-
- Ars:
- svn_version: LLVM revision number.
- llvm_src_dir: llvm-project directory path.
- patches_json_fp: Filepath to the PATCHES.json file.
- """
- with patches_json_fp.open(encoding="utf-8") as f:
- patch_entries = patch_utils.json_to_patch_entries(
- patches_json_fp.parent,
- f,
- )
- modified_entries = UpdateVersionRangesWithEntries(
- svn_version, llvm_src_dir, patch_entries
- )
- with patch_utils.atomic_write(patches_json_fp, encoding="utf-8") as f:
- _WriteJsonChanges([p.to_dict() for p in patch_entries], f)
- for entry in modified_entries:
- print(
- f"Stopped applying {entry.rel_patch_path} ({entry.title()}) "
- f"for r{svn_version}"
- )
-
-
-def UpdateVersionRangesWithEntries(
- svn_version: int,
- llvm_src_dir: Path,
- patch_entries: Iterable[patch_utils.PatchEntry],
-) -> List[patch_utils.PatchEntry]:
- """Test-able helper for UpdateVersionRanges.
-
- Args:
- svn_version: LLVM revision number.
- llvm_src_dir: llvm-project directory path.
- patch_entries: PatchEntry objects to modify.
-
- Returns:
- A list of PatchEntry objects which were modified.
-
- Post:
- Modifies patch_entries in place.
- """
- modified_entries: List[patch_utils.PatchEntry] = []
- with patch_utils.git_clean_context(llvm_src_dir):
- for pe in patch_entries:
- test_result = pe.test_apply(llvm_src_dir)
- if not test_result:
- if pe.version_range is None:
- pe.version_range = {}
- pe.version_range["until"] = svn_version
- modified_entries.append(pe)
- else:
- # We have to actually apply the patch so that future patches
- # will stack properly.
- if not pe.apply(llvm_src_dir).succeeded:
- raise RuntimeError(
- "Could not apply patch that dry ran successfully"
- )
- return modified_entries
-
-
def CheckPatchApplies(
svn_version: int,
llvm_src_dir: Path,
@@ -374,10 +267,14 @@
PrintPatchResults(result)
def _remove(args):
- RemoveOldPatches(args.svn_version, llvm_src_dir, patches_json_fp)
+ patch_utils.remove_old_patches(
+ args.svn_version, llvm_src_dir, patches_json_fp
+ )
def _disable(args):
- UpdateVersionRanges(args.svn_version, llvm_src_dir, patches_json_fp)
+ patch_utils.update_version_ranges(
+ args.svn_version, llvm_src_dir, patches_json_fp
+ )
def _test_single(args):
if not args.test_patch:
diff --git a/llvm_tools/patch_manager_unittest.py b/llvm_tools/patch_manager_unittest.py
index 19c2d8a..42697d9 100755
--- a/llvm_tools/patch_manager_unittest.py
+++ b/llvm_tools/patch_manager_unittest.py
@@ -62,50 +62,6 @@
mock_isfile.assert_called_once()
@mock.patch("builtins.print")
- def testRemoveOldPatches(self, _):
- """Can remove old patches from PATCHES.json."""
- one_patch_dict = {
- "metadata": {
- "title": "[some label] hello world",
- },
- "platforms": [
- "chromiumos",
- ],
- "rel_patch_path": "x/y/z",
- "version_range": {
- "from": 4,
- "until": 5,
- },
- }
- 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)
- patch_manager.RemoveOldPatches(svn_version, Path(), json_filepath)
- with json_filepath.open("r", encoding="utf-8") as f:
- result = json.load(f)
- assertion_f(result)
-
- with tempfile.TemporaryDirectory(
- prefix="patch_manager_unittest"
- ) as dirname:
- for r, a in cases:
- _t(dirname, r, a)
-
- @mock.patch("builtins.print")
@mock.patch.object(patch_utils, "git_clean_context")
def testCheckPatchApplies(self, _, mock_git_clean_context):
"""Tests whether we can apply a single patch for a given svn_version."""
@@ -253,53 +209,6 @@
patch_manager.GitBisectionCode.SKIP,
)
- @mock.patch("patch_utils.git_clean_context", mock.MagicMock)
- def testUpdateVersionRanges(self):
- """Test the UpdateVersionRanges function."""
- with tempfile.TemporaryDirectory(
- prefix="patch_manager_unittest"
- ) as dirname:
- dirpath = Path(dirname)
- patches = [
- patch_utils.PatchEntry(
- workdir=dirpath,
- rel_patch_path="x.patch",
- metadata=None,
- platforms=None,
- version_range={
- "from": 0,
- "until": 2,
- },
- ),
- patch_utils.PatchEntry(
- workdir=dirpath,
- rel_patch_path="y.patch",
- metadata=None,
- platforms=None,
- version_range={
- "from": 0,
- "until": 2,
- },
- ),
- ]
- patches[0].apply = mock.MagicMock(
- return_value=patch_utils.PatchResult(
- succeeded=False, failed_hunks={"a/b/c": []}
- )
- )
- patches[1].apply = mock.MagicMock(
- return_value=patch_utils.PatchResult(succeeded=True)
- )
- results = patch_manager.UpdateVersionRangesWithEntries(
- 1, dirpath, patches
- )
- # We should only have updated the version_range of the first patch,
- # as that one failed to apply.
- self.assertEqual(len(results), 1)
- self.assertEqual(results[0].version_range, {"from": 0, "until": 1})
- self.assertEqual(patches[0].version_range, {"from": 0, "until": 1})
- self.assertEqual(patches[1].version_range, {"from": 0, "until": 2})
-
if __name__ == "__main__":
unittest.main()
diff --git a/llvm_tools/patch_utils.py b/llvm_tools/patch_utils.py
index ca912f2..b86e192 100644
--- a/llvm_tools/patch_utils.py
+++ b/llvm_tools/patch_utils.py
@@ -12,7 +12,7 @@
import re
import subprocess
import sys
-from typing import Any, Dict, IO, List, Optional, Tuple, Union
+from typing import Any, Dict, IO, Iterable, List, Optional, Tuple, Union
CHECKED_FILE_RE = re.compile(r"^checking file\s+(.*)$")
@@ -456,3 +456,137 @@
yield
finally:
clean_src_tree(git_root_dir)
+
+
+def _write_json_changes(patches: List[Dict[str, Any]], file_io: IO[str]):
+ """Write JSON changes to file, does not acquire new file lock."""
+ json.dump(patches, file_io, indent=4, separators=(",", ": "))
+ # Need to add a newline as json.dump omits it.
+ file_io.write("\n")
+
+
+def update_version_ranges(
+ svn_version: int, llvm_src_dir: Path, patches_json_fp: Path
+) -> PatchInfo:
+ """Reduce the version ranges of failing patches.
+
+ Patches which fail to apply will have their 'version_range.until'
+ field reduced to the passed in svn_version.
+
+ Modifies the contents of patches_json_fp.
+
+ Args:
+ svn_version: LLVM revision number.
+ llvm_src_dir: llvm-project directory path.
+ patches_json_fp: Filepath to the PATCHES.json file.
+
+ Returns:
+ PatchInfo for applied and disabled patches.
+ """
+ with patches_json_fp.open(encoding="utf-8") as f:
+ patch_entries = json_to_patch_entries(
+ patches_json_fp.parent,
+ f,
+ )
+ modified_entries, applied_patches = update_version_ranges_with_entries(
+ svn_version, llvm_src_dir, patch_entries
+ )
+ with atomic_write(patches_json_fp, encoding="utf-8") as f:
+ _write_json_changes([p.to_dict() for p in patch_entries], f)
+ for entry in modified_entries:
+ print(
+ f"Stopped applying {entry.rel_patch_path} ({entry.title()}) "
+ f"for r{svn_version}"
+ )
+ return PatchInfo(
+ non_applicable_patches=[],
+ applied_patches=[p.rel_patch_path for p in applied_patches],
+ failed_patches=[],
+ disabled_patches=[p.rel_patch_path for p in modified_entries],
+ removed_patches=[],
+ modified_metadata=patches_json_fp if modified_entries else None,
+ )
+
+
+def update_version_ranges_with_entries(
+ svn_version: int,
+ llvm_src_dir: Path,
+ patch_entries: Iterable[PatchEntry],
+) -> Tuple[List[PatchEntry], List[PatchEntry]]:
+ """Test-able helper for UpdateVersionRanges.
+
+ Args:
+ svn_version: LLVM revision number.
+ llvm_src_dir: llvm-project directory path.
+ patch_entries: PatchEntry objects to modify.
+
+ Returns:
+ Tuple of (modified entries, applied patches)
+
+ Post:
+ Modifies patch_entries in place.
+ """
+ modified_entries: List[PatchEntry] = []
+ applied_patches: List[PatchEntry] = []
+ with git_clean_context(llvm_src_dir):
+ for pe in patch_entries:
+ test_result = pe.test_apply(llvm_src_dir)
+ if not test_result:
+ if pe.version_range is None:
+ pe.version_range = {}
+ pe.version_range["until"] = svn_version
+ modified_entries.append(pe)
+ else:
+ # We have to actually apply the patch so that future patches
+ # will stack properly.
+ if not pe.apply(llvm_src_dir).succeeded:
+ raise RuntimeError(
+ "Could not apply patch that dry ran successfully"
+ )
+ applied_patches.append(pe)
+
+ return modified_entries, applied_patches
+
+
+def remove_old_patches(
+ svn_version: int, llvm_src_dir: Path, patches_json_fp: Path
+) -> PatchInfo:
+ """Remove patches that don't and will never apply for the future.
+
+ Patches are determined to be "old" via the "is_old" method for
+ each patch entry.
+
+ Args:
+ svn_version: LLVM SVN version.
+ llvm_src_dir: LLVM source directory.
+ patches_json_fp: Location to edit patches on.
+
+ Returns:
+ PatchInfo for modified patches.
+ """
+ with patches_json_fp.open(encoding="utf-8") as f:
+ patches_list = json.load(f)
+ patch_entries = (
+ PatchEntry.from_dict(llvm_src_dir, elem) for elem in patches_list
+ )
+ 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(patches_json_fp, encoding="utf-8") as f:
+ _write_json_changes(filtered_entries, f)
+ 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()}")
+
+ patches_dir_path = llvm_src_dir / patches_json_fp.parent
+ return PatchInfo(
+ non_applicable_patches=[],
+ applied_patches=[],
+ failed_patches=[],
+ disabled_patches=[],
+ removed_patches=[
+ patches_dir_path / p.rel_patch_path for p in removed_entries
+ ],
+ modified_metadata=patches_json_fp if removed_entries else None,
+ )
diff --git a/llvm_tools/patch_utils_unittest.py b/llvm_tools/patch_utils_unittest.py
index 8fe45c2..b8c2139 100755
--- a/llvm_tools/patch_utils_unittest.py
+++ b/llvm_tools/patch_utils_unittest.py
@@ -6,9 +6,11 @@
"""Unit tests for the patch_utils.py file."""
import io
+import json
from pathlib import Path
import subprocess
import tempfile
+from typing import Callable
import unittest
from unittest import mock
@@ -99,7 +101,7 @@
def test_can_parse_from_json(self):
"""Test that patches be loaded from json."""
- json = """
+ patches_json = """
[
{
"metadata": {},
@@ -121,7 +123,7 @@
}
]
"""
- result = pu.json_to_patch_entries(Path(), io.StringIO(json))
+ result = pu.json_to_patch_entries(Path(), io.StringIO(patches_json))
self.assertEqual(len(result), 4)
def test_parsed_hunks(self):
@@ -215,6 +217,97 @@
test_file.write_text("abc")
self.assertTrue(pu.is_git_dirty(dirpath))
+ @mock.patch("patch_utils.git_clean_context", mock.MagicMock)
+ def test_update_version_ranges(self):
+ """Test the UpdateVersionRanges function."""
+ with tempfile.TemporaryDirectory(
+ prefix="patch_manager_unittest"
+ ) as dirname:
+ dirpath = Path(dirname)
+ patches = [
+ pu.PatchEntry(
+ workdir=dirpath,
+ rel_patch_path="x.patch",
+ metadata=None,
+ platforms=None,
+ version_range={
+ "from": 0,
+ "until": 2,
+ },
+ ),
+ pu.PatchEntry(
+ workdir=dirpath,
+ rel_patch_path="y.patch",
+ metadata=None,
+ platforms=None,
+ version_range={
+ "from": 0,
+ "until": 2,
+ },
+ ),
+ ]
+ patches[0].apply = mock.MagicMock(
+ return_value=pu.PatchResult(
+ succeeded=False, failed_hunks={"a/b/c": []}
+ )
+ )
+ patches[1].apply = mock.MagicMock(
+ return_value=pu.PatchResult(succeeded=True)
+ )
+ results, _ = pu.update_version_ranges_with_entries(
+ 1, dirpath, patches
+ )
+ # We should only have updated the version_range of the first patch,
+ # as that one failed to apply.
+ self.assertEqual(len(results), 1)
+ self.assertEqual(results[0].version_range, {"from": 0, "until": 1})
+ self.assertEqual(patches[0].version_range, {"from": 0, "until": 1})
+ self.assertEqual(patches[1].version_range, {"from": 0, "until": 2})
+
+ @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",
+ },
+ "platforms": [
+ "chromiumos",
+ ],
+ "rel_patch_path": "x/y/z",
+ "version_range": {
+ "from": 4,
+ "until": 5,
+ },
+ }
+ 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)
+
+ with tempfile.TemporaryDirectory(
+ prefix="patch_utils_unittest"
+ ) as dirname:
+ for r, a in cases:
+ _t(dirname, r, a)
+
@staticmethod
def _default_json_dict():
return {
diff --git a/llvm_tools/update_chromeos_llvm_hash.py b/llvm_tools/update_chromeos_llvm_hash.py
index c52c732..75c6ce6 100755
--- a/llvm_tools/update_chromeos_llvm_hash.py
+++ b/llvm_tools/update_chromeos_llvm_hash.py
@@ -411,7 +411,7 @@
"""Removes the patches from $FILESDIR of a package.
Args:
- patches: A list of absolute pathes of patches to remove
+ patches: A list of absolute paths of patches to remove
Raises:
ValueError: Failed to remove a patch in $FILESDIR.
@@ -728,13 +728,26 @@
src_path = Path(dirname)
with patch_utils.git_clean_context(src_path):
- patches_info = patch_utils.apply_all_from_json(
- svn_version=svn_version,
- llvm_src_dir=src_path,
- patches_json_fp=patches_json_fp,
- continue_on_failure=mode
- == failure_modes.FailureModes.CONTINUE,
- )
+ if (
+ mode == failure_modes.FailureModes.FAIL
+ or mode == failure_modes.FailureModes.CONTINUE
+ ):
+ patches_info = patch_utils.apply_all_from_json(
+ svn_version=svn_version,
+ llvm_src_dir=src_path,
+ patches_json_fp=patches_json_fp,
+ continue_on_failure=mode
+ == failure_modes.FailureModes.CONTINUE,
+ )
+ elif mode == failure_modes.FailureModes.REMOVE_PATCHES:
+ patches_info = patch_utils.remove_old_patches(
+ svn_version, src_path, patches_json_fp
+ )
+ elif mode == failure_modes.FailureModes.DISABLE_PATCHES:
+ patches_info = patch_utils.update_version_ranges(
+ svn_version, src_path, patches_json_fp
+ )
+
package_info[cur_package] = patches_info._asdict()
return package_info
