Skip to content

Fail clearly when git metadata is read-only #83

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
anmarchenko wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

Fail clearly when git metadata is read-only #83

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
49 changes: 45 additions & 4 deletions internal/git/git.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,29 +2,70 @@ package git

import (
"fmt"
"os"
"os/exec"
"strings"
)

// LookPathFunc is the function used to look up executables in PATH.
// It can be overridden in tests.
var LookPathFunc = exec.LookPath

// RunGitCommandFunc is the function used to run git commands.
// RunGitOutputFunc is the function used to run git commands and capture output.
// It can be overridden in tests.
var RunGitCommandFunc = func(args ...string) error {
return exec.Command("git", args...).Run()
var RunGitOutputFunc = func(args ...string) ([]byte, error) {
return exec.Command("git", args...).CombinedOutput()
}

var createTempFileFunc = os.CreateTemp

// CheckAvailable verifies that git is available and the current directory is a git repository.
// Returns an error if git is not installed or the current directory is not a git repository.
func CheckAvailable() error {
if _, err := LookPathFunc("git"); err != nil {
return fmt.Errorf("git executable not found: git is required for ddtest to work")
}

if err := RunGitCommandFunc("rev-parse", "--git-dir"); err != nil {
gitDir, err := getGitDirectory()
if err != nil {
return fmt.Errorf("current directory is not a git repository: git is required for ddtest to work")
}

if err := checkWritable(gitDir); err != nil {
return err
}

return nil
}

func getGitDirectory() (string, error) {
out, err := RunGitOutputFunc("rev-parse", "--absolute-git-dir")

@chatgpt-codex-connector chatgpt-codex-connector Bot Jun 12, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Check the common Git directory for worktrees

In a linked worktree, git rev-parse --absolute-git-dir returns the per-worktree admin directory (for example .git/worktrees/<name>), while Git documents --git-common-dir as the common repository metadata location and the TIA fetch paths later write repository objects/shallow state during git fetch (civisibility/utils/git.go:330 and :450). If the common .git directory is read-only but this per-worktree directory is writable, this preflight passes and ddtest still fails later without the clear startup error this change is adding; check the absolute common git dir as well/instead. Git docs: https://git-scm.com/docs/git-rev-parse/2.13.7.html#Documentation/git-rev-parse.txt---git-common-dir

Useful? React with 👍 / 👎.

@anmarchenko anmarchenko Jun 12, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

this is interesting to know, but for CI worktree is more like an edge case for CI environments that we target - but we'll keep it mind, worktrees get more usage with AI agents

if err != nil {
return "", err
}

gitDir := strings.TrimSpace(string(out))
if gitDir == "" {
return "", fmt.Errorf("git returned an empty git directory")
}

return gitDir, nil
}

func checkWritable(path string) error {
file, err := createTempFileFunc(path, ".ddtest-write-check-")
if err != nil {
return fmt.Errorf("git metadata directory is not writable: ddtest needs write access to %s to fetch repository metadata for Test Impact Analysis: %w", path, err)
}

fileName := file.Name()
if err := file.Close(); err != nil {
_ = os.Remove(fileName)
return fmt.Errorf("git metadata directory write check failed: %s: %w", path, err)
}
if err := os.Remove(fileName); err != nil {
return fmt.Errorf("git metadata directory write check cleanup failed: %s: %w", path, err)
}

return nil
}
141 changes: 88 additions & 53 deletions internal/git/git_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,19 +2,45 @@ package git

import (
"errors"
"fmt"
"os"
"os/exec"
"strings"
"testing"
)

func TestCheckAvailable_Success(t *testing.T) {
// Save original functions
func resetGitHooks(t *testing.T) {
t.Helper()

origLookPath := LookPathFunc
origRunGit := RunGitCommandFunc
defer func() {
origRunGitOutput := RunGitOutputFunc
origCreateTempFile := createTempFileFunc
t.Cleanup(func() {
LookPathFunc = origLookPath
RunGitCommandFunc = origRunGit
}()
RunGitOutputFunc = origRunGitOutput
createTempFileFunc = origCreateTempFile
})
}

func mockGitDirectory(t *testing.T) string {
t.Helper()

gitDir := t.TempDir()
RunGitOutputFunc = func(args ...string) ([]byte, error) {
switch strings.Join(args, " ") {
case "rev-parse --absolute-git-dir":
return []byte(gitDir + "\n"), nil
default:
return nil, fmt.Errorf("unexpected git command: %v", args)
}
}

return gitDir
}

func TestCheckAvailable_Success(t *testing.T) {
resetGitHooks(t)
mockGitDirectory(t)

// Mock both functions to succeed
LookPathFunc = func(file string) (string, error) {
Expand All @@ -23,9 +49,6 @@ func TestCheckAvailable_Success(t *testing.T) {
}
return "", errors.New("not found")
}
RunGitCommandFunc = func(args ...string) error {
return nil
}

err := CheckAvailable()
if err != nil {
Expand All @@ -34,21 +57,12 @@ func TestCheckAvailable_Success(t *testing.T) {
}

func TestCheckAvailable_GitNotInstalled(t *testing.T) {
// Save original functions
origLookPath := LookPathFunc
origRunGit := RunGitCommandFunc
defer func() {
LookPathFunc = origLookPath
RunGitCommandFunc = origRunGit
}()
resetGitHooks(t)

// Mock LookPath to fail
LookPathFunc = func(file string) (string, error) {
return "", errors.New("executable file not found in $PATH")
}
RunGitCommandFunc = func(args ...string) error {
return nil
}

err := CheckAvailable()
if err == nil {
Expand All @@ -63,13 +77,7 @@ func TestCheckAvailable_GitNotInstalled(t *testing.T) {
}

func TestCheckAvailable_NotAGitRepo(t *testing.T) {
// Save original functions
origLookPath := LookPathFunc
origRunGit := RunGitCommandFunc
defer func() {
LookPathFunc = origLookPath
RunGitCommandFunc = origRunGit
}()
resetGitHooks(t)

// Mock LookPath to succeed, but git command to fail
LookPathFunc = func(file string) (string, error) {
Expand All @@ -78,8 +86,8 @@ func TestCheckAvailable_NotAGitRepo(t *testing.T) {
}
return "", errors.New("not found")
}
RunGitCommandFunc = func(args ...string) error {
return errors.New("fatal: not a git repository")
RunGitOutputFunc = func(args ...string) ([]byte, error) {
return nil, errors.New("fatal: not a git repository")
}

err := CheckAvailable()
Expand All @@ -95,37 +103,65 @@ func TestCheckAvailable_NotAGitRepo(t *testing.T) {
}

func TestCheckAvailable_CorrectGitCommand(t *testing.T) {
// Save original functions
origLookPath := LookPathFunc
origRunGit := RunGitCommandFunc
defer func() {
LookPathFunc = origLookPath
RunGitCommandFunc = origRunGit
}()
resetGitHooks(t)

var capturedArgs []string
gitDir := t.TempDir()
var capturedOutputArgs []string

LookPathFunc = func(file string) (string, error) {
return "/usr/bin/git", nil
}
RunGitCommandFunc = func(args ...string) error {
capturedArgs = args
return nil
RunGitOutputFunc = func(args ...string) ([]byte, error) {
capturedOutputArgs = append([]string(nil), args...)
switch strings.Join(args, " ") {
case "rev-parse --absolute-git-dir":
return []byte(gitDir + "\n"), nil
default:
return nil, fmt.Errorf("unexpected git command: %v", args)
}
}

_ = CheckAvailable()
if err := CheckAvailable(); err != nil {
t.Fatalf("expected no error, got %v", err)
}

expectedArgs := []string{"rev-parse", "--git-dir"}
if len(capturedArgs) != len(expectedArgs) {
t.Errorf("expected args %v, got %v", expectedArgs, capturedArgs)
expectedArgs := []string{"rev-parse", "--absolute-git-dir"}
if len(capturedOutputArgs) != len(expectedArgs) {
t.Fatalf("expected args %v, got %v", expectedArgs, capturedOutputArgs)
}
for i, arg := range expectedArgs {
if capturedArgs[i] != arg {
t.Errorf("expected arg[%d] = %q, got %q", i, arg, capturedArgs[i])
if capturedOutputArgs[i] != arg {
t.Errorf("expected arg[%d] = %q, got %q", i, arg, capturedOutputArgs[i])
}
}
}

func TestCheckAvailable_GitDirectoryNotWritable(t *testing.T) {
resetGitHooks(t)
gitDir := mockGitDirectory(t)

LookPathFunc = func(file string) (string, error) {
return "/usr/bin/git", nil
}
createTempFileFunc = func(dir, pattern string) (*os.File, error) {
return nil, os.ErrPermission
}

err := CheckAvailable()
if err == nil {
t.Fatal("expected error when git directory is not writable")
}
if !strings.Contains(err.Error(), "git metadata directory is not writable") {
t.Errorf("expected git metadata writability error, got: %v", err)
}
if !strings.Contains(err.Error(), gitDir) {
t.Errorf("expected git directory path %q in error message, got: %v", gitDir, err)
}
if !strings.Contains(err.Error(), "ddtest needs write access") {
t.Errorf("expected actionable write access message, got: %v", err)
}
}

func TestCheckAvailable_Integration(t *testing.T) {
// This test uses real git commands to verify the function works in a real environment
// Skip if git is not available
Expand All @@ -134,22 +170,21 @@ func TestCheckAvailable_Integration(t *testing.T) {
}

// Save original functions to ensure we use real ones
origLookPath := LookPathFunc
origRunGit := RunGitCommandFunc
defer func() {
LookPathFunc = origLookPath
RunGitCommandFunc = origRunGit
}()
resetGitHooks(t)

// Reset to real implementations
LookPathFunc = exec.LookPath
RunGitCommandFunc = func(args ...string) error {
return exec.Command("git", args...).Run()
RunGitOutputFunc = func(args ...string) ([]byte, error) {
return exec.Command("git", args...).CombinedOutput()
}
createTempFileFunc = os.CreateTemp

// Since tests run in the project directory which is a git repo, this should succeed
err := CheckAvailable()
if err != nil {
if strings.Contains(err.Error(), "git metadata directory is not writable") {
t.Skipf("git metadata directory is not writable in this environment: %v", err)
}
t.Errorf("expected no error in git repository, got %v", err)
}
}
Loading