Skip to content

hook install/status: .git/config is parsed with configparser, which rejects git's legal duplicate keys (spurious "could not read core.hooksPath" on VS Code repos) #1907

Description

@rjmarin

Summary

_hooks_dir() reads .git/config with configparser.RawConfigParser. Git's config format is not INI: it permits the same key more than once in a section (multivars). configparser rejects that with DuplicateOptionError, so every graphify hook install|status|uninstall prints an alarming warning on a perfectly valid repo.

Correctness is saved by the git rev-parse --git-path hooks fallback, so this is a false alarm rather than a breakage — but the warning is scary, it fires on a very common setup, and the block that produces it turns out to be redundant.

Version: graphifyy==0.9.15, macOS arm64, git 2.x, husky 9.1.7.

Repro

VS Code writes vscode-merge-base into .git/config, and routinely writes it twice:

[branch "main"]
	remote = origin
	vscode-merge-base = origin/main
	vscode-merge-base = origin/main
	merge = refs/heads/main

Git is entirely happy with this:

$ git config --get-all branch.main.vscode-merge-base
origin/main
origin/main

configparser is not:

$ python -c "import configparser; c=configparser.RawConfigParser(); c.read('.git/config', encoding='utf-8')"
configparser.DuplicateOptionError: While reading from '.git/config' [line 15]:
option 'vscode-merge-base' in section 'branch "main"' already exists

So every hook command emits:

$ graphify hook install
[graphify hooks] could not read core.hooksPath from /path/to/repo/.git/config: While reading from PosixPath('/path/to/repo/.git/config') [line 15]: option 'vscode-merge-base' in section 'branch "main"' already exists
post-commit: installed at /path/to/repo/.husky/post-commit
post-checkout: installed at /path/to/repo/.husky/post-checkout

Reproduce from scratch:

git init repo && cd repo
printf '[branch "main"]\n\tdupe = a\n\tdupe = a\n' >> .git/config
graphify hook status    # → could not read core.hooksPath ...

Source

graphify/hooks.py:400:

def _hooks_dir(root: Path) -> Path:
    try:
        cfg = configparser.RawConfigParser()
        cfg.read(root / ".git" / "config", encoding="utf-8")
        custom = cfg.get("core", "hookspath", fallback="").strip()
        ...
    except (configparser.Error, OSError) as exc:
        print(f"[graphify hooks] could not read core.hooksPath from ...: {exc}", file=sys.stderr)
    # fallback
    res = _sp.run(["git", "-C", str(root), "rev-parse", "--git-path", "hooks"], ...)

The comment above the except says it was narrowed deliberately (PR747-NEW-2) so corrupt-config/tampering signals surface instead of being swallowed. That intent is good — the problem is that a duplicate key isn't corruption or tampering, it's valid git config, so the signal is crying wolf on ordinary repos.

The configparser block appears to be strictly redundant

git rev-parse --git-path hooks — already there as the fallback — honors core.hooksPath in every case the configparser path tries to cover, and several it can't:

case configparser git rev-parse --git-path hooks
local core.hooksPath
duplicate key anywhere in file ❌ raises
--global / --system core.hooksPath ❌ never reads those files → "" → falls through
[include] / [includeIf] ❌ not resolved → falls through
linked worktree (.git is a file) ❌ path doesn't exist

Verified on my repo (core.hooksPath = .husky/_):

$ git rev-parse --git-path hooks
.husky/_

which _user_hooks_dir() then correctly maps to .husky/ per #987. So in this repo the only thing that produced the right answer was the fallback; the configparser block contributed nothing but the warning.

Suggested fix

Drop the configparser block and let git rev-parse --git-path hooks be the single source of truth (it's already the fallback, already handles worktrees, already handles global/system/includes). If a config read genuinely fails, git's own non-zero exit + stderr is the honest signal to surface.

If the configparser path is worth keeping for the no-git-binary case, strict=False stops the duplicate-key crash:

-        cfg = configparser.RawConfigParser()
+        cfg = configparser.RawConfigParser(strict=False)

though that still leaves global/system/include configs invisible, so deleting seems cleaner than patching.

Related: #1385 and #987 are both core.hooksPath resolution bugs in this same function — consolidating on git rev-parse --git-path hooks would likely retire that whole class.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Fields

    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions