From d6847c4827dd9d2847f210979cb49e7e97b4060b Mon Sep 17 00:00:00 2001 From: Marc Jay Date: Tue, 9 Apr 2019 23:53:39 +0100 Subject: [PATCH 1/6] Add wildcard matching to no-commit-to-branch hook so that commits can be blocked on, for example, all release branches with 'release/*' --- README.md | 2 +- pre_commit_hooks/no_commit_to_branch.py | 8 +++++--- tests/no_commit_to_branch_test.py | 13 +++++++++++++ 3 files changed, 19 insertions(+), 4 deletions(-) diff --git a/README.md b/README.md index a937771..abcd030 100644 --- a/README.md +++ b/README.md @@ -79,7 +79,7 @@ Add this to your `.pre-commit-config.yaml` - `name-tests-test` - Assert that files in tests/ end in `_test.py`. - Use `args: ['--django']` to match `test*.py` instead. - `no-commit-to-branch` - Protect specific branches from direct checkins. - - Use `args: [--branch, staging, --branch, master]` to set the branch. + - Use `args: [--branch, staging, --branch, master, --branch, release/*]` to set the branch. `master` is the default if no argument is set. - `-b` / `--branch` may be specified multiple times to protect multiple branches. diff --git a/pre_commit_hooks/no_commit_to_branch.py b/pre_commit_hooks/no_commit_to_branch.py index 6b68c91..8729891 100644 --- a/pre_commit_hooks/no_commit_to_branch.py +++ b/pre_commit_hooks/no_commit_to_branch.py @@ -1,6 +1,7 @@ from __future__ import print_function import argparse +import fnmatch from typing import Optional from typing import Sequence from typing import Set @@ -11,11 +12,12 @@ from pre_commit_hooks.util import cmd_output def is_on_branch(protected): # type: (Set[str]) -> bool try: - branch = cmd_output('git', 'symbolic-ref', 'HEAD') + ref_name = cmd_output('git', 'symbolic-ref', 'HEAD') except CalledProcessError: return False - chunks = branch.strip().split('/') - return '/'.join(chunks[2:]) in protected + chunks = ref_name.strip().split('/') + branch_name = '/'.join(chunks[2:]) + return any(fnmatch.fnmatch(branch_name, s) for s in protected) def main(argv=None): # type: (Optional[Sequence[str]]) -> int diff --git a/tests/no_commit_to_branch_test.py b/tests/no_commit_to_branch_test.py index e978ba2..a83d8de 100644 --- a/tests/no_commit_to_branch_test.py +++ b/tests/no_commit_to_branch_test.py @@ -44,6 +44,19 @@ def test_forbid_multiple_branches(temp_git_dir, branch_name): assert main(('--branch', 'b1', '--branch', 'b2')) +def test_branch_wildcard_fail(temp_git_dir): + with temp_git_dir.as_cwd(): + cmd_output('git', 'checkout', '-b', 'another/branch') + assert is_on_branch({'another/*'}) is True + + +@pytest.mark.parametrize('branch_name', ('master', 'another/branch')) +def test_branch_wildcard_multiple_branches_fail(temp_git_dir, branch_name): + with temp_git_dir.as_cwd(): + cmd_output('git', 'checkout', '-b', branch_name) + assert main(('--branch', 'master', '--branch', 'another/*')) + + def test_main_default_call(temp_git_dir): with temp_git_dir.as_cwd(): cmd_output('git', 'checkout', '-b', 'anotherbranch') From 8d2785b9d66fc1489b9a30669f4ebd5ce1ac7ffc Mon Sep 17 00:00:00 2001 From: Marc Jay Date: Sat, 20 Apr 2019 13:46:49 +0100 Subject: [PATCH 2/6] Amend approach for no-commit-to-branch to use regex matching based on feedback. Adds --pattern optional argument which can be used alongside --branch to block commits to a branch which matches a supplied regex expression --- README.md | 4 +++- pre_commit_hooks/no_commit_to_branch.py | 19 +++++++++++++++---- tests/no_commit_to_branch_test.py | 8 ++++---- 3 files changed, 22 insertions(+), 9 deletions(-) diff --git a/README.md b/README.md index abcd030..96325b9 100644 --- a/README.md +++ b/README.md @@ -79,10 +79,12 @@ Add this to your `.pre-commit-config.yaml` - `name-tests-test` - Assert that files in tests/ end in `_test.py`. - Use `args: ['--django']` to match `test*.py` instead. - `no-commit-to-branch` - Protect specific branches from direct checkins. - - Use `args: [--branch, staging, --branch, master, --branch, release/*]` to set the branch. + - Use `args: [--branch, staging, --branch, master]` to set the branch. `master` is the default if no argument is set. - `-b` / `--branch` may be specified multiple times to protect multiple branches. + - `-p` / `--pattern` can be used to protect branches that match a supplied regex + (e.g. `--pattern, release/.*`). May be specified multiple times. - `pretty-format-json` - Checks that all your JSON files are pretty. "Pretty" here means that keys are sorted and indented. You can configure this with the following commandline options: diff --git a/pre_commit_hooks/no_commit_to_branch.py b/pre_commit_hooks/no_commit_to_branch.py index 8729891..585eadc 100644 --- a/pre_commit_hooks/no_commit_to_branch.py +++ b/pre_commit_hooks/no_commit_to_branch.py @@ -1,7 +1,7 @@ from __future__ import print_function import argparse -import fnmatch +import re from typing import Optional from typing import Sequence from typing import Set @@ -10,14 +10,17 @@ from pre_commit_hooks.util import CalledProcessError from pre_commit_hooks.util import cmd_output -def is_on_branch(protected): # type: (Set[str]) -> bool +def is_on_branch(protected, patterns=set()): + # type: (Set[str], Set[str]) -> bool try: ref_name = cmd_output('git', 'symbolic-ref', 'HEAD') except CalledProcessError: return False chunks = ref_name.strip().split('/') branch_name = '/'.join(chunks[2:]) - return any(fnmatch.fnmatch(branch_name, s) for s in protected) + return branch_name in protected or any( + re.match(p, branch_name) for p in patterns + ) def main(argv=None): # type: (Optional[Sequence[str]]) -> int @@ -26,10 +29,18 @@ def main(argv=None): # type: (Optional[Sequence[str]]) -> int '-b', '--branch', action='append', help='branch to disallow commits to, may be specified multiple times', ) + parser.add_argument( + '-p', '--pattern', action='append', + help=( + 'regex pattern for branch name to disallow commits to, ' + 'May be specified multiple times' + ), + ) args = parser.parse_args(argv) protected = set(args.branch or ('master',)) - return int(is_on_branch(protected)) + patterns = set(args.pattern or ()) + return int(is_on_branch(protected, patterns)) if __name__ == '__main__': diff --git a/tests/no_commit_to_branch_test.py b/tests/no_commit_to_branch_test.py index a83d8de..a2ab1f1 100644 --- a/tests/no_commit_to_branch_test.py +++ b/tests/no_commit_to_branch_test.py @@ -44,17 +44,17 @@ def test_forbid_multiple_branches(temp_git_dir, branch_name): assert main(('--branch', 'b1', '--branch', 'b2')) -def test_branch_wildcard_fail(temp_git_dir): +def test_branch_pattern_fail(temp_git_dir): with temp_git_dir.as_cwd(): cmd_output('git', 'checkout', '-b', 'another/branch') - assert is_on_branch({'another/*'}) is True + assert is_on_branch(set(), {'another/.*'}) is True @pytest.mark.parametrize('branch_name', ('master', 'another/branch')) -def test_branch_wildcard_multiple_branches_fail(temp_git_dir, branch_name): +def test_branch_pattern_multiple_branches_fail(temp_git_dir, branch_name): with temp_git_dir.as_cwd(): cmd_output('git', 'checkout', '-b', branch_name) - assert main(('--branch', 'master', '--branch', 'another/*')) + assert main(('--branch', 'master', '--pattern', 'another/.*')) def test_main_default_call(temp_git_dir): From 7b959d140fe77075e5688697f5cf5eb1eeed8c97 Mon Sep 17 00:00:00 2001 From: Marc Jay Date: Sat, 20 Apr 2019 13:52:59 +0100 Subject: [PATCH 3/6] Tidy up indentation in README changes for no-commit-to-branch --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 96325b9..df060a2 100644 --- a/README.md +++ b/README.md @@ -84,7 +84,7 @@ Add this to your `.pre-commit-config.yaml` - `-b` / `--branch` may be specified multiple times to protect multiple branches. - `-p` / `--pattern` can be used to protect branches that match a supplied regex - (e.g. `--pattern, release/.*`). May be specified multiple times. + (e.g. `--pattern, release/.*`). May be specified multiple times. - `pretty-format-json` - Checks that all your JSON files are pretty. "Pretty" here means that keys are sorted and indented. You can configure this with the following commandline options: From 6568414fc568257fd9fa705e886e3f5e3adcffc0 Mon Sep 17 00:00:00 2001 From: Marc Jay Date: Sat, 20 Apr 2019 14:06:59 +0100 Subject: [PATCH 4/6] Clarify default behaviour in README for no-commit-to-branch --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index df060a2..5bf28d3 100644 --- a/README.md +++ b/README.md @@ -80,7 +80,7 @@ Add this to your `.pre-commit-config.yaml` - Use `args: ['--django']` to match `test*.py` instead. - `no-commit-to-branch` - Protect specific branches from direct checkins. - Use `args: [--branch, staging, --branch, master]` to set the branch. - `master` is the default if no argument is set. + `master` is the default if no branch argument is set. - `-b` / `--branch` may be specified multiple times to protect multiple branches. - `-p` / `--pattern` can be used to protect branches that match a supplied regex From a7af81244972ae6ac30bd55260af46b7ce25a6e1 Mon Sep 17 00:00:00 2001 From: Marc Jay Date: Sat, 20 Apr 2019 23:07:14 +0100 Subject: [PATCH 5/6] Make optional argument use an immutable set for the default value in no-commit-to-branch. Make other sets immutable to satisfy type-checking and be consistent --- pre_commit_hooks/no_commit_to_branch.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/pre_commit_hooks/no_commit_to_branch.py b/pre_commit_hooks/no_commit_to_branch.py index 585eadc..a1015f4 100644 --- a/pre_commit_hooks/no_commit_to_branch.py +++ b/pre_commit_hooks/no_commit_to_branch.py @@ -2,16 +2,16 @@ from __future__ import print_function import argparse import re +from typing import FrozenSet from typing import Optional from typing import Sequence -from typing import Set from pre_commit_hooks.util import CalledProcessError from pre_commit_hooks.util import cmd_output -def is_on_branch(protected, patterns=set()): - # type: (Set[str], Set[str]) -> bool +def is_on_branch(protected, patterns=frozenset()): + # type: (FrozenSet[str], FrozenSet[str]) -> bool try: ref_name = cmd_output('git', 'symbolic-ref', 'HEAD') except CalledProcessError: @@ -33,13 +33,13 @@ def main(argv=None): # type: (Optional[Sequence[str]]) -> int '-p', '--pattern', action='append', help=( 'regex pattern for branch name to disallow commits to, ' - 'May be specified multiple times' + 'may be specified multiple times' ), ) args = parser.parse_args(argv) - protected = set(args.branch or ('master',)) - patterns = set(args.pattern or ()) + protected = frozenset(args.branch or ('master',)) + patterns = frozenset(args.pattern or ()) return int(is_on_branch(protected, patterns)) From 053feb1e6f78595fed43f3b61899532ea9b45934 Mon Sep 17 00:00:00 2001 From: Anthony Sottile Date: Sat, 20 Apr 2019 16:21:58 -0700 Subject: [PATCH 6/6] Use AbstractSet to appease mypy --- pre_commit_hooks/no_commit_to_branch.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pre_commit_hooks/no_commit_to_branch.py b/pre_commit_hooks/no_commit_to_branch.py index a1015f4..3131059 100644 --- a/pre_commit_hooks/no_commit_to_branch.py +++ b/pre_commit_hooks/no_commit_to_branch.py @@ -2,7 +2,7 @@ from __future__ import print_function import argparse import re -from typing import FrozenSet +from typing import AbstractSet from typing import Optional from typing import Sequence @@ -11,7 +11,7 @@ from pre_commit_hooks.util import cmd_output def is_on_branch(protected, patterns=frozenset()): - # type: (FrozenSet[str], FrozenSet[str]) -> bool + # type: (AbstractSet[str], AbstractSet[str]) -> bool try: ref_name = cmd_output('git', 'symbolic-ref', 'HEAD') except CalledProcessError: