From b7d94d1c8ec232331e8871ad229fcae25534688d Mon Sep 17 00:00:00 2001 From: dawn Date: Mon, 24 Aug 2026 12:36:35 +0900 Subject: [PATCH] gitutil,spindle,knotserver/git,knotmirror/xrpc,cmd/prefill-zoekt: consolidate git subprocess handling Signed-off-by: dawn --- cmd/prefill-zoekt/main.go | 3 +- gitutil/archive.go | 13 +- gitutil/command.go | 65 +++++++ gitutil/command_test.go | 23 +++ gitutil/sync.go | 131 ++++++++++++++ .../git/git_test.go => gitutil/sync_test.go | 62 ++++++- knotmirror/git.go | 16 +- knotmirror/xrpc/git_list_branches.go | 10 +- knotmirror/xrpc/git_list_commits.go | 10 +- knotmirror/xrpc/git_list_tags.go | 4 +- knotmirror/xrpc/gitea/batch.go | 6 +- knotmirror/xrpc/gitea/commit.go | 5 +- knotmirror/xrpc/gitea/gitea.go | 5 +- knotserver/git/cmd.go | 7 +- knotserver/git/diff.go | 3 +- knotserver/git/fork.go | 4 +- knotserver/git/last_commit.go | 3 +- knotserver/git/merge.go | 14 +- knotserver/git/service/service.go | 6 +- spindle/git/git.go | 164 ------------------ spindle/server.go | 11 +- spindle/tapclient.go | 4 +- 22 files changed, 335 insertions(+), 234 deletions(-) create mode 100644 gitutil/command.go create mode 100644 gitutil/command_test.go create mode 100644 gitutil/sync.go rename spindle/git/git_test.go => gitutil/sync_test.go (56%) delete mode 100644 spindle/git/git.go diff --git a/cmd/prefill-zoekt/main.go b/cmd/prefill-zoekt/main.go index d70d3b3e..ae8e3304 100644 --- a/cmd/prefill-zoekt/main.go +++ b/cmd/prefill-zoekt/main.go @@ -28,6 +28,7 @@ import ( "github.com/bluesky-social/indigo/atproto/syntax" "github.com/samber/lo" "github.com/sourcegraph/zoekt" + "tangled.org/core/gitutil" "tangled.org/core/repoident" ) @@ -121,7 +122,7 @@ func resolveHead(knot repoident.KnotURL, repoDid repoident.RepoDid, allowHTTP bo lo.Ternary(allowHTTP, []string{"-c", "http.sslVerify=false"}, nil), "ls-remote", "--symref", remote, "HEAD", ) - out, err := exec.Command("git", args...).Output() + out, err := gitutil.Contain(exec.Command("git", args...)).Output() if err != nil { return zoekt.RepositoryBranch{}, fmt.Errorf("git ls-remote --symref %s HEAD: %w", remote, err) } diff --git a/gitutil/archive.go b/gitutil/archive.go index 1bade909..806e06d4 100644 --- a/gitutil/archive.go +++ b/gitutil/archive.go @@ -11,11 +11,8 @@ import ( "mime" "net/http" "net/url" - "os/exec" "slices" "strings" - "syscall" - "time" "unicode" "github.com/go-git/go-git/v5/plumbing" @@ -250,25 +247,17 @@ func (b *ResponseBody) Fail() { b.inner.WriteHeader(http.StatusInternalServerError) } -const archiveWaitDelay = 10 * time.Second - func WriteArchive(ctx context.Context, w io.Writer, repoPath string, archive ServedArchive) error { ctx, cancel := context.WithCancel(ctx) defer cancel() - cmd := exec.CommandContext(ctx, "git", "archive", + cmd := Command(ctx, "archive", "--format="+archive.format.String(), "--prefix="+archive.prefix.String()+"/", "--", archive.rev.String()) cmd.Dir = repoPath stderr := new(bytes.Buffer) cmd.Stderr = stderr - cmd.SysProcAttr = &syscall.SysProcAttr{Setpgid: true} - cmd.WaitDelay = archiveWaitDelay - cmd.Cancel = func() error { - err := syscall.Kill(-cmd.Process.Pid, syscall.SIGKILL) - return lo.Ternary(err == syscall.ESRCH, nil, err) - } stdout, err := cmd.StdoutPipe() if err != nil { diff --git a/gitutil/command.go b/gitutil/command.go new file mode 100644 index 00000000..b0b27b32 --- /dev/null +++ b/gitutil/command.go @@ -0,0 +1,65 @@ +package gitutil + +import ( + "bytes" + "context" + "fmt" + "os/exec" + "strings" + "syscall" + "time" +) + +// git runs network work in child processes, so killing only git leaves them running + +// WaitDelay stops waits on output held open by git's children +const WaitDelay = 10 * time.Second + +// Command kills git and its children when ctx is cancelled +func Command(ctx context.Context, args ...string) *exec.Cmd { + return Contain(exec.CommandContext(ctx, "git", args...)) +} + +// Contain is for sandbox-rebuilt commands and direct git binaries +func Contain(cmd *exec.Cmd) *exec.Cmd { + if cmd.SysProcAttr == nil { + cmd.SysProcAttr = &syscall.SysProcAttr{} + } + cmd.SysProcAttr.Setpgid = true + cmd.WaitDelay = WaitDelay + // exec rejects Cancel on commands not created with CommandContext + if cmd.Cancel != nil { + cmd.Cancel = func() error { + if cmd.Process == nil { + return nil + } + err := syscall.Kill(-cmd.Process.Pid, syscall.SIGKILL) + if err == syscall.ESRCH { + return nil + } + return err + } + } + return cmd +} + +func Run(ctx context.Context, args ...string) error { + var stderr bytes.Buffer + cmd := Command(ctx, args...) + cmd.Stderr = &stderr + if err := cmd.Run(); err != nil { + return fmt.Errorf("%w: %s", err, strings.TrimSpace(stderr.String())) + } + return nil +} + +func Output(ctx context.Context, args ...string) ([]byte, error) { + var stdout, stderr bytes.Buffer + cmd := Command(ctx, args...) + cmd.Stdout = &stdout + cmd.Stderr = &stderr + if err := cmd.Run(); err != nil { + return nil, fmt.Errorf("%w: %s", err, strings.TrimSpace(stderr.String())) + } + return stdout.Bytes(), nil +} diff --git a/gitutil/command_test.go b/gitutil/command_test.go new file mode 100644 index 00000000..1ff82eb4 --- /dev/null +++ b/gitutil/command_test.go @@ -0,0 +1,23 @@ +package gitutil + +import ( + "os/exec" + "testing" +) + +// os/exec refuses to start a command that carries a Cancel it did not get from +// CommandContext, and the sandbox-wrapped commands in knotserver are exactly +// that shape, so containment has to degrade instead of breaking them +func TestContainContextlessCommand(t *testing.T) { + cmd := Contain(exec.Command("git", "version")) + out, err := cmd.CombinedOutput() + if err != nil { + t.Fatalf("%v: %s", err, out) + } + if !cmd.SysProcAttr.Setpgid { + t.Fatal("a contained command has to lead its own process group") + } + if cmd.WaitDelay == 0 { + t.Fatal("a contained command has to bound its wait") + } +} diff --git a/gitutil/sync.go b/gitutil/sync.go new file mode 100644 index 00000000..0578618b --- /dev/null +++ b/gitutil/sync.go @@ -0,0 +1,131 @@ +package gitutil + +import ( + "context" + "fmt" + "os" + "path/filepath" + "strings" + "sync" + + "github.com/hashicorp/go-version" +) + +// don't let two requests mutate the same checkout at once +var repoLocks keyedMutex + +type keyedMutex struct { + mu sync.Mutex + m map[string]*sync.Mutex +} + +func (k *keyedMutex) lock(key string) func() { + k.mu.Lock() + if k.m == nil { + k.m = make(map[string]*sync.Mutex) + } + mu, ok := k.m[key] + if !ok { + mu = &sync.Mutex{} + k.m[key] = mu + } + k.mu.Unlock() + + mu.Lock() + return mu.Unlock +} + +func Version() (*version.Version, error) { + out, err := Output(context.Background(), "version") + if err != nil { + return nil, err + } + fields := strings.Fields(string(out)) + if len(fields) < 3 { + return nil, fmt.Errorf("invalid git version: %s", out) + } + + // git version 2.29.3.windows.1 + versionString := fields[2] + if pos := strings.Index(versionString, "windows"); pos >= 1 { + versionString = versionString[:pos-1] + } + return version.NewVersion(versionString) +} + +// SparseSync updates a sparse checkout, cloning it first if needed +func SparseSync(ctx context.Context, cloneUri, path, rev string, sparse ...string) error { + defer repoLocks.lock(path)() + + exist, err := isDir(path) + if err != nil { + return err + } + if exist { + gitDirExist, err := isDir(path + "/.git") + if err != nil { + return err + } + if !gitDirExist { + if err := os.RemoveAll(path); err != nil { + return fmt.Errorf("cleanup invalid git dir: %w", err) + } + exist = false + } + } + if rev == "" { + rev = "HEAD" + } + clone := func() error { + if err := Run(ctx, "clone", "--no-checkout", "--depth=1", "--filter=tree:0", "--revision="+rev, cloneUri, path); err != nil { + return fmt.Errorf("git clone: %w", err) + } + if err := Run(ctx, append([]string{"-C", path, "sparse-checkout", "set", "--no-cone"}, sparse...)...); err != nil { + return fmt.Errorf("git sparse-checkout set: %w", err) + } + return nil + } + if !exist { + if err := clone(); err != nil { + return err + } + } else { + fetch := func() error { + return Run(ctx, "-C", path, "fetch", "--depth=1", "--filter=tree:0", "origin", rev) + } + if err := fetch(); err != nil { + // a cancelled fetch can leave lock files behind + removeStaleLocks(path) + if retryErr := fetch(); retryErr != nil { + if rmErr := os.RemoveAll(path); rmErr != nil { + return fmt.Errorf("git fetch: %w (cleanup failed: %v)", retryErr, rmErr) + } + if cloneErr := clone(); cloneErr != nil { + return fmt.Errorf("git fetch: %w (re-clone failed: %v)", retryErr, cloneErr) + } + } + } + } + if err := Run(ctx, "-C", path, "checkout", rev); err != nil { + return fmt.Errorf("git checkout: %w", err) + } + return nil +} + +func removeStaleLocks(path string) { + locks, _ := filepath.Glob(filepath.Join(path, ".git", "*.lock")) + for _, lock := range locks { + os.Remove(lock) + } +} + +func isDir(path string) (bool, error) { + info, err := os.Stat(path) + if err == nil && info.IsDir() { + return true, nil + } + if os.IsNotExist(err) { + return false, nil + } + return false, err +} diff --git a/spindle/git/git_test.go b/gitutil/sync_test.go similarity index 56% rename from spindle/git/git_test.go rename to gitutil/sync_test.go index e418b9b3..df0b63a2 100644 --- a/spindle/git/git_test.go +++ b/gitutil/sync_test.go @@ -1,10 +1,12 @@ -package git +package gitutil import ( "bufio" "context" "io" "net" + "os" + "os/exec" "path/filepath" "strings" "sync" @@ -12,6 +14,50 @@ import ( "time" ) +const testSparseDir = "/.tangled/workflows" + +func TestSparseSync(t *testing.T) { + // ignore the developer's git config + t.Setenv("GIT_CONFIG_GLOBAL", os.DevNull) + t.Setenv("GIT_CONFIG_SYSTEM", os.DevNull) + t.Setenv("GIT_AUTHOR_NAME", "spindle") + t.Setenv("GIT_AUTHOR_EMAIL", "spindle@tangled.org") + t.Setenv("GIT_COMMITTER_NAME", "spindle") + t.Setenv("GIT_COMMITTER_EMAIL", "spindle@tangled.org") + + src := t.TempDir() + run := func(args ...string) string { + t.Helper() + cmd := exec.Command("git", append([]string{"-C", src}, args...)...) + out, err := cmd.CombinedOutput() + if err != nil { + t.Fatalf("git %v: %v: %s", args, err, out) + } + return strings.TrimSpace(string(out)) + } + + run("init", "-b", "main") + if err := os.MkdirAll(filepath.Join(src, ".tangled", "workflows"), 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(src, ".tangled", "workflows", "ci.yml"), []byte("when: []\n"), 0o644); err != nil { + t.Fatal(err) + } + run("add", ".") + run("commit", "-m", "workflows") + rev := run("rev-parse", "HEAD") + + path := filepath.Join(t.TempDir(), "repo") + for _, stage := range []string{"clone", "fetch"} { + if err := SparseSync(context.Background(), src, path, rev, testSparseDir); err != nil { + t.Fatalf("%s: %v", stage, err) + } + if _, err := os.Stat(filepath.Join(path, ".tangled", "workflows", "ci.yml")); err != nil { + t.Fatalf("%s: sparse file missing from the checkout: %v", stage, err) + } + } +} + // hangingKnot keeps the child process alive until the client closes its socket func hangingKnot(t *testing.T) (url string, asked, dropped <-chan struct{}) { t.Helper() @@ -58,7 +104,7 @@ func hangingKnot(t *testing.T) (url string, asked, dropped <-chan struct{}) { return "http://" + ln.Addr().String(), askedCh, droppedCh } -func TestRunGitCancelKillsChild(t *testing.T) { +func TestRunCancelKillsRemoteHelper(t *testing.T) { knot, asked, dropped := hangingKnot(t) path := filepath.Join(t.TempDir(), "repo") @@ -66,7 +112,7 @@ func TestRunGitCancelKillsChild(t *testing.T) { defer cancel() done := make(chan error, 1) - go func() { done <- runGit(ctx, "clone", "--no-checkout", "--depth=1", knot+"/repo.git", path) }() + go func() { done <- Run(ctx, "clone", "--no-checkout", "--depth=1", knot+"/repo.git", path) }() select { case <-asked: @@ -84,13 +130,13 @@ func TestRunGitCancelKillsChild(t *testing.T) { t.Fatal("a cancelled clone reported success") } case <-time.After(15 * time.Second): - t.Fatal("runGit is still blocked after cancellation") + t.Fatal("Run is still blocked after cancellation: a surviving helper holds the inherited stderr pipe open") } select { case <-dropped: case <-time.After(15 * time.Second): - t.Fatal("git child outlived the cancelled clone") + t.Fatal("git-remote-http outlived the cancelled clone") } } @@ -102,7 +148,7 @@ func TestSparseSyncCancelReleasesRepoLock(t *testing.T) { defer cancel() done := make(chan error, 1) - go func() { done <- SparseSyncGitRepo(ctx, knot+"/repo.git", path, "HEAD") }() + go func() { done <- SparseSync(ctx, knot+"/repo.git", path, "HEAD", testSparseDir) }() select { case <-asked: @@ -117,7 +163,7 @@ func TestSparseSyncCancelReleasesRepoLock(t *testing.T) { select { case <-done: case <-time.After(15 * time.Second): - t.Fatal("sync is still blocked after cancellation") + t.Fatal("SparseSync is still blocked after cancellation") } // another request must be able to use this repo @@ -125,7 +171,7 @@ func TestSparseSyncCancelReleasesRepoLock(t *testing.T) { cancelDead() next := make(chan error, 1) - go func() { next <- SparseSyncGitRepo(dead, knot+"/repo.git", path, "HEAD") }() + go func() { next <- SparseSync(dead, knot+"/repo.git", path, "HEAD", testSparseDir) }() select { case <-next: diff --git a/knotmirror/git.go b/knotmirror/git.go index 5540fa41..8b6c6963 100644 --- a/knotmirror/git.go +++ b/knotmirror/git.go @@ -7,7 +7,6 @@ import ( "log" "net/url" "os" - "os/exec" "path/filepath" "regexp" "strings" @@ -16,6 +15,7 @@ import ( "github.com/go-git/go-git/v5" gitconfig "github.com/go-git/go-git/v5/config" "github.com/go-git/go-git/v5/plumbing/transport" + "tangled.org/core/gitutil" "tangled.org/core/knotmirror/models" ) @@ -68,7 +68,7 @@ func (c *CliGitMirrorManager) Clone(ctx context.Context, repo *models.Repo) erro } func (c *CliGitMirrorManager) clone(ctx context.Context, path, url string) error { - cmd := exec.CommandContext(ctx, "git", "clone", "--mirror", url, path) + cmd := gitutil.Command(ctx, "clone", "--mirror", url, path) if out, err := cmd.CombinedOutput(); err != nil { if ctx.Err() != nil { return ctx.Err() @@ -93,7 +93,7 @@ func (c *CliGitMirrorManager) Fetch(ctx context.Context, repo *models.Repo) erro } func (c *CliGitMirrorManager) fetch(ctx context.Context, path, url string) error { - cmd := exec.CommandContext(ctx, "git", "-C", path, "fetch", "--prune", url, "+refs/*:refs/*") + cmd := gitutil.Command(ctx, "-C", path, "fetch", "--prune", url, "+refs/*:refs/*") if out, err := cmd.CombinedOutput(); err != nil { if ctx.Err() != nil { return ctx.Err() @@ -102,7 +102,7 @@ func (c *CliGitMirrorManager) fetch(ctx context.Context, path, url string) error } // TODO(boltless): make this dedicated event instead - lsRemoteCmd := exec.CommandContext(ctx, "git", "ls-remote", "--symref", url, "HEAD") + lsRemoteCmd := gitutil.Command(ctx, "ls-remote", "--symref", url, "HEAD") out, err := lsRemoteCmd.CombinedOutput() if err != nil { if ctx.Err() != nil { @@ -123,7 +123,7 @@ func (c *CliGitMirrorManager) fetch(ctx context.Context, path, url string) error } } if headRef != "" { - symrefCmd := exec.CommandContext(ctx, "git", "-C", path, "symbolic-ref", "HEAD", headRef) + symrefCmd := gitutil.Command(ctx, "-C", path, "symbolic-ref", "HEAD", headRef) if out, err := symrefCmd.CombinedOutput(); err != nil { if ctx.Err() != nil { return ctx.Err() @@ -139,7 +139,7 @@ func (c *CliGitMirrorManager) fetch(ctx context.Context, path, url string) error func writeCommitGraph(ctx context.Context, path string, timeout time.Duration) { ctx, cancel := context.WithTimeout(ctx, timeout) defer cancel() - if err := exec.CommandContext(ctx, "git", "-C", path, "commit-graph", "write", "--reachable", "--split").Run(); err != nil { + if err := gitutil.Command(ctx, "-C", path, "commit-graph", "write", "--reachable", "--split").Run(); err != nil { log.Println("failed to run commit-graph", err) } } @@ -170,14 +170,14 @@ func (c *CliGitMirrorManager) Sync(ctx context.Context, repo *models.Repo) error func (c *CliGitMirrorManager) DefaultBranch(ctx context.Context, repo *models.Repo) (branch, error) { path := c.makeRepoPath(repo) - nameCmd := exec.CommandContext(ctx, "git", "-C", path, "symbolic-ref", "--short", "HEAD") + nameCmd := gitutil.Command(ctx, "-C", path, "symbolic-ref", "--short", "HEAD") nameOut, err := nameCmd.Output() if err != nil { return branch{}, err } // --verify --quiet exits 1 with no output on an empty repo (unborn HEAD). - revCmd := exec.CommandContext(ctx, "git", "-C", path, "rev-parse", "--verify", "--quiet", "HEAD") + revCmd := gitutil.Command(ctx, "-C", path, "rev-parse", "--verify", "--quiet", "HEAD") revOut, err := revCmd.Output() if err != nil { return branch{}, err diff --git a/knotmirror/xrpc/git_list_branches.go b/knotmirror/xrpc/git_list_branches.go index ed5a7234..e4efc430 100644 --- a/knotmirror/xrpc/git_list_branches.go +++ b/knotmirror/xrpc/git_list_branches.go @@ -7,7 +7,6 @@ import ( "io" "net/http" "os" - "os/exec" "path/filepath" "slices" "strconv" @@ -16,6 +15,7 @@ import ( "github.com/bluesky-social/indigo/atproto/atclient" "github.com/bluesky-social/indigo/atproto/syntax" "github.com/go-git/go-git/v5/plumbing" + "tangled.org/core/gitutil" "tangled.org/core/knotmirror/xrpc/gitea" "tangled.org/core/types" ) @@ -73,7 +73,7 @@ func (x *Xrpc) listBranches(ctx context.Context, repo syntax.DID, limit int, cur // ignore error: an empty default branch just means nothing is marked default defaultBranch := func(repoPath string) string { - out, err := exec.Command("git", "-C", repoPath, "rev-parse", "--abbrev-ref", "HEAD").Output() + out, err := gitutil.Command(ctx, "-C", repoPath, "rev-parse", "--abbrev-ref", "HEAD").Output() if err != nil { return "" } @@ -86,8 +86,8 @@ func (x *Xrpc) listBranches(ctx context.Context, repo syntax.DID, limit int, cur oid string } refs, err := func(repoPath string) ([]branchRef, error) { - out, err := exec.Command( - "git", + out, err := gitutil.Command( + ctx, "-C", repoPath, "for-each-ref", "--format=%(refname:short)"+fieldSeparator+"%(objectname)", @@ -168,7 +168,7 @@ func (x *Xrpc) listBranches(ctx context.Context, repo syntax.DID, limit int, cur // -> total total, err := func(repoPath string) (int, error) { - out, err := exec.Command("git", "-C", repoPath, "for-each-ref", "--format=%(refname)", "refs/heads").Output() + out, err := gitutil.Command(ctx, "-C", repoPath, "for-each-ref", "--format=%(refname)", "refs/heads").Output() if err != nil { return 0, err } diff --git a/knotmirror/xrpc/git_list_commits.go b/knotmirror/xrpc/git_list_commits.go index 1151cdf5..af290dde 100644 --- a/knotmirror/xrpc/git_list_commits.go +++ b/knotmirror/xrpc/git_list_commits.go @@ -6,13 +6,13 @@ import ( "fmt" "io" "net/http" - "os/exec" "strconv" "github.com/bluesky-social/indigo/atproto/atclient" "github.com/bluesky-social/indigo/atproto/syntax" "github.com/go-git/go-git/v5/plumbing" "github.com/go-git/go-git/v5/plumbing/object" + "tangled.org/core/gitutil" "tangled.org/core/knotmirror/xrpc/gitea" "tangled.org/core/types" ) @@ -73,8 +73,8 @@ func (x *Xrpc) listCommits(ctx context.Context, repo syntax.DID, ref string, lim // -> []hash logs, err := func(repoPath, rev string) ([]byte, error) { - out, err := exec.Command( - "git", + out, err := gitutil.Command( + ctx, "-C", repoPath, "rev-list", rev, @@ -126,8 +126,8 @@ func (x *Xrpc) listCommits(ctx context.Context, repo syntax.DID, ref string, lim // -> total total, err := func(repoPath, rev string) (int, error) { - out, err := exec.Command( - "git", + out, err := gitutil.Command( + ctx, "-C", repoPath, "rev-list", rev, diff --git a/knotmirror/xrpc/git_list_tags.go b/knotmirror/xrpc/git_list_tags.go index 6cd352ab..0ebbd05a 100644 --- a/knotmirror/xrpc/git_list_tags.go +++ b/knotmirror/xrpc/git_list_tags.go @@ -5,13 +5,13 @@ import ( "context" "fmt" "net/http" - "os/exec" "strconv" "github.com/bluesky-social/indigo/atproto/atclient" "github.com/bluesky-social/indigo/atproto/syntax" "github.com/go-git/go-git/v5/plumbing" "github.com/go-git/go-git/v5/plumbing/object" + "tangled.org/core/gitutil" "tangled.org/core/knotserver/git" "tangled.org/core/types" ) @@ -96,7 +96,7 @@ func (x *Xrpc) listTags(ctx context.Context, repo syntax.DID, limit int, cursor // -> total total, err := func(repoPath string) (int, error) { - out, err := exec.Command("git", "-C", repoPath, "for-each-ref", "--format=%(refname)", "refs/tags").Output() + out, err := gitutil.Command(ctx, "-C", repoPath, "for-each-ref", "--format=%(refname)", "refs/tags").Output() if err != nil { return 0, err } diff --git a/knotmirror/xrpc/gitea/batch.go b/knotmirror/xrpc/gitea/batch.go index 07a33615..59a3384b 100644 --- a/knotmirror/xrpc/gitea/batch.go +++ b/knotmirror/xrpc/gitea/batch.go @@ -9,7 +9,6 @@ import ( "fmt" "io" "math" - "os/exec" "strconv" "strings" @@ -19,6 +18,7 @@ import ( "github.com/go-git/go-git/v5/plumbing/filemode" "github.com/go-git/go-git/v5/plumbing/hash" "github.com/go-git/go-git/v5/plumbing/object" + "tangled.org/core/gitutil" ) func GetCommit(ctx context.Context, repoPath, rev string) (*object.Commit, error) { @@ -129,7 +129,7 @@ func CatFileBatchCheck(ctx context.Context, repoPath string) (io.WriteCloser, *b go func() { stderr := &strings.Builder{} - cmd := exec.CommandContext(ctx, "git", "-C", repoPath, "cat-file", "--batch-check") + cmd := gitutil.Command(ctx, "-C", repoPath, "cat-file", "--batch-check") cmd.Stdin = batchStdinReader cmd.Stdout = batchStdoutWriter cmd.Stderr = stderr @@ -167,7 +167,7 @@ func CatFileBatch(ctx context.Context, repoPath string) (io.WriteCloser, *bufio. go func() { stderr := &strings.Builder{} - cmd := exec.CommandContext(ctx, "git", "-C", repoPath, "cat-file", "--batch") + cmd := gitutil.Command(ctx, "-C", repoPath, "cat-file", "--batch") cmd.Stdin = batchStdinReader cmd.Stdout = batchStdoutWriter cmd.Stderr = stderr diff --git a/knotmirror/xrpc/gitea/commit.go b/knotmirror/xrpc/gitea/commit.go index 00ea8495..2098cbfd 100644 --- a/knotmirror/xrpc/gitea/commit.go +++ b/knotmirror/xrpc/gitea/commit.go @@ -4,11 +4,11 @@ import ( "context" "errors" "fmt" - "os/exec" "strings" "github.com/go-git/go-git/v5/plumbing" "github.com/go-git/go-git/v5/plumbing/object" + "tangled.org/core/gitutil" ) func GetCommitByPathWithID(ctx context.Context, oid plumbing.Hash, repoPath, relpath string) (*object.Commit, error) { @@ -20,8 +20,7 @@ func GetCommitByPathWithID(ctx context.Context, oid plumbing.Hash, repoPath, rel relpath = `\` + relpath } - out, err := exec.CommandContext(ctx, - "git", + out, err := gitutil.Command(ctx, "-C", repoPath, "log", "-1", diff --git a/knotmirror/xrpc/gitea/gitea.go b/knotmirror/xrpc/gitea/gitea.go index 41697774..c56b6e38 100644 --- a/knotmirror/xrpc/gitea/gitea.go +++ b/knotmirror/xrpc/gitea/gitea.go @@ -10,12 +10,12 @@ import ( "errors" "fmt" "io" - "os/exec" "path" "strings" "github.com/djherbis/buffer" "github.com/djherbis/nio/v3" + "tangled.org/core/gitutil" "tangled.org/core/sets" ) @@ -34,8 +34,7 @@ func LogNameStatusRepo(ctx context.Context, repository, headRef, treepath string _ = stdoutWriter.Close() } - cmd := exec.CommandContext(ctx, - "git", + cmd := gitutil.Command(ctx, "log", "--name-status", "-c", diff --git a/knotserver/git/cmd.go b/knotserver/git/cmd.go index 21eabe1c..41d35f86 100644 --- a/knotserver/git/cmd.go +++ b/knotserver/git/cmd.go @@ -3,7 +3,8 @@ package git import ( "fmt" "os/exec" - "syscall" + + "tangled.org/core/gitutil" ) const ( @@ -28,9 +29,7 @@ func (g *GitRepo) runGitCmd(command string, extraArgs ...string) ([]byte, error) cmd.Dir = g.path } - if cmd.SysProcAttr == nil { - cmd.SysProcAttr = &syscall.SysProcAttr{Setpgid: true} - } + gitutil.Contain(cmd) out, err := cmd.Output() if err != nil { diff --git a/knotserver/git/diff.go b/knotserver/git/diff.go index 46f4c0cd..16e9d73e 100644 --- a/knotserver/git/diff.go +++ b/knotserver/git/diff.go @@ -12,6 +12,7 @@ import ( "github.com/bluekeyes/go-gitdiff/gitdiff" "github.com/go-git/go-git/v5/plumbing" "github.com/go-git/go-git/v5/plumbing/object" + "tangled.org/core/gitutil" "tangled.org/core/patchutil" "tangled.org/core/types" ) @@ -137,7 +138,7 @@ func (g *GitRepo) formatSinglePatch(commit plumbing.Hash, extraArgs ...string) ( } args = append(args, extraArgs...) - cmd := exec.Command("git", args...) + cmd := gitutil.Contain(exec.Command("git", args...)) cmd.Stdout = &stdout cmd.Stderr = os.Stderr err := cmd.Run() diff --git a/knotserver/git/fork.go b/knotserver/git/fork.go index 6f78ddd1..72d5bdeb 100644 --- a/knotserver/git/fork.go +++ b/knotserver/git/fork.go @@ -11,6 +11,7 @@ import ( "github.com/go-git/go-git/v5" "github.com/go-git/go-git/v5/config" + "tangled.org/core/gitutil" knotconfig "tangled.org/core/knotserver/config" "tangled.org/core/knotserver/sandbox" ) @@ -41,7 +42,7 @@ func ForkWithSandbox(repoPath, source string, cfg *knotconfig.Config, sb sandbox u = o } - cloneCmd := exec.Command("git", "clone", "--bare", u.String(), repoPath) + cloneCmd := gitutil.Contain(exec.Command("git", "clone", "--bare", u.String(), repoPath)) cloneCmd.Env = append(cloneCmd.Env, "GIT_TERMINAL_PROMPT=0") if err := cloneCmd.Run(); err != nil { return fmt.Errorf("failed to bare clone repository: %w", err) @@ -61,6 +62,7 @@ func ForkWithSandbox(repoPath, source string, cfg *knotconfig.Config, sb sandbox } else { configureCmd.Dir = repoPath } + gitutil.Contain(configureCmd) if err := configureCmd.Run(); err != nil { return fmt.Errorf("failed to configure hidden refs: %w", err) } diff --git a/knotserver/git/last_commit.go b/knotserver/git/last_commit.go index d984bbed..26017a88 100644 --- a/knotserver/git/last_commit.go +++ b/knotserver/git/last_commit.go @@ -15,6 +15,7 @@ import ( "github.com/dgraph-io/ristretto" "github.com/go-git/go-git/v5/plumbing" + "tangled.org/core/gitutil" "tangled.org/core/sets" "tangled.org/core/types" ) @@ -53,7 +54,7 @@ func (g *GitRepo) streamingGitLog(ctx context.Context, extraArgs ...string) (io. args = append(args, g.h.String()) args = append(args, extraArgs...) - cmd := exec.CommandContext(ctx, "git", args...) + cmd := gitutil.Command(ctx, args...) cmd.Dir = g.path stdout, err := cmd.StdoutPipe() diff --git a/knotserver/git/merge.go b/knotserver/git/merge.go index 013dca4a..2d78f866 100644 --- a/knotserver/git/merge.go +++ b/knotserver/git/merge.go @@ -13,6 +13,7 @@ import ( "github.com/dgraph-io/ristretto" "github.com/go-git/go-git/v5" "github.com/go-git/go-git/v5/plumbing" + "tangled.org/core/gitutil" "tangled.org/core/patchutil" "tangled.org/core/types" ) @@ -160,10 +161,15 @@ func (g *GitRepo) applyPatch(patchData, patchFile string, opts MergeOptions) err // wrapCmd optionally sandboxes a command to g.path. wrapCmd := func(cmd *exec.Cmd) (*exec.Cmd, error) { if g.sandbox != nil { - return g.sandbox.Wrap(g.path, cmd) + var err error + cmd, err = g.sandbox.Wrap(g.path, cmd) + if err != nil { + return nil, err + } + } else { + cmd.Dir = g.path } - cmd.Dir = g.path - return cmd, nil + return gitutil.Contain(cmd), nil } // configure default git user before merge @@ -279,6 +285,7 @@ func (g *GitRepo) applySingleMailbox(singlePatch types.FormatPatch) (plumbing.Ha rawCmd.Dir = g.path cmd = rawCmd } + gitutil.Contain(cmd) cmd.Stderr = &stderr head, err := g.r.Head() @@ -456,6 +463,7 @@ func (g *GitRepo) MergeWithOptions(patchData string, targetBranch string, opts M } else { pushCmd.Dir = tmpDir } + gitutil.Contain(pushCmd) if err := pushCmd.Run(); err != nil { return &ErrMerge{ Message: "failed to push changes to bare repository", diff --git a/knotserver/git/service/service.go b/knotserver/git/service/service.go index a26cac6e..738ed158 100644 --- a/knotserver/git/service/service.go +++ b/knotserver/git/service/service.go @@ -9,8 +9,8 @@ import ( "os/exec" "strings" "sync" - "syscall" + "tangled.org/core/gitutil" "tangled.org/core/knotserver/sandbox" ) @@ -37,9 +37,7 @@ func (c *ServiceCommand) RunService(cmd *exec.Cmd) error { cmd.Dir = c.Dir } - if cmd.SysProcAttr == nil { - cmd.SysProcAttr = &syscall.SysProcAttr{Setpgid: true} - } + gitutil.Contain(cmd) var stderr bytes.Buffer cmd.Stderr = &stderr diff --git a/spindle/git/git.go b/spindle/git/git.go deleted file mode 100644 index e5438565..00000000 --- a/spindle/git/git.go +++ /dev/null @@ -1,164 +0,0 @@ -package git - -import ( - "bytes" - "context" - "fmt" - "os" - "os/exec" - "path/filepath" - "strings" - "sync" - "syscall" - "time" - - "github.com/hashicorp/go-version" -) - -// repoLocks serializes git operations per repo directory. Concurrent triggers -// on the same repo (a push landing while a manual run is dispatched, two "Run -// CI" clicks, etc.) resolve to the same path with different revisions; running -// clone/fetch/checkout there in parallel collides on .git/index.lock and can -// corrupt the dir. Locking is keyed by path so unrelated repos don't serialize. -var repoLocks keyedMutex - -type keyedMutex struct { - mu sync.Mutex - m map[string]*sync.Mutex -} - -// lock acquires the mutex for key and returns its unlock func. -func (k *keyedMutex) lock(key string) func() { - k.mu.Lock() - if k.m == nil { - k.m = make(map[string]*sync.Mutex) - } - mu, ok := k.m[key] - if !ok { - mu = &sync.Mutex{} - k.m[key] = mu - } - k.mu.Unlock() - - mu.Lock() - return mu.Unlock -} - -func Version() (*version.Version, error) { - var buf bytes.Buffer - cmd := exec.Command("git", "version") - cmd.Stdout = &buf - cmd.Stderr = os.Stderr - err := cmd.Run() - if err != nil { - return nil, err - } - fields := strings.Fields(buf.String()) - if len(fields) < 3 { - return nil, fmt.Errorf("invalid git version: %s", buf.String()) - } - - // version string is like: "git version 2.29.3" or "git version 2.29.3.windows.1" - versionString := fields[2] - if pos := strings.Index(versionString, "windows"); pos >= 1 { - versionString = versionString[:pos-1] - } - return version.NewVersion(versionString) -} - -const WorkflowDir = `/.tangled/workflows` - -func runGit(ctx context.Context, args ...string) error { - var stderr bytes.Buffer - cmd := exec.CommandContext(ctx, "git", args...) - cmd.SysProcAttr = &syscall.SysProcAttr{Setpgid: true} - cmd.WaitDelay = 10 * time.Second - cmd.Cancel = func() error { - if cmd.Process == nil { - return nil - } - err := syscall.Kill(-cmd.Process.Pid, syscall.SIGKILL) - if err == syscall.ESRCH { - return nil - } - return err - } - cmd.Stderr = &stderr - if err := cmd.Run(); err != nil { - return fmt.Errorf("%w: %s", err, strings.TrimSpace(stderr.String())) - } - return nil -} - -func SparseSyncGitRepo(ctx context.Context, cloneUri, path, rev string) error { - defer repoLocks.lock(path)() - - exist, err := isDir(path) - if err != nil { - return err - } - if exist { - gitDirExist, err := isDir(path + "/.git") - if err != nil { - return err - } - if !gitDirExist { - if err := os.RemoveAll(path); err != nil { - return fmt.Errorf("cleanup invalid git dir: %w", err) - } - exist = false - } - } - if rev == "" { - rev = "HEAD" - } - if !exist { - if err := runGit(ctx, "clone", "--no-checkout", "--depth=1", "--filter=tree:0", "--revision="+rev, cloneUri, path); err != nil { - return fmt.Errorf("git clone: %w", err) - } - if err := runGit(ctx, "-C", path, "sparse-checkout", "set", "--no-cone", WorkflowDir); err != nil { - return fmt.Errorf("git sparse-checkout set: %w", err) - } - } else { - if err := runGit(ctx, "-C", path, "fetch", "--depth=1", "--filter=tree:0", "origin", rev); err != nil { - // remove any locks if the repo was left in a mid fetch state - removeStaleLocks(path) - if retryErr := runGit(ctx, "-C", path, "fetch", "--depth=1", "--filter=tree:0", "origin", rev); retryErr != nil { - // if still broken, wipe and refetch - if rmErr := os.RemoveAll(path); rmErr != nil { - return fmt.Errorf("git fetch: %w (cleanup failed: %v)", retryErr, rmErr) - } - if cloneErr := runGit(ctx, "clone", "--no-checkout", "--depth=1", "--filter=tree:0", "--revision="+rev, cloneUri, path); cloneErr != nil { - return fmt.Errorf("git fetch: %w (re-clone failed: %v)", retryErr, cloneErr) - } - if cloneErr := runGit(ctx, "-C", path, "sparse-checkout", "set", "--no-cone", WorkflowDir); cloneErr != nil { - return fmt.Errorf("git sparse-checkout set: %w", cloneErr) - } - } - } - } - if err := runGit(ctx, "-C", path, "checkout", rev); err != nil { - return fmt.Errorf("git checkout: %w", err) - } - return nil -} - -func removeStaleLocks(path string) { - // removes shallow.lock, index.lock, etc., all are stale locks - // worst case scenario we fall through to wipe and refetch anyway - locks, _ := filepath.Glob(filepath.Join(path, ".git", "*.lock")) - for _, lock := range locks { - os.Remove(lock) - } -} - -func isDir(path string) (bool, error) { - info, err := os.Stat(path) - if err == nil && info.IsDir() { - return true, nil - } - if os.IsNotExist(err) { - return false, nil - } - return false, err -} diff --git a/spindle/server.go b/spindle/server.go index 1a8a2355..509e6451 100644 --- a/spindle/server.go +++ b/spindle/server.go @@ -26,6 +26,7 @@ import ( "tangled.org/core/eventconsumer" "tangled.org/core/eventconsumer/cursor" "tangled.org/core/eventstream" + "tangled.org/core/gitutil" "tangled.org/core/idresolver" "tangled.org/core/jetstream" knotdb "tangled.org/core/knotserver/db" @@ -41,7 +42,6 @@ import ( "tangled.org/core/spindle/engine" "tangled.org/core/spindle/engines/dummy" "tangled.org/core/spindle/engines/nixery" - "tangled.org/core/spindle/git" "tangled.org/core/spindle/mill" "tangled.org/core/spindle/mill/executor" "tangled.org/core/spindle/models" @@ -57,6 +57,9 @@ var defaultMotd []byte const ( rbacDomain = "thisserver" + + // sparse-checkout wants a leading slash to anchor the pattern at the root + sparseWorkflowDir = "/" + workflow.WorkflowDir ) type Spindle struct { @@ -539,7 +542,7 @@ func (s *Spindle) processKnotStream(ctx context.Context, src eventconsumer.Sourc // NOTE: we are blindly trusting the knot that it will return only repos it own repoCloneUri := s.newRepoCloneUrl(src.Host, repoDid) repoPath := s.newRepoPath(repoDid) - if err := git.SparseSyncGitRepo(ctx, repoCloneUri, repoPath, event.NewSha); err != nil { + if err := gitutil.SparseSync(ctx, repoCloneUri, repoPath, event.NewSha, sparseWorkflowDir); err != nil { return fmt.Errorf("sync git repo: %w", err) } l.Info("synced git repo") @@ -883,7 +886,7 @@ func fingerprintWorkflowDefinition(rawPipeline workflow.RawPipeline) string { } func (s *Spindle) loadPipeline(ctx context.Context, repoUri, repoPath, rev string) (workflow.RawPipeline, error) { - if err := git.SparseSyncGitRepo(ctx, repoUri, repoPath, rev); err != nil { + if err := gitutil.SparseSync(ctx, repoUri, repoPath, rev, sparseWorkflowDir); err != nil { return nil, fmt.Errorf("syncing git repo: %w", err) } gr, err := kgit.Open(repoPath, rev) @@ -937,7 +940,7 @@ func (s *Spindle) newRepoCloneUrl(knot string, did syntax.DID) string { const RequiredVersion = "2.49.0" func ensureGitVersion() error { - v, err := git.Version() + v, err := gitutil.Version() if err != nil { return fmt.Errorf("fetching git version: %w", err) } diff --git a/spindle/tapclient.go b/spindle/tapclient.go index bfe9d90f..ea966b29 100644 --- a/spindle/tapclient.go +++ b/spindle/tapclient.go @@ -19,10 +19,10 @@ import ( "tangled.org/core/api/tangled" avmodels "tangled.org/core/appview/models" "tangled.org/core/eventconsumer" + "tangled.org/core/gitutil" "tangled.org/core/log" "tangled.org/core/rbac" "tangled.org/core/spindle/db" - "tangled.org/core/spindle/git" "tangled.org/core/spindle/models" "tangled.org/core/spindle/netguard" "tangled.org/core/tapc" @@ -186,7 +186,7 @@ func (t *Tap) processRepo(ctx context.Context, evt *tapc.RecordEventData) error // setup sparse sync repoCloneUri := t.spindle.newRepoCloneUrl(repo.Knot, repo.RepoDid) repoPath := t.spindle.newRepoPath(repo.RepoDid) - if err := git.SparseSyncGitRepo(ctx, repoCloneUri, repoPath, ""); err != nil { + if err := gitutil.SparseSync(ctx, repoCloneUri, repoPath, "", sparseWorkflowDir); err != nil { return fmt.Errorf("setting up sparse-clone git repo: %w", err) } -- 2.51.2