Skip to content

Fix commit hooks respecting core.hooksPath#2159

Open
Siesta0217 wants to merge 1 commit into
gitpython-developers:mainfrom
Siesta0217:fix-core-hooks-path-commit-hooks
Open

Fix commit hooks respecting core.hooksPath#2159
Siesta0217 wants to merge 1 commit into
gitpython-developers:mainfrom
Siesta0217:fix-core-hooks-path-commit-hooks

Conversation

@Siesta0217

Copy link
Copy Markdown

Summary

  • Resolve commit hook paths via git rev-parse --git-path hooks/<name> so GitPython follows Git's core.hooksPath configuration.
  • Keep the existing default .git/hooks behavior when core.hooksPath is not configured.
  • Add regression coverage for a pre-commit hook stored in a custom hooks path.

Fixes #2083

Test Plan

  • python -m pytest test/test_index.py::TestIndex::test_pre_commit_hook_respects_core_hooks_path test/test_index.py::TestIndex::test_pre_commit_hook_success -q -o 'addopts='
  • python -m pytest test/test_index.py::TestIndex::test_run_commit_hook test/test_index.py::TestIndex::test_pre_commit_hook_fail test/test_index.py::TestIndex::test_commit_msg_hook_success -q -o 'addopts='
  • ruff check git/index/fun.py test/test_index.py

Use git rev-parse --git-path when resolving commit hook paths so GitPython follows Git's core.hooksPath configuration.\n\nAdd regression coverage for a pre-commit hook stored in a custom hooks path.\n\nFixes gitpython-developers#2083

@Byron Byron left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks a lot.

The test is good, but the implementation needs adjustments.

Comment thread git/index/fun.py
Comment on lines 85 to 95

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This doesn't do what's advertised in the PR description, as it should be conditional to the presence of core.hookspath.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR updates GitPython’s commit-hook execution to resolve hook locations via Git itself, so hooks honor core.hooksPath, and adds a regression test to confirm a hook runs from a custom hooks directory.

Changes:

  • Add _commit_hook_path() that resolves hooks/<name> via git rev-parse --git-path, intended to respect core.hooksPath.
  • Switch run_commit_hook() to use _commit_hook_path() instead of hardcoding .git/hooks/<name>.
  • Add a regression test verifying a pre-commit hook runs from a configured custom hooks path.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
git/index/fun.py Adds Git-based hook path resolution and uses it in run_commit_hook().
test/test_index.py Adds a regression test for core.hooksPath hook execution.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread git/index/fun.py
Comment on lines +68 to +74
""":return: path to the named commit hook, respecting Git's core.hooksPath."""
hp = index.repo.git.rev_parse("--git-path", f"hooks/{name}")
if osp.isabs(hp):
return hp

base_dir = index.repo.working_dir or index.repo.git_dir
return osp.abspath(osp.join(base_dir, hp))
Comment thread test/test_index.py
Comment on lines +1188 to 1189

@pytest.mark.xfail(
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

Commit hooks don't respect core.hooksPath in config.

4 participants