diff options
author | Jian Cai <jiancai@google.com> | 2021-04-29 14:23:49 -0700 |
---|---|---|
committer | Jian Cai <jiancai@google.com> | 2021-05-10 20:17:15 +0000 |
commit | fd0b18e0c6c4736b4818b7d30546ae37b17dee8f (patch) | |
tree | 82f90a7c9177b26f60145b7edb6ac250e200a157 | |
parent | 6fed0d0594c243585878e3bdea112838785c2a31 (diff) | |
download | toolchain-utils-fd0b18e0c6c4736b4818b7d30546ae37b17dee8f.tar.gz |
llvm_tool: support differential reviews in the cherry-pick tool.
Rename cherrypick_cl.py to get_llvm_upstream.py, and add --differential
option for creating local patches based on differential reviews. This
should help testing experimental Clang/LLVM patches on ChromeOS.
BUG=chromium:1202311
TEST=Successfully run the script with --sha $SHA --differential
$DIFFERENTIAL_REVISION
Change-Id: I7bd1704d1079352db52fc4690ef453436fe854de
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/third_party/toolchain-utils/+/2861111
Tested-by: Jian Cai <jiancai@google.com>
Reviewed-by: George Burgess <gbiv@chromium.org>
-rw-r--r-- | llvm_tools/README.md | 10 | ||||
-rwxr-xr-x | llvm_tools/get_upstream_patch.py (renamed from llvm_tools/cherrypick_cl.py) | 129 | ||||
-rwxr-xr-x | llvm_tools/nightly_revert_checker.py | 8 | ||||
-rwxr-xr-x | llvm_tools/nightly_revert_checker_test.py | 8 |
4 files changed, 104 insertions, 51 deletions
diff --git a/llvm_tools/README.md b/llvm_tools/README.md index fabb37a9..f15040ec 100644 --- a/llvm_tools/README.md +++ b/llvm_tools/README.md @@ -472,18 +472,20 @@ r387778 **Tip**: if you put a symlink called `git-llvm-rev` to this script somewhere on your `$PATH`, you can also use it as `git llvm-rev`. -### `cherrypick_cl.py` +### `get_upstream_patch.py` #### Usage -This script updates the proper ChromeOS packages with an LLVM cherrypick of your choosing, and -copies the cherrypick into patch folders of the packages. +This script updates the proper ChromeOS packages with LLVM patches of your choosing, and +copies the patches into patch folders of the packages. This tool supports both git hash +of commits as well as differential reviews. Usage: ``` -./cherrypick_cl.py --chroot_path /abs/path/to/chroot --start_sha llvm +./get_upstream_patch.py --chroot_path /abs/path/to/chroot --start_sha llvm --sha 174c3eb69f19ff2d6a3eeae31d04afe77e62c021 --sha 174c3eb69f19ff2d6a3eeae31d04afe77e62c021 +--differential D123456 ``` It tries to autodetect a lot of things (e.g., packages changed by each sha, diff --git a/llvm_tools/cherrypick_cl.py b/llvm_tools/get_upstream_patch.py index ebf7c0f3..e1f321de 100755 --- a/llvm_tools/cherrypick_cl.py +++ b/llvm_tools/get_upstream_patch.py @@ -6,7 +6,7 @@ # pylint: disable=cros-logging-import -"""Adds a cherrypick to LLVM's PATCHES.json.""" +"""Get an upstream patch to LLVM's PATCHES.json.""" from __future__ import print_function @@ -31,14 +31,38 @@ class CherrypickError(ValueError): """A ValueError that highlights the cherry-pick has been seen before""" -def add_cherrypick(patches_json_path: str, patches_dir: str, - relative_patches_dir: str, start_version: git_llvm_rev.Rev, - llvm_dir: str, rev: git_llvm_rev.Rev, sha: str, - package: str): +def add_patch(patches_json_path: str, patches_dir: str, + relative_patches_dir: str, start_version: git_llvm_rev.Rev, + llvm_dir: str, rev: t.Union[git_llvm_rev.Rev, str], sha: str, + package: str): + """Gets the start and end intervals in 'json_file'. + + Args: + patches_json_path: The absolute path to PATCHES.json. + patches_dir: The aboslute path to the directory patches are in. + relative_patches_dir: The relative path to PATCHES.json. + start_version: The base LLVM revision this patch applies to. + llvm_dir: The path to LLVM checkout. + rev: An LLVM revision (git_llvm_rev.Rev) for a cherrypicking, or a + differential revision (str) otherwise. + sha: The LLVM git sha that corresponds to the patch. For differential + revisions, the git sha from the local commit created by 'arc patch' + is used. + package: The LLVM project name this patch applies to. + + Raises: + CherrypickError: A ValueError that highlights the cherry-pick has been + seen before. + """ + with open(patches_json_path, encoding='utf-8') as f: patches_json = json.load(f) - file_name = sha + '.patch' + is_cherrypick = isinstance(rev, git_llvm_rev.Rev) + if is_cherrypick: + file_name = f'{sha}.patch' + else: + file_name = f'{rev}.patch' rel_patch_path = os.path.join(relative_patches_dir, file_name) for p in patches_json: @@ -46,9 +70,11 @@ def add_cherrypick(patches_json_path: str, patches_dir: str, if rel_path == rel_patch_path: raise CherrypickError( f'Patch at {rel_path} already exists in PATCHES.json') - if sha in rel_path: - logging.warning( - 'Similarly-named patch already exists in PATCHES.json: %r', rel_path) + if is_cherrypick: + if sha in rel_path: + logging.warning( + 'Similarly-named patch already exists in PATCHES.json: %r', + rel_path) with open(os.path.join(patches_dir, file_name), 'wb') as f: cmd = ['git', 'show', sha] @@ -63,12 +89,14 @@ def add_cherrypick(patches_json_path: str, patches_dir: str, commit_subject = subprocess.check_output( ['git', 'log', '-n1', '--format=%s', sha], cwd=llvm_dir, encoding='utf-8') - patches_json.append({ + patch_metadata = { 'comment': commit_subject.strip(), 'rel_patch_path': rel_patch_path, 'start_version': start_version.number, - 'end_version': rev.number, - }) + } + if isinstance(rev, git_llvm_rev.Rev): + patch_metadata['end_version'] = rev.number + patches_json.append(patch_metadata) temp_file = patches_json_path + '.tmp' with open(temp_file, 'w', encoding='utf-8') as f: @@ -148,19 +176,19 @@ def get_package_names(sha: str, llvm_dir: str) -> list: return packages -def add_cherrypicks_for_packages(packages: t.List[str], symlinks: t.List[str], - start_rev: git_llvm_rev.Rev, - rev: git_llvm_rev.Rev, sha: str, - llvm_config: git_llvm_rev.LLVMConfig): +def create_patch_for_packages(packages: t.List[str], symlinks: t.List[str], + start_rev: git_llvm_rev.Rev, + rev: t.Union[git_llvm_rev.Rev, str], sha: str, + llvm_dir: str): """Create a patch and add its metadata for each package""" for package, symlink in zip(packages, symlinks): symlink_dir = os.path.dirname(symlink) patches_json_path = os.path.join(symlink_dir, 'files/PATCHES.json') relative_patches_dir = 'cherry' if package == 'llvm' else '' patches_dir = os.path.join(symlink_dir, 'files', relative_patches_dir) - logging.info('Cherrypicking %s (%s) into %s', rev, sha, package) - add_cherrypick(patches_json_path, patches_dir, relative_patches_dir, - start_rev, llvm_config.dir, rev, sha, package) + logging.info('Getting %s (%s) into %s', rev, sha, package) + add_patch(patches_json_path, patches_dir, relative_patches_dir, start_rev, + llvm_dir, rev, sha, package) def make_cl(symlinks_to_uprev: t.List[str], llvm_symlink_dir: str, branch: str, @@ -185,23 +213,33 @@ def resolve_symbolic_sha(start_sha: str, llvm_symlink_dir: str) -> str: return start_sha -def find_commits_and_make_cl(chroot_path: str, shas: t.List[str], +def find_patches_and_make_cl(chroot_path: str, patches: t.List[str], start_rev: git_llvm_rev.Rev, llvm_config: git_llvm_rev.LLVMConfig, llvm_symlink_dir: str, create_cl: bool, reviewers: t.Optional[t.List[str]], cc: t.Optional[t.List[str]]): if create_cl: - branch = f'cherry-pick-{datetime.now().strftime("%Y%m%d%H%M%S%f")}' + branch = f'get-upstream-{datetime.now().strftime("%Y%m%d%H%M%S%f")}' git.CreateBranch(llvm_symlink_dir, branch) symlinks_to_uprev = [] commit_messages = [ - 'llvm: cherry-pick CLs from upstream\n', + 'llvm: get patches from upstream\n', ] - for sha in shas: - sha = resolve_llvm_ref(llvm_config.dir, sha) - rev = git_llvm_rev.translate_sha_to_rev(llvm_config, sha) + for patch in patches: + # git hash should only have lower-case letters + is_differential = patch.startswith('D') + if is_differential: + subprocess.check_output( + ['arc', 'patch', '--nobranch', '--revision', patch], + cwd=llvm_config.dir, + ) + sha = resolve_llvm_ref(llvm_config.dir, 'HEAD') + rev = patch + else: + sha = resolve_llvm_ref(llvm_config.dir, patch) + rev = git_llvm_rev.translate_sha_to_rev(llvm_config, sha) # Find out the llvm projects changed in this commit packages = get_package_names(sha, llvm_config.dir) # Find out the ebuild symlinks of the corresponding ChromeOS packages @@ -210,29 +248,38 @@ def find_commits_and_make_cl(chroot_path: str, shas: t.List[str], for package in packages ]) symlinks = chroot.ConvertChrootPathsToAbsolutePaths(chroot_path, symlinks) - - add_cherrypicks_for_packages(packages, symlinks, start_rev, rev, sha, - llvm_config) + # Create a local patch for all the affected llvm projects + create_patch_for_packages(packages, symlinks, start_rev, rev, sha, + llvm_config.dir) if create_cl: symlinks_to_uprev.extend(symlinks) + + if is_differential: + msg = f'\n\nreviews.llvm.org/{patch}\n' + else: + msg = f'\n\nreviews.llvm.org/rG{sha}\n' commit_messages.extend([ - '\n\nreviews.llvm.org/rG%s\n' % sha, + msg, subprocess.check_output(['git', 'log', '-n1', '--oneline', sha], cwd=llvm_config.dir, encoding='utf-8') ]) + if is_differential: + subprocess.check_output(['git', 'reset', '--hard', 'HEAD^'], + cwd=llvm_config.dir) + if create_cl: make_cl(symlinks_to_uprev, llvm_symlink_dir, branch, commit_messages, reviewers, cc) -def do_cherrypick(chroot_path: str, - create_cl: bool, - start_sha: str, - shas: t.List[str], - reviewers: t.List[str] = None, - cc: t.List[str] = None): +def get_from_upstream(chroot_path: str, + create_cl: bool, + start_sha: str, + patches: t.List[str], + reviewers: t.List[str] = None, + cc: t.List[str] = None): llvm_symlink = chroot.ConvertChrootPathsToAbsolutePaths( chroot_path, chroot.GetChrootEbuildPaths(chroot_path, ['sys-devel/llvm']))[0] @@ -253,9 +300,9 @@ def do_cherrypick(chroot_path: str, remote='origin', dir=get_llvm_hash.GetAndUpdateLLVMProjectInLLVMTools()) start_sha = resolve_llvm_ref(llvm_config.dir, start_sha) - find_commits_and_make_cl( + find_patches_and_make_cl( chroot_path=chroot_path, - shas=shas, + patches=patches, start_rev=git_llvm_rev.translate_sha_to_rev(llvm_config, start_sha), llvm_config=llvm_config, llvm_symlink_dir=llvm_symlink_dir, @@ -288,17 +335,21 @@ def main(): action='append', help='The LLVM git SHA to cherry-pick.') parser.add_argument( + '--differential', + action='append', + help='The LLVM differential revision to apply. Example: D1234') + parser.add_argument( '--create_cl', default=False, action='store_true', help='Automatically create a CL if specified') args = parser.parse_args() - do_cherrypick( + get_from_upstream( chroot_path=args.chroot_path, create_cl=args.create_cl, start_sha=args.start_sha, - shas=args.sha) + patches=args.sha + args.differential) if __name__ == '__main__': diff --git a/llvm_tools/nightly_revert_checker.py b/llvm_tools/nightly_revert_checker.py index d8566c3f..1a7bc793 100755 --- a/llvm_tools/nightly_revert_checker.py +++ b/llvm_tools/nightly_revert_checker.py @@ -29,7 +29,7 @@ import cros_utils.tiny_render as tiny_render import get_llvm_hash import git_llvm_rev import revert_checker -import cherrypick_cl +import get_upstream_patch State = t.Any @@ -232,14 +232,14 @@ def do_cherrypick(chroot_path: str, llvm_dir: str, seen.add(friendly_name) for sha, reverted_sha in reverts: try: - cherrypick_cl.do_cherrypick( + get_upstream_patch.get_from_upstream( chroot_path=chroot_path, create_cl=True, start_sha=reverted_sha, - shas=[sha], + patches=[sha], reviewers=reviewers, cc=cc) - except cherrypick_cl.CherrypickError as e: + except get_upstream_patch.CherrypickError as e: logging.info('%s, skipping...', str(e)) return new_state diff --git a/llvm_tools/nightly_revert_checker_test.py b/llvm_tools/nightly_revert_checker_test.py index 5efe2aeb..a8ab4195 100755 --- a/llvm_tools/nightly_revert_checker_test.py +++ b/llvm_tools/nightly_revert_checker_test.py @@ -12,8 +12,8 @@ import io import unittest from unittest.mock import patch -import cherrypick_cl import cros_utils.tiny_render as tiny_render +import get_upstream_patch import nightly_revert_checker import revert_checker @@ -156,7 +156,7 @@ class Test(unittest.TestCase): self.assertIn('Failed to detect SHAs', str(e.exception)) @patch('revert_checker.find_reverts') - @patch('cherrypick_cl.do_cherrypick') + @patch('get_upstream_patch.get_from_upstream') def test_do_cherrypick_is_called(self, do_cherrypick, find_reverts): find_reverts.return_value = [ revert_checker.Revert('12345abcdef', 'fedcba54321') @@ -173,13 +173,13 @@ class Test(unittest.TestCase): find_reverts.assert_called_once() @patch('revert_checker.find_reverts') - @patch('cherrypick_cl.do_cherrypick') + @patch('get_upstream_patch.get_from_upstream') def test_do_cherrypick_handles_cherrypick_error(self, do_cherrypick, find_reverts): find_reverts.return_value = [ revert_checker.Revert('12345abcdef', 'fedcba54321') ] - do_cherrypick.side_effect = cherrypick_cl.CherrypickError( + do_cherrypick.side_effect = get_upstream_patch.CherrypickError( 'Patch at 12345abcdef already exists in PATCHES.json') nightly_revert_checker.do_cherrypick( chroot_path='/path/to/chroot', |