Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(908)

Side by Side Diff: PRESUBMIT.py

Issue 1408783008: Add PRESUBMIT check for native API changes. (Closed) Base URL: https://chromium.googlesource.com/external/webrtc.git@master
Patch Set: Updated paths, ensured partial paths don't give false positives Created 5 years ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View unified diff | Download patch
« no previous file with comments | « no previous file | no next file » | no next file with comments »
Toggle Intra-line Diffs ('i') | Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
OLDNEW
1 # Copyright (c) 2012 The WebRTC project authors. All Rights Reserved. 1 # Copyright (c) 2012 The WebRTC project authors. All Rights Reserved.
2 # 2 #
3 # Use of this source code is governed by a BSD-style license 3 # Use of this source code is governed by a BSD-style license
4 # that can be found in the LICENSE file in the root of the source 4 # that can be found in the LICENSE file in the root of the source
5 # tree. An additional intellectual property rights grant can be found 5 # tree. An additional intellectual property rights grant can be found
6 # in the file PATENTS. All contributing project authors may 6 # in the file PATENTS. All contributing project authors may
7 # be found in the AUTHORS file in the root of the source tree. 7 # be found in the AUTHORS file in the root of the source tree.
8 8
9 import json 9 import json
10 import os 10 import os
11 import platform 11 import platform
12 import re 12 import re
13 import subprocess 13 import subprocess
14 import sys 14 import sys
15 15
16 16
17 NATIVE_API_DIRS = (
18 'talk/app/webrtc',
19 'webrtc',
20 'webrtc/common_audio/include', # DEPRECATED (will go away).
Andrew MacDonald 2015/11/25 19:10:40 I think there are several users of common_audio. A
kjellander_webrtc 2015/11/25 22:16:51 Yes, but AFAIK the plan is to get rid of that and
Andrew MacDonald 2015/11/25 23:09:53 OK. +aluebs for visibility as I believe beamer/ is
aluebs-webrtc 2015/11/25 23:54:45 Yes, common_audio is extensively used in beamer, i
kjellander_webrtc 2015/11/26 08:37:53 Please discuss this with Henrik L (added to review
21 'webrtc/modules/audio_coding/main/include',
Andrew MacDonald 2015/11/25 19:10:40 nit: Before we publish this as an "official" direc
kjellander_webrtc 2015/11/25 22:16:51 Alright, I'll take care of this one too. I created
Andrew MacDonald 2015/11/25 23:09:53 Thanks! RIP main.
22 'webrtc/modules/audio_conference_mixer/include', # DEPRECATED (will go away).
23 'webrtc/modules/audio_device/include',
24 'webrtc/modules/audio_processing/include',
25 'webrtc/modules/bitrate_controller/include',
26 'webrtc/modules/include',
27 'webrtc/modules/remote_bitrate_estimator/include',
28 'webrtc/modules/rtp_rtcp/include',
29 'webrtc/modules/rtp_rtcp/source', # DEPRECATED (will go away).
30 'webrtc/modules/utility/include',
31 'webrtc/modules/video_coding/codecs/h264/include',
32 'webrtc/modules/video_coding/codecs/i420/include',
33 'webrtc/modules/video_coding/codecs/vp8/include',
34 'webrtc/modules/video_coding/codecs/vp9/include',
35 'webrtc/modules/video_coding/include',
36 'webrtc/system_wrappers/include',
37 'webrtc/voice_engine/include',
38 )
39
40
41 def _VerifyNativeApiHeadersListIsValid(input_api, output_api):
42 """Ensures the list of native API header directories is up to date."""
43 non_existing_paths = []
44 native_api_full_paths = [
45 input_api.os_path.join(input_api.PresubmitLocalPath(),
46 *path.split('/')) for path in NATIVE_API_DIRS]
47 for path in native_api_full_paths:
48 if not os.path.isdir(path):
49 non_existing_paths.append(path)
50 if non_existing_paths:
51 return [output_api.PresubmitError(
52 'Directories to native API headers have changed which has made the '
53 'list in PRESUBMIT.py outdated.\nPlease update it to the current '
54 'location of our native APIs.',
55 non_existing_paths)]
56 return []
57
58
59 def _CheckNativeApiHeaderChanges(input_api, output_api):
60 """Checks to remind proper changing of native APIs."""
61 files = []
62 for f in input_api.AffectedSourceFiles(input_api.FilterSourceFile):
63 if f.LocalPath().endswith('.h'):
64 for path in NATIVE_API_DIRS:
65 if os.path.dirname(f.LocalPath()) == path:
66 files.append(f)
67
68 if files:
69 return [output_api.PresubmitPromptWarning(
70 'You seem to be changing native API header files. Please make sure '
71 'you:\n'
72 ' 1. Make compatible changes that doesn\'t break existing clients.\n'
73 ' 2. Mark the old APIs as deprecated.\n'
74 ' 3. Create a timeline and plan for when the deprecated method will '
75 'be removed.\n'
76 ' 4. Update/inform existing clients to update their code to the new '
phoglund 2015/11/26 09:52:07 There is really no way for anyone to know what the
Andrew MacDonald 2015/11/26 17:13:20 And probably webrtc-users, since I doubt many Goog
kjellander_webrtc 2015/11/27 07:06:59 Right, I guess externals cannot post to this list
77 'API.\n'
78 ' 5. (much later) remove the deprecated API.\n'
79 'Related files:',
80 files)]
81 return []
82
83
17 def _CheckNoIOStreamInHeaders(input_api, output_api): 84 def _CheckNoIOStreamInHeaders(input_api, output_api):
18 """Checks to make sure no .h files include <iostream>.""" 85 """Checks to make sure no .h files include <iostream>."""
19 files = [] 86 files = []
20 pattern = input_api.re.compile(r'^#include\s*<iostream>', 87 pattern = input_api.re.compile(r'^#include\s*<iostream>',
21 input_api.re.MULTILINE) 88 input_api.re.MULTILINE)
22 for f in input_api.AffectedSourceFiles(input_api.FilterSourceFile): 89 for f in input_api.AffectedSourceFiles(input_api.FilterSourceFile):
23 if not f.LocalPath().endswith('.h'): 90 if not f.LocalPath().endswith('.h'):
24 continue 91 continue
25 contents = input_api.ReadFile(f) 92 contents = input_api.ReadFile(f)
26 if pattern.search(contents): 93 if pattern.search(contents):
(...skipping 265 matching lines...) Expand 10 before | Expand all | Expand 10 after
292 black_list=(r'.+\.gyp$', r'.+\.gypi$', r'.+\.gn$', r'.+\.gni$', 'DEPS')) 359 black_list=(r'.+\.gyp$', r'.+\.gypi$', r'.+\.gn$', r'.+\.gni$', 'DEPS'))
293 results.extend(input_api.canned_checks.CheckLongLines( 360 results.extend(input_api.canned_checks.CheckLongLines(
294 input_api, output_api, maxlen=80, source_file_filter=long_lines_sources)) 361 input_api, output_api, maxlen=80, source_file_filter=long_lines_sources))
295 results.extend(input_api.canned_checks.CheckChangeHasNoTabs( 362 results.extend(input_api.canned_checks.CheckChangeHasNoTabs(
296 input_api, output_api)) 363 input_api, output_api))
297 results.extend(input_api.canned_checks.CheckChangeHasNoStrayWhitespace( 364 results.extend(input_api.canned_checks.CheckChangeHasNoStrayWhitespace(
298 input_api, output_api)) 365 input_api, output_api))
299 results.extend(input_api.canned_checks.CheckChangeTodoHasOwner( 366 results.extend(input_api.canned_checks.CheckChangeTodoHasOwner(
300 input_api, output_api)) 367 input_api, output_api))
301 results.extend(_CheckApprovedFilesLintClean(input_api, output_api)) 368 results.extend(_CheckApprovedFilesLintClean(input_api, output_api))
369 results.extend(_CheckNativeApiHeaderChanges(input_api, output_api))
302 results.extend(_CheckNoIOStreamInHeaders(input_api, output_api)) 370 results.extend(_CheckNoIOStreamInHeaders(input_api, output_api))
303 results.extend(_CheckNoFRIEND_TEST(input_api, output_api)) 371 results.extend(_CheckNoFRIEND_TEST(input_api, output_api))
304 results.extend(_CheckGypChanges(input_api, output_api)) 372 results.extend(_CheckGypChanges(input_api, output_api))
305 results.extend(_CheckUnwantedDependencies(input_api, output_api)) 373 results.extend(_CheckUnwantedDependencies(input_api, output_api))
306 results.extend(_RunPythonTests(input_api, output_api)) 374 results.extend(_RunPythonTests(input_api, output_api))
307 return results 375 return results
308 376
309 377
310 def CheckChangeOnUpload(input_api, output_api): 378 def CheckChangeOnUpload(input_api, output_api):
311 results = [] 379 results = []
312 results.extend(_CommonChecks(input_api, output_api)) 380 results.extend(_CommonChecks(input_api, output_api))
313 results.extend( 381 results.extend(
314 input_api.canned_checks.CheckGNFormatted(input_api, output_api)) 382 input_api.canned_checks.CheckGNFormatted(input_api, output_api))
315 return results 383 return results
316 384
317 385
318 def CheckChangeOnCommit(input_api, output_api): 386 def CheckChangeOnCommit(input_api, output_api):
319 results = [] 387 results = []
320 results.extend(_CommonChecks(input_api, output_api)) 388 results.extend(_CommonChecks(input_api, output_api))
389 results.extend(_VerifyNativeApiHeadersListIsValid(input_api, output_api))
321 results.extend(input_api.canned_checks.CheckOwners(input_api, output_api)) 390 results.extend(input_api.canned_checks.CheckOwners(input_api, output_api))
322 results.extend(input_api.canned_checks.CheckChangeWasUploaded( 391 results.extend(input_api.canned_checks.CheckChangeWasUploaded(
323 input_api, output_api)) 392 input_api, output_api))
324 results.extend(input_api.canned_checks.CheckChangeHasDescription( 393 results.extend(input_api.canned_checks.CheckChangeHasDescription(
325 input_api, output_api)) 394 input_api, output_api))
326 results.extend(input_api.canned_checks.CheckChangeHasBugField( 395 results.extend(input_api.canned_checks.CheckChangeHasBugField(
327 input_api, output_api)) 396 input_api, output_api))
328 results.extend(input_api.canned_checks.CheckChangeHasTestField( 397 results.extend(input_api.canned_checks.CheckChangeHasTestField(
329 input_api, output_api)) 398 input_api, output_api))
330 results.extend(input_api.canned_checks.CheckTreeIsOpen( 399 results.extend(input_api.canned_checks.CheckTreeIsOpen(
(...skipping 18 matching lines...) Expand all
349 for builder in masters[master]: 418 for builder in masters[master]:
350 if 'presubmit' in builder: 419 if 'presubmit' in builder:
351 # Do not trigger presubmit builders, since they're likely to fail 420 # Do not trigger presubmit builders, since they're likely to fail
352 # (e.g. OWNERS checks before finished code review), and we're running 421 # (e.g. OWNERS checks before finished code review), and we're running
353 # local presubmit anyway. 422 # local presubmit anyway.
354 pass 423 pass
355 else: 424 else:
356 try_config[master][builder] = ['defaulttests'] 425 try_config[master][builder] = ['defaulttests']
357 426
358 return try_config 427 return try_config
OLDNEW
« no previous file with comments | « no previous file | no next file » | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698