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>
diff --git a/llvm_tools/README.md b/llvm_tools/README.md
index fabb37a..f15040e 100644
--- a/llvm_tools/README.md
+++ b/llvm_tools/README.md
@@ -472,18 +472,20 @@
**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
similarity index 70%
rename from llvm_tools/cherrypick_cl.py
rename to llvm_tools/get_upstream_patch.py
index ebf7c0f..e1f321d 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 @@
"""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 @@
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 @@
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 @@
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 @@
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 @@
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 @@
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 @@
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 d8566c3..1a7bc79 100755
--- a/llvm_tools/nightly_revert_checker.py
+++ b/llvm_tools/nightly_revert_checker.py
@@ -29,7 +29,7 @@
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 @@
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 5efe2ae..a8ab419 100755
--- a/llvm_tools/nightly_revert_checker_test.py
+++ b/llvm_tools/nightly_revert_checker_test.py
@@ -12,8 +12,8 @@
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 @@
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 @@
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',
