diff options
Diffstat (limited to 'deps/v8/PRESUBMIT.py')
| -rw-r--r-- | deps/v8/PRESUBMIT.py | 177 |
1 files changed, 121 insertions, 56 deletions
diff --git a/deps/v8/PRESUBMIT.py b/deps/v8/PRESUBMIT.py index 201bf55f71..67986d8303 100644 --- a/deps/v8/PRESUBMIT.py +++ b/deps/v8/PRESUBMIT.py @@ -32,6 +32,7 @@ for more details about the presubmit API built into gcl. """ import json +import os import re import sys @@ -134,8 +135,68 @@ def _CheckUnwantedDependencies(input_api, output_api): # Restore sys.path to what it was before. sys.path = original_sys_path + def _FilesImpactedByDepsChange(files): + all_files = [f.AbsoluteLocalPath() for f in files] + deps_files = [p for p in all_files if IsDepsFile(p)] + impacted_files = union([_CollectImpactedFiles(path) for path in deps_files]) + impacted_file_objs = [ImpactedFile(path) for path in impacted_files] + return impacted_file_objs + + def IsDepsFile(p): + return os.path.isfile(p) and os.path.basename(p) == 'DEPS' + + def union(list_of_lists): + """Ensure no duplicates""" + return set(sum(list_of_lists, [])) + + def _CollectImpactedFiles(deps_file): + # TODO(liviurau): Do not walk paths twice. Then we have no duplicates. + # Higher level DEPS changes may dominate lower level DEPS changes. + # TODO(liviurau): Check if DEPS changed in the right way. + # 'include_rules' impact c++ files but 'vars' or 'deps' do not. + # Maybe we just eval both old and new DEPS content and check + # if the list are the same. + result = [] + parent_dir = os.path.dirname(deps_file) + for relative_f in input_api.change.AllFiles(parent_dir): + abs_f = os.path.join(parent_dir, relative_f) + if CppChecker.IsCppFile(abs_f): + result.append(abs_f) + return result + + class ImpactedFile(object): + """Duck type version of AffectedFile needed to check files under directories + where a DEPS file changed. Extend the interface along the line of + AffectedFile if you need it for other checks.""" + + def __init__(self, path): + self._path = path + + def LocalPath(self): + path = self._path.replace(os.sep, '/') + return os.path.normpath(path) + + def ChangedContents(self): + with open(self._path) as f: + # TODO(liviurau): read only '#include' lines + lines = f.readlines() + return enumerate(lines, start=1) + + def _FilterDuplicates(impacted_files, affected_files): + """"We include all impacted files but exclude affected files that are also + impacted. Files impacted by DEPS changes take precedence before files + affected by direct changes.""" + result = impacted_files[:] + only_paths = set([imf.LocalPath() for imf in impacted_files]) + for af in affected_files: + if not af.LocalPath() in only_paths: + result.append(af) + return result + added_includes = [] - for f in input_api.AffectedFiles(): + affected_files = input_api.AffectedFiles() + impacted_by_deps = _FilesImpactedByDepsChange(affected_files) + for f in _FilterDuplicates(impacted_by_deps, affected_files): if not CppChecker.IsCppFile(f.LocalPath()): continue @@ -301,39 +362,43 @@ def _CheckNoProductionCodeUsingTestOnlyFunctions(input_api, output_api): return [] +def _CheckGenderNeutralInLicenses(input_api, output_api): + # License files are taken as is, even if they include gendered pronouns. + def LicenseFilter(path): + input_api.FilterSourceFile(path, black_list=_LICENSE_FILE) + + return input_api.canned_checks.CheckGenderNeutral( + input_api, output_api, source_file_filter=LicenseFilter) + + +def _RunTestsWithVPythonSpec(input_api, output_api): + return input_api.RunTests( + input_api.canned_checks.CheckVPythonSpec(input_api, output_api)) + + def _CommonChecks(input_api, output_api): """Checks common to both upload and commit.""" - results = [] # TODO(machenbach): Replace some of those checks, e.g. owners and copyright, # with the canned PanProjectChecks. Need to make sure that the checks all # pass on all existing files. - results.extend(input_api.canned_checks.CheckOwnersFormat( - input_api, output_api)) - results.extend(input_api.canned_checks.CheckOwners( - input_api, output_api)) - results.extend(_CheckCommitMessageBugEntry(input_api, output_api)) - results.extend(input_api.canned_checks.CheckPatchFormatted( - input_api, output_api)) - - # License files are taken as is, even if they include gendered pronouns. - license_filter = lambda path: input_api.FilterSourceFile( - path, black_list=_LICENSE_FILE) - results.extend(input_api.canned_checks.CheckGenderNeutral( - input_api, output_api, source_file_filter=license_filter)) - - results.extend(_V8PresubmitChecks(input_api, output_api)) - results.extend(_CheckUnwantedDependencies(input_api, output_api)) - results.extend( - _CheckNoProductionCodeUsingTestOnlyFunctions(input_api, output_api)) - results.extend(_CheckHeadersHaveIncludeGuards(input_api, output_api)) - results.extend( - _CheckNoInlineHeaderIncludesInNormalHeaders(input_api, output_api)) - results.extend(_CheckJSONFiles(input_api, output_api)) - results.extend(_CheckMacroUndefs(input_api, output_api)) - results.extend(_CheckNoexceptAnnotations(input_api, output_api)) - results.extend(input_api.RunTests( - input_api.canned_checks.CheckVPythonSpec(input_api, output_api))) - return results + checks = [ + input_api.canned_checks.CheckOwnersFormat, + input_api.canned_checks.CheckOwners, + _CheckCommitMessageBugEntry, + input_api.canned_checks.CheckPatchFormatted, + _CheckGenderNeutralInLicenses, + _V8PresubmitChecks, + _CheckUnwantedDependencies, + _CheckNoProductionCodeUsingTestOnlyFunctions, + _CheckHeadersHaveIncludeGuards, + _CheckNoInlineHeaderIncludesInNormalHeaders, + _CheckJSONFiles, + _CheckMacroUndefs, + _CheckNoexceptAnnotations, + _RunTestsWithVPythonSpec, + ] + + return sum([check(input_api, output_api) for check in checks], []) def _SkipTreeCheck(input_api, output_api): @@ -395,7 +460,7 @@ def _CheckMacroUndefs(input_api, output_api): """ Checks that each #define in a .cc file is eventually followed by an #undef. - TODO(clemensh): This check should eventually be enabled for all cc files via + TODO(clemensb): This check should eventually be enabled for all cc files via tools/presubmit.py (https://crbug.com/v8/6811). """ def FilterFile(affected_file): @@ -404,13 +469,29 @@ def _CheckMacroUndefs(input_api, output_api): white_list = (r'.+\.cc',r'.+\.cpp',r'.+\.c') return input_api.FilterSourceFile(affected_file, white_list=white_list) + def Touches(line): + return line.startswith('+') or line.startswith('-') + + def InvolvesMacros(text): + return define_pattern.match(text) or undef_pattern.match(text) + def TouchesMacros(f): - for line in f.GenerateScmDiff().splitlines(): - if not line.startswith('+') and not line.startswith('-'): - continue - if define_pattern.match(line[1:]) or undef_pattern.match(line[1:]): - return True - return False + return any(Touches(line) and InvolvesMacros(line[1:]) + for line in f.GenerateScmDiff().splitlines()) + + def CollectUndefsWithNoDef(defined_macros, errors, f, line, line_nr): + define_match = define_pattern.match(line) + if define_match: + name = define_match.group(1) + defined_macros[name] = line_nr + undef_match = undef_pattern.match(line) + if undef_match and not "// NOLINT" in line: + name = undef_match.group(1) + if name in defined_macros: + del defined_macros[name] + else: + errors.append('{}:{}: Macro named \'{}\' was not defined before.' + .format(f.LocalPath(), line_nr, name)) define_pattern = input_api.re.compile(r'#define (\w+)') undef_pattern = input_api.re.compile(r'#undef (\w+)') @@ -422,25 +503,9 @@ def _CheckMacroUndefs(input_api, output_api): defined_macros = dict() with open(f.LocalPath()) as fh: - line_nr = 0 - for line in fh: - line_nr += 1 - - define_match = define_pattern.match(line) - if define_match: - name = define_match.group(1) - defined_macros[name] = line_nr - - undef_match = undef_pattern.match(line) - if undef_match: - if "// NOLINT" in line: - continue - name = undef_match.group(1) - if not name in defined_macros: - errors.append('{}:{}: Macro named \'{}\' was not defined before.' - .format(f.LocalPath(), line_nr, name)) - else: - del defined_macros[name] + for line_nr, line in enumerate(fh, start=1): + CollectUndefsWithNoDef(defined_macros, errors, f, line, line_nr) + for name, line_nr in sorted(defined_macros.items(), key=lambda e: e[1]): errors.append('{}:{}: Macro missing #undef: {}' .format(f.LocalPath(), line_nr, name)) @@ -463,7 +528,7 @@ def _CheckNoexceptAnnotations(input_api, output_api): Omitting it at some places can result in weird compiler errors if this is mixed with other classes that have the annotation. - TODO(clemensh): This check should eventually be enabled for all files via + TODO(clemensb): This check should eventually be enabled for all files via tools/presubmit.py (https://crbug.com/v8/8616). """ |