diff options
author | android-build-team Robot <android-build-team-robot@google.com> | 2021-02-08 23:48:59 +0000 |
---|---|---|
committer | android-build-team Robot <android-build-team-robot@google.com> | 2021-02-08 23:48:59 +0000 |
commit | eb66132ac67bb09dac3746fc49fc0afe2badcdb7 (patch) | |
tree | 452a13ed8070f7cee303a3daa9388dea0efff390 | |
parent | ed2806ac60cc86b01b3c7a623d896683ee980492 (diff) | |
parent | a3ae3b069e87f3bcc3073409a2d7c64e50abe20b (diff) | |
download | repohooks-eb66132ac67bb09dac3746fc49fc0afe2badcdb7.tar.gz |
Snap for 7132927 from a3ae3b069e87f3bcc3073409a2d7c64e50abe20b to mainline-captiveportallogin-releaseandroid-mainline-11.0.0_r36android-mainline-11.0.0_r30android-mainline-11.0.0_r12android11-mainline-captiveportallogin-release
Change-Id: I5dda4a4137f7d019431cc4b93584816a253e907d
-rw-r--r-- | README.md | 10 | ||||
-rw-r--r-- | rh/config.py | 34 | ||||
-rw-r--r-- | rh/hooks.py | 22 | ||||
-rwxr-xr-x | rh/hooks_unittest.py | 14 | ||||
-rw-r--r-- | rh/utils.py | 17 | ||||
-rwxr-xr-x | tools/android_test_mapping_format.py | 11 |
6 files changed, 55 insertions, 53 deletions
@@ -170,6 +170,9 @@ some dog = tool --no-cat-in-commit-message ${PREUPLOAD_COMMIT_MESSAGE} This section allows for turning on common/builtin hooks. There are a bunch of canned hooks already included geared towards AOSP style guidelines. +* `aidl_format`: Run AIDL files (.aidl) through `aidl-format`. +* `android_test_mapping_format`: Validate TEST_MAPPING files in Android source + code. Refer to go/test-mapping for more details. * `bpfmt`: Run Blueprint files (.bp) through `bpfmt`. * `checkpatch`: Run commits through the Linux kernel's `checkpatch.pl` script. * `clang_format`: Run git-clang-format against the commit. The default style is @@ -195,8 +198,6 @@ canned hooks already included geared towards AOSP style guidelines. * `pylint3`: Run Python code through `pylint` using Python 3. * `rustfmt`: Run Rust code through `rustfmt`. * `xmllint`: Run XML code through `xmllint`. -* `android_test_mapping_format`: Validate TEST_MAPPING files in Android source - code. Refer to go/test-mapping for more details. Note: Builtin hooks tend to match specific filenames (e.g. `.json`). If no files match in a specific commit, then the hook will be skipped for that commit. @@ -263,6 +264,9 @@ executables can be overridden through `[Tool Paths]`. This is helpful to provide consistent behavior for developers across different OS and Linux distros/versions. The following tools are recognized: +* `aidl-format`: used for the `aidl_format` builtin hook. +* `android-test-mapping-format`: used for the `android_test_mapping_format` + builtin hook. * `bpfmt`: used for the `bpfmt` builtin hook. * `clang-format`: used for the `clang_format` builtin hook. * `cpplint`: used for the `cpplint` builtin hook. @@ -272,8 +276,6 @@ distros/versions. The following tools are recognized: * `google-java-format-diff`: used for the `google_java_format` builtin hook. * `pylint`: used for the `pylint` builtin hook. * `rustfmt`: used for the `rustfmt` builtin hook. -* `android-test-mapping-format`: used for the `android_test_mapping_format` - builtin hook. See [Placeholders](#Placeholders) for variables you can expand automatically. diff --git a/rh/config.py b/rh/config.py index ed75007..2c7b4af 100644 --- a/rh/config.py +++ b/rh/config.py @@ -63,21 +63,9 @@ class RawConfigParser(configparser.RawConfigParser): return default raise - 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 default is not _UNSET: - return default - raise - def items(self, section=_UNSET, default=_UNSET): """Return a list of (key, value) tuples for the options in |section|.""" 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: @@ -87,15 +75,6 @@ class RawConfigParser(configparser.RawConfigParser): 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): """A single (abstract) config used for `repo upload` hooks.""" @@ -138,7 +117,8 @@ class PreUploadConfig(object): def custom_hook(self, hook): """The command to execute for |hook|.""" - return shlex.split(self.config.get(self.CUSTOM_HOOKS_SECTION, hook, '')) + return shlex.split(self.config.get( + self.CUSTOM_HOOKS_SECTION, hook, fallback='')) @property def builtin_hooks(self): @@ -148,13 +128,13 @@ class PreUploadConfig(object): def builtin_hook_option(self, hook): """The options to pass to |hook|.""" - return shlex.split(self.config.get(self.BUILTIN_HOOKS_OPTIONS_SECTION, - hook, '')) + return shlex.split(self.config.get( + self.BUILTIN_HOOKS_OPTIONS_SECTION, hook, fallback='')) 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, '')) + return shlex.split(self.config.get( + self.BUILTIN_HOOKS_EXCLUDE_SECTION, hook, fallback='')) @property def tool_paths(self): @@ -186,7 +166,7 @@ class PreUploadConfig(object): """Whether to skip hooks for merged commits.""" return rh.shell.boolean_shell_value( self.config.get(self.OPTIONS_SECTION, - self.OPTION_IGNORE_MERGED_COMMITS, None), + self.OPTION_IGNORE_MERGED_COMMITS, fallback=None), False) def update(self, preupload_config): diff --git a/rh/hooks.py b/rh/hooks.py index 8cea251..0b3bb29 100644 --- a/rh/hooks.py +++ b/rh/hooks.py @@ -974,9 +974,30 @@ def check_android_test_mapping(project, commit, _desc, diff, options=None): return _check_cmd('android-test-mapping-format', project, commit, cmd) +def check_aidl_format(project, commit, _desc, diff, options=None): + """Checks that AIDL files are formatted with aidl-format.""" + # All *.aidl files except for those under aidl_api directory. + filtered = _filter_diff(diff, [r'\.aidl$'], [r'/aidl_api/']) + if not filtered: + return None + aidl_format = options.tool_path('aidl-format') + cmd = [aidl_format, '-d'] + options.args((), filtered) + ret = [] + for d in filtered: + data = rh.git.get_file_content(commit, d.file) + result = _run(cmd, input=data) + if result.stdout: + fixup_func = _fixup_func_caller([aidl_format, '-w', d.file]) + ret.append(rh.results.HookResult( + 'aidl-format', project, commit, error=result.stdout, + files=(d.file,), fixup_func=fixup_func)) + return ret + + # Hooks that projects can opt into. # Note: Make sure to keep the top level README.md up to date when adding more! BUILTIN_HOOKS = { + 'aidl_format': check_aidl_format, 'android_test_mapping_format': check_android_test_mapping, 'bpfmt': check_bpfmt, 'checkpatch': check_checkpatch, @@ -1002,6 +1023,7 @@ BUILTIN_HOOKS = { # Additional tools that the hooks can call with their default values. # Note: Make sure to keep the top level README.md up to date when adding more! TOOL_PATHS = { + 'aidl-format': 'aidl-format', 'android-test-mapping-format': os.path.join(TOOLS_DIR, 'android_test_mapping_format.py'), 'bpfmt': 'bpfmt', diff --git a/rh/hooks_unittest.py b/rh/hooks_unittest.py index 716e1da..8466319 100755 --- a/rh/hooks_unittest.py +++ b/rh/hooks_unittest.py @@ -823,6 +823,20 @@ class BuiltinHooksTests(unittest.TestCase): self.project, 'commit', 'desc', diff, options=self.options) self.assertIsNotNone(ret) + def test_aidl_format(self, mock_check, _mock_run): + """Verify the aidl_format builtin hook.""" + # First call should do nothing as there are no files to check. + ret = rh.hooks.check_aidl_format( + self.project, 'commit', 'desc', (), options=self.options) + self.assertIsNone(ret) + self.assertFalse(mock_check.called) + + # Second call will have some results. + diff = [rh.git.RawDiffEntry(file='IFoo.go')] + ret = rh.hooks.check_gofmt( + self.project, 'commit', 'desc', diff, options=self.options) + self.assertIsNotNone(ret) + if __name__ == '__main__': unittest.main() diff --git a/rh/utils.py b/rh/utils.py index 1b36c7a..52a6828 100644 --- a/rh/utils.py +++ b/rh/utils.py @@ -183,12 +183,8 @@ def _kill_child_process(proc, int_timeout, kill_timeout, cmd, original_handler, print('Ignoring unhandled exception in _kill_child_process: %s' % e, file=sys.stderr) - # Ensure our child process has been reaped. - kwargs = {} - if sys.version_info.major >= 3: - # ... but don't wait forever. - kwargs['timeout'] = 60 - proc.wait_lock_breaker(**kwargs) + # Ensure our child process has been reaped, but don't wait forever. + proc.wait_lock_breaker(timeout=60) if not rh.signals.relay_signal(original_handler, signum, frame): # Mock up our own, matching exit code for signaling. @@ -310,13 +306,8 @@ def run(cmd, redirect_stdout=False, redirect_stderr=False, cwd=None, input=None, kill_timeout = float(kill_timeout) def _get_tempfile(): - kwargs = {} - if sys.version_info.major < 3: - kwargs['bufsize'] = 0 - else: - kwargs['buffering'] = 0 try: - return tempfile.TemporaryFile(**kwargs) + return tempfile.TemporaryFile(buffering=0) except EnvironmentError as e: if e.errno != errno.ENOENT: raise @@ -325,7 +316,7 @@ def run(cmd, redirect_stdout=False, redirect_stderr=False, cwd=None, input=None, # issue in this particular case since our usage gurantees deletion, # and since this is primarily triggered during hard cgroups # shutdown. - return tempfile.TemporaryFile(dir='/tmp', **kwargs) + return tempfile.TemporaryFile(dir='/tmp', buffering=0) # Modify defaults based on parameters. # Note that tempfiles must be unbuffered else attempts to read diff --git a/tools/android_test_mapping_format.py b/tools/android_test_mapping_format.py index b87b886..1e654e6 100755 --- a/tools/android_test_mapping_format.py +++ b/tools/android_test_mapping_format.py @@ -53,13 +53,6 @@ TEST_MAPPING_URL = ( _COMMENTS_RE = re.compile(r'^\s*//') -if sys.version_info.major < 3: - # pylint: disable=basestring-builtin,undefined-variable - string_types = basestring -else: - string_types = str - - class Error(Exception): """Base exception for all custom exceptions in this module.""" @@ -125,14 +118,14 @@ def _validate_test(test, test_mapping_file): 'Failed test config: %s' % (test_mapping_file, test)) preferred_targets = test.get(PREFERRED_TARGETS, []) if (not isinstance(preferred_targets, list) or - any(not isinstance(t, string_types) for t in preferred_targets)): + any(not isinstance(t, str) for t in preferred_targets)): raise InvalidTestMappingError( 'Invalid test config in test mapping file %s. `preferred_targets` ' 'setting in test config can only be a list of strings. Failed test ' 'config: %s' % (test_mapping_file, test)) file_patterns = test.get(FILE_PATTERNS, []) if (not isinstance(file_patterns, list) or - any(not isinstance(p, string_types) for p in file_patterns)): + any(not isinstance(p, str) for p in file_patterns)): raise InvalidTestMappingError( 'Invalid test config in test mapping file %s. `file_patterns` ' 'setting in test config can only be a list of strings. Failed test ' |