Home | History | Annotate | Download | only in skqp 
      1 # Copyright (c) 2013 The Chromium Authors. All rights reserved.
      2 # Use of this source code is governed by a BSD-style license that can be
      3 # found in the LICENSE file.
      4 
      5 
      6 """Top-level presubmit script for Skia.
      7 
      8 See http://dev.chromium.org/developers/how-tos/depottools/presubmit-scripts
      9 for more details about the presubmit API built into gcl.
     10 """
     11 
     12 import collections
     13 import csv
     14 import fnmatch
     15 import os
     16 import re
     17 import subprocess
     18 import sys
     19 import traceback
     20 
     21 
     22 REVERT_CL_SUBJECT_PREFIX = 'Revert '
     23 
     24 SKIA_TREE_STATUS_URL = 'http://skia-tree-status.appspot.com'
     25 
     26 # Please add the complete email address here (and not just 'xyz@' or 'xyz').
     27 PUBLIC_API_OWNERS = (
     28     'reed (at] chromium.org',
     29     'reed (at] google.com',
     30     'bsalomon (at] chromium.org',
     31     'bsalomon (at] google.com',
     32     'djsollen (at] chromium.org',
     33     'djsollen (at] google.com',
     34     'hcm (at] chromium.org',
     35     'hcm (at] google.com',
     36 )
     37 
     38 AUTO_COMMIT_BOTS = (
     39     'update-docs (at] skia.org',
     40     'update-skps (at] skia.org'
     41 )
     42 
     43 AUTHORS_FILE_NAME = 'AUTHORS'
     44 
     45 DOCS_PREVIEW_URL = 'https://skia.org/?cl='
     46 GOLD_TRYBOT_URL = 'https://gold.skia.org/search?issue='
     47 
     48 # Path to CQ bots feature is described in https://bug.skia.org/4364
     49 PATH_PREFIX_TO_EXTRA_TRYBOTS = {
     50     'src/opts/': ('skia.primary:'
     51       'Test-Debian9-Clang-GCE-CPU-AVX2-x86_64-Release-All-SKNX_NO_SIMD'),
     52     'include/private/SkAtomics.h': ('skia.primary:'
     53       'Test-Debian9-Clang-GCE-CPU-AVX2-x86_64-Release-All-TSAN,'
     54       'Test-Ubuntu17-Clang-Golo-GPU-QuadroP400-x86_64-Release-All-TSAN'
     55     ),
     56 
     57     # Below are examples to show what is possible with this feature.
     58     # 'src/svg/': 'master1:abc;master2:def',
     59     # 'src/svg/parser/': 'master3:ghi,jkl;master4:mno',
     60     # 'src/image/SkImage_Base.h': 'master5:pqr,stu;master1:abc1;master2:def',
     61 }
     62 
     63 SERVICE_ACCOUNT_SUFFIX = '@skia-buildbots.google.com.iam.gserviceaccount.com'
     64 
     65 
     66 def _CheckChangeHasEol(input_api, output_api, source_file_filter=None):
     67   """Checks that files end with atleast one \n (LF)."""
     68   eof_files = []
     69   for f in input_api.AffectedSourceFiles(source_file_filter):
     70     contents = input_api.ReadFile(f, 'rb')
     71     # Check that the file ends in atleast one newline character.
     72     if len(contents) > 1 and contents[-1:] != '\n':
     73       eof_files.append(f.LocalPath())
     74 
     75   if eof_files:
     76     return [output_api.PresubmitPromptWarning(
     77       'These files should end in a newline character:',
     78       items=eof_files)]
     79   return []
     80 
     81 
     82 def _PythonChecks(input_api, output_api):
     83   """Run checks on any modified Python files."""
     84   pylint_disabled_files = (
     85       'infra/bots/recipes.py',
     86   )
     87   pylint_disabled_warnings = (
     88       'F0401',  # Unable to import.
     89       'E0611',  # No name in module.
     90       'W0232',  # Class has no __init__ method.
     91       'E1002',  # Use of super on an old style class.
     92       'W0403',  # Relative import used.
     93       'R0201',  # Method could be a function.
     94       'E1003',  # Using class name in super.
     95       'W0613',  # Unused argument.
     96       'W0105',  # String statement has no effect.
     97   )
     98   # Run Pylint on only the modified python files. Unfortunately it still runs
     99   # Pylint on the whole file instead of just the modified lines.
    100   affected_python_files = []
    101   for affected_file in input_api.AffectedSourceFiles(None):
    102     affected_file_path = affected_file.LocalPath()
    103     if affected_file_path.endswith('.py'):
    104       if affected_file_path not in pylint_disabled_files:
    105         affected_python_files.append(affected_file_path)
    106   return input_api.canned_checks.RunPylint(
    107       input_api, output_api,
    108       disabled_warnings=pylint_disabled_warnings,
    109       white_list=affected_python_files)
    110 
    111 
    112 def _IfDefChecks(input_api, output_api):
    113   """Ensures if/ifdef are not before includes. See skbug/3362 for details."""
    114   comment_block_start_pattern = re.compile('^\s*\/\*.*$')
    115   comment_block_middle_pattern = re.compile('^\s+\*.*')
    116   comment_block_end_pattern = re.compile('^\s+\*\/.*$')
    117   single_line_comment_pattern = re.compile('^\s*//.*$')
    118   def is_comment(line):
    119     return (comment_block_start_pattern.match(line) or
    120             comment_block_middle_pattern.match(line) or
    121             comment_block_end_pattern.match(line) or
    122             single_line_comment_pattern.match(line))
    123 
    124   empty_line_pattern = re.compile('^\s*$')
    125   def is_empty_line(line):
    126     return empty_line_pattern.match(line)
    127 
    128   failing_files = []
    129   for affected_file in input_api.AffectedSourceFiles(None):
    130     affected_file_path = affected_file.LocalPath()
    131     if affected_file_path.endswith('.cpp') or affected_file_path.endswith('.h'):
    132       f = open(affected_file_path)
    133       for line in f.xreadlines():
    134         if is_comment(line) or is_empty_line(line):
    135           continue
    136         # The below will be the first real line after comments and newlines.
    137         if line.startswith('#if 0 '):
    138           pass
    139         elif line.startswith('#if ') or line.startswith('#ifdef '):
    140           failing_files.append(affected_file_path)
    141         break
    142 
    143   results = []
    144   if failing_files:
    145     results.append(
    146         output_api.PresubmitError(
    147             'The following files have #if or #ifdef before includes:\n%s\n\n'
    148             'See https://bug.skia.org/3362 for why this should be fixed.' %
    149                 '\n'.join(failing_files)))
    150   return results
    151 
    152 
    153 def _CopyrightChecks(input_api, output_api, source_file_filter=None):
    154   results = []
    155   year_pattern = r'\d{4}'
    156   year_range_pattern = r'%s(-%s)?' % (year_pattern, year_pattern)
    157   years_pattern = r'%s(,%s)*,?' % (year_range_pattern, year_range_pattern)
    158   copyright_pattern = (
    159       r'Copyright (\([cC]\) )?%s \w+' % years_pattern)
    160 
    161   for affected_file in input_api.AffectedSourceFiles(source_file_filter):
    162     if 'third_party' in affected_file.LocalPath():
    163       continue
    164     contents = input_api.ReadFile(affected_file, 'rb')
    165     if not re.search(copyright_pattern, contents):
    166       results.append(output_api.PresubmitError(
    167           '%s is missing a correct copyright header.' % affected_file))
    168   return results
    169 
    170 
    171 def _ToolFlags(input_api, output_api):
    172   """Make sure `{dm,nanobench}_flags.py test` passes if modified."""
    173   results = []
    174   sources = lambda x: ('dm_flags.py'        in x.LocalPath() or
    175                        'nanobench_flags.py' in x.LocalPath())
    176   for f in input_api.AffectedSourceFiles(sources):
    177     if 0 != subprocess.call(['python', f.LocalPath(), 'test']):
    178       results.append(output_api.PresubmitError('`python %s test` failed' % f))
    179   return results
    180 
    181 
    182 def _InfraTests(input_api, output_api):
    183   """Run the infra tests."""
    184   results = []
    185   if not any(f.LocalPath().startswith('infra')
    186              for f in input_api.AffectedFiles()):
    187     return results
    188 
    189   cmd = ['python', os.path.join('infra', 'bots', 'infra_tests.py')]
    190   try:
    191     subprocess.check_output(cmd)
    192   except subprocess.CalledProcessError as e:
    193     results.append(output_api.PresubmitError(
    194         '`%s` failed:\n%s' % (' '.join(cmd), e.output)))
    195   return results
    196 
    197 
    198 def _CheckGNFormatted(input_api, output_api):
    199   """Make sure any .gn files we're changing have been formatted."""
    200   results = []
    201   for f in input_api.AffectedFiles():
    202     if (not f.LocalPath().endswith('.gn') and
    203         not f.LocalPath().endswith('.gni')):
    204       continue
    205 
    206     gn = 'gn.bat' if 'win32' in sys.platform else 'gn'
    207     cmd = [gn, 'format', '--dry-run', f.LocalPath()]
    208     try:
    209       subprocess.check_output(cmd)
    210     except subprocess.CalledProcessError:
    211       fix = 'gn format ' + f.LocalPath()
    212       results.append(output_api.PresubmitError(
    213           '`%s` failed, try\n\t%s' % (' '.join(cmd), fix)))
    214   return results
    215 
    216 
    217 def _CommonChecks(input_api, output_api):
    218   """Presubmit checks common to upload and commit."""
    219   results = []
    220   sources = lambda x: (x.LocalPath().endswith('.h') or
    221                        x.LocalPath().endswith('.py') or
    222                        x.LocalPath().endswith('.sh') or
    223                        x.LocalPath().endswith('.m') or
    224                        x.LocalPath().endswith('.mm') or
    225                        x.LocalPath().endswith('.go') or
    226                        x.LocalPath().endswith('.c') or
    227                        x.LocalPath().endswith('.cc') or
    228                        x.LocalPath().endswith('.cpp'))
    229   results.extend(
    230       _CheckChangeHasEol(
    231           input_api, output_api, source_file_filter=sources))
    232   results.extend(
    233       input_api.canned_checks.CheckChangeHasNoCR(
    234           input_api, output_api, source_file_filter=sources))
    235   results.extend(
    236       input_api.canned_checks.CheckChangeHasNoStrayWhitespace(
    237           input_api, output_api, source_file_filter=sources))
    238   results.extend(_PythonChecks(input_api, output_api))
    239   results.extend(_IfDefChecks(input_api, output_api))
    240   results.extend(_CopyrightChecks(input_api, output_api,
    241                                   source_file_filter=sources))
    242   results.extend(_ToolFlags(input_api, output_api))
    243   return results
    244 
    245 
    246 def CheckChangeOnUpload(input_api, output_api):
    247   """Presubmit checks for the change on upload.
    248 
    249   The following are the presubmit checks:
    250   * Check change has one and only one EOL.
    251   """
    252   results = []
    253   results.extend(_CommonChecks(input_api, output_api))
    254   # Run on upload, not commit, since the presubmit bot apparently doesn't have
    255   # coverage or Go installed.
    256   results.extend(_InfraTests(input_api, output_api))
    257 
    258   results.extend(_CheckGNFormatted(input_api, output_api))
    259   return results
    260 
    261 
    262 def _CheckTreeStatus(input_api, output_api, json_url):
    263   """Check whether to allow commit.
    264 
    265   Args:
    266     input_api: input related apis.
    267     output_api: output related apis.
    268     json_url: url to download json style status.
    269   """
    270   tree_status_results = input_api.canned_checks.CheckTreeIsOpen(
    271       input_api, output_api, json_url=json_url)
    272   if not tree_status_results:
    273     # Check for caution state only if tree is not closed.
    274     connection = input_api.urllib2.urlopen(json_url)
    275     status = input_api.json.loads(connection.read())
    276     connection.close()
    277     if ('caution' in status['message'].lower() and
    278         os.isatty(sys.stdout.fileno())):
    279       # Display a prompt only if we are in an interactive shell. Without this
    280       # check the commit queue behaves incorrectly because it considers
    281       # prompts to be failures.
    282       short_text = 'Tree state is: ' + status['general_state']
    283       long_text = status['message'] + '\n' + json_url
    284       tree_status_results.append(
    285           output_api.PresubmitPromptWarning(
    286               message=short_text, long_text=long_text))
    287   else:
    288     # Tree status is closed. Put in message about contacting sheriff.
    289     connection = input_api.urllib2.urlopen(
    290         SKIA_TREE_STATUS_URL + '/current-sheriff')
    291     sheriff_details = input_api.json.loads(connection.read())
    292     if sheriff_details:
    293       tree_status_results[0]._message += (
    294           '\n\nPlease contact the current Skia sheriff (%s) if you are trying '
    295           'to submit a build fix\nand do not know how to submit because the '
    296           'tree is closed') % sheriff_details['username']
    297   return tree_status_results
    298 
    299 
    300 class CodeReview(object):
    301   """Abstracts which codereview tool is used for the specified issue."""
    302 
    303   def __init__(self, input_api):
    304     self._issue = input_api.change.issue
    305     self._gerrit = input_api.gerrit
    306 
    307   def GetOwnerEmail(self):
    308     return self._gerrit.GetChangeOwner(self._issue)
    309 
    310   def GetSubject(self):
    311     return self._gerrit.GetChangeInfo(self._issue)['subject']
    312 
    313   def GetDescription(self):
    314     return self._gerrit.GetChangeDescription(self._issue)
    315 
    316   def IsDryRun(self):
    317     return self._gerrit.GetChangeInfo(
    318         self._issue)['labels']['Commit-Queue'].get('value', 0) == 1
    319 
    320   def GetReviewers(self):
    321     code_review_label = (
    322         self._gerrit.GetChangeInfo(self._issue)['labels']['Code-Review'])
    323     return [r['email'] for r in code_review_label.get('all', [])]
    324 
    325   def GetApprovers(self):
    326     approvers = []
    327     code_review_label = (
    328         self._gerrit.GetChangeInfo(self._issue)['labels']['Code-Review'])
    329     for m in code_review_label.get('all', []):
    330       if m.get("value") == 1:
    331         approvers.append(m["email"])
    332     return approvers
    333 
    334 
    335 def _CheckOwnerIsInAuthorsFile(input_api, output_api):
    336   results = []
    337   if input_api.change.issue:
    338     cr = CodeReview(input_api)
    339 
    340     owner_email = cr.GetOwnerEmail()
    341 
    342     # Service accounts don't need to be in AUTHORS.
    343     if owner_email.endswith(SERVICE_ACCOUNT_SUFFIX):
    344       return results
    345 
    346     try:
    347       authors_content = ''
    348       for line in open(AUTHORS_FILE_NAME):
    349         if not line.startswith('#'):
    350           authors_content += line
    351       email_fnmatches = re.findall('<(.*)>', authors_content)
    352       for email_fnmatch in email_fnmatches:
    353         if fnmatch.fnmatch(owner_email, email_fnmatch):
    354           # Found a match, the user is in the AUTHORS file break out of the loop
    355           break
    356       else:
    357         results.append(
    358           output_api.PresubmitError(
    359             'The email %s is not in Skia\'s AUTHORS file.\n'
    360             'Issue owner, this CL must include an addition to the Skia AUTHORS '
    361             'file.'
    362             % owner_email))
    363     except IOError:
    364       # Do not fail if authors file cannot be found.
    365       traceback.print_exc()
    366       input_api.logging.error('AUTHORS file not found!')
    367 
    368   return results
    369 
    370 
    371 def _CheckLGTMsForPublicAPI(input_api, output_api):
    372   """Check LGTMs for public API changes.
    373 
    374   For public API files make sure there is an LGTM from the list of owners in
    375   PUBLIC_API_OWNERS.
    376   """
    377   results = []
    378   requires_owner_check = False
    379   for affected_file in input_api.AffectedFiles():
    380     affected_file_path = affected_file.LocalPath()
    381     file_path, file_ext = os.path.splitext(affected_file_path)
    382     # We only care about files that end in .h and are under the top-level
    383     # include dir, but not include/private.
    384     if (file_ext == '.h' and
    385         'include' == file_path.split(os.path.sep)[0] and
    386         'private' not in file_path):
    387       requires_owner_check = True
    388 
    389   if not requires_owner_check:
    390     return results
    391 
    392   lgtm_from_owner = False
    393   if input_api.change.issue:
    394     cr = CodeReview(input_api)
    395 
    396     if re.match(REVERT_CL_SUBJECT_PREFIX, cr.GetSubject(), re.I):
    397       # It is a revert CL, ignore the public api owners check.
    398       return results
    399 
    400     if cr.IsDryRun():
    401       # Ignore public api owners check for dry run CLs since they are not
    402       # going to be committed.
    403       return results
    404 
    405     if input_api.gerrit:
    406       for reviewer in cr.GetReviewers():
    407         if reviewer in PUBLIC_API_OWNERS:
    408           # If an owner is specified as an reviewer in Gerrit then ignore the
    409           # public api owners check.
    410           return results
    411     else:
    412       match = re.search(r'^TBR=(.*)$', cr.GetDescription(), re.M)
    413       if match:
    414         tbr_section = match.group(1).strip().split(' ')[0]
    415         tbr_entries = tbr_section.split(',')
    416         for owner in PUBLIC_API_OWNERS:
    417           if owner in tbr_entries or owner.split('@')[0] in tbr_entries:
    418             # If an owner is specified in the TBR= line then ignore the public
    419             # api owners check.
    420             return results
    421 
    422     if cr.GetOwnerEmail() in PUBLIC_API_OWNERS:
    423       # An owner created the CL that is an automatic LGTM.
    424       lgtm_from_owner = True
    425 
    426     for approver in cr.GetApprovers():
    427       if approver in PUBLIC_API_OWNERS:
    428         # Found an lgtm in a message from an owner.
    429         lgtm_from_owner = True
    430         break
    431 
    432   if not lgtm_from_owner:
    433     results.append(
    434         output_api.PresubmitError(
    435             "If this CL adds to or changes Skia's public API, you need an LGTM "
    436             "from any of %s.  If this CL only removes from or doesn't change "
    437             "Skia's public API, please add a short note to the CL saying so. "
    438             "Add one of the owners as a reviewer to your CL as well as to the "
    439             "TBR= line.  If you don't know if this CL affects Skia's public "
    440             "API, treat it like it does." % str(PUBLIC_API_OWNERS)))
    441   return results
    442 
    443 
    444 def _FooterExists(footers, key, value):
    445   for k, v in footers:
    446     if k == key and v == value:
    447       return True
    448   return False
    449 
    450 
    451 def PostUploadHook(cl, change, output_api):
    452   """git cl upload will call this hook after the issue is created/modified.
    453 
    454   This hook does the following:
    455   * Adds a link to preview docs changes if there are any docs changes in the CL.
    456   * Adds 'No-Try: true' if the CL contains only docs changes.
    457   * Adds 'No-Tree-Checks: true' for non master branch changes since they do not
    458     need to be gated on the master branch's tree.
    459   * Adds 'No-Try: true' for non master branch changes since trybots do not yet
    460     work on them.
    461   * Adds 'No-Presubmit: true' for non master branch changes since those don't
    462     run the presubmit checks.
    463   * Adds extra trybots for the paths defined in PATH_TO_EXTRA_TRYBOTS.
    464   """
    465 
    466   results = []
    467   atleast_one_docs_change = False
    468   all_docs_changes = True
    469   for affected_file in change.AffectedFiles():
    470     affected_file_path = affected_file.LocalPath()
    471     file_path, _ = os.path.splitext(affected_file_path)
    472     if 'site' == file_path.split(os.path.sep)[0]:
    473       atleast_one_docs_change = True
    474     else:
    475       all_docs_changes = False
    476     if atleast_one_docs_change and not all_docs_changes:
    477       break
    478 
    479   issue = cl.issue
    480   if issue:
    481     # Skip PostUploadHooks for all auto-commit bots. New patchsets (caused
    482     # due to PostUploadHooks) invalidates the CQ+2 vote from the
    483     # "--use-commit-queue" flag to "git cl upload".
    484     if cl.GetIssueOwner() in AUTO_COMMIT_BOTS:
    485       return results
    486 
    487     original_description_lines, footers = cl.GetDescriptionFooters()
    488     new_description_lines = list(original_description_lines)
    489 
    490     # If the change includes only doc changes then add No-Try: true in the
    491     # CL's description if it does not exist yet.
    492     if all_docs_changes and not _FooterExists(footers, 'No-Try', 'true'):
    493       new_description_lines.append('No-Try: true')
    494       results.append(
    495           output_api.PresubmitNotifyResult(
    496               'This change has only doc changes. Automatically added '
    497               '\'No-Try: true\' to the CL\'s description'))
    498 
    499     # If there is atleast one docs change then add preview link in the CL's
    500     # description if it does not already exist there.
    501     docs_preview_link = '%s%s' % (DOCS_PREVIEW_URL, issue)
    502     docs_preview_line = 'Docs-Preview: %s' % docs_preview_link
    503     if (atleast_one_docs_change and
    504         not _FooterExists(footers, 'Docs-Preview', docs_preview_link)):
    505       # Automatically add a link to where the docs can be previewed.
    506       new_description_lines.append(docs_preview_line)
    507       results.append(
    508           output_api.PresubmitNotifyResult(
    509               'Automatically added a link to preview the docs changes to the '
    510               'CL\'s description'))
    511 
    512     # If the target ref is not master then add 'No-Tree-Checks: true' and
    513     # 'No-Try: true' to the CL's description if it does not already exist there.
    514     target_ref = cl.GetRemoteBranch()[1]
    515     if target_ref != 'refs/remotes/origin/master':
    516       if not _FooterExists(footers, 'No-Tree-Checks', 'true'):
    517         new_description_lines.append('No-Tree-Checks: true')
    518         results.append(
    519             output_api.PresubmitNotifyResult(
    520                 'Branch changes do not need to rely on the master branch\'s '
    521                 'tree status. Automatically added \'No-Tree-Checks: true\' to '
    522                 'the CL\'s description'))
    523       if not _FooterExists(footers, 'No-Try', 'true'):
    524         new_description_lines.append('No-Try: true')
    525         results.append(
    526             output_api.PresubmitNotifyResult(
    527                 'Trybots do not yet work for non-master branches. '
    528                 'Automatically added \'No-Try: true\' to the CL\'s '
    529                 'description'))
    530       if not _FooterExists(footers, 'No-Presubmit', 'true'):
    531         new_description_lines.append('No-Presubmit: true')
    532         results.append(
    533             output_api.PresubmitNotifyResult(
    534                 'Branch changes do not run the presubmit checks.'))
    535 
    536     # Automatically set Cq-Include-Trybots if any of the changed files here
    537     # begin with the paths of interest.
    538     bots_to_include = []
    539     for affected_file in change.AffectedFiles():
    540       affected_file_path = affected_file.LocalPath()
    541       for path_prefix, extra_bots in PATH_PREFIX_TO_EXTRA_TRYBOTS.iteritems():
    542         if affected_file_path.startswith(path_prefix):
    543           results.append(
    544               output_api.PresubmitNotifyResult(
    545                   'Your CL modifies the path %s.\nAutomatically adding %s to '
    546                   'the CL description.' % (affected_file_path, extra_bots)))
    547           bots_to_include.append(extra_bots)
    548     if bots_to_include:
    549       output_api.EnsureCQIncludeTrybotsAreAdded(
    550           cl, bots_to_include, new_description_lines)
    551 
    552     # If the description has changed update it.
    553     if new_description_lines != original_description_lines:
    554       # Add a new line separating the new contents from the old contents.
    555       new_description_lines.insert(len(original_description_lines), '')
    556       cl.UpdateDescriptionFooters(new_description_lines, footers)
    557 
    558     return results
    559 
    560 
    561 def CheckChangeOnCommit(input_api, output_api):
    562   """Presubmit checks for the change on commit.
    563 
    564   The following are the presubmit checks:
    565   * Check change has one and only one EOL.
    566   * Ensures that the Skia tree is open in
    567     http://skia-tree-status.appspot.com/. Shows a warning if it is in 'Caution'
    568     state and an error if it is in 'Closed' state.
    569   """
    570   results = []
    571   results.extend(_CommonChecks(input_api, output_api))
    572   results.extend(
    573       _CheckTreeStatus(input_api, output_api, json_url=(
    574           SKIA_TREE_STATUS_URL + '/banner-status?format=json')))
    575   results.extend(_CheckLGTMsForPublicAPI(input_api, output_api))
    576   results.extend(_CheckOwnerIsInAuthorsFile(input_api, output_api))
    577   # Checks for the presence of 'DO NOT''SUBMIT' in CL description and in
    578   # content of files.
    579   results.extend(
    580       input_api.canned_checks.CheckDoNotSubmit(input_api, output_api))
    581   return results
    582