Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions git/AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@ not about one product's workflow.
- Do not call `exec.Command("git", ...)` directly in package code unless the
direct call is the behavior being tested.
- Pass `context.Context` through git operations that can block.
- Keep best-effort helper probes bounded; they must not stall the caller's real
git command when optional ambient state is slow or broken.
- Return git failures with captured stderr. Do not hide git's message behind a
generic error.
- Use `gitlock` around mutations that can race with another process touching
Expand Down
7 changes: 6 additions & 1 deletion git/cmd/gitcmd.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import (
"strconv"
"strings"
"sync"
"time"

gitenv "go.kenn.io/kit/git/env"
)
Expand Down Expand Up @@ -183,6 +184,8 @@ var (
emptyGlobalConfigPath string
)

var safeDirectoryProbeTimeout = 2 * time.Second

// nullGlobalConfigPath returns a path suitable for GIT_CONFIG_GLOBAL that makes
// git read an empty (no-op) global config.
//
Expand Down Expand Up @@ -237,10 +240,12 @@ func readSafeDirectories(ctx context.Context, env []string, dir string) []string
for _, scope := range scopes {
// --includes is required for explicit-scope reads to honor include.path
// and includeIf directives the way git's default config sequence does.
cmd := gitCommand(ctx, true, "config", scope, "--includes", "-z", "--get-all", "safe.directory")
probeCtx, cancel := context.WithTimeout(ctx, safeDirectoryProbeTimeout)
cmd := gitCommand(probeCtx, true, "config", scope, "--includes", "-z", "--get-all", "safe.directory")
cmd.Dir = dir
cmd.Env = env
out, err := cmd.Output()
cancel()
if err != nil || len(out) == 0 {
continue
}
Expand Down
43 changes: 43 additions & 0 deletions git/cmd/gitcmd_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,13 @@ import (
"context"
"encoding/base64"
"os"
"os/exec"
"path/filepath"
"runtime"
"slices"
"strings"
"testing"
"time"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
Expand Down Expand Up @@ -128,6 +131,23 @@ func TestReadSafeDirectoriesHonorsNoSystem(t *testing.T) {
assert.Equal(t, []string{"/home/repo"}, got)
}

func TestReadSafeDirectoriesBoundsProbeRuntime(t *testing.T) {
origTimeout := safeDirectoryProbeTimeout
safeDirectoryProbeTimeout = 50 * time.Millisecond
t.Cleanup(func() { safeDirectoryProbeTimeout = origTimeout })

binDir := buildSleepingGit(t)
pathEnv := binDir + string(os.PathListSeparator) + os.Getenv("PATH")
t.Setenv("PATH", pathEnv)
env := append(os.Environ(), "PATH="+pathEnv, "GIT_CONFIG_NOSYSTEM=0")

start := time.Now()
got := readSafeDirectories(context.Background(), env, "")

assert.Empty(t, got)
assert.Less(t, time.Since(start), time.Second, "safe.directory probes are best-effort and must not stall git commands")
}

func TestReadSafeDirectoriesConditionalInclude(t *testing.T) {
// Regression test: the probes must run in the command's directory with
// --includes so includeIf "gitdir:..." entries resolve for the repository
Expand Down Expand Up @@ -298,6 +318,29 @@ func captureGitEnv(t *testing.T, runner Runner) string {
return string(envBytes)
}

func buildSleepingGit(t *testing.T) string {
t.Helper()
binDir := t.TempDir()
exeName := "git"
if runtime.GOOS == "windows" {
exeName += ".exe"
}
exePath := filepath.Join(binDir, exeName)
srcPath := filepath.Join(t.TempDir(), "main.go")
require.NoError(t, os.WriteFile(srcPath, []byte(`package main

import "time"

func main() {
time.Sleep(10 * time.Second)
}
`), 0o600))
cmd := exec.Command("go", "build", "-o", exePath, srcPath)
out, err := cmd.CombinedOutput()
require.NoError(t, err, string(out))
return binDir
}

func gitConfigValue(env, key string) string {
values := map[string]string{}
keys := map[string]string{}
Expand Down
Loading