forbid-new-submodules: fix triggering failure when only a submodule is committed (without any other file); support --from-ref and --to-ref; fixes #609
diff --git a/.pre-commit-hooks.yaml b/.pre-commit-hooks.yaml
index 91dbdf0..646a0bb 100644
--- a/.pre-commit-hooks.yaml
+++ b/.pre-commit-hooks.yaml
@@ -154,6 +154,7 @@
language: python
entry: forbid-new-submodules
description: Prevent addition of new git submodules
+ types: [directory]
- id: mixed-line-ending
name: Mixed line ending
description: Replaces or checks mixed line ending
diff --git a/pre_commit_hooks/forbid_new_submodules.py b/pre_commit_hooks/forbid_new_submodules.py
index c144d72..9d1aa2f 100644
--- a/pre_commit_hooks/forbid_new_submodules.py
+++ b/pre_commit_hooks/forbid_new_submodules.py
@@ -1,3 +1,5 @@
+import argparse
+import os
from typing import Optional
from typing import Sequence
@@ -5,10 +7,23 @@
def main(argv: Optional[Sequence[str]] = None) -> int:
- # `argv` is ignored, pre-commit will send us a list of files that we
- # don't care about
+ parser = argparse.ArgumentParser()
+ parser.add_argument('filenames', nargs='*')
+ args = parser.parse_args(argv)
+
+ if (
+ 'PRE_COMMIT_FROM_REF' in os.environ and
+ 'PRE_COMMIT_TO_REF' in os.environ
+ ):
+ diff_arg = '...'.join((
+ os.environ['PRE_COMMIT_FROM_REF'],
+ os.environ['PRE_COMMIT_TO_REF'],
+ ))
+ else:
+ diff_arg = '--staged'
added_diff = cmd_output(
- 'git', 'diff', '--staged', '--diff-filter=A', '--raw',
+ 'git', 'diff', '--diff-filter=A', '--raw', diff_arg, '--',
+ *args.filenames,
)
retv = 0
for line in added_diff.splitlines():
diff --git a/testing/util.py b/testing/util.py
index 8e468d6..5043754 100644
--- a/testing/util.py
+++ b/testing/util.py
@@ -1,4 +1,5 @@
import os.path
+import subprocess
TESTING_DIR = os.path.abspath(os.path.dirname(__file__))
@@ -6,3 +7,8 @@
def get_resource_path(path):
return os.path.join(TESTING_DIR, 'resources', path)
+
+
+def git_commit(*args, **kwargs):
+ cmd = ('git', 'commit', '--no-gpg-sign', '--no-verify', '--no-edit', *args)
+ subprocess.check_call(cmd, **kwargs)
diff --git a/tests/check_added_large_files_test.py b/tests/check_added_large_files_test.py
index d98d99b..d38c4f6 100644
--- a/tests/check_added_large_files_test.py
+++ b/tests/check_added_large_files_test.py
@@ -5,6 +5,7 @@
from pre_commit_hooks.check_added_large_files import find_large_added_files
from pre_commit_hooks.check_added_large_files import main
from pre_commit_hooks.util import cmd_output
+from testing.util import git_commit
def test_nothing_added(temp_git_dir):
@@ -104,7 +105,7 @@
# First add the file we're going to move
temp_git_dir.join('a.bin').write('a' * 10000)
cmd_output('git', 'add', '--', '.')
- cmd_output('git', 'commit', '--no-gpg-sign', '-am', 'foo')
+ git_commit('-am', 'foo')
# Now move it and make sure the hook still succeeds
cmd_output('git', 'mv', 'a.bin', 'b.bin')
assert main(('--maxkb', '9', 'b.bin')) == 0
diff --git a/tests/check_case_conflict_test.py b/tests/check_case_conflict_test.py
index c8c9d12..d9211b5 100644
--- a/tests/check_case_conflict_test.py
+++ b/tests/check_case_conflict_test.py
@@ -6,6 +6,7 @@
from pre_commit_hooks.check_case_conflict import main
from pre_commit_hooks.check_case_conflict import parents
from pre_commit_hooks.util import cmd_output
+from testing.util import git_commit
skip_win32 = pytest.mark.skipif(
sys.platform == 'win32',
@@ -85,7 +86,7 @@
with temp_git_dir.as_cwd():
temp_git_dir.join('f.py').write("print('hello world')")
cmd_output('git', 'add', 'f.py')
- cmd_output('git', 'commit', '--no-gpg-sign', '-n', '-m', 'Add f.py')
+ git_commit('-m', 'Add f.py')
temp_git_dir.join('F.py').write("print('hello world')")
cmd_output('git', 'add', 'F.py')
@@ -98,7 +99,7 @@
with temp_git_dir.as_cwd():
temp_git_dir.mkdir('dir').join('x').write('foo')
cmd_output('git', 'add', '-A')
- cmd_output('git', 'commit', '--no-gpg-sign', '-n', '-m', 'Add f.py')
+ git_commit('-m', 'Add f.py')
temp_git_dir.join('DIR').write('foo')
cmd_output('git', 'add', '-A')
diff --git a/tests/check_merge_conflict_test.py b/tests/check_merge_conflict_test.py
index fccf41f..de07bc1 100644
--- a/tests/check_merge_conflict_test.py
+++ b/tests/check_merge_conflict_test.py
@@ -6,6 +6,7 @@
from pre_commit_hooks.check_merge_conflict import main
from pre_commit_hooks.util import cmd_output
from testing.util import get_resource_path
+from testing.util import git_commit
@pytest.fixture
@@ -20,19 +21,19 @@
with repo1.as_cwd():
repo1_f1.ensure()
cmd_output('git', 'add', '.')
- cmd_output('git', 'commit', '--no-gpg-sign', '-m', 'commit1')
+ git_commit('-m', 'commit1')
cmd_output('git', 'clone', str(repo1), str(repo2))
# Commit in master
with repo1.as_cwd():
repo1_f1.write('parent\n')
- cmd_output('git', 'commit', '--no-gpg-sign', '-am', 'master commit2')
+ git_commit('-am', 'master commit2')
# Commit in clone and pull
with repo2.as_cwd():
repo2_f1.write('child\n')
- cmd_output('git', 'commit', '--no-gpg-sign', '-am', 'clone commit2')
+ git_commit('-am', 'clone commit2')
cmd_output('git', 'pull', '--no-rebase', retcode=None)
# We should end up in a merge conflict!
f1 = repo2_f1.read()
@@ -75,20 +76,20 @@
with repo1.as_cwd():
repo1_f1.ensure()
cmd_output('git', 'add', '.')
- cmd_output('git', 'commit', '--no-gpg-sign', '-m', 'commit1')
+ git_commit('-m', 'commit1')
cmd_output('git', 'clone', str(repo1), str(repo2))
# Commit in master
with repo1.as_cwd():
repo1_f1.write('parent\n')
- cmd_output('git', 'commit', '--no-gpg-sign', '-am', 'master commit2')
+ git_commit('-am', 'master commit2')
# Commit in clone and pull without committing
with repo2.as_cwd():
repo2_f2.write('child\n')
cmd_output('git', 'add', '.')
- cmd_output('git', 'commit', '--no-gpg-sign', '-m', 'clone commit2')
+ git_commit('-m', 'clone commit2')
cmd_output('git', 'pull', '--no-commit', '--no-rebase')
# We should end up in a pending merge
assert repo2_f1.read() == 'parent\n'
diff --git a/tests/destroyed_symlinks_test.py b/tests/destroyed_symlinks_test.py
index d2c9031..cde06cf 100644
--- a/tests/destroyed_symlinks_test.py
+++ b/tests/destroyed_symlinks_test.py
@@ -5,6 +5,7 @@
from pre_commit_hooks.destroyed_symlinks import find_destroyed_symlinks
from pre_commit_hooks.destroyed_symlinks import main
+from testing.util import git_commit
TEST_SYMLINK = 'test_symlink'
TEST_SYMLINK_TARGET = '/doesnt/really/matters'
@@ -23,9 +24,7 @@
with open(TEST_FILE, 'w') as f:
print('some random content', file=f)
subprocess.check_call(('git', 'add', '.'))
- subprocess.check_call(
- ('git', 'commit', '--no-gpg-sign', '-m', 'initial'),
- )
+ git_commit('-m', 'initial')
assert b'120000 ' in subprocess.check_output(
('git', 'cat-file', '-p', 'HEAD^{tree}'),
)
diff --git a/tests/forbid_new_submodules_test.py b/tests/forbid_new_submodules_test.py
index 4871ae7..0326d94 100644
--- a/tests/forbid_new_submodules_test.py
+++ b/tests/forbid_new_submodules_test.py
@@ -1,22 +1,20 @@
+import os
import subprocess
+from unittest import mock
import pytest
from pre_commit_hooks.forbid_new_submodules import main
+from testing.util import git_commit
@pytest.fixture
def git_dir_with_git_dir(tmpdir):
with tmpdir.as_cwd():
subprocess.check_call(('git', 'init', '.'))
- subprocess.check_call((
- 'git', 'commit', '-m', 'init', '--allow-empty', '--no-gpg-sign',
- ))
+ git_commit('--allow-empty', '-m', 'init')
subprocess.check_call(('git', 'init', 'foo'))
- subprocess.check_call(
- ('git', 'commit', '-m', 'init', '--allow-empty', '--no-gpg-sign'),
- cwd=str(tmpdir.join('foo')),
- )
+ git_commit('--allow-empty', '-m', 'init', cwd=str(tmpdir.join('foo')))
yield
@@ -31,7 +29,24 @@
)
def test_main_new_submodule(git_dir_with_git_dir, capsys, cmd):
subprocess.check_call(cmd)
- assert main() == 1
+ assert main(('random_non-related_file',)) == 0
+ assert main(('foo',)) == 1
+ out, _ = capsys.readouterr()
+ assert out.startswith('foo: new submodule introduced\n')
+
+
+def test_main_new_submodule_committed(git_dir_with_git_dir, capsys):
+ rev_parse_cmd = ('git', 'rev-parse', 'HEAD')
+ from_ref = subprocess.check_output(rev_parse_cmd).decode().strip()
+ subprocess.check_call(('git', 'submodule', 'add', './foo'))
+ git_commit('-m', 'new submodule')
+ to_ref = subprocess.check_output(rev_parse_cmd).decode().strip()
+ with mock.patch.dict(
+ os.environ,
+ {'PRE_COMMIT_FROM_REF': from_ref, 'PRE_COMMIT_TO_REF': to_ref},
+ ):
+ assert main(('random_non-related_file',)) == 0
+ assert main(('foo',)) == 1
out, _ = capsys.readouterr()
assert out.startswith('foo: new submodule introduced\n')
@@ -39,4 +54,4 @@
def test_main_no_new_submodule(git_dir_with_git_dir):
open('test.py', 'a+').close()
subprocess.check_call(('git', 'add', 'test.py'))
- assert main() == 0
+ assert main(('test.py',)) == 0
diff --git a/tests/no_commit_to_branch_test.py b/tests/no_commit_to_branch_test.py
index 610e660..9fcb580 100644
--- a/tests/no_commit_to_branch_test.py
+++ b/tests/no_commit_to_branch_test.py
@@ -3,6 +3,7 @@
from pre_commit_hooks.no_commit_to_branch import is_on_branch
from pre_commit_hooks.no_commit_to_branch import main
from pre_commit_hooks.util import cmd_output
+from testing.util import git_commit
def test_other_branch(temp_git_dir):
@@ -62,7 +63,7 @@
def test_not_on_a_branch(temp_git_dir):
with temp_git_dir.as_cwd():
- cmd_output('git', 'commit', '--no-gpg-sign', '--allow-empty', '-m1')
+ git_commit('--allow-empty', '-m1')
head = cmd_output('git', 'rev-parse', 'HEAD').strip()
cmd_output('git', 'checkout', head)
# we're not on a branch!
