From d46c288cfdeadb070fa71c744b9970b3f1c7a623 Mon Sep 17 00:00:00 2001 From: Daniel Baumann Date: Thu, 12 Aug 2021 14:03:27 +0200 Subject: Merging upstream version 2.14.0. Signed-off-by: Daniel Baumann --- .pre-commit-config.yaml | 9 ++-- .pre-commit-hooks.yaml | 2 +- CHANGELOG.md | 19 +++++++ pre_commit/commands/hook_impl.py | 7 ++- pre_commit/commands/install_uninstall.py | 14 ++--- pre_commit/commands/run.py | 6 ++- pre_commit/languages/docker.py | 22 ++++++-- pre_commit/main.py | 3 ++ setup.cfg | 4 +- testing/util.py | 2 + tests/clientlib_test.py | 4 +- tests/commands/install_uninstall_test.py | 17 +++++- tests/commands/run_test.py | 1 + tests/languages/docker_test.py | 92 ++++++++++++++++++++------------ tests/repository_test.py | 2 +- 15 files changed, 148 insertions(+), 56 deletions(-) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 78cbfac..8c06de5 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -21,16 +21,16 @@ repos: hooks: - id: autopep8 - repo: https://github.com/pre-commit/pre-commit - rev: v2.13.0 + rev: v2.14.0 hooks: - id: validate_manifest - repo: https://github.com/asottile/pyupgrade - rev: v2.16.0 + rev: v2.23.1 hooks: - id: pyupgrade args: [--py36-plus] - repo: https://github.com/asottile/reorder_python_imports - rev: v2.5.0 + rev: v2.6.0 hooks: - id: reorder-python-imports args: [--py3-plus] @@ -44,9 +44,10 @@ repos: hooks: - id: setup-cfg-fmt - repo: https://github.com/pre-commit/mirrors-mypy - rev: v0.812 + rev: v0.910 hooks: - id: mypy + additional_dependencies: [types-all] exclude: ^testing/resources/ - repo: meta hooks: diff --git a/.pre-commit-hooks.yaml b/.pre-commit-hooks.yaml index ef269d1..3d1ffbc 100644 --- a/.pre-commit-hooks.yaml +++ b/.pre-commit-hooks.yaml @@ -1,5 +1,5 @@ - id: validate_manifest - name: Validate Pre-Commit Manifest + name: validate pre-commit manifest description: This validator validates a pre-commit hooks manifest file entry: pre-commit-validate-manifest language: python diff --git a/CHANGELOG.md b/CHANGELOG.md index e47a1c5..eaeaa27 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,22 @@ +2.14.0 - 2021-08-06 +=================== + +### Features +- During `pre-push` hooks, expose local branch as `PRE_COMMIT_LOCAL_BRANCH`. + - #1947 PR by @FlorentClarret. + - #1410 issue by @MaicoTimmerman. +- Improve container id detection for docker-beside-docker with custom hostname. + - #1919 PR by @adarnimrod. + - #1918 issue by @adarnimrod. + +### Fixes +- Read legacy hooks in an encoding-agnostic way. + - #1943 PR by @asottile. + - #1942 issue by @sbienkow-ninja. +- Fix execution of docker hooks for docker-in-docker. + - #1997 PR by @asottile. + - #1978 issue by @robin-moss. + 2.13.0 - 2021-05-21 =================== diff --git a/pre_commit/commands/hook_impl.py b/pre_commit/commands/hook_impl.py index a766ee9..c544167 100644 --- a/pre_commit/commands/hook_impl.py +++ b/pre_commit/commands/hook_impl.py @@ -70,6 +70,7 @@ def _ns( *, all_files: bool = False, remote_branch: Optional[str] = None, + local_branch: Optional[str] = None, from_ref: Optional[str] = None, to_ref: Optional[str] = None, remote_name: Optional[str] = None, @@ -82,6 +83,7 @@ def _ns( color=color, hook_stage=hook_type.replace('pre-', ''), remote_branch=remote_branch, + local_branch=local_branch, from_ref=from_ref, to_ref=to_ref, remote_name=remote_name, @@ -110,7 +112,7 @@ def _pre_push_ns( remote_url = args[1] for line in stdin.decode().splitlines(): - _, local_sha, remote_branch, remote_sha = line.split() + local_branch, local_sha, remote_branch, remote_sha = line.split() if local_sha == Z40: continue elif remote_sha != Z40 and _rev_exists(remote_sha): @@ -118,6 +120,7 @@ def _pre_push_ns( 'pre-push', color, from_ref=remote_sha, to_ref=local_sha, remote_branch=remote_branch, + local_branch=local_branch, remote_name=remote_name, remote_url=remote_url, ) else: @@ -139,6 +142,7 @@ def _pre_push_ns( all_files=True, remote_name=remote_name, remote_url=remote_url, remote_branch=remote_branch, + local_branch=local_branch, ) else: rev_cmd = ('git', 'rev-parse', f'{first_ancestor}^') @@ -148,6 +152,7 @@ def _pre_push_ns( from_ref=source, to_ref=local_sha, remote_name=remote_name, remote_url=remote_url, remote_branch=remote_branch, + local_branch=local_branch, ) # nothing to push diff --git a/pre_commit/commands/install_uninstall.py b/pre_commit/commands/install_uninstall.py index 684b598..73c8d60 100644 --- a/pre_commit/commands/install_uninstall.py +++ b/pre_commit/commands/install_uninstall.py @@ -21,13 +21,13 @@ logger = logging.getLogger(__name__) # This is used to identify the hook file we install PRIOR_HASHES = ( - '4d9958c90bc262f47553e2c073f14cfe', - 'd8ee923c46731b42cd95cc869add4062', - '49fd668cb42069aa1b6048464be5d395', - '79f09a650522a87b0da915d0d983b2de', - 'e358c9dae00eac5d06b38dfdb1e33a8c', + b'4d9958c90bc262f47553e2c073f14cfe', + b'd8ee923c46731b42cd95cc869add4062', + b'49fd668cb42069aa1b6048464be5d395', + b'79f09a650522a87b0da915d0d983b2de', + b'e358c9dae00eac5d06b38dfdb1e33a8c', ) -CURRENT_HASH = '138fd403232d2ddd5efb44317e38bf03' +CURRENT_HASH = b'138fd403232d2ddd5efb44317e38bf03' TEMPLATE_START = '# start templated\n' TEMPLATE_END = '# end templated\n' # Homebrew/homebrew-core#35825: be more timid about appropriate `PATH` @@ -48,7 +48,7 @@ def _hook_paths( def is_our_script(filename: str) -> bool: if not os.path.exists(filename): # pragma: win32 no cover (symlink) return False - with open(filename) as f: + with open(filename, 'rb') as f: contents = f.read() return any(h in contents for h in (CURRENT_HASH,) + PRIOR_HASHES) diff --git a/pre_commit/commands/run.py b/pre_commit/commands/run.py index 0fef50d..d906d5b 100644 --- a/pre_commit/commands/run.py +++ b/pre_commit/commands/run.py @@ -371,7 +371,11 @@ def run( environ['PRE_COMMIT_FROM_REF'] = args.from_ref environ['PRE_COMMIT_TO_REF'] = args.to_ref - if args.remote_name and args.remote_url and args.remote_branch: + if ( + args.remote_name and args.remote_url and + args.remote_branch and args.local_branch + ): + environ['PRE_COMMIT_LOCAL_BRANCH'] = args.local_branch environ['PRE_COMMIT_REMOTE_BRANCH'] = args.remote_branch environ['PRE_COMMIT_REMOTE_NAME'] = args.remote_name environ['PRE_COMMIT_REMOTE_URL'] = args.remote_url diff --git a/pre_commit/languages/docker.py b/pre_commit/languages/docker.py index 5b21ec9..644d8d2 100644 --- a/pre_commit/languages/docker.py +++ b/pre_commit/languages/docker.py @@ -1,7 +1,6 @@ import hashlib import json import os -import socket from typing import Sequence from typing import Tuple @@ -9,6 +8,7 @@ import pre_commit.constants as C from pre_commit.hook import Hook from pre_commit.languages import helpers from pre_commit.prefix import Prefix +from pre_commit.util import CalledProcessError from pre_commit.util import clean_path_on_failure from pre_commit.util import cmd_output_b @@ -26,12 +26,28 @@ def _is_in_docker() -> bool: return False +def _get_container_id() -> str: + # It's assumed that we already check /proc/1/cgroup in _is_in_docker. The + # cpuset cgroup controller existed since cgroups were introduced so this + # way of getting the container ID is pretty reliable. + with open('/proc/1/cgroup', 'rb') as f: + for line in f.readlines(): + if line.split(b':')[1] == b'cpuset': + return os.path.basename(line.split(b':')[2]).strip().decode() + raise RuntimeError('Failed to find the container ID in /proc/1/cgroup.') + + def _get_docker_path(path: str) -> str: if not _is_in_docker(): return path - hostname = socket.gethostname() - _, out, _ = cmd_output_b('docker', 'inspect', hostname) + container_id = _get_container_id() + + try: + _, out, _ = cmd_output_b('docker', 'inspect', container_id) + except CalledProcessError: + # self-container was not visible from here (perhaps docker-in-docker) + return path container, = json.loads(out) for mount in container['Mounts']: diff --git a/pre_commit/main.py b/pre_commit/main.py index c66cfb9..ad3d873 100644 --- a/pre_commit/main.py +++ b/pre_commit/main.py @@ -99,6 +99,9 @@ def _add_run_options(parser: argparse.ArgumentParser) -> None: parser.add_argument( '--remote-branch', help='Remote branch ref used by `git push`.', ) + parser.add_argument( + '--local-branch', help='Local branch ref used by `git push`.', + ) parser.add_argument( '--from-ref', '--source', '-s', help=( diff --git a/setup.cfg b/setup.cfg index ae5cc7c..50f1605 100644 --- a/setup.cfg +++ b/setup.cfg @@ -1,6 +1,6 @@ [metadata] name = pre_commit -version = 2.13.0 +version = 2.14.0 description = A framework for managing and maintaining multi-language pre-commit hooks. long_description = file: README.md long_description_content_type = text/markdown @@ -63,6 +63,8 @@ disallow_any_generics = true disallow_incomplete_defs = true disallow_untyped_defs = true no_implicit_optional = true +warn_redundant_casts = true +warn_unused_ignores = true [mypy-testing.*] disallow_untyped_defs = false diff --git a/testing/util.py b/testing/util.py index 1364453..12f22b5 100644 --- a/testing/util.py +++ b/testing/util.py @@ -62,6 +62,7 @@ def run_opts( verbose=False, hook=None, remote_branch='', + local_branch='', from_ref='', to_ref='', remote_name='', @@ -81,6 +82,7 @@ def run_opts( verbose=verbose, hook=hook, remote_branch=remote_branch, + local_branch=local_branch, from_ref=from_ref, to_ref=to_ref, remote_name=remote_name, diff --git a/tests/clientlib_test.py b/tests/clientlib_test.py index ff3cce3..09bdb3e 100644 --- a/tests/clientlib_test.py +++ b/tests/clientlib_test.py @@ -427,7 +427,7 @@ def test_minimum_pre_commit_version_passing(): @pytest.mark.parametrize('schema', (CONFIG_SCHEMA, CONFIG_REPO_DICT)) def test_warn_additional(schema): allowed_keys = {item.key for item in schema.items if hasattr(item, 'key')} - warn_additional, = [ + warn_additional, = ( x for x in schema.items if isinstance(x, cfgv.WarnAdditionalKeys) - ] + ) assert allowed_keys == set(warn_additional.keys) diff --git a/tests/commands/install_uninstall_test.py b/tests/commands/install_uninstall_test.py index bd28654..314b8b9 100644 --- a/tests/commands/install_uninstall_test.py +++ b/tests/commands/install_uninstall_test.py @@ -39,7 +39,7 @@ def test_is_script(): def test_is_previous_pre_commit(tmpdir): f = tmpdir.join('foo') - f.write(f'{PRIOR_HASHES[0]}\n') + f.write(f'{PRIOR_HASHES[0].decode()}\n') assert is_our_script(f.strpath) @@ -390,6 +390,19 @@ def test_install_existing_hook_no_overwrite_idempotent(tempdir_factory, store): NORMAL_PRE_COMMIT_RUN.assert_matches(output[len('legacy hook\n'):]) +def test_install_with_existing_non_utf8_script(tmpdir, store): + cmd_output('git', 'init', str(tmpdir)) + tmpdir.join('.git/hooks').ensure_dir() + tmpdir.join('.git/hooks/pre-commit').write_binary( + b'#!/usr/bin/env bash\n' + b'# garbage: \xa0\xef\x12\xf2\n' + b'echo legacy hook\n', + ) + + with tmpdir.as_cwd(): + assert install(C.CONFIG_FILE, store, hook_types=['pre-commit']) == 0 + + FAIL_OLD_HOOK = re_assert.Matches( r'fail!\n' r'\[INFO\] Initializing environment for .+\.\n' @@ -460,7 +473,7 @@ def test_replace_old_commit_script(tempdir_factory, store): # Install a script that looks like our old script pre_commit_contents = resource_text('hook-tmpl') new_contents = pre_commit_contents.replace( - CURRENT_HASH, PRIOR_HASHES[-1], + CURRENT_HASH.decode(), PRIOR_HASHES[-1].decode(), ) os.makedirs(os.path.join(path, '.git/hooks'), exist_ok=True) diff --git a/tests/commands/run_test.py b/tests/commands/run_test.py index e184340..da7569e 100644 --- a/tests/commands/run_test.py +++ b/tests/commands/run_test.py @@ -487,6 +487,7 @@ def test_all_push_options_ok(cap_out, store, repo_with_passing_hook): args = run_opts( from_ref='master', to_ref='master', remote_branch='master', + local_branch='master', remote_name='origin', remote_url='https://example.com/repo', ) ret, printed = _do_run(cap_out, store, repo_with_passing_hook, args) diff --git a/tests/languages/docker_test.py b/tests/languages/docker_test.py index 01b5e27..ec6bb83 100644 --- a/tests/languages/docker_test.py +++ b/tests/languages/docker_test.py @@ -8,6 +8,42 @@ from unittest import mock import pytest from pre_commit.languages import docker +from pre_commit.util import CalledProcessError + +DOCKER_CGROUP_EXAMPLE = b'''\ +12:hugetlb:/docker/c33988ec7651ebc867cb24755eaf637a6734088bc7eef59d5799293a9e5450f7 +11:blkio:/docker/c33988ec7651ebc867cb24755eaf637a6734088bc7eef59d5799293a9e5450f7 +10:freezer:/docker/c33988ec7651ebc867cb24755eaf637a6734088bc7eef59d5799293a9e5450f7 +9:cpu,cpuacct:/docker/c33988ec7651ebc867cb24755eaf637a6734088bc7eef59d5799293a9e5450f7 +8:pids:/docker/c33988ec7651ebc867cb24755eaf637a6734088bc7eef59d5799293a9e5450f7 +7:rdma:/ +6:net_cls,net_prio:/docker/c33988ec7651ebc867cb24755eaf637a6734088bc7eef59d5799293a9e5450f7 +5:cpuset:/docker/c33988ec7651ebc867cb24755eaf637a6734088bc7eef59d5799293a9e5450f7 +4:devices:/docker/c33988ec7651ebc867cb24755eaf637a6734088bc7eef59d5799293a9e5450f7 +3:memory:/docker/c33988ec7651ebc867cb24755eaf637a6734088bc7eef59d5799293a9e5450f7 +2:perf_event:/docker/c33988ec7651ebc867cb24755eaf637a6734088bc7eef59d5799293a9e5450f7 +1:name=systemd:/docker/c33988ec7651ebc867cb24755eaf637a6734088bc7eef59d5799293a9e5450f7 +0::/system.slice/containerd.service +''' # noqa: E501 + +# The ID should match the above cgroup example. +CONTAINER_ID = 'c33988ec7651ebc867cb24755eaf637a6734088bc7eef59d5799293a9e5450f7' # noqa: E501 + +NON_DOCKER_CGROUP_EXAMPLE = b'''\ +12:perf_event:/ +11:hugetlb:/ +10:devices:/ +9:blkio:/ +8:rdma:/ +7:cpuset:/ +6:cpu,cpuacct:/ +5:freezer:/ +4:memory:/ +3:pids:/ +2:net_cls,net_prio:/ +1:name=systemd:/init.scope +0::/init.scope +''' def test_docker_fallback_user(): @@ -37,45 +73,25 @@ def _mock_open(data): def test_in_docker_docker_in_file(): - docker_cgroup_example = b'''\ -12:hugetlb:/docker/c33988ec7651ebc867cb24755eaf637a6734088bc7eef59d5799293a9e5450f7 -11:blkio:/docker/c33988ec7651ebc867cb24755eaf637a6734088bc7eef59d5799293a9e5450f7 -10:freezer:/docker/c33988ec7651ebc867cb24755eaf637a6734088bc7eef59d5799293a9e5450f7 -9:cpu,cpuacct:/docker/c33988ec7651ebc867cb24755eaf637a6734088bc7eef59d5799293a9e5450f7 -8:pids:/docker/c33988ec7651ebc867cb24755eaf637a6734088bc7eef59d5799293a9e5450f7 -7:rdma:/ -6:net_cls,net_prio:/docker/c33988ec7651ebc867cb24755eaf637a6734088bc7eef59d5799293a9e5450f7 -5:cpuset:/docker/c33988ec7651ebc867cb24755eaf637a6734088bc7eef59d5799293a9e5450f7 -4:devices:/docker/c33988ec7651ebc867cb24755eaf637a6734088bc7eef59d5799293a9e5450f7 -3:memory:/docker/c33988ec7651ebc867cb24755eaf637a6734088bc7eef59d5799293a9e5450f7 -2:perf_event:/docker/c33988ec7651ebc867cb24755eaf637a6734088bc7eef59d5799293a9e5450f7 -1:name=systemd:/docker/c33988ec7651ebc867cb24755eaf637a6734088bc7eef59d5799293a9e5450f7 -0::/system.slice/containerd.service -''' # noqa: E501 - with _mock_open(docker_cgroup_example): + with _mock_open(DOCKER_CGROUP_EXAMPLE): assert docker._is_in_docker() is True def test_in_docker_docker_not_in_file(): - non_docker_cgroup_example = b'''\ -12:perf_event:/ -11:hugetlb:/ -10:devices:/ -9:blkio:/ -8:rdma:/ -7:cpuset:/ -6:cpu,cpuacct:/ -5:freezer:/ -4:memory:/ -3:pids:/ -2:net_cls,net_prio:/ -1:name=systemd:/init.scope -0::/init.scope -''' - with _mock_open(non_docker_cgroup_example): + with _mock_open(NON_DOCKER_CGROUP_EXAMPLE): assert docker._is_in_docker() is False +def test_get_container_id(): + with _mock_open(DOCKER_CGROUP_EXAMPLE): + assert docker._get_container_id() == CONTAINER_ID + + +def test_get_container_id_failure(): + with _mock_open(b''), pytest.raises(RuntimeError): + docker._get_container_id() + + def test_get_docker_path_not_in_docker_returns_same(): with mock.patch.object(docker, '_is_in_docker', return_value=False): assert docker._get_docker_path('abc') == 'abc' @@ -84,7 +100,10 @@ def test_get_docker_path_not_in_docker_returns_same(): @pytest.fixture def in_docker(): with mock.patch.object(docker, '_is_in_docker', return_value=True): - yield + with mock.patch.object( + docker, '_get_container_id', return_value=CONTAINER_ID, + ): + yield def _linux_commonpath(): @@ -153,3 +172,10 @@ def test_get_docker_path_in_docker_windows(in_docker): path = r'c:\folder\test\something' expected = r'c:\users\user\test\something' assert docker._get_docker_path(path) == expected + + +def test_get_docker_path_in_docker_docker_in_docker(in_docker): + # won't be able to discover "self" container in true docker-in-docker + err = CalledProcessError(1, (), 0, b'', b'') + with mock.patch.object(docker, 'cmd_output_b', side_effect=err): + assert docker._get_docker_path('/project') == '/project' diff --git a/tests/repository_test.py b/tests/repository_test.py index b6f7fb2..af829c2 100644 --- a/tests/repository_test.py +++ b/tests/repository_test.py @@ -51,7 +51,7 @@ def _get_hook_no_install(repo_config, store, hook_id): config = cfgv.validate(config, CONFIG_SCHEMA) config = cfgv.apply_defaults(config, CONFIG_SCHEMA) hooks = all_hooks(config, store) - hook, = [hook for hook in hooks if hook.id == hook_id] + hook, = (hook for hook in hooks if hook.id == hook_id) return hook -- cgit v1.2.3