fix: bd doctor falsely reports pre-push hook is not a bd hook (#799)

The CheckSyncBranchHookCompatibility was checking core.hooksPath first,
while CheckGitHooks always uses .git/hooks/. This caused inconsistent
results when core.hooksPath was set globally.

Fix: Remove core.hooksPath check from CheckSyncBranchHookCompatibility
to match CheckGitHooks behavior. Both now consistently use .git/hooks/.

Note: A future improvement could make both respect core.hooksPath.
This commit is contained in:
Steve Yegge
2025-12-29 14:55:30 -08:00
parent c2c2ef5d07
commit 05c8bbe4f9
2 changed files with 13 additions and 22 deletions

View File

@@ -276,22 +276,9 @@ func CheckSyncBranchHookCompatibility(path string) DoctorCheck {
gitDir = filepath.Join(path, gitDir)
}
// Check for pre-push hook in standard location or shared hooks location
var hookPath string
// First check if core.hooksPath is configured (shared hooks)
hooksPathCmd := exec.Command("git", "config", "--get", "core.hooksPath")
hooksPathCmd.Dir = path
if hooksPathOutput, err := hooksPathCmd.Output(); err == nil {
sharedHooksDir := strings.TrimSpace(string(hooksPathOutput))
if !filepath.IsAbs(sharedHooksDir) {
sharedHooksDir = filepath.Join(path, sharedHooksDir)
}
hookPath = filepath.Join(sharedHooksDir, "pre-push")
} else {
// Use standard .git/hooks location
hookPath = filepath.Join(gitDir, "hooks", "pre-push")
}
// Use standard .git/hooks location for consistency with CheckGitHooks (issue #799)
// Note: core.hooksPath is intentionally NOT checked here to match CheckGitHooks behavior.
hookPath := filepath.Join(gitDir, "hooks", "pre-push")
hookContent, err := os.ReadFile(hookPath) // #nosec G304 - path is controlled
if err != nil {

View File

@@ -744,16 +744,20 @@ func TestCheckSyncBranchHookCompatibility_OldHookFormat(t *testing.T) {
expectedStatus: "warning",
expectInMsg: "Could not determine",
},
// Note: core.hooksPath is NOT respected by this check (or CheckGitHooks)
// Both functions use .git/hooks/ for consistency. This is a known limitation.
// A future fix could make both respect core.hooksPath.
{
name: "hook in shared hooks directory (core.hooksPath)",
name: "hook in standard location with core.hooksPath set elsewhere",
setup: func(t *testing.T, dir string) {
setupGitRepoInDir(t, dir)
// create shared hooks directory
sharedHooksDir := filepath.Join(dir, ".git-hooks")
os.MkdirAll(sharedHooksDir, 0755)
// Put hook in standard .git/hooks location
gitDir := filepath.Join(dir, ".git")
hooksDir := filepath.Join(gitDir, "hooks")
os.MkdirAll(hooksDir, 0755)
hookContent := "#!/bin/sh\n# bd-hooks-version: 0.29.0\nbd sync\n"
os.WriteFile(filepath.Join(sharedHooksDir, "pre-push"), []byte(hookContent), 0755)
// configure core.hooksPath
os.WriteFile(filepath.Join(hooksDir, "pre-push"), []byte(hookContent), 0755)
// configure core.hooksPath (ignored by this check)
cmd := exec.Command("git", "config", "core.hooksPath", ".git-hooks")
cmd.Dir = dir
_ = cmd.Run()