-
-
Notifications
You must be signed in to change notification settings - Fork 966
Fix commit hooks to respect core.hooksPath configuration #2090
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from 1 commit
7977004
1a10226
7b1acdb
6e58a69
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1055,6 +1055,86 @@ def test_run_commit_hook(self, rw_repo): | |
| output = Path(rw_repo.git_dir, "output.txt").read_text(encoding="utf-8") | ||
| self.assertEqual(output, "ran fake hook\n") | ||
|
|
||
| @pytest.mark.xfail( | ||
| type(_win_bash_status) is WinBashStatus.Absent, | ||
| reason="Can't run a hook on Windows without bash.exe.", | ||
| raises=HookExecutionError, | ||
| ) | ||
| @pytest.mark.xfail( | ||
| type(_win_bash_status) is WinBashStatus.WslNoDistro, | ||
| reason="Currently uses the bash.exe of WSL, even with no WSL distro installed", | ||
| raises=HookExecutionError, | ||
| ) | ||
| @with_rw_repo("HEAD", bare=False) | ||
| def test_run_commit_hook_respects_core_hookspath(self, rw_repo): | ||
| """Test that run_commit_hook() respects core.hooksPath configuration. | ||
|
|
||
| This addresses issue #2083 where commit hooks were always looked for in | ||
| $GIT_DIR/hooks instead of respecting the core.hooksPath config setting. | ||
| """ | ||
| index = rw_repo.index | ||
|
|
||
| # Create a custom hooks directory outside of .git | ||
| custom_hooks_dir = Path(rw_repo.working_tree_dir) / "custom-hooks" | ||
| custom_hooks_dir.mkdir() | ||
|
|
||
| # Create a hook in the custom location | ||
| custom_hook = custom_hooks_dir / "fake-hook" | ||
| custom_hook.write_text(HOOKS_SHEBANG + "echo 'ran from custom hooks path' >output.txt") | ||
| custom_hook.chmod(0o744) | ||
|
|
||
| # Set core.hooksPath in the repo config | ||
| with rw_repo.config_writer() as config: | ||
| config.set_value("core", "hooksPath", str(custom_hooks_dir)) | ||
|
|
||
| # Run the hook - it should use the custom path | ||
| run_commit_hook("fake-hook", index) | ||
|
|
||
| output_file = Path(rw_repo.working_tree_dir) / "output.txt" | ||
| self.assertTrue(output_file.exists(), "Hook should have created output.txt") | ||
| output = output_file.read_text(encoding="utf-8") | ||
| self.assertEqual(output, "ran from custom hooks path\n") | ||
|
Comment on lines
1069
to
1096
|
||
|
|
||
| @pytest.mark.xfail( | ||
| type(_win_bash_status) is WinBashStatus.Absent, | ||
| reason="Can't run a hook on Windows without bash.exe.", | ||
| raises=HookExecutionError, | ||
| ) | ||
| @pytest.mark.xfail( | ||
| type(_win_bash_status) is WinBashStatus.WslNoDistro, | ||
| reason="Currently uses the bash.exe of WSL, even with no WSL distro installed", | ||
| raises=HookExecutionError, | ||
| ) | ||
| @with_rw_repo("HEAD", bare=False) | ||
| def test_run_commit_hook_respects_relative_core_hookspath(self, rw_repo): | ||
| """Test that run_commit_hook() handles relative core.hooksPath correctly. | ||
|
|
||
| Per git-config docs, relative paths for core.hooksPath are relative to | ||
| the directory where hooks are run (typically the working tree root). | ||
| """ | ||
| index = rw_repo.index | ||
|
|
||
| # Create a custom hooks directory with a relative path | ||
| custom_hooks_dir = Path(rw_repo.working_tree_dir) / "relative-hooks" | ||
| custom_hooks_dir.mkdir() | ||
|
|
||
| # Create a hook in the custom location | ||
| custom_hook = custom_hooks_dir / "fake-hook" | ||
| custom_hook.write_text(HOOKS_SHEBANG + "echo 'ran from relative hooks path' >output.txt") | ||
| custom_hook.chmod(0o744) | ||
|
|
||
| # Set core.hooksPath to a relative path | ||
| with rw_repo.config_writer() as config: | ||
| config.set_value("core", "hooksPath", "relative-hooks") | ||
|
|
||
| # Run the hook - it should resolve the relative path correctly | ||
| run_commit_hook("fake-hook", index) | ||
|
|
||
| output_file = Path(rw_repo.working_tree_dir) / "output.txt" | ||
| self.assertTrue(output_file.exists(), "Hook should have created output.txt") | ||
| output = output_file.read_text(encoding="utf-8") | ||
| self.assertEqual(output, "ran from relative hooks path\n") | ||
|
Comment on lines
1109
to
1136
|
||
|
|
||
| @ddt.data((False,), (True,)) | ||
| @with_rw_directory | ||
| def test_hook_uses_shell_not_from_cwd(self, rw_dir, case): | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The import statement
from git.repo.base import Repois only used for type hinting in the function signature on line 71. This import is already conditionally imported at the module level within theTYPE_CHECKINGblock (see lines 48-52), making this local import unnecessary. Consider removing this local import and relying on the string annotation"Repo"which is already being used.