GeezerVison

The World Wide Web, restored to 800 pixels of glory.

GeezerVison is showing github.com — Fix commit hooks respecting core.hooksPath by Siesta0217 · Pull Request #2159 · gitpython-developers/GitPython

Conversation

Siesta0217 commented May 31, 2026 •

edited by Byron

Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link

Copy Markdown

Tasks

refackiew

Created by Codex on behalf of Byron. Byron will review before this is ready to merge.

Summary

Read the effective core.hooksPath setting case-insensitively through GitPython configuration only when it is present and non-empty.
Preserve the existing .git/hooks behavior—and avoid a Git subprocess—when core.hooksPath is unset or empty.
Resolve configured relative hook directories from the directory where commit hooks run, while supporting absolute and home-relative paths.
Keep the Windows Bash fallback working for hooks outside the repository worktree.
Cover default lookup, mixed-case configuration, non-bare relative paths, and out-of-worktree Windows command construction.

Fixes #2083

Test Plan

31 index tests passed locally.
Ruff lint and format checks passed.
mypy passed for git/index/fun.py.
Full GitHub Actions matrix will be revalidated after the latest review fix.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

EliahKagan reacted with thumbs up emoji

All reactions

👍 1 reaction

Byron requested a review from Copilot

June 16, 2026 23:26

Copilot started reviewing on behalf of Byron

June 16, 2026 23:27

View session

Byron requested changes

Jun 16, 2026

View reviewed changes

Byron left a comment

Copy link

Copy Markdown

Member

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.

Thanks a lot.

The test is good, but the implementation needs adjustments.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Comment thread

git/index/fun.py Resolved

Uh oh!

There was an error while loading. Please reload this page.

Copilot AI reviewed

Jun 16, 2026

View reviewed changes

Copilot AI left a comment

Copy link

Copy Markdown

Contributor

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.

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.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Comment thread

git/index/fun.py Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread

test/test_index.py Resolved

Uh oh!

There was an error while loading. Please reload this page.


          fix: respect core.hooksPath for commit hooks

406b98e

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

Copilot AI review requested due to automatic review settings

July 20, 2026 08:18

Byron force-pushed the fix-core-hooks-path-commit-hooks branch from 9b3198c to 406b98e Compare

July 20, 2026 08:18

Copilot started reviewing on behalf of Byron

July 20, 2026 08:22

View session

Copilot AI reviewed

Jul 20, 2026

View reviewed changes

Copilot AI left a comment

Copy link

Copy Markdown

Contributor

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.

Pull request overview

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

Comments suppressed due to low confidence (1)

git/index/fun.py:99

On Windows, run_commit_hook assumes the hook path is under index.repo.working_dir when it builds relative_hp for bash.exe. With core.hooksPath, git rev-parse --git-path hooks/<name> can legitimately return an absolute path outside the working directory (or a relative path that escapes it), which makes Path(hp).relative_to(...) raise ValueError and turns a runnable hook into a HookExecutionError. Consider using os.path.relpath with a fallback to an absolute POSIX path instead of Path.relative_to so hooks configured outside the worktree don't crash.

    hp = _commit_hook_path(name, index)
    if not os.access(hp, os.X_OK):
        return

    env = os.environ.copy()

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Copilot AI review requested due to automatic review settings

July 20, 2026 08:27

Copilot started reviewing on behalf of Byron

July 20, 2026 08:30

View session

Copilot AI reviewed

Jul 20, 2026

View reviewed changes

Copilot AI left a comment

Copy link

Copy Markdown

Contributor

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.

Pull request overview

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Comment thread

git/index/fun.py Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread

git/index/fun.py Resolved

Uh oh!

There was an error while loading. Please reload this page.

Copilot AI review requested due to automatic review settings

July 20, 2026 08:40

Copilot started reviewing on behalf of Byron

July 20, 2026 08:43

View session

Byron force-pushed the fix-core-hooks-path-commit-hooks branch from 0702dea to f7aa470 Compare

July 20, 2026 08:44

Copilot AI reviewed

Jul 20, 2026

View reviewed changes

Copilot AI left a comment

Copy link

Copy Markdown

Contributor

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.

Pull request overview

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Copilot AI review requested due to automatic review settings

July 20, 2026 08:49

Copilot started reviewing on behalf of Siesta0217

July 20, 2026 08:49

View session

Copilot AI reviewed

Jul 20, 2026

View reviewed changes

Copilot AI left a comment

Copy link

Copy Markdown

Contributor

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.

Pull request overview

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Comment thread

git/index/fun.py Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread

test/test_index.py Resolved

Uh oh!

There was an error while loading. Please reload this page.

Copilot AI review requested due to automatic review settings

July 20, 2026 09:00

Byron force-pushed the fix-core-hooks-path-commit-hooks branch from 11a0c30 to fafbdc8 Compare

July 20, 2026 09:01

Copilot started reviewing on behalf of Byron

July 20, 2026 09:02

View session

Copilot AI reviewed

Jul 20, 2026

View reviewed changes

Copilot AI left a comment

Copy link

Copy Markdown

Contributor

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.

Pull request overview

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Comment thread

git/index/fun.py Resolved

Uh oh!

There was an error while loading. Please reload this page.

Copilot AI review requested due to automatic review settings

July 20, 2026 09:10

Copilot started reviewing on behalf of Byron

July 20, 2026 09:12

View session

Copilot AI reviewed

Jul 20, 2026

View reviewed changes

Copilot AI left a comment

Copy link

Copy Markdown

Contributor

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.

Pull request overview

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Comment thread

git/index/fun.py Resolved

Uh oh!

There was an error while loading. Please reload this page.


          Address review feedback about hook resolution

9bc287a

- Review feedback: resolve core.hooksPath only when configured, avoid an
unconditional git rev-parse dependency, and cover relative paths in a non-bare
repository.
- Read the effective Git configuration directly so unconfigured repositories
retain the legacy .git/hooks lookup and configured relative paths resolve from
the directory where hooks execute. Exercise both guarantees in the hook tests.
- Review feedback: core.hooksPath can point outside index.repo.working_dir,
where Path.relative_to() raises on Windows and prevents a valid hook from
running.
- Build the Bash argument with os.path.relpath so hooks in parent or other
absolute locations remain executable. Fall back to the absolute POSIX-form
path when Windows cannot form a relative path across drives, and add a focused
command-construction regression test.

Byron force-pushed the fix-core-hooks-path-commit-hooks branch from 5252136 to 9bc287a Compare

July 20, 2026 11:01

Copilot AI review requested due to automatic review settings

July 20, 2026 11:01

Copilot started reviewing on behalf of Byron

July 20, 2026 11:03

View session

Copilot AI reviewed

Jul 20, 2026

View reviewed changes

Copilot AI left a comment

Copy link

Copy Markdown

Contributor

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.

Pull request overview

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

Comments suppressed due to low confidence (1)

git/index/fun.py:71

_commit_hook_path() uses config.get("core", "hooksPath"), but GitConfigParser is explicitly case-sensitive for section and option names. This means valid Git configs like [Core] hooksPath=... or core.hookspath=... will be ignored and hooks will incorrectly fall back to .git/hooks, so the fix won’t work in many real-world configurations.

def _commit_hook_path(name: str, index: "IndexFile") -> str:
    """:return: path to the named commit hook, respecting Git's core.hooksPath."""
    with index.repo.config_reader() as config:
        hooks_dir = config.get("core", "hooksPath", fallback="")

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Byron merged commit 354eb2f into gitpython-developers:main

Jul 20, 2026

30 checks passed

Uh oh!

There was an error while loading. Please reload this page.

Byron mentioned this pull request

Jul 20, 2026

fix: respect core.hooksPath git config in commit hooks #2173

Closed

coderabbitai Bot mentioned this pull request

Aug 10, 2026

test: answer eight unresolved review findings, and prove the new gates bite myguard-labs/nginx-cache-turbo-module#232

Merged

Reviewers

Byron

Byron requested changes

Copilot code review Copilot

Copilot review effort

Applies to this pull request for everyone.Learn more about Copilot code review.

Copilot left review comments

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

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

Uh oh!

There was an error while loading. Please reload this page.

5 participants

Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.

You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.

GeezerVison

Conversation

Siesta0217 commented May 31, 2026 • edited by Byron Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Tasks

Summary

Test Plan

Uh oh!

Byron left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Milestone

Development

Uh oh!

5 participants

Siesta0217 commented May 31, 2026 •

edited by Byron

Loading

Uh oh!

There was an error while loading. Please reload this page.