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 """ |
(...skipping 262 matching lines...) Expand 10 before | Expand all | Expand 10 after Loading... |
273 SKIA_TREE_STATUS_URL + '/current-sheriff') | 273 SKIA_TREE_STATUS_URL + '/current-sheriff') |
274 sheriff_details = input_api.json.loads(connection.read()) | 274 sheriff_details = input_api.json.loads(connection.read()) |
275 if sheriff_details: | 275 if sheriff_details: |
276 tree_status_results[0]._message += ( | 276 tree_status_results[0]._message += ( |
277 '\n\nPlease contact the current Skia sheriff (%s) if you are trying ' | 277 '\n\nPlease contact the current Skia sheriff (%s) if you are trying ' |
278 'to submit a build fix\nand do not know how to submit because the ' | 278 'to submit a build fix\nand do not know how to submit because the ' |
279 'tree is closed') % sheriff_details['username'] | 279 'tree is closed') % sheriff_details['username'] |
280 return tree_status_results | 280 return tree_status_results |
281 | 281 |
282 | 282 |
| 283 class CodeReview(object): |
| 284 """Abstracts which codereview tool is used for the specified issue.""" |
| 285 |
| 286 def __init__(self, input_api): |
| 287 self._issue = input_api.change.issue |
| 288 self._gerrit = input_api.gerrit |
| 289 self._rietveld_properties = None |
| 290 if not self._gerrit: |
| 291 self._rietveld_properties = input_api.rietveld.get_issue_properties( |
| 292 issue=int(self._issue), messages=True) |
| 293 |
| 294 def GetOwnerEmail(self): |
| 295 if self._gerrit: |
| 296 return self._gerrit.GetChangeOwner(self._issue) |
| 297 else: |
| 298 return self._rietveld_properties['owner_email'] |
| 299 |
| 300 def GetSubject(self): |
| 301 if self._gerrit: |
| 302 return self._gerrit.GetChangeInfo(self._issue)['subject'] |
| 303 else: |
| 304 return self._rietveld_properties['subject'] |
| 305 |
| 306 def GetDescription(self): |
| 307 if self._gerrit: |
| 308 return self._gerrit.GetChangeDescription(self._issue) |
| 309 else: |
| 310 return self._rietveld_properties['description'] |
| 311 |
| 312 def IsDryRun(self): |
| 313 if self._gerrit: |
| 314 return self._gerrit.GetChangeInfo( |
| 315 self._issue)['labels']['Commit-Queue'].get('value', 0) == 1 |
| 316 else: |
| 317 return self._rietveld_properties['cq_dry_run'] |
| 318 |
| 319 def GetApprovers(self): |
| 320 approvers = [] |
| 321 if self._gerrit: |
| 322 for m in self._gerrit.GetChangeInfo( |
| 323 self._issue)['labels']['Code-Review']['all']: |
| 324 if m.get("value") == 1: |
| 325 approvers.append(m["email"]) |
| 326 else: |
| 327 for m in self._rietveld_properties.get('messages', []): |
| 328 if 'lgtm' in m['text'].lower(): |
| 329 approvers.append(m['sender']) |
| 330 return approvers |
| 331 |
| 332 |
283 def _CheckOwnerIsInAuthorsFile(input_api, output_api): | 333 def _CheckOwnerIsInAuthorsFile(input_api, output_api): |
284 results = [] | 334 results = [] |
285 issue = input_api.change.issue | 335 if input_api.change.issue: |
286 if issue and input_api.rietveld: | 336 cr = CodeReview(input_api) |
287 issue_properties = input_api.rietveld.get_issue_properties( | |
288 issue=int(issue), messages=False) | |
289 owner_email = issue_properties['owner_email'] | |
290 | 337 |
| 338 owner_email = cr.GetOwnerEmail() |
291 try: | 339 try: |
292 authors_content = '' | 340 authors_content = '' |
293 for line in open(AUTHORS_FILE_NAME): | 341 for line in open(AUTHORS_FILE_NAME): |
294 if not line.startswith('#'): | 342 if not line.startswith('#'): |
295 authors_content += line | 343 authors_content += line |
296 email_fnmatches = re.findall('<(.*)>', authors_content) | 344 email_fnmatches = re.findall('<(.*)>', authors_content) |
297 for email_fnmatch in email_fnmatches: | 345 for email_fnmatch in email_fnmatches: |
298 if fnmatch.fnmatch(owner_email, email_fnmatch): | 346 if fnmatch.fnmatch(owner_email, email_fnmatch): |
299 # Found a match, the user is in the AUTHORS file break out of the loop | 347 # Found a match, the user is in the AUTHORS file break out of the loop |
300 break | 348 break |
(...skipping 27 matching lines...) Expand all Loading... |
328 # include dir, but not include/private. | 376 # include dir, but not include/private. |
329 if (file_ext == '.h' and | 377 if (file_ext == '.h' and |
330 'include' == file_path.split(os.path.sep)[0] and | 378 'include' == file_path.split(os.path.sep)[0] and |
331 'private' not in file_path): | 379 'private' not in file_path): |
332 requires_owner_check = True | 380 requires_owner_check = True |
333 | 381 |
334 if not requires_owner_check: | 382 if not requires_owner_check: |
335 return results | 383 return results |
336 | 384 |
337 lgtm_from_owner = False | 385 lgtm_from_owner = False |
338 issue = input_api.change.issue | 386 if input_api.change.issue: |
339 if issue and input_api.rietveld: | 387 cr = CodeReview(input_api) |
340 issue_properties = input_api.rietveld.get_issue_properties( | 388 |
341 issue=int(issue), messages=True) | 389 if re.match(REVERT_CL_SUBJECT_PREFIX, cr.GetSubject(), re.I): |
342 if re.match(REVERT_CL_SUBJECT_PREFIX, issue_properties['subject'], re.I): | |
343 # It is a revert CL, ignore the public api owners check. | 390 # It is a revert CL, ignore the public api owners check. |
344 return results | 391 return results |
345 | 392 |
346 if issue_properties['cq_dry_run']: | 393 if cr.IsDryRun(): |
347 # Ignore public api owners check for dry run CLs since they are not | 394 # Ignore public api owners check for dry run CLs since they are not |
348 # going to be committed. | 395 # going to be committed. |
349 return results | 396 return results |
350 | 397 |
351 match = re.search(r'^TBR=(.*)$', issue_properties['description'], re.M) | 398 match = re.search(r'^TBR=(.*)$', cr.GetDescription(), re.M) |
352 if match: | 399 if match: |
353 tbr_entries = match.group(1).strip().split(',') | 400 tbr_entries = match.group(1).strip().split(',') |
354 for owner in PUBLIC_API_OWNERS: | 401 for owner in PUBLIC_API_OWNERS: |
355 if owner in tbr_entries or owner.split('@')[0] in tbr_entries: | 402 if owner in tbr_entries or owner.split('@')[0] in tbr_entries: |
356 # If an owner is specified in the TBR= line then ignore the public | 403 # If an owner is specified in the TBR= line then ignore the public |
357 # api owners check. | 404 # api owners check. |
358 return results | 405 return results |
359 | 406 |
360 if issue_properties['owner_email'] in PUBLIC_API_OWNERS: | 407 if cr.GetOwnerEmail() in PUBLIC_API_OWNERS: |
361 # An owner created the CL that is an automatic LGTM. | 408 # An owner created the CL that is an automatic LGTM. |
362 lgtm_from_owner = True | 409 lgtm_from_owner = True |
363 | 410 |
364 messages = issue_properties.get('messages') | 411 for approver in cr.GetApprovers(): |
365 if messages: | 412 if approver in PUBLIC_API_OWNERS: |
366 for message in messages: | 413 # Found an lgtm in a message from an owner. |
367 if (message['sender'] in PUBLIC_API_OWNERS and | 414 lgtm_from_owner = True |
368 'lgtm' in message['text'].lower()): | 415 break |
369 # Found an lgtm in a message from an owner. | |
370 lgtm_from_owner = True | |
371 break | |
372 | 416 |
373 if not lgtm_from_owner: | 417 if not lgtm_from_owner: |
374 results.append( | 418 results.append( |
375 output_api.PresubmitError( | 419 output_api.PresubmitError( |
376 "If this CL adds to or changes Skia's public API, you need an LGTM " | 420 "If this CL adds to or changes Skia's public API, you need an LGTM " |
377 "from any of %s. If this CL only removes from or doesn't change " | 421 "from any of %s. If this CL only removes from or doesn't change " |
378 "Skia's public API, please add a short note to the CL saying so " | 422 "Skia's public API, please add a short note to the CL saying so " |
379 "and add one of those reviewers on a TBR= line. If you don't know " | 423 "and add one of those reviewers on a TBR= line. If you don't know " |
380 "if this CL affects Skia's public API, treat it like it does." | 424 "if this CL affects Skia's public API, treat it like it does." |
381 % str(PUBLIC_API_OWNERS))) | 425 % str(PUBLIC_API_OWNERS))) |
(...skipping 10 matching lines...) Expand all Loading... |
392 * Adds 'NOTREECHECKS=true' for non master branch changes since they do not | 436 * Adds 'NOTREECHECKS=true' for non master branch changes since they do not |
393 need to be gated on the master branch's tree. | 437 need to be gated on the master branch's tree. |
394 * Adds 'NOTRY=true' for non master branch changes since trybots do not yet | 438 * Adds 'NOTRY=true' for non master branch changes since trybots do not yet |
395 work on them. | 439 work on them. |
396 * Adds 'NOPRESUBMIT=true' for non master branch changes since those don't | 440 * Adds 'NOPRESUBMIT=true' for non master branch changes since those don't |
397 run the presubmit checks. | 441 run the presubmit checks. |
398 * Adds extra trybots for the paths defined in PATH_TO_EXTRA_TRYBOTS. | 442 * Adds extra trybots for the paths defined in PATH_TO_EXTRA_TRYBOTS. |
399 """ | 443 """ |
400 | 444 |
401 results = [] | 445 results = [] |
| 446 if cl.IsGerrit(): |
| 447 results.append( |
| 448 output_api.PresubmitNotifyResult( |
| 449 'Post upload hooks are not yet supported for Gerrit CLs')) |
| 450 return results |
| 451 |
402 atleast_one_docs_change = False | 452 atleast_one_docs_change = False |
403 all_docs_changes = True | 453 all_docs_changes = True |
404 for affected_file in change.AffectedFiles(): | 454 for affected_file in change.AffectedFiles(): |
405 affected_file_path = affected_file.LocalPath() | 455 affected_file_path = affected_file.LocalPath() |
406 file_path, _ = os.path.splitext(affected_file_path) | 456 file_path, _ = os.path.splitext(affected_file_path) |
407 if 'site' == file_path.split(os.path.sep)[0]: | 457 if 'site' == file_path.split(os.path.sep)[0]: |
408 atleast_one_docs_change = True | 458 atleast_one_docs_change = True |
409 else: | 459 else: |
410 all_docs_changes = False | 460 all_docs_changes = False |
411 if atleast_one_docs_change and not all_docs_changes: | 461 if atleast_one_docs_change and not all_docs_changes: |
(...skipping 141 matching lines...) Expand 10 before | Expand all | Expand 10 after Loading... |
553 state and an error if it is in 'Closed' state. | 603 state and an error if it is in 'Closed' state. |
554 """ | 604 """ |
555 results = [] | 605 results = [] |
556 results.extend(_CommonChecks(input_api, output_api)) | 606 results.extend(_CommonChecks(input_api, output_api)) |
557 results.extend( | 607 results.extend( |
558 _CheckTreeStatus(input_api, output_api, json_url=( | 608 _CheckTreeStatus(input_api, output_api, json_url=( |
559 SKIA_TREE_STATUS_URL + '/banner-status?format=json'))) | 609 SKIA_TREE_STATUS_URL + '/banner-status?format=json'))) |
560 results.extend(_CheckLGTMsForPublicAPI(input_api, output_api)) | 610 results.extend(_CheckLGTMsForPublicAPI(input_api, output_api)) |
561 results.extend(_CheckOwnerIsInAuthorsFile(input_api, output_api)) | 611 results.extend(_CheckOwnerIsInAuthorsFile(input_api, output_api)) |
562 return results | 612 return results |
OLD | NEW |