diff options
author | Adrian Dole <adriandole@google.com> | 2022-10-05 22:11:44 +0000 |
---|---|---|
committer | Chromeos LUCI <chromeos-scoped@luci-project-accounts.iam.gserviceaccount.com> | 2022-10-14 17:34:29 +0000 |
commit | b20b2304b692de6a423acb0035303d53c4ee3e3d (patch) | |
tree | cd097883e36d7aca59ed444e5a3036fce6cbb2c0 | |
parent | 5971b3261cd08aaa8821f2b57a6e1ad2c0ac511e (diff) | |
download | toolchain-utils-b20b2304b692de6a423acb0035303d53c4ee3e3d.tar.gz |
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>
-rwxr-xr-x | llvm_tools/patch_manager.py | 117 | ||||
-rwxr-xr-x | llvm_tools/patch_manager_unittest.py | 91 | ||||
-rw-r--r-- | llvm_tools/patch_utils.py | 136 | ||||
-rwxr-xr-x | llvm_tools/patch_utils_unittest.py | 97 | ||||
-rwxr-xr-x | llvm_tools/update_chromeos_llvm_hash.py | 29 |
5 files changed, 258 insertions, 212 deletions
diff --git a/llvm_tools/patch_manager.py b/llvm_tools/patch_manager.py index 11e82227..4d4e8385 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 @@ def GetHEADSVNVersion(src_path): 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 @@ def GetCommitHashesForBisection(src_path, good_svn_version, bad_svn_version): 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 @@ def main(sys_argv: List[str]): 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 19c2d8af..42697d91 100755 --- a/llvm_tools/patch_manager_unittest.py +++ b/llvm_tools/patch_manager_unittest.py @@ -62,50 +62,6 @@ class PatchManagerTest(unittest.TestCase): 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 @@ class PatchManagerTest(unittest.TestCase): 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 ca912f2b..b86e1925 100644 --- a/llvm_tools/patch_utils.py +++ b/llvm_tools/patch_utils.py @@ -12,7 +12,7 @@ from pathlib import Path 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 @@ def git_clean_context(git_root_dir: Path): 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 8fe45c2c..b8c21390 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 @@ class TestPatchUtils(unittest.TestCase): def test_can_parse_from_json(self): """Test that patches be loaded from json.""" - json = """ + patches_json = """ [ { "metadata": {}, @@ -121,7 +123,7 @@ class TestPatchUtils(unittest.TestCase): } ] """ - 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 @@ Hunk #1 SUCCEEDED at 96 with fuzz 1. 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 c52c7328..75c6ce6c 100755 --- a/llvm_tools/update_chromeos_llvm_hash.py +++ b/llvm_tools/update_chromeos_llvm_hash.py @@ -411,7 +411,7 @@ def RemovePatchesFromFilesDir(patches): """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 @@ def UpdatePackagesPatchMetadataFile( 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 |