From 1692c83a9b9ffcb5c5edfae959cbe8a33e6b8e8c Mon Sep 17 00:00:00 2001 From: Edmund Kohlwey Date: Wed, 10 Jun 2026 09:55:08 -0400 Subject: [PATCH] test/shamhub: Scope bare-repo git commands with --git-dir git 2.50 no longer auto-discovers a bare repository from cwd alone, so running commands like `git config`, `git show-ref`, and `git rev-parse` via WithDir(repoDir(...)) fails with "fatal: not in a git directory". This broke every shamhub-backed script test. Add a `(*ShamHub).gitCmd(ctx, owner, repo, args...)` helper that prefixes the command with `--git-dir ` and replace each WithDir-on- bare-repo call site with it. Behavior is unchanged on older git versions; tests now pass on 2.50+. --- internal/forge/shamhub/change.go | 4 +--- internal/forge/shamhub/ref.go | 5 +---- internal/forge/shamhub/repo.go | 3 +-- internal/forge/shamhub/shamhub.go | 11 ++++++++++ internal/forge/shamhub/states.go | 35 +++++++++--------------------- internal/forge/shamhub/template.go | 9 +++----- 6 files changed, 27 insertions(+), 40 deletions(-) diff --git a/internal/forge/shamhub/change.go b/internal/forge/shamhub/change.go index 7cf5bbd36..0f49c0e7a 100644 --- a/internal/forge/shamhub/change.go +++ b/internal/forge/shamhub/change.go @@ -7,7 +7,6 @@ import ( "strings" "go.abhg.dev/gs/internal/forge" - "go.abhg.dev/gs/internal/xec" ) // ListChanges reports all changes known to the forge. @@ -222,8 +221,7 @@ type ChangeBranch struct { } func (sh *ShamHub) toChangeBranch(b *shamBranch) (*ChangeBranch, error) { - out, err := xec.Command(context.Background(), sh.log, sh.gitExe, "rev-parse", b.Name). - WithDir(sh.repoDir(b.Owner, b.Repo)). + out, err := sh.gitCmd(context.Background(), b.Owner, b.Repo, "rev-parse", b.Name). Output() if err != nil { return nil, fmt.Errorf("get SHA for %v/%v:%v: %w", b.Owner, b.Repo, b.Name, err) diff --git a/internal/forge/shamhub/ref.go b/internal/forge/shamhub/ref.go index d70558c7f..9a45d4108 100644 --- a/internal/forge/shamhub/ref.go +++ b/internal/forge/shamhub/ref.go @@ -3,8 +3,6 @@ package shamhub import ( "context" "fmt" - - "go.abhg.dev/gs/internal/xec" ) type refExistsRequest struct { @@ -38,8 +36,7 @@ func (r *forgeRepository) RefExists(ctx context.Context, ref string) (bool, erro } func (sh *ShamHub) refExists(ctx context.Context, owner, repo, ref string) bool { - return xec.Command(ctx, sh.log, sh.gitExe, "show-ref", "--verify", "--quiet", ref). - WithDir(sh.repoDir(owner, repo)). + return sh.gitCmd(ctx, owner, repo, "show-ref", "--verify", "--quiet", ref). Run() == nil } diff --git a/internal/forge/shamhub/repo.go b/internal/forge/shamhub/repo.go index f7456943a..6a7bd2b6b 100644 --- a/internal/forge/shamhub/repo.go +++ b/internal/forge/shamhub/repo.go @@ -98,8 +98,7 @@ func (sh *ShamHub) newRepository(owner, repo string, forkOf *repoID) (string, er } // Configure the repository to accept pushes. - if err := xec.Command(ctx, sh.log, sh.gitExe, "config", "http.receivepack", "true"). - WithDir(repoDir). + if err := sh.gitCmd(ctx, owner, repo, "config", "http.receivepack", "true"). CaptureStdout(). Run(); err != nil { return "", fmt.Errorf("configure repository: %w", err) diff --git a/internal/forge/shamhub/shamhub.go b/internal/forge/shamhub/shamhub.go index b72603c32..d54453a71 100644 --- a/internal/forge/shamhub/shamhub.go +++ b/internal/forge/shamhub/shamhub.go @@ -5,6 +5,7 @@ package shamhub import ( + "context" "fmt" "net/http/cgi" "net/http/httptest" @@ -137,6 +138,16 @@ func (sh *ShamHub) repoDir(owner, repo string) string { return filepath.Join(sh.gitRoot, owner, repo+".git") } +// gitCmd builds a git command scoped to the bare repository for owner/repo. +// Use this instead of WithDir(repoDir(...)): git 2.50 no longer discovers a +// bare repo from the current working directory alone, so an explicit +// --git-dir is required for `git config`, `git show-ref`, `git rev-parse`, +// and similar commands. +func (sh *ShamHub) gitCmd(ctx context.Context, owner, repo string, args ...string) *xec.Cmd { + return xec.Command(ctx, sh.log, sh.gitExe, + append([]string{"--git-dir", sh.repoDir(owner, repo)}, args...)...) +} + func (sh *ShamHub) changeURL(owner, repo string, change int) string { return fmt.Sprintf("%s/%s/%s/change/%d", sh.GitURL(), owner, repo, change) } diff --git a/internal/forge/shamhub/states.go b/internal/forge/shamhub/states.go index d4690c493..7d67a94f4 100644 --- a/internal/forge/shamhub/states.go +++ b/internal/forge/shamhub/states.go @@ -10,7 +10,6 @@ import ( "go.abhg.dev/gs/internal/forge" "go.abhg.dev/gs/internal/git" - "go.abhg.dev/gs/internal/xec" ) type statesRequest struct { @@ -208,10 +207,6 @@ func (sh *ShamHub) MergeChange(req MergeChangeRequest) error { return fmt.Errorf("change %d (%v/%v) is not open", req.Number, req.Owner, req.Repo) } - // Determine if this is a cross-fork merge by checking if the head branch - // exists in the target repository or needs to be fetched from a fork - targetRepoDir := sh.repoDir(req.Owner, req.Repo) - baseRef := change.Base headRef := change.Head @@ -219,12 +214,10 @@ func (sh *ShamHub) MergeChange(req MergeChangeRequest) error { defer cancel() if req.Time.IsZero() { - out, err := xec.Command( - ctx, sh.log, sh.gitExe, + out, err := sh.gitCmd( + ctx, headRef.Owner, headRef.Repo, "log", "-1", "--format=%cI", headRef.Name, - ). - WithDir(sh.repoDir(headRef.Owner, headRef.Repo)). - Output() + ).Output() if err != nil { return fmt.Errorf("read head commit time: %w", err) } @@ -246,10 +239,8 @@ func (sh *ShamHub) MergeChange(req MergeChangeRequest) error { // If a HeadHash is provided, verify the head matches. if req.HeadHash != "" { - headRepoDir := sh.repoDir(headRef.Owner, headRef.Repo) - out, err := xec.Command(ctx, sh.log, sh.gitExe, + out, err := sh.gitCmd(ctx, headRef.Owner, headRef.Repo, "rev-parse", headRef.Name). - WithDir(headRepoDir). Output() if err != nil { return fmt.Errorf("resolve head ref: %w", err) @@ -267,8 +258,7 @@ func (sh *ShamHub) MergeChange(req MergeChangeRequest) error { if headRef.Owner != req.Owner || headRef.Repo != req.Repo { // Fetch the head branch from the fork forkRepoDir := sh.repoDir(headRef.Owner, headRef.Repo) - if err := xec.Command(ctx, sh.log, sh.gitExe, "fetch", forkRepoDir, headRef.Name+":"+headRef.Name). - WithDir(targetRepoDir). + if err := sh.gitCmd(ctx, req.Owner, req.Repo, "fetch", forkRepoDir, headRef.Name+":"+headRef.Name). Run(); err != nil { return fmt.Errorf("fetch from fork: %w", err) } @@ -284,8 +274,7 @@ func (sh *ShamHub) MergeChange(req MergeChangeRequest) error { // // This requires at least Git 2.38. tree, err := func() (string, error) { - out, err := xec.Command(ctx, sh.log, sh.gitExe, "merge-tree", "--write-tree", baseRef.Name, headRef.Name). - WithDir(targetRepoDir). + out, err := sh.gitCmd(ctx, req.Owner, req.Repo, "merge-tree", "--write-tree", baseRef.Name, headRef.Name). Output() if err != nil { return "", fmt.Errorf("merge-tree: %w", err) @@ -313,8 +302,7 @@ func (sh *ShamHub) MergeChange(req MergeChangeRequest) error { } args = append(args, "-m", msg, tree) - out, err := xec.Command(ctx, sh.log, sh.gitExe, args...). - WithDir(sh.repoDir(req.Owner, req.Repo)). + out, err := sh.gitCmd(ctx, req.Owner, req.Repo, args...). AppendEnv(commitEnv...). Output() if err != nil { @@ -328,8 +316,7 @@ func (sh *ShamHub) MergeChange(req MergeChangeRequest) error { } headHash, err := func() (string, error) { - out, err := xec.Command(ctx, sh.log, sh.gitExe, "rev-parse", headRef.Name). - WithDir(sh.repoDir(headRef.Owner, headRef.Repo)). + out, err := sh.gitCmd(ctx, headRef.Owner, headRef.Repo, "rev-parse", headRef.Name). Output() if err != nil { return "", fmt.Errorf("rev-parse head: %w", err) @@ -344,8 +331,7 @@ func (sh *ShamHub) MergeChange(req MergeChangeRequest) error { // Update the ref to point to the new commit. err = func() error { ref := "refs/heads/" + sh.changes[changeIdx].Base.Name - if err := xec.Command(ctx, sh.log, sh.gitExe, "update-ref", ref, commit). - WithDir(sh.repoDir(req.Owner, req.Repo)). + if err := sh.gitCmd(ctx, req.Owner, req.Repo, "update-ref", ref, commit). Run(); err != nil { return fmt.Errorf("update-ref: %w", err) } @@ -358,8 +344,7 @@ func (sh *ShamHub) MergeChange(req MergeChangeRequest) error { if req.DeleteBranch { err := func() error { - if err := xec.Command(ctx, sh.log, sh.gitExe, "branch", "-D", change.Head.Name). - WithDir(sh.repoDir(change.Head.Owner, change.Head.Repo)). + if err := sh.gitCmd(ctx, change.Head.Owner, change.Head.Repo, "branch", "-D", change.Head.Name). Run(); err != nil { return fmt.Errorf("delete branch: %w", err) } diff --git a/internal/forge/shamhub/template.go b/internal/forge/shamhub/template.go index 7c068f5ff..297a66b64 100644 --- a/internal/forge/shamhub/template.go +++ b/internal/forge/shamhub/template.go @@ -9,7 +9,6 @@ import ( "go.abhg.dev/gs/internal/forge" "go.abhg.dev/gs/internal/scanutil" - "go.abhg.dev/gs/internal/xec" ) var _changeTemplatePaths = []string{ @@ -49,9 +48,8 @@ func (sh *ShamHub) handleChangeTemplate(ctx context.Context, req *changeTemplate var res changeTemplateResponse var matches []matchingTemplatePath - treeCmd := xec.Command(ctx, nil, sh.gitExe, - "ls-tree", "-r", "-z", "--name-only", "HEAD"). - WithDir(sh.repoDir(owner, repo)) + treeCmd := sh.gitCmd(ctx, owner, repo, + "ls-tree", "-r", "-z", "--name-only", "HEAD") for treePath, err := range treeCmd.Scan(scanutil.SplitNull) { if err != nil { return nil, nil @@ -69,9 +67,8 @@ func (sh *ShamHub) handleChangeTemplate(ctx context.Context, req *changeTemplate }) for _, templatePath := range matches { - out, err := xec.Command(ctx, sh.log, sh.gitExe, + out, err := sh.gitCmd(ctx, owner, repo, "cat-file", "blob", "HEAD:"+templatePath.filePath). - WithDir(sh.repoDir(owner, repo)). Output() if err != nil { continue