'bsalomon@google.com',
'djsollen@chromium.org',
'djsollen@google.com',
+ 'hcm@chromium.org',
+ 'hcm@google.com',
)
AUTHORS_FILE_NAME = 'AUTHORS'
# Path to CQ bots feature is described in https://bug.skia.org/4364
PATH_PREFIX_TO_EXTRA_TRYBOTS = {
- # pylint: disable=line-too-long
- 'cmake/': 'master.client.skia.compile:Build-Mac-Clang-x86_64-Release-CMake-Trybot,Build-Ubuntu-GCC-x86_64-Release-CMake-Trybot',
- # pylint: disable=line-too-long
- 'src/opts/': 'master.client.skia:Test-Ubuntu-GCC-GCE-CPU-AVX2-x86_64-Release-SKNX_NO_SIMD-Trybot',
-
- 'include/private/SkAtomics.h': ('master.client.skia:'
- 'Test-Ubuntu-GCC-GCE-CPU-AVX2-x86_64-Release-TSAN-Trybot,'
- 'Test-Ubuntu-GCC-Golo-GPU-GT610-x86_64-Release-TSAN-Trybot'
+ 'src/opts/':
+ 'skia.primary:Test-Ubuntu-GCC-GCE-CPU-AVX2-x86_64-Release-SKNX_NO_SIMD',
+ 'include/private/SkAtomics.h': ('skia.primary:'
+ 'Test-Ubuntu-Clang-GCE-CPU-AVX2-x86_64-Release-TSAN,'
+ 'Test-Ubuntu-Clang-Golo-GPU-GT610-x86_64-Release-TSAN-Trybot'
),
# Below are examples to show what is possible with this feature.
return results
-def _RecipeSimulationTest(input_api, output_api):
- """Run the recipe simulation test."""
+def _InfraTests(input_api, output_api):
+ """Run the infra tests."""
results = []
if not any(f.LocalPath().startswith('infra')
for f in input_api.AffectedFiles()):
return results
- recipes_py = os.path.join('infra', 'bots', 'recipes.py')
- cmd = ['python', recipes_py, 'simulation_test']
+ cmd = ['python', os.path.join('infra', 'bots', 'infra_tests.py')]
try:
subprocess.check_output(cmd)
except subprocess.CalledProcessError as e:
'`%s` failed:\n%s' % (' '.join(cmd), e.output)))
return results
+
def _CheckGNFormatted(input_api, output_api):
"""Make sure any .gn files we're changing have been formatted."""
results = []
for f in input_api.AffectedFiles():
- if not f.LocalPath().endswith('.gn'):
+ if (not f.LocalPath().endswith('.gn') and
+ not f.LocalPath().endswith('.gni')):
continue
- cmd = ['gn', 'format', '--dry-run', f.LocalPath()]
+ gn = 'gn.bat' if 'win32' in sys.platform else 'gn'
+ cmd = [gn, 'format', '--dry-run', f.LocalPath()]
try:
subprocess.check_output(cmd)
except subprocess.CalledProcessError:
"""Presubmit checks common to upload and commit."""
results = []
sources = lambda x: (x.LocalPath().endswith('.h') or
- x.LocalPath().endswith('.gypi') or
- x.LocalPath().endswith('.gyp') or
x.LocalPath().endswith('.py') or
x.LocalPath().endswith('.sh') or
x.LocalPath().endswith('.m') or
results = []
results.extend(_CommonChecks(input_api, output_api))
# Run on upload, not commit, since the presubmit bot apparently doesn't have
- # coverage installed.
- results.extend(_RecipeSimulationTest(input_api, output_api))
+ # coverage or Go installed.
+ results.extend(_InfraTests(input_api, output_api))
+
results.extend(_CheckGNFormatted(input_api, output_api))
return results
return tree_status_results
+class CodeReview(object):
+ """Abstracts which codereview tool is used for the specified issue."""
+
+ def __init__(self, input_api):
+ self._issue = input_api.change.issue
+ self._gerrit = input_api.gerrit
+ self._rietveld_properties = None
+ if not self._gerrit:
+ self._rietveld_properties = input_api.rietveld.get_issue_properties(
+ issue=int(self._issue), messages=True)
+
+ def GetOwnerEmail(self):
+ if self._gerrit:
+ return self._gerrit.GetChangeOwner(self._issue)
+ else:
+ return self._rietveld_properties['owner_email']
+
+ def GetSubject(self):
+ if self._gerrit:
+ return self._gerrit.GetChangeInfo(self._issue)['subject']
+ else:
+ return self._rietveld_properties['subject']
+
+ def GetDescription(self):
+ if self._gerrit:
+ return self._gerrit.GetChangeDescription(self._issue)
+ else:
+ return self._rietveld_properties['description']
+
+ def IsDryRun(self):
+ if self._gerrit:
+ return self._gerrit.GetChangeInfo(
+ self._issue)['labels']['Commit-Queue'].get('value', 0) == 1
+ else:
+ return self._rietveld_properties['cq_dry_run']
+
+ def GetReviewers(self):
+ if self._gerrit:
+ code_review_label = (
+ self._gerrit.GetChangeInfo(self._issue)['labels']['Code-Review'])
+ return [r['email'] for r in code_review_label.get('all', [])]
+ else:
+ return self._rietveld_properties['reviewers']
+
+ def GetApprovers(self):
+ approvers = []
+ if self._gerrit:
+ code_review_label = (
+ self._gerrit.GetChangeInfo(self._issue)['labels']['Code-Review'])
+ for m in code_review_label.get('all', []):
+ if m.get("value") == 1:
+ approvers.append(m["email"])
+ else:
+ for m in self._rietveld_properties.get('messages', []):
+ if 'lgtm' in m['text'].lower():
+ approvers.append(m['sender'])
+ return approvers
+
+
def _CheckOwnerIsInAuthorsFile(input_api, output_api):
results = []
- issue = input_api.change.issue
- if issue and input_api.rietveld:
- issue_properties = input_api.rietveld.get_issue_properties(
- issue=int(issue), messages=False)
- owner_email = issue_properties['owner_email']
+ if input_api.change.issue:
+ cr = CodeReview(input_api)
+ owner_email = cr.GetOwnerEmail()
try:
authors_content = ''
for line in open(AUTHORS_FILE_NAME):
return results
lgtm_from_owner = False
- issue = input_api.change.issue
- if issue and input_api.rietveld:
- issue_properties = input_api.rietveld.get_issue_properties(
- issue=int(issue), messages=True)
- if re.match(REVERT_CL_SUBJECT_PREFIX, issue_properties['subject'], re.I):
+ if input_api.change.issue:
+ cr = CodeReview(input_api)
+
+ if re.match(REVERT_CL_SUBJECT_PREFIX, cr.GetSubject(), re.I):
# It is a revert CL, ignore the public api owners check.
return results
- if issue_properties['cq_dry_run']:
+ if cr.IsDryRun():
# Ignore public api owners check for dry run CLs since they are not
# going to be committed.
return results
- match = re.search(r'^TBR=(.*)$', issue_properties['description'], re.M)
- if match:
- tbr_entries = match.group(1).strip().split(',')
- for owner in PUBLIC_API_OWNERS:
- if owner in tbr_entries or owner.split('@')[0] in tbr_entries:
- # If an owner is specified in the TBR= line then ignore the public
- # api owners check.
+ if input_api.gerrit:
+ for reviewer in cr.GetReviewers():
+ if reviewer in PUBLIC_API_OWNERS:
+ # If an owner is specified as an reviewer in Gerrit then ignore the
+ # public api owners check.
return results
-
- if issue_properties['owner_email'] in PUBLIC_API_OWNERS:
+ else:
+ match = re.search(r'^TBR=(.*)$', cr.GetDescription(), re.M)
+ if match:
+ tbr_section = match.group(1).strip().split(' ')[0]
+ tbr_entries = tbr_section.split(',')
+ for owner in PUBLIC_API_OWNERS:
+ if owner in tbr_entries or owner.split('@')[0] in tbr_entries:
+ # If an owner is specified in the TBR= line then ignore the public
+ # api owners check.
+ return results
+
+ if cr.GetOwnerEmail() in PUBLIC_API_OWNERS:
# An owner created the CL that is an automatic LGTM.
lgtm_from_owner = True
- messages = issue_properties.get('messages')
- if messages:
- for message in messages:
- if (message['sender'] in PUBLIC_API_OWNERS and
- 'lgtm' in message['text'].lower()):
- # Found an lgtm in a message from an owner.
- lgtm_from_owner = True
- break
+ for approver in cr.GetApprovers():
+ if approver in PUBLIC_API_OWNERS:
+ # Found an lgtm in a message from an owner.
+ lgtm_from_owner = True
+ break
if not lgtm_from_owner:
results.append(
output_api.PresubmitError(
"If this CL adds to or changes Skia's public API, you need an LGTM "
"from any of %s. If this CL only removes from or doesn't change "
- "Skia's public API, please add a short note to the CL saying so "
- "and add one of those reviewers on a TBR= line. If you don't know "
- "if this CL affects Skia's public API, treat it like it does."
- % str(PUBLIC_API_OWNERS)))
+ "Skia's public API, please add a short note to the CL saying so. "
+ "Add one of the owners as a reviewer to your CL. For Rietveld CLs "
+ "you also need to add one of those owners on a TBR= line. If you "
+ "don't know if this CL affects Skia's public API, treat it like it "
+ "does." % str(PUBLIC_API_OWNERS)))
return results
"""git cl upload will call this hook after the issue is created/modified.
This hook does the following:
- * Adds a link to the CL's Gold trybot results.
* Adds a link to preview docs changes if there are any docs changes in the CL.
* Adds 'NOTRY=true' if the CL contains only docs changes.
* Adds 'NOTREECHECKS=true' for non master branch changes since they do not
break
issue = cl.issue
- rietveld_obj = cl.RpcServer()
- if issue and rietveld_obj:
- original_description = rietveld_obj.get_description(issue)
- new_description = original_description
-
- # Add GOLD_TRYBOT_URL if it does not exist yet.
- if not re.search(r'^GOLD_TRYBOT_URL=', new_description, re.M | re.I):
- new_description += '\nGOLD_TRYBOT_URL= %s%s' % (GOLD_TRYBOT_URL, issue)
- results.append(
- output_api.PresubmitNotifyResult(
- 'Added link to Gold trybot runs to the CL\'s description.\n'
- 'Note: Results may take sometime to be populated after trybots '
- 'complete.'))
+ if issue:
+ original_description_lines, footers = cl.GetDescriptionFooters()
+ new_description_lines = list(original_description_lines)
# If the change includes only doc changes then add NOTRY=true in the
# CL's description if it does not exist yet.
- if all_docs_changes and not re.search(
- r'^NOTRY=true$', new_description, re.M | re.I):
- new_description += '\nNOTRY=true'
+ if all_docs_changes and 'NOTRY=true' not in new_description_lines:
+ new_description_lines.append('NOTRY=true')
results.append(
output_api.PresubmitNotifyResult(
'This change has only doc changes. Automatically added '
# If there is atleast one docs change then add preview link in the CL's
# description if it does not already exist there.
- if atleast_one_docs_change and not re.search(
- r'^DOCS_PREVIEW=.*', new_description, re.M | re.I):
+ docs_preview_line = 'DOCS_PREVIEW= %s%s' % (DOCS_PREVIEW_URL, issue)
+ if (atleast_one_docs_change and
+ docs_preview_line not in new_description_lines):
# Automatically add a link to where the docs can be previewed.
- new_description += '\nDOCS_PREVIEW= %s%s' % (DOCS_PREVIEW_URL, issue)
+ new_description_lines.append(docs_preview_line)
results.append(
output_api.PresubmitNotifyResult(
'Automatically added a link to preview the docs changes to the '
# If the target ref is not master then add NOTREECHECKS=true and NOTRY=true
# to the CL's description if it does not already exist there.
- target_ref = rietveld_obj.get_issue_properties(issue, False).get(
- 'target_ref', '')
- if target_ref != 'refs/heads/master':
- if not re.search(
- r'^NOTREECHECKS=true$', new_description, re.M | re.I):
- new_description += "\nNOTREECHECKS=true"
+ target_ref = cl.GetRemoteBranch()[1]
+ if target_ref != 'refs/remotes/origin/master':
+ if 'NOTREECHECKS=true' not in new_description_lines:
+ new_description_lines.append('NOTREECHECKS=true')
results.append(
output_api.PresubmitNotifyResult(
'Branch changes do not need to rely on the master branch\'s '
'tree status. Automatically added \'NOTREECHECKS=true\' to the '
'CL\'s description'))
- if not re.search(
- r'^NOTRY=true$', new_description, re.M | re.I):
- new_description += "\nNOTRY=true"
+ if 'NOTRY=true' not in new_description_lines:
+ new_description_lines.append('NOTRY=true')
results.append(
output_api.PresubmitNotifyResult(
'Trybots do not yet work for non-master branches. '
'Automatically added \'NOTRY=true\' to the CL\'s description'))
- if not re.search(
- r'^NOPRESUBMIT=true$', new_description, re.M | re.I):
- new_description += "\nNOPRESUBMIT=true"
+ if 'NOPRESUBMIT=true' not in new_description_lines:
+ new_description_lines.append('NOPRESUBMIT=true')
results.append(
output_api.PresubmitNotifyResult(
'Branch changes do not run the presubmit checks.'))
_MergeCQExtraTrybotsMaps(
cq_master_to_trybots, _GetCQExtraTrybotsMap(extra_bots))
if cq_master_to_trybots:
- new_description = _AddCQExtraTrybotsToDesc(
- cq_master_to_trybots, new_description)
+ _AddCQExtraTrybotsToDesc(cq_master_to_trybots, new_description_lines)
# If the description has changed update it.
- if new_description != original_description:
- rietveld_obj.update_description(issue, new_description)
+ if new_description_lines != original_description_lines:
+ # Add a new line separating the new contents from the old contents.
+ new_description_lines.insert(len(original_description_lines), '')
+ cl.UpdateDescriptionFooters(new_description_lines, footers)
return results
-def _AddCQExtraTrybotsToDesc(cq_master_to_trybots, description):
+def _AddCQExtraTrybotsToDesc(cq_master_to_trybots, description_lines):
"""Adds the specified master and trybots to the CQ_INCLUDE_TRYBOTS keyword.
If the keyword already exists in the description then it appends to it only
If the keyword does not exist then it creates a new section in the
description.
"""
- match = re.search(r'^CQ_INCLUDE_TRYBOTS=(.*)$', description, re.M | re.I)
- if match:
- original_trybots_map = _GetCQExtraTrybotsMap(match.group(1))
+ found = None
+ foundIdx = -1
+ for idx, line in enumerate(description_lines):
+ if line.startswith('CQ_INCLUDE_TRYBOTS'):
+ found = line
+ foundIdx = idx
+
+ if found:
+ original_trybots_map = _GetCQExtraTrybotsMap(found)
_MergeCQExtraTrybotsMaps(cq_master_to_trybots, original_trybots_map)
- new_description = description.replace(
- match.group(0), _GetCQExtraTrybotsStr(cq_master_to_trybots))
+ new_line = _GetCQExtraTrybotsStr(cq_master_to_trybots)
+ if new_line != found:
+ description_lines[foundIdx] = new_line
else:
- new_description = description + "\n%s" % (
- _GetCQExtraTrybotsStr(cq_master_to_trybots))
- return new_description
+ description_lines.append(_GetCQExtraTrybotsStr(cq_master_to_trybots))
def _MergeCQExtraTrybotsMaps(dest_map, map_to_be_consumed):
projects / platform / upstream / libSkiaSharp.git / blobdiff