Merge c2b9b3fe7dce1a9122dfcc37d531f767b63c2946 on remote branch
Change-Id: I90ded4504eae26daa676611e5dedb47aca760d2b
diff --git a/PREUPLOAD.cfg b/PREUPLOAD.cfg
index 03350c7..81f505d 100644
--- a/PREUPLOAD.cfg
+++ b/PREUPLOAD.cfg
@@ -1,21 +1,14 @@
[Hook Scripts]
# Only list fast unittests here.
-py2_config_unittest = python2 ./rh/config_unittest.py
-py3_config_unittest = python3 ./rh/config_unittest.py
-py2_hooks_unittest = python2 ./rh/hooks_unittest.py
-py3_hooks_unittest = python3 ./rh/hooks_unittest.py
-py2_shell_unittest = python2 ./rh/shell_unittest.py
-py3_shell_unittest = python3 ./rh/shell_unittest.py
-py2_utils_unittest = python2 ./rh/utils_unittest.py
-py3_utils_unittest = python3 ./rh/utils_unittest.py
-py2_android_test_mapping_format_unittest = python2 ./tools/android_test_mapping_format_unittest.py
-py3_android_test_mapping_format_unittest = python3 ./tools/android_test_mapping_format_unittest.py
-py2_config_test = python2 ./rh/config_test.py --check-env --commit-id ${PREUPLOAD_COMMIT} --commit-msg ${PREUPLOAD_COMMIT_MESSAGE} --repo-root ${REPO_ROOT} -- ${PREUPLOAD_FILES}
-py3_config_test = python3 ./rh/config_test.py --check-env --commit-id ${PREUPLOAD_COMMIT} --commit-msg ${PREUPLOAD_COMMIT_MESSAGE} --repo-root ${REPO_ROOT} -- ${PREUPLOAD_FILES}
+config_unittest = ./rh/config_unittest.py
+hooks_unittest = ./rh/hooks_unittest.py
+shell_unittest = ./rh/shell_unittest.py
+utils_unittest = ./rh/utils_unittest.py
+android_test_mapping_format_unittest = ./tools/android_test_mapping_format_unittest.py
+config_test = ./rh/config_test.py --check-env --commit-id ${PREUPLOAD_COMMIT} --commit-msg ${PREUPLOAD_COMMIT_MESSAGE} --repo-root ${REPO_ROOT} -- ${PREUPLOAD_FILES}
[Builtin Hooks]
commit_msg_bug_field = true
commit_msg_changeid_field = true
commit_msg_test_field = true
-pylint2 = true
pylint3 = true
diff --git a/README.md b/README.md
index d35a4c4..dd6ffc6 100644
--- a/README.md
+++ b/README.md
@@ -227,6 +227,34 @@
cpplint = --filter=-x ${PREUPLOAD_FILES}
```
+## [Builtin Hooks Exclude Paths]
+
+*** note
+This section can only be added to the repo project-wide settings
+[GLOBAL-PREUPLOAD.cfg].
+***
+
+Used to explicitly exclude some projects when processing a hook. With this
+section, it is possible to define a hook that should apply to the majority of
+projects except a few.
+
+An entry must completely match the project's `REPO_PATH`. The paths can use the
+[shell-style wildcards](https://docs.python.org/library/fnmatch.html) and
+quotes. For advanced cases, it is possible to use a [regular
+expression](https://docs.python.org/howto/regex.html) by using the `^` prefix.
+
+```
+[Builtin Hooks Exclude Paths]
+# Run cpplint on all projects except ones under external/ and vendor/.
+# The "external" and "vendor" projects, if they exist, will still run cpplint.
+cpplint = external/* vendor/*
+
+# Run rustfmt on all projects except ones under external/. All projects under
+# hardware/ will be excluded except for ones starting with hardware/google (due to
+# the negative regex match).
+rustfmt = external/ ^hardware/(!?google)
+```
+
## [Tool Paths]
Some builtin hooks need to call external executables to work correctly. By
diff --git a/pre-upload.py b/pre-upload.py
index 57f699b..e7ef564 100755
--- a/pre-upload.py
+++ b/pre-upload.py
@@ -1,4 +1,4 @@
-#!/usr/bin/python
+#!/usr/bin/env python3
# -*- coding:utf-8 -*-
# Copyright 2016 The Android Open Source Project
#
@@ -29,18 +29,9 @@
# Assert some minimum Python versions as we don't test or support any others.
-# We only support Python 2.7, and require 2.7.5+/3.4+ to include signal fix:
-# https://bugs.python.org/issue14173
-if sys.version_info < (2, 7, 5):
- print('repohooks: error: Python-2.7.5+ is required', file=sys.stderr)
- sys.exit(1)
-elif sys.version_info.major == 3 and sys.version_info < (3, 5):
+if sys.version_info < (3, 5):
print('repohooks: error: Python-3.5+ is required', file=sys.stderr)
sys.exit(1)
-elif sys.version_info < (3, 6):
- # We want to get people off of old versions of Python.
- print('repohooks: warning: Python-3.6+ is going to be required; '
- 'please upgrade soon to maintain support.', file=sys.stderr)
_path = os.path.dirname(os.path.realpath(__file__))
@@ -212,7 +203,7 @@
# Load the config for this git repo.
'.',
)
- return rh.config.PreUploadConfig(paths=paths, global_paths=global_paths)
+ return rh.config.PreUploadSettings(paths=paths, global_paths=global_paths)
def _attempt_fixes(fixup_func_list, commit_list):
@@ -283,16 +274,17 @@
(e,))
return False
+ project = rh.Project(name=project_name, dir=proj_dir, remote=remote)
+ rel_proj_dir = os.path.relpath(proj_dir, rh.git.find_repo_root())
+
os.environ.update({
'REPO_LREV': rh.git.get_commit_for_ref(upstream_branch),
- 'REPO_PATH': os.path.relpath(proj_dir, rh.git.find_repo_root()),
+ 'REPO_PATH': rel_proj_dir,
'REPO_PROJECT': project_name,
'REPO_REMOTE': remote,
'REPO_RREV': rh.git.get_remote_revision(upstream_branch, remote),
})
- project = rh.Project(name=project_name, dir=proj_dir, remote=remote)
-
if not commit_list:
commit_list = rh.git.get_commits(
ignore_merged_commits=config.ignore_merged_commits)
@@ -310,8 +302,10 @@
commit_summary = desc.split('\n', 1)[0]
output.commit_start(commit=commit, commit_summary=commit_summary)
- for name, hook in hooks:
+ for name, hook, exclusion_scope in hooks:
output.hook_start(name)
+ if rel_proj_dir in exclusion_scope:
+ break
hook_results = hook(project, commit, desc, diff)
(error, warning) = _process_hook_results(hook_results)
if error is not None or warning is not None:
diff --git a/rh/config.py b/rh/config.py
index 6e96fc7..e2ad713 100644
--- a/rh/config.py
+++ b/rh/config.py
@@ -18,6 +18,7 @@
from __future__ import print_function
import functools
+import itertools
import os
import shlex
import sys
@@ -41,99 +42,97 @@
"""Config file has unknown sections/keys or other values."""
+# Sentinel so we can handle None-vs-unspecified.
+_UNSET = object()
+
+
class RawConfigParser(configparser.RawConfigParser):
"""Like RawConfigParser but with some default helpers."""
- @staticmethod
- def _check_args(name, cnt_min, cnt_max, args):
- cnt = len(args)
- if cnt not in (0, cnt_max - cnt_min):
- raise TypeError('%s() takes %i or %i arguments (got %i)' %
- (name, cnt_min, cnt_max, cnt,))
- return cnt
-
- # pylint can't seem to grok our use of *args here.
+ # pylint doesn't like it when we extend the API.
# pylint: disable=arguments-differ
- def options(self, section, *args):
- """Return the options in |section| (with default |args|).
+ def options(self, section, default=_UNSET):
+ """Return the options in |section|.
Args:
section: The section to look up.
- args: What to return if |section| does not exist.
+ default: What to return if |section| does not exist.
"""
- cnt = self._check_args('options', 2, 3, args)
try:
return configparser.RawConfigParser.options(self, section)
except configparser.NoSectionError:
- if cnt == 1:
- return args[0]
+ if default is not _UNSET:
+ return default
raise
- def get(self, section, option, *args):
- """Return the value for |option| in |section| (with default |args|)."""
- cnt = self._check_args('get', 3, 4, args)
+ def get(self, section, option, default=_UNSET):
+ """Return the value for |option| in |section| (with |default|)."""
try:
return configparser.RawConfigParser.get(self, section, option)
except (configparser.NoSectionError, configparser.NoOptionError):
- if cnt == 1:
- return args[0]
+ if default is not _UNSET:
+ return default
raise
- def items(self, section, *args):
+ def items(self, section=_UNSET, default=_UNSET):
"""Return a list of (key, value) tuples for the options in |section|."""
- cnt = self._check_args('items', 2, 3, args)
+ if section is _UNSET:
+ # Python 3 compat logic. Return a dict of section-to-options.
+ if sys.version_info.major < 3:
+ return [(x, self.items(x)) for x in self.sections()]
+ return super(RawConfigParser, self).items()
+
try:
return configparser.RawConfigParser.items(self, section)
except configparser.NoSectionError:
- if cnt == 1:
- return args[0]
+ if default is not _UNSET:
+ return default
raise
+ if sys.version_info.major < 3:
+ def read_dict(self, dictionary):
+ """Store |dictionary| into ourselves."""
+ for section, settings in dictionary.items():
+ for option, value in settings:
+ if not self.has_section(section):
+ self.add_section(section)
+ self.set(section, option, value)
+
class PreUploadConfig(object):
- """Config file used for per-project `repo upload` hooks."""
-
- FILENAME = 'PREUPLOAD.cfg'
- GLOBAL_FILENAME = 'GLOBAL-PREUPLOAD.cfg'
+ """A single (abstract) config used for `repo upload` hooks."""
CUSTOM_HOOKS_SECTION = 'Hook Scripts'
BUILTIN_HOOKS_SECTION = 'Builtin Hooks'
BUILTIN_HOOKS_OPTIONS_SECTION = 'Builtin Hooks Options'
+ BUILTIN_HOOKS_EXCLUDE_SECTION = 'Builtin Hooks Exclude Paths'
TOOL_PATHS_SECTION = 'Tool Paths'
OPTIONS_SECTION = 'Options'
+ VALID_SECTIONS = {
+ CUSTOM_HOOKS_SECTION,
+ BUILTIN_HOOKS_SECTION,
+ BUILTIN_HOOKS_OPTIONS_SECTION,
+ BUILTIN_HOOKS_EXCLUDE_SECTION,
+ TOOL_PATHS_SECTION,
+ OPTIONS_SECTION,
+ }
OPTION_IGNORE_MERGED_COMMITS = 'ignore_merged_commits'
- VALID_OPTIONS = (OPTION_IGNORE_MERGED_COMMITS,)
+ VALID_OPTIONS = {OPTION_IGNORE_MERGED_COMMITS}
- def __init__(self, paths=('',), global_paths=()):
+ def __init__(self, config=None, source=None):
"""Initialize.
- All the config files found will be merged together in order.
-
Args:
- paths: The directories to look for config files.
- global_paths: The directories to look for global config files.
+ config: A configparse.ConfigParser instance.
+ source: Where this config came from. This is used in error messages to
+ facilitate debugging. It is not necessarily a valid path.
"""
- config = RawConfigParser()
-
- def _search(paths, filename):
- for path in paths:
- path = os.path.join(path, filename)
- if os.path.exists(path):
- self.paths.append(path)
- try:
- config.read(path)
- except configparser.ParsingError as e:
- raise ValidationError('%s: %s' % (path, e))
-
- self.paths = []
- _search(global_paths, self.GLOBAL_FILENAME)
- _search(paths, self.FILENAME)
-
- self.config = config
-
- self._validate()
+ self.config = config if config else RawConfigParser()
+ self.source = source
+ if config:
+ self._validate()
@property
def custom_hooks(self):
@@ -155,26 +154,35 @@
return shlex.split(self.config.get(self.BUILTIN_HOOKS_OPTIONS_SECTION,
hook, ''))
+ def builtin_hook_exclude_paths(self, hook):
+ """List of paths for which |hook| should not be executed."""
+ return shlex.split(self.config.get(self.BUILTIN_HOOKS_EXCLUDE_SECTION,
+ hook, ''))
+
@property
def tool_paths(self):
"""List of all tool paths."""
return dict(self.config.items(self.TOOL_PATHS_SECTION, ()))
def callable_hooks(self):
- """Yield a name and callback for each hook to be executed."""
+ """Yield a CallableHook for each hook to be executed."""
+ scope = rh.hooks.ExclusionScope([])
for hook in self.custom_hooks:
options = rh.hooks.HookOptions(hook,
self.custom_hook(hook),
self.tool_paths)
- yield (hook, functools.partial(rh.hooks.check_custom,
- options=options))
+ func = functools.partial(rh.hooks.check_custom, options=options)
+ yield rh.hooks.CallableHook(hook, func, scope)
for hook in self.builtin_hooks:
options = rh.hooks.HookOptions(hook,
self.builtin_hook_option(hook),
self.tool_paths)
- yield (hook, functools.partial(rh.hooks.BUILTIN_HOOKS[hook],
- options=options))
+ func = functools.partial(rh.hooks.BUILTIN_HOOKS[hook],
+ options=options)
+ scope = rh.hooks.ExclusionScope(
+ self.builtin_hook_exclude_paths(hook))
+ yield rh.hooks.CallableHook(hook, func, scope)
@property
def ignore_merged_commits(self):
@@ -184,28 +192,25 @@
self.OPTION_IGNORE_MERGED_COMMITS, None),
False)
+ def update(self, preupload_config):
+ """Merge settings from |preupload_config| into ourself."""
+ self.config.read_dict(preupload_config.config)
+
def _validate(self):
"""Run consistency checks on the config settings."""
config = self.config
# Reject unknown sections.
- valid_sections = set((
- self.CUSTOM_HOOKS_SECTION,
- self.BUILTIN_HOOKS_SECTION,
- self.BUILTIN_HOOKS_OPTIONS_SECTION,
- self.TOOL_PATHS_SECTION,
- self.OPTIONS_SECTION,
- ))
- bad_sections = set(config.sections()) - valid_sections
+ bad_sections = set(config.sections()) - self.VALID_SECTIONS
if bad_sections:
raise ValidationError('%s: unknown sections: %s' %
- (self.paths, bad_sections))
+ (self.source, bad_sections))
# Reject blank custom hooks.
for hook in self.custom_hooks:
if not config.get(self.CUSTOM_HOOKS_SECTION, hook):
raise ValidationError('%s: custom hook "%s" cannot be blank' %
- (self.paths, hook))
+ (self.source, hook))
# Reject unknown builtin hooks.
valid_builtin_hooks = set(rh.hooks.BUILTIN_HOOKS.keys())
@@ -214,7 +219,7 @@
bad_hooks = hooks - valid_builtin_hooks
if bad_hooks:
raise ValidationError('%s: unknown builtin hooks: %s' %
- (self.paths, bad_hooks))
+ (self.source, bad_hooks))
elif config.has_section(self.BUILTIN_HOOKS_OPTIONS_SECTION):
raise ValidationError('Builtin hook options specified, but missing '
'builtin hook settings')
@@ -224,7 +229,7 @@
bad_hooks = hooks - valid_builtin_hooks
if bad_hooks:
raise ValidationError('%s: unknown builtin hook options: %s' %
- (self.paths, bad_hooks))
+ (self.source, bad_hooks))
# Verify hooks are valid shell strings.
for hook in self.custom_hooks:
@@ -232,7 +237,7 @@
self.custom_hook(hook)
except ValueError as e:
raise ValidationError('%s: hook "%s" command line is invalid: '
- '%s' % (self.paths, hook, e))
+ '%s' % (self.source, hook, e))
# Verify hook options are valid shell strings.
for hook in self.builtin_hooks:
@@ -240,7 +245,7 @@
self.builtin_hook_option(hook)
except ValueError as e:
raise ValidationError('%s: hook options "%s" are invalid: %s' %
- (self.paths, hook, e))
+ (self.source, hook, e))
# Reject unknown tools.
valid_tools = set(rh.hooks.TOOL_PATHS.keys())
@@ -249,13 +254,105 @@
bad_tools = tools - valid_tools
if bad_tools:
raise ValidationError('%s: unknown tools: %s' %
- (self.paths, bad_tools))
+ (self.source, bad_tools))
# Reject unknown options.
- valid_options = set(self.VALID_OPTIONS)
if config.has_section(self.OPTIONS_SECTION):
options = set(config.options(self.OPTIONS_SECTION))
- bad_options = options - valid_options
+ bad_options = options - self.VALID_OPTIONS
if bad_options:
raise ValidationError('%s: unknown options: %s' %
- (self.paths, bad_options))
+ (self.source, bad_options))
+
+
+class PreUploadFile(PreUploadConfig):
+ """A single config (file) used for `repo upload` hooks.
+
+ This is an abstract class that requires subclasses to define the FILENAME
+ constant.
+
+ Attributes:
+ path: The path of the file.
+ """
+ FILENAME = None
+
+ def __init__(self, path):
+ """Initialize.
+
+ Args:
+ path: The config file to load.
+ """
+ super(PreUploadFile, self).__init__(source=path)
+
+ self.path = path
+ try:
+ self.config.read(path)
+ except configparser.ParsingError as e:
+ raise ValidationError('%s: %s' % (path, e))
+
+ self._validate()
+
+ @classmethod
+ def from_paths(cls, paths):
+ """Search for files within paths that matches the class FILENAME.
+
+ Args:
+ paths: List of directories to look for config files.
+
+ Yields:
+ For each valid file found, an instance is created and returned.
+ """
+ for path in paths:
+ path = os.path.join(path, cls.FILENAME)
+ if os.path.exists(path):
+ yield cls(path)
+
+
+class LocalPreUploadFile(PreUploadFile):
+ """A single config file for a project (PREUPLOAD.cfg)."""
+ FILENAME = 'PREUPLOAD.cfg'
+
+ def _validate(self):
+ super(LocalPreUploadFile, self)._validate()
+
+ # Reject Exclude Paths section for local config.
+ if self.config.has_section(self.BUILTIN_HOOKS_EXCLUDE_SECTION):
+ raise ValidationError('%s: [%s] is not valid in local files' %
+ (self.path,
+ self.BUILTIN_HOOKS_EXCLUDE_SECTION))
+
+
+class GlobalPreUploadFile(PreUploadFile):
+ """A single config file for a repo (GLOBAL-PREUPLOAD.cfg)."""
+ FILENAME = 'GLOBAL-PREUPLOAD.cfg'
+
+
+class PreUploadSettings(PreUploadConfig):
+ """Settings for `repo upload` hooks.
+
+ This encompasses multiple config files and provides the final (merged)
+ settings for a particular project.
+ """
+
+ def __init__(self, paths=('',), global_paths=()):
+ """Initialize.
+
+ All the config files found will be merged together in order.
+
+ Args:
+ paths: The directories to look for config files.
+ global_paths: The directories to look for global config files.
+ """
+ super(PreUploadSettings, self).__init__()
+
+ self.paths = []
+ for config in itertools.chain(
+ GlobalPreUploadFile.from_paths(global_paths),
+ LocalPreUploadFile.from_paths(paths)):
+ self.paths.append(config.path)
+ self.update(config)
+
+
+ # We validated configs in isolation, now do one final pass altogether.
+ self.source = '{%s}' % '|'.join(self.paths)
+ self._validate()
diff --git a/rh/config_unittest.py b/rh/config_unittest.py
index d51afdc..4b27c5a 100755
--- a/rh/config_unittest.py
+++ b/rh/config_unittest.py
@@ -39,37 +39,53 @@
class PreUploadConfigTests(unittest.TestCase):
"""Tests for the PreUploadConfig class."""
+ def testMissing(self):
+ """Instantiating a non-existent config file should be fine."""
+ rh.config.PreUploadConfig()
+
+
+class FileTestCase(unittest.TestCase):
+ """Helper class for tests cases to setup configuration files."""
+
def setUp(self):
self.tempdir = tempfile.mkdtemp()
def tearDown(self):
shutil.rmtree(self.tempdir)
- def _write_config(self, data, filename=None):
- """Helper to write out a config file for testing."""
- if filename is None:
- filename = rh.config.PreUploadConfig.FILENAME
+ def _write_config(self, data, filename='temp.cfg'):
+ """Helper to write out a config file for testing.
+
+ Returns:
+ Path to the file where the configuration was written.
+ """
path = os.path.join(self.tempdir, filename)
with open(path, 'w') as fp:
fp.write(data)
+ return path
+
+ def _write_local_config(self, data):
+ """Helper to write out a local config file for testing."""
+ return self._write_config(
+ data, filename=rh.config.LocalPreUploadFile.FILENAME)
def _write_global_config(self, data):
"""Helper to write out a global config file for testing."""
- self._write_config(
- data, filename=rh.config.PreUploadConfig.GLOBAL_FILENAME)
+ return self._write_config(
+ data, filename=rh.config.GlobalPreUploadFile.FILENAME)
- def testMissing(self):
- """Instantiating a non-existent config file should be fine."""
- rh.config.PreUploadConfig()
+
+class PreUploadFileTests(FileTestCase):
+ """Tests for the PreUploadFile class."""
def testEmpty(self):
"""Instantiating an empty config file should be fine."""
- self._write_config('')
- rh.config.PreUploadConfig(paths=(self.tempdir,))
+ path = self._write_config('')
+ rh.config.PreUploadFile(path)
def testValid(self):
"""Verify a fully valid file works."""
- self._write_config("""# This be a comment me matey.
+ path = self._write_config("""# This be a comment me matey.
[Hook Scripts]
name = script --with "some args"
@@ -82,39 +98,56 @@
[Options]
ignore_merged_commits = true
""")
- rh.config.PreUploadConfig(paths=(self.tempdir,))
+ rh.config.PreUploadFile(path)
def testUnknownSection(self):
"""Reject unknown sections."""
- self._write_config('[BOOGA]')
- self.assertRaises(rh.config.ValidationError, rh.config.PreUploadConfig,
- paths=(self.tempdir,))
+ path = self._write_config('[BOOGA]')
+ self.assertRaises(rh.config.ValidationError, rh.config.PreUploadFile,
+ path)
def testUnknownBuiltin(self):
"""Reject unknown builtin hooks."""
- self._write_config('[Builtin Hooks]\nbooga = borg!')
- self.assertRaises(rh.config.ValidationError, rh.config.PreUploadConfig,
- paths=(self.tempdir,))
+ path = self._write_config('[Builtin Hooks]\nbooga = borg!')
+ self.assertRaises(rh.config.ValidationError, rh.config.PreUploadFile,
+ path)
def testEmptyCustomHook(self):
"""Reject empty custom hooks."""
- self._write_config('[Hook Scripts]\nbooga = \t \n')
- self.assertRaises(rh.config.ValidationError, rh.config.PreUploadConfig,
- paths=(self.tempdir,))
+ path = self._write_config('[Hook Scripts]\nbooga = \t \n')
+ self.assertRaises(rh.config.ValidationError, rh.config.PreUploadFile,
+ path)
def testInvalidIni(self):
"""Reject invalid ini files."""
- self._write_config('[Hook Scripts]\n =')
- self.assertRaises(rh.config.ValidationError, rh.config.PreUploadConfig,
- paths=(self.tempdir,))
+ path = self._write_config('[Hook Scripts]\n =')
+ self.assertRaises(rh.config.ValidationError, rh.config.PreUploadFile,
+ path)
def testInvalidString(self):
"""Catch invalid string quoting."""
- self._write_config("""[Hook Scripts]
+ path = self._write_config("""[Hook Scripts]
name = script --'bad-quotes
""")
- self.assertRaises(rh.config.ValidationError, rh.config.PreUploadConfig,
- paths=(self.tempdir,))
+ self.assertRaises(rh.config.ValidationError, rh.config.PreUploadFile,
+ path)
+
+
+class LocalPreUploadFileTests(FileTestCase):
+ """Test for the LocalPreUploadFile class."""
+
+ def testInvalidSectionConfig(self):
+ """Reject local config that uses invalid sections."""
+ path = self._write_config("""[Builtin Hooks Exclude Paths]
+cpplint = external/ 'test directory' ^vendor/(?!google/)
+""")
+ self.assertRaises(rh.config.ValidationError,
+ rh.config.LocalPreUploadFile,
+ path)
+
+
+class PreUploadSettingsTests(FileTestCase):
+ """Tests for the PreUploadSettings class."""
def testGlobalConfigs(self):
"""Verify global configs stack properly."""
@@ -122,14 +155,21 @@
commit_msg_bug_field = true
commit_msg_changeid_field = true
commit_msg_test_field = false""")
- self._write_config("""[Builtin Hooks]
+ self._write_local_config("""[Builtin Hooks]
commit_msg_bug_field = false
commit_msg_test_field = true""")
- config = rh.config.PreUploadConfig(paths=(self.tempdir,),
- global_paths=(self.tempdir,))
+ config = rh.config.PreUploadSettings(paths=(self.tempdir,),
+ global_paths=(self.tempdir,))
self.assertEqual(config.builtin_hooks,
['commit_msg_changeid_field', 'commit_msg_test_field'])
+ def testGlobalExcludeScope(self):
+ """Verify exclude scope is valid for global config."""
+ self._write_global_config("""[Builtin Hooks Exclude Paths]
+cpplint = external/ 'test directory' ^vendor/(?!google/)
+""")
+ rh.config.PreUploadSettings(global_paths=(self.tempdir,))
+
if __name__ == '__main__':
unittest.main()
diff --git a/rh/git.py b/rh/git.py
index baf669c..da9d55c 100644
--- a/rh/git.py
+++ b/rh/git.py
@@ -167,12 +167,12 @@
Returns:
A list of modified/added (and perhaps deleted) files
"""
- return raw_diff(os.getcwd(), '%s^!' % commit)
+ return raw_diff(os.getcwd(), '%s^-' % commit)
def get_commits(ignore_merged_commits=False):
"""Returns a list of commits for this review."""
- cmd = ['git', 'log', '%s..' % get_upstream_branch(), '--format=%H']
+ cmd = ['git', 'rev-list', '%s..' % get_upstream_branch()]
if ignore_merged_commits:
cmd.append('--first-parent')
return rh.utils.run(cmd, capture_output=True).stdout.split()
@@ -180,7 +180,7 @@
def get_commit_desc(commit):
"""Returns the full commit message of a commit."""
- cmd = ['git', 'log', '--format=%B', commit + '^!']
+ cmd = ['git', 'diff-tree', '-s', '--always', '--format=%B', commit]
return rh.utils.run(cmd, capture_output=True).stdout
diff --git a/rh/hooks.py b/rh/hooks.py
index 7c13bf1..491da91 100644
--- a/rh/hooks.py
+++ b/rh/hooks.py
@@ -17,6 +17,8 @@
from __future__ import print_function
+import collections
+import fnmatch
import json
import os
import platform
@@ -141,6 +143,42 @@
return _get_build_os_name()
+class ExclusionScope(object):
+ """Exclusion scope for a hook.
+
+ An exclusion scope can be used to determine if a hook has been disabled for
+ a specific project.
+ """
+
+ def __init__(self, scope):
+ """Initialize.
+
+ Args:
+ scope: A list of shell-style wildcards (fnmatch) or regular
+ expression. Regular expressions must start with the ^ character.
+ """
+ self._scope = []
+ for path in scope:
+ if path.startswith('^'):
+ self._scope.append(re.compile(path))
+ else:
+ self._scope.append(path)
+
+ def __contains__(self, proj_dir):
+ """Checks if |proj_dir| matches the excluded paths.
+
+ Args:
+ proj_dir: The relative path of the project.
+ """
+ for exclusion_path in self._scope:
+ if hasattr(exclusion_path, 'match'):
+ if exclusion_path.match(proj_dir):
+ return True
+ elif fnmatch.fnmatch(proj_dir, exclusion_path):
+ return True
+ return False
+
+
class HookOptions(object):
"""Holder class for hook options."""
@@ -199,6 +237,10 @@
return self.expand_vars([tool_path])[0]
+# A callable hook.
+CallableHook = collections.namedtuple('CallableHook', ('name', 'hook', 'scope'))
+
+
def _run(cmd, **kwargs):
"""Helper command for checks that tend to gather output."""
kwargs.setdefault('combine_stdout_stderr', True)
@@ -792,8 +834,27 @@
return None
rustfmt = options.tool_path('rustfmt')
- cmd = [rustfmt] + options.args(('--check', '${PREUPLOAD_FILES}',), filtered)
- return _check_cmd('rustfmt', project, commit, cmd)
+ cmd = [rustfmt] + options.args((), filtered)
+ ret = []
+ for d in filtered:
+ data = rh.git.get_file_content(commit, d.file)
+ result = _run(cmd, input=data)
+ # If the parsing failed, stdout will contain enough details on the
+ # location of the error.
+ if result.returncode:
+ ret.append(rh.results.HookResult(
+ 'rustfmt', project, commit, error=result.stdout,
+ files=(d.file,)))
+ continue
+ # TODO(b/164111102): rustfmt stable does not support --check on stdin.
+ # If no error is reported, compare stdin with stdout.
+ if data != result.stdout:
+ msg = ('To fix, please run: %s' %
+ rh.shell.cmd_to_str(cmd + [d.file]))
+ ret.append(rh.results.HookResult(
+ 'rustfmt', project, commit, error=msg,
+ files=(d.file,)))
+ return ret
def check_xmllint(project, commit, _desc, diff, options=None):
diff --git a/rh/hooks_unittest.py b/rh/hooks_unittest.py
index 373e09f..12059f8 100755
--- a/rh/hooks_unittest.py
+++ b/rh/hooks_unittest.py
@@ -182,6 +182,28 @@
self.assertEqual(self.replacer.get('BUILD_OS'), m.return_value)
+class ExclusionScopeTests(unittest.TestCase):
+ """Verify behavior of ExclusionScope class."""
+
+ def testEmpty(self):
+ """Verify the in operator for an empty scope."""
+ scope = rh.hooks.ExclusionScope([])
+ self.assertNotIn('external/*', scope)
+
+ def testGlob(self):
+ """Verify the in operator for a scope using wildcards."""
+ scope = rh.hooks.ExclusionScope(['vendor/*', 'external/*'])
+ self.assertIn('external/tools', scope)
+
+ def testRegex(self):
+ """Verify the in operator for a scope using regular expressions."""
+ scope = rh.hooks.ExclusionScope(['^vendor/(?!google)',
+ 'external/*'])
+ self.assertIn('vendor/', scope)
+ self.assertNotIn('vendor/google/', scope)
+ self.assertIn('vendor/other/', scope)
+
+
class HookOptionsTests(unittest.TestCase):
"""Verify behavior of HookOptions object."""
@@ -293,7 +315,7 @@
"""
# First call should do nothing as there are no files to check.
ret = func(self.project, 'commit', 'desc', (), options=self.options)
- self.assertEqual(ret, None)
+ self.assertIsNone(ret)
self.assertFalse(mock_check.called)
# Second call should include some checks.
@@ -690,21 +712,21 @@
# First call should do nothing as there are no files to check.
ret = rh.hooks.check_gofmt(
self.project, 'commit', 'desc', (), options=self.options)
- self.assertEqual(ret, None)
+ self.assertIsNone(ret)
self.assertFalse(mock_check.called)
# Second call will have some results.
diff = [rh.git.RawDiffEntry(file='foo.go')]
ret = rh.hooks.check_gofmt(
self.project, 'commit', 'desc', diff, options=self.options)
- self.assertNotEqual(ret, None)
+ self.assertIsNotNone(ret)
def test_jsonlint(self, mock_check, _mock_run):
"""Verify the jsonlint builtin hook."""
# First call should do nothing as there are no files to check.
ret = rh.hooks.check_json(
self.project, 'commit', 'desc', (), options=self.options)
- self.assertEqual(ret, None)
+ self.assertIsNone(ret)
self.assertFalse(mock_check.called)
# TODO: Actually pass some valid/invalid json data down.
@@ -725,8 +747,17 @@
('foo.py',))
def test_rustfmt(self, mock_check, _mock_run):
- self._test_file_filter(mock_check, rh.hooks.check_rustfmt,
- ('foo.rs',))
+ # First call should do nothing as there are no files to check.
+ ret = rh.hooks.check_rustfmt(
+ self.project, 'commit', 'desc', (), options=self.options)
+ self.assertEqual(ret, None)
+ self.assertFalse(mock_check.called)
+
+ # Second call will have some results.
+ diff = [rh.git.RawDiffEntry(file='lib.rs')]
+ ret = rh.hooks.check_rustfmt(
+ self.project, 'commit', 'desc', diff, options=self.options)
+ self.assertNotEqual(ret, None)
def test_xmllint(self, mock_check, _mock_run):
"""Verify the xmllint builtin hook."""
diff --git a/tools/android_test_mapping_format.py b/tools/android_test_mapping_format.py
index 14b02f5..47e09c5 100755
--- a/tools/android_test_mapping_format.py
+++ b/tools/android_test_mapping_format.py
@@ -1,4 +1,4 @@
-#!/usr/bin/python
+#!/usr/bin/env python3
# -*- coding:utf-8 -*-
# Copyright 2018 The Android Open Source Project
#
diff --git a/tools/clang-format.py b/tools/clang-format.py
index 7fa5b10..958f543 100755
--- a/tools/clang-format.py
+++ b/tools/clang-format.py
@@ -1,4 +1,4 @@
-#!/usr/bin/python
+#!/usr/bin/env python3
# -*- coding:utf-8 -*-
# Copyright 2016 The Android Open Source Project
#
diff --git a/tools/cpplint.py b/tools/cpplint.py
index c08cf6f..e99d661 100755
--- a/tools/cpplint.py
+++ b/tools/cpplint.py
@@ -52,6 +52,12 @@
import string
import sys
import unicodedata
+import sysconfig
+
+try:
+ xrange # Python 2
+except NameError:
+ xrange = range # Python 3
_USAGE = """
@@ -570,7 +576,7 @@
# Automatically append to extensions list so it does not have to be set 2 times
_valid_extensions.update(_hpp_headers)
except ValueError:
- PrintUsage('Header extensions must be comma seperated list.')
+ PrintUsage('Header extensions must be comma separated list.')
def IsHeaderExtension(file_extension):
return file_extension in _hpp_headers
@@ -1378,7 +1384,7 @@
def RemoveMultiLineCommentsFromRange(lines, begin, end):
"""Clears a range of lines for multi-line comments."""
- # Having // dummy comments makes the lines non-empty, so we will not get
+ # Having // <empty> comments makes the lines non-empty, so we will not get
# unnecessary blank line warnings later in the code.
for i in range(begin, end):
lines[i] = '/**/'
@@ -1752,7 +1758,7 @@
"""Logs an error if no Copyright message appears at the top of the file."""
# We'll say it should occur by line 10. Don't forget there's a
- # dummy line at the front.
+ # placeholder line at the front.
for line in xrange(1, min(len(lines), 11)):
if re.search(r'Copyright', lines[line], re.I): break
else: # means no copyright line was found
@@ -1847,8 +1853,8 @@
PathSplitToList(_root))
if _root_debug:
- sys.stderr.write("_root lstrip (maybe_path=%s, file_path_from_root=%s," +
- " _root=%s)\n" %(maybe_path, file_path_from_root, _root))
+ sys.stderr.write(("_root lstrip (maybe_path=%s, file_path_from_root=%s," +
+ " _root=%s)\n") %(maybe_path, file_path_from_root, _root))
if maybe_path:
return os.path.join(*maybe_path)
@@ -1861,8 +1867,8 @@
PathSplitToList(root_abspath))
if _root_debug:
- sys.stderr.write("_root prepend (maybe_path=%s, full_path=%s, " +
- "root_abspath=%s)\n" %(maybe_path, full_path, root_abspath))
+ sys.stderr.write(("_root prepend (maybe_path=%s, full_path=%s, " +
+ "root_abspath=%s)\n") %(maybe_path, full_path, root_abspath))
if maybe_path:
return os.path.join(*maybe_path)
@@ -3278,8 +3284,8 @@
line = clean_lines.elided[linenum]
# You shouldn't have spaces before your brackets, except maybe after
- # 'delete []' or 'return []() {};'
- if Search(r'\w\s+\[', line) and not Search(r'(?:delete|return)\s+\[', line):
+ # 'delete []', 'return []() {};', or 'auto [abc, ...] = ...;'.
+ if Search(r'\w\s+\[', line) and not Search(r'(?:auto&?|delete|return)\s+\[', line):
error(filename, linenum, 'whitespace/braces', 5,
'Extra space before [')
@@ -3861,9 +3867,9 @@
# Block bodies should not be followed by a semicolon. Due to C++11
# brace initialization, there are more places where semicolons are
- # required than not, so we use a whitelist approach to check these
- # rather than a blacklist. These are the places where "};" should
- # be replaced by just "}":
+ # required than not, so we explicitly list the allowed rules rather
+ # than listing the disallowed ones. These are the places where "};"
+ # should be replaced by just "}":
# 1. Some flavor of block following closing parenthesis:
# for (;;) {};
# while (...) {};
@@ -3919,11 +3925,11 @@
# - INTERFACE_DEF
# - EXCLUSIVE_LOCKS_REQUIRED, SHARED_LOCKS_REQUIRED, LOCKS_EXCLUDED:
#
- # We implement a whitelist of safe macros instead of a blacklist of
+ # We implement a list of safe macros instead of a list of
# unsafe macros, even though the latter appears less frequently in
# google code and would have been easier to implement. This is because
- # the downside for getting the whitelist wrong means some extra
- # semicolons, while the downside for getting the blacklist wrong
+ # the downside for getting the allowed checks wrong means some extra
+ # semicolons, while the downside for getting disallowed checks wrong
# would result in compile errors.
#
# In addition to macros, we also don't want to warn on
@@ -4287,6 +4293,16 @@
if unicodedata.east_asian_width(uc) in ('W', 'F'):
width += 2
elif not unicodedata.combining(uc):
+ # Issue 337
+ # https://mail.python.org/pipermail/python-list/2012-August/628809.html
+ if (sys.version_info.major, sys.version_info.minor) <= (3, 2):
+ # https://github.com/python/cpython/blob/2.7/Include/unicodeobject.h#L81
+ is_wide_build = sysconfig.get_config_var("Py_UNICODE_SIZE") >= 4
+ # https://github.com/python/cpython/blob/2.7/Objects/unicodeobject.c#L564
+ is_low_surrogate = 0xDC00 <= ord(uc) <= 0xDFFF
+ if not is_wide_build and is_low_surrogate:
+ width -= 1
+
width += 1
return width
else:
@@ -5109,19 +5125,19 @@
#
# We also accept & in static_assert, which looks like a function but
# it's actually a declaration expression.
- whitelisted_functions = (r'(?:[sS]wap(?:<\w:+>)?|'
+ allowed_functions = (r'(?:[sS]wap(?:<\w:+>)?|'
r'operator\s*[<>][<>]|'
r'static_assert|COMPILE_ASSERT'
r')\s*\(')
- if Search(whitelisted_functions, line):
+ if Search(allowed_functions, line):
return
elif not Search(r'\S+\([^)]*$', line):
- # Don't see a whitelisted function on this line. Actually we
+ # Don't see an allowed function on this line. Actually we
# didn't see any function name on this line, so this is likely a
# multi-line parameter list. Try a bit harder to catch this case.
for i in xrange(2):
if (linenum > i and
- Search(whitelisted_functions, clean_lines.elided[linenum - i - 1])):
+ Search(allowed_functions, clean_lines.elided[linenum - i - 1])):
return
decls = ReplaceAll(r'{[^}]*}', ' ', line) # exclude function body
@@ -6189,7 +6205,7 @@
try:
_valid_extensions = set(val.split(','))
except ValueError:
- PrintUsage('Extensions must be comma seperated list.')
+ PrintUsage('Extensions must be comma separated list.')
elif opt == '--headers':
ProcessHppHeadersOption(val)
diff --git a/tools/google-java-format.py b/tools/google-java-format.py
index ed5ce28..5a537c0 100755
--- a/tools/google-java-format.py
+++ b/tools/google-java-format.py
@@ -1,4 +1,4 @@
-#!/usr/bin/python
+#!/usr/bin/env python3
# -*- coding:utf-8 -*-
# Copyright 2016 The Android Open Source Project
#