OLD | NEW |
1 # Copyright (c) 2013 The Chromium Authors. All rights reserved. | 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 | 2 # Use of this source code is governed by a BSD-style license that can be |
3 # found in the LICENSE file. | 3 # found in the LICENSE file. |
4 | 4 |
5 | 5 |
6 """Top-level presubmit script for Skia. | 6 """Top-level presubmit script for Skia. |
7 | 7 |
8 See http://dev.chromium.org/developers/how-tos/depottools/presubmit-scripts | 8 See http://dev.chromium.org/developers/how-tos/depottools/presubmit-scripts |
9 for more details about the presubmit API built into gcl. | 9 for more details about the presubmit API built into gcl. |
10 """ | 10 """ |
11 | 11 |
12 import fnmatch | 12 import fnmatch |
13 import os | 13 import os |
14 import re | 14 import re |
| 15 import subprocess |
15 import sys | 16 import sys |
16 import traceback | 17 import traceback |
17 | 18 |
18 | 19 |
19 REVERT_CL_SUBJECT_PREFIX = 'Revert ' | 20 REVERT_CL_SUBJECT_PREFIX = 'Revert ' |
20 | 21 |
21 SKIA_TREE_STATUS_URL = 'http://skia-tree-status.appspot.com' | 22 SKIA_TREE_STATUS_URL = 'http://skia-tree-status.appspot.com' |
22 | 23 |
23 # Please add the complete email address here (and not just 'xyz@' or 'xyz'). | 24 # Please add the complete email address here (and not just 'xyz@' or 'xyz'). |
24 PUBLIC_API_OWNERS = ( | 25 PUBLIC_API_OWNERS = ( |
25 'reed@chromium.org', | 26 'reed@chromium.org', |
26 'reed@google.com', | 27 'reed@google.com', |
27 'bsalomon@chromium.org', | 28 'bsalomon@chromium.org', |
28 'bsalomon@google.com', | 29 'bsalomon@google.com', |
29 'djsollen@chromium.org', | 30 'djsollen@chromium.org', |
30 'djsollen@google.com', | 31 'djsollen@google.com', |
31 ) | 32 ) |
32 | 33 |
33 AUTHORS_FILE_NAME = 'AUTHORS' | 34 AUTHORS_FILE_NAME = 'AUTHORS' |
34 | 35 |
| 36 DOCS_PREVIEW_URL = 'https://skia.org/?cl=' |
| 37 |
35 | 38 |
36 def _CheckChangeHasEol(input_api, output_api, source_file_filter=None): | 39 def _CheckChangeHasEol(input_api, output_api, source_file_filter=None): |
37 """Checks that files end with atleast one \n (LF).""" | 40 """Checks that files end with atleast one \n (LF).""" |
38 eof_files = [] | 41 eof_files = [] |
39 for f in input_api.AffectedSourceFiles(source_file_filter): | 42 for f in input_api.AffectedSourceFiles(source_file_filter): |
40 contents = input_api.ReadFile(f, 'rb') | 43 contents = input_api.ReadFile(f, 'rb') |
41 # Check that the file ends in atleast one newline character. | 44 # Check that the file ends in atleast one newline character. |
42 if len(contents) > 1 and contents[-1:] != '\n': | 45 if len(contents) > 1 and contents[-1:] != '\n': |
43 eof_files.append(f.LocalPath()) | 46 eof_files.append(f.LocalPath()) |
44 | 47 |
(...skipping 187 matching lines...) Expand 10 before | Expand all | Expand 10 after Loading... |
232 break | 235 break |
233 | 236 |
234 if not lgtm_from_owner: | 237 if not lgtm_from_owner: |
235 results.append( | 238 results.append( |
236 output_api.PresubmitError( | 239 output_api.PresubmitError( |
237 'Since the CL is editing public API, you must have an LGTM from ' | 240 'Since the CL is editing public API, you must have an LGTM from ' |
238 'one of: %s' % str(PUBLIC_API_OWNERS))) | 241 'one of: %s' % str(PUBLIC_API_OWNERS))) |
239 return results | 242 return results |
240 | 243 |
241 | 244 |
| 245 def PostUploadHook(cl, change, output_api): |
| 246 """git cl upload will call this hook after the issue is created/modified. |
| 247 |
| 248 This hook does the following: |
| 249 * Adds a link to preview docs changes if there are any docs changes in the CL. |
| 250 * Adds 'NOTRY=true' if the CL contains only docs changes. |
| 251 * Adds 'NOTREECHECKS=true' for non master branch changes since they do not |
| 252 need to be gated on the master branch's tree. |
| 253 * Adds 'NOTRY=true' for non master branch changes since trybots do not yet |
| 254 work on them. |
| 255 """ |
| 256 |
| 257 results = [] |
| 258 atleast_one_docs_change = False |
| 259 all_docs_changes = True |
| 260 for affected_file in change.AffectedFiles(): |
| 261 affected_file_path = affected_file.LocalPath() |
| 262 file_path, _ = os.path.splitext(affected_file_path) |
| 263 if 'site' == file_path.split(os.path.sep)[0]: |
| 264 atleast_one_docs_change = True |
| 265 else: |
| 266 all_docs_changes = False |
| 267 if atleast_one_docs_change and not all_docs_changes: |
| 268 break |
| 269 |
| 270 issue = cl.issue |
| 271 rietveld_obj = cl.RpcServer() |
| 272 if issue and rietveld_obj: |
| 273 original_description = rietveld_obj.get_description(issue) |
| 274 new_description = original_description |
| 275 |
| 276 # If the change includes only doc changes then add NOTRY=true in the |
| 277 # CL's description if it does not exist yet. |
| 278 if all_docs_changes and not re.search( |
| 279 r'^NOTRY=true$', new_description, re.M | re.I): |
| 280 new_description += '\nNOTRY=true' |
| 281 results.append( |
| 282 output_api.PresubmitNotifyResult( |
| 283 'This change has only doc changes. Automatically added ' |
| 284 '\'NOTRY=true\' to the CL\'s description')) |
| 285 |
| 286 # If there is atleast one docs change then add preview link in the CL's |
| 287 # description if it does not already exist there. |
| 288 if atleast_one_docs_change and not re.search( |
| 289 r'^DOCS_PREVIEW=.*', new_description, re.M | re.I): |
| 290 # Automatically add a link to where the docs can be previewed. |
| 291 new_description += '\nDOCS_PREVIEW= %s%s' % (DOCS_PREVIEW_URL, issue) |
| 292 results.append( |
| 293 output_api.PresubmitNotifyResult( |
| 294 'Automatically added a link to preview the docs changes to the ' |
| 295 'CL\'s description')) |
| 296 |
| 297 # If the target ref is not master then add NOTREECHECKS=true and NOTRY=true |
| 298 # to the CL's description if it does not already exist there. |
| 299 target_ref = rietveld_obj.get_issue_properties(issue, False).get( |
| 300 'target_ref', '') |
| 301 if target_ref != 'refs/heads/master': |
| 302 if not re.search( |
| 303 r'^NOTREECHECKS=true$', new_description, re.M | re.I): |
| 304 new_description += "\nNOTREECHECKS=true" |
| 305 results.append( |
| 306 output_api.PresubmitNotifyResult( |
| 307 'Branch changes do not need to rely on the master branch\'s ' |
| 308 'tree status. Automatically added \'NOTREECHECKS=true\' to the ' |
| 309 'CL\'s description')) |
| 310 if not re.search( |
| 311 r'^NOTRY=true$', new_description, re.M | re.I): |
| 312 new_description += "\nNOTRY=true" |
| 313 results.append( |
| 314 output_api.PresubmitNotifyResult( |
| 315 'Trybots do not yet work for non-master branches. ' |
| 316 'Automatically added \'NOTRY=true\' to the CL\'s description')) |
| 317 |
| 318 |
| 319 # If the description has changed update it. |
| 320 if new_description != original_description: |
| 321 rietveld_obj.update_description(issue, new_description) |
| 322 |
| 323 return results |
| 324 |
| 325 |
242 def CheckChangeOnCommit(input_api, output_api): | 326 def CheckChangeOnCommit(input_api, output_api): |
243 """Presubmit checks for the change on commit. | 327 """Presubmit checks for the change on commit. |
244 | 328 |
245 The following are the presubmit checks: | 329 The following are the presubmit checks: |
246 * Check change has one and only one EOL. | 330 * Check change has one and only one EOL. |
247 * Ensures that the Skia tree is open in | 331 * Ensures that the Skia tree is open in |
248 http://skia-tree-status.appspot.com/. Shows a warning if it is in 'Caution' | 332 http://skia-tree-status.appspot.com/. Shows a warning if it is in 'Caution' |
249 state and an error if it is in 'Closed' state. | 333 state and an error if it is in 'Closed' state. |
250 """ | 334 """ |
251 results = [] | 335 results = [] |
252 results.extend(_CommonChecks(input_api, output_api)) | 336 results.extend(_CommonChecks(input_api, output_api)) |
253 results.extend( | 337 results.extend( |
254 _CheckTreeStatus(input_api, output_api, json_url=( | 338 _CheckTreeStatus(input_api, output_api, json_url=( |
255 SKIA_TREE_STATUS_URL + '/banner-status?format=json'))) | 339 SKIA_TREE_STATUS_URL + '/banner-status?format=json'))) |
256 results.extend(_CheckLGTMsForPublicAPI(input_api, output_api)) | 340 results.extend(_CheckLGTMsForPublicAPI(input_api, output_api)) |
257 results.extend(_CheckOwnerIsInAuthorsFile(input_api, output_api)) | 341 results.extend(_CheckOwnerIsInAuthorsFile(input_api, output_api)) |
258 return results | 342 return results |
OLD | NEW |