fix: improve git sync

.gitignore

@@ -9,7 +9,4 @@ TODO
 .air.toml
 build-errors.log
 site/
-tests/e2e/venv/
-tests/e2e/__pycache__/
-tests/e2e/.pytest_cache/
-tests/e2e/watch.sh
+coverage.out

.mockery.yaml

@@ -0,0 +1,13 @@
+keeptree: False
+with-expecter: True
+inpackage: True
+resolve-type-alias: False
+issue-845-fix: True
+dir: "{{.InterfaceDirRelative}}"
+outpkg: "{{.PackageName}}"
+filename: "mock_{{.InterfaceName}}.go"
+
+packages:
+  github.com/pilat/devbox/internal/git:
+    interfaces:
+      CommandRunner:

Makefile

@@ -1,7 +1,10 @@
-.PHONY: all tidy vendor lint test test-e2e
+.PHONY: all tidy vendor mocks lint test test-e2e
 
 all: tidy vendor lint test
 
+mocks:
+	mockery
+
 tidy:
 	go mod tidy
 

cmd/devbox/update.go

@@ -117,34 +117,70 @@ func runSourcesUpdate(ctx context.Context, p *project.Project) error {
 		for name := range p.Sources {
 			cw.Event(progress.Event{
 				ID:         "Source " + name,
-				StatusText: "Syncing",
+				StatusText: "Pending",
 			})
 		}
 
+		// Create a cancellable context for fail-fast behavior
+		ctx, cancelSync := context.WithCancel(ctx)
+		defer cancelSync()
+
+		// Semaphore to limit concurrent goroutines to 4
+		const maxConcurrency = 4
+		sem := make(chan struct{}, maxConcurrency)
+
 		var errCh = make(chan error, len(p.Sources))
 		for name, src := range p.Sources {
 			go func(name string, src project.SourceConfig) {
-				repoDir := filepath.Join(p.WorkingDir, app.SourcesDir, name)
-
-				git := git.New(repoDir)
-				err := git.Sync(ctx, src.URL, src.Branch, src.SparseCheckout)
+				// Acquire semaphore
+				select {
+				case sem <- struct{}{}:
+					defer func() { <-sem }()
+				case <-ctx.Done():
+					errCh <- ctx.Err()
+					return
+				}
 
 				cw.Event(progress.Event{
 					ID:         "Source " + name,
-					StatusText: "Synced",
-					Status:     progress.Done,
+					StatusText: "Syncing",
 				})
 
+				repoDir := filepath.Join(p.WorkingDir, app.SourcesDir, name)
+
+				g := git.New(repoDir)
+				err := g.Sync(ctx, src.URL, src.Branch, src.SparseCheckout)
+
+				if err != nil {
+					cw.Event(progress.Event{
+						ID:         "Source " + name,
+						StatusText: "Failed",
+						Status:     progress.Error,
+					})
+					cancelSync() // Fail-fast: cancel all other syncs
+				} else {
+					cw.Event(progress.Event{
+						ID:         "Source " + name,
+						StatusText: "Synced",
+						Status:     progress.Done,
+					})
+				}
+
 				errCh <- err
 			}(name, src)
 		}
 
+		var firstErr error
 		for i := 0; i < len(p.Sources); i++ {
-			if err := <-errCh; err != nil {
-				return fmt.Errorf("failed to sync source: %w", err)
+			if err := <-errCh; err != nil && firstErr == nil {
+				firstErr = err
 			}
 		}
 
+		if firstErr != nil {
+			return fmt.Errorf("failed to sync source: %w", firstErr)
+		}
+
 		return nil
 	}
 

go.mod

@@ -137,6 +137,7 @@ require (
 	github.com/sirupsen/logrus v1.9.4-0.20230606125235-dd1b4c2e81af // indirect
 	github.com/skratchdot/open-golang v0.0.0-20200116055534-eef842397966 // indirect
 	github.com/spf13/pflag v1.0.5 // indirect
+	github.com/stretchr/objx v0.5.2 // indirect
 	github.com/theupdateframework/notary v0.7.0 // indirect
 	github.com/tilt-dev/fsnotify v1.4.8-0.20220602155310-fff9c274a375 // indirect
 	github.com/tonistiigi/dchapes-mode v0.0.0-20241001053921-ca0759fec205 // indirect

go.sum

@@ -444,6 +444,8 @@ github.com/stretchr/objx v0.1.1/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+
 github.com/stretchr/objx v0.2.0/go.mod h1:qt09Ya8vawLte6SNmTgCsAVtYtaKzEcn8ATUoHMkEqE=
 github.com/stretchr/objx v0.4.0/go.mod h1:YvHI0jy2hoMjB+UWwv71VJQ9isScKT/TqJzVSSt89Yw=
 github.com/stretchr/objx v0.5.0/go.mod h1:Yh+to48EsGEfYuaHDzXPcE3xhTkx73EhmCGUpEOglKo=
+github.com/stretchr/objx v0.5.2 h1:xuMeJ0Sdp5ZMRXx/aWO6RZxdr3beISkG5/G/aIRr3pY=
+github.com/stretchr/objx v0.5.2/go.mod h1:FRsXN1f5AsAjCGJKqEizvkpNtU+EGNCLh3NxZ/8L+MA=
 github.com/stretchr/testify v1.2.2/go.mod h1:a8OnRcib4nhh0OaRAV+Yts87kKdq0PP7pXfy6kDkUVs=
 github.com/stretchr/testify v1.3.0/go.mod h1:M5WIy9Dh21IEIfnGCwXGc5bZfKNJtfHm1UVUgZn+9EI=
 github.com/stretchr/testify v1.4.0/go.mod h1:j7eGeouHqKxXV5pUuKE4zz7dFj8WfuZ+81PSLYec5m4=

internal/git/exec.go

@@ -0,0 +1,29 @@
+package git
+
+import (
+	"context"
+	"os"
+	"os/exec"
+)
+
+// CommandRunner executes shell commands.
+type CommandRunner interface {
+	Run(ctx context.Context, name string, args ...string) (string, error)
+	RunWithTTY(ctx context.Context, name string, args ...string) (string, error)
+}
+
+type defaultRunner struct{}
+
+func (r *defaultRunner) Run(ctx context.Context, name string, args ...string) (string, error) {
+	cmd := exec.CommandContext(ctx, name, args...)
+	cmd.Env = append(os.Environ(), "GIT_TERMINAL_PROMPT=0")
+	out, err := cmd.CombinedOutput()
+	return string(out), err
+}
+
+func (r *defaultRunner) RunWithTTY(ctx context.Context, name string, args ...string) (string, error) {
+	cmd := exec.CommandContext(ctx, name, args...)
+	cmd.Stdin = os.Stdin
+	out, err := cmd.CombinedOutput()
+	return string(out), err
+}

internal/git/git.go

@@ -4,30 +4,31 @@ import (
 	"context"
 	"fmt"
 	"os"
-	"os/exec"
 	"path/filepath"
 	"strings"
 )
 
 type svc struct {
 	targetPath string
+	runner     CommandRunner
 }
 
 func New(targetFolder string) *svc {
 	return &svc{
 		targetPath: targetFolder,
+		runner:     &defaultRunner{},
 	}
 }
 
 func (s *svc) Clone(ctx context.Context, url, branch string) error {
-	cmds := []string{"clone", url, s.targetPath}
+	args := []string{"clone", url, s.targetPath}
 	if branch != "" {
-		cmds = append(cmds, "--branch", branch)
+		args = append(args, "--branch", branch)
 	}
 
-	out, err := s.exec(ctx, "git", cmds...)
+	out, err := s.runner.RunWithTTY(ctx, "git", args...)
 	if err != nil {
-		return fmt.Errorf("failed to clone: %s %w", out, err)
+		return fmt.Errorf("failed to clone: %s\n%s\n%w", out, gitConfigHint(url), err)
 	}
 
 	return nil
@@ -52,7 +53,7 @@ func (s *svc) SetLocalExclude(patterns []string) error {
 }
 
 func (s *svc) Sync(ctx context.Context, url, branch string, sparseCheckout []string) error {
-	// if there is no `.git` directory we should not to try to reset because it will try to lock a repo above
+	// if there is no `.git` directory we should not try to reset because it will try to lock a repo above
 	if _, err := os.Stat(filepath.Join(s.targetPath, ".git")); os.IsNotExist(err) {
 		_ = os.RemoveAll(s.targetPath)
 	}
@@ -68,30 +69,30 @@ func (s *svc) Sync(ctx context.Context, url, branch string, sparseCheckout []str
 		}
 	} else {
 		_ = os.MkdirAll(s.targetPath, os.ModePerm)
-		out, err := s.exec(ctx, "git", "clone", "--no-checkout", "--depth", "1", url, s.targetPath)
+		out, err := s.runner.RunWithTTY(ctx, "git", "clone", "--no-checkout", "--depth", "1", url, s.targetPath)
 		if err != nil {
-			return fmt.Errorf("failed to clone: %s %w", out, err)
+			return fmt.Errorf("failed to clone: %s\n%s\n%w", out, gitConfigHint(url), err)
 		}
 	}
 
 	if len(sparseCheckout) > 0 {
-		out, err := s.exec(ctx, "git", "-C", s.targetPath, "sparse-checkout", "init", "--cone")
+		out, err := s.runner.Run(ctx, "git", "-C", s.targetPath, "sparse-checkout", "init", "--cone")
 		if err != nil {
 			return fmt.Errorf("failed to init sparse-checkout: %s %w", out, err)
 		}
 
-		out, err = s.exec(ctx, "git", append([]string{"-C", s.targetPath, "sparse-checkout", "set"}, sparseCheckout...)...)
+		out, err = s.runner.Run(ctx, "git", append([]string{"-C", s.targetPath, "sparse-checkout", "set"}, sparseCheckout...)...)
 		if err != nil {
 			return fmt.Errorf("failed to set sparse-checkout: %s %w", out, err)
 		}
 	} else {
-		out, err := s.exec(ctx, "git", "-C", s.targetPath, "sparse-checkout", "disable")
+		out, err := s.runner.Run(ctx, "git", "-C", s.targetPath, "sparse-checkout", "disable")
 		if err != nil {
 			return fmt.Errorf("failed to disable sparse-checkout: %s %w", out, err)
 		}
 	}
 
-	out, err := s.exec(ctx, "git", "-C", s.targetPath, "checkout", branch)
+	out, err := s.runner.Run(ctx, "git", "-C", s.targetPath, "checkout", branch)
 	if err != nil {
 		return fmt.Errorf("failed to checkout: %s %w", out, err)
 	}
@@ -104,7 +105,7 @@ func (s *svc) Pull(ctx context.Context) error {
 		return fmt.Errorf("failed to reset repo: %w", err)
 	}
 
-	out, err := s.exec(ctx, "git", "-C", s.targetPath, "pull", "--rebase")
+	out, err := s.runner.RunWithTTY(ctx, "git", "-C", s.targetPath, "pull", "--rebase")
 	if err != nil {
 		return fmt.Errorf("failed to pull: %s %w", out, err)
 	}
@@ -113,7 +114,7 @@ func (s *svc) Pull(ctx context.Context) error {
 }
 
 func (s *svc) GetInfo(ctx context.Context) (*CommitInfo, error) {
-	out, err := s.exec(ctx, "git", "-C", s.targetPath, "log", "-1", "--pretty=format:%H%n%aN%n%ad%n%s")
+	out, err := s.runner.Run(ctx, "git", "-C", s.targetPath, "log", "-1", "--pretty=format:%H%n%aN%n%ad%n%s")
 	if err != nil {
 		return nil, fmt.Errorf("failed to get commit info: %s", out)
 	}
@@ -132,7 +133,7 @@ func (s *svc) GetInfo(ctx context.Context) (*CommitInfo, error) {
 }
 
 func (s *svc) GetRemote(ctx context.Context) (string, error) {
-	out, err := s.exec(ctx, "git", "-C", s.targetPath, "config", "--get", "remote.origin.url")
+	out, err := s.runner.Run(ctx, "git", "-C", s.targetPath, "config", "--get", "remote.origin.url")
 	if err != nil {
 		return "", fmt.Errorf("failed to get remote: %s %w", out, err)
 	}
@@ -141,7 +142,7 @@ func (s *svc) GetRemote(ctx context.Context) (string, error) {
 }
 
 func (s *svc) GetTopLevel(ctx context.Context) (string, error) {
-	out, err := s.exec(ctx, "git", "-C", s.targetPath, "rev-parse", "--show-toplevel")
+	out, err := s.runner.Run(ctx, "git", "-C", s.targetPath, "rev-parse", "--show-toplevel")
 	if err != nil {
 		return "", fmt.Errorf("failed to get top level: %s %w", out, err)
 	}
@@ -152,27 +153,38 @@ func (s *svc) GetTopLevel(ctx context.Context) (string, error) {
 func (s *svc) reset(ctx context.Context) error {
 	_ = os.Remove(filepath.Join(s.targetPath, ".git/index.lock"))
 
-	out, err := s.exec(ctx, "git", "-C", s.targetPath, "reset", "--hard")
+	out, err := s.runner.Run(ctx, "git", "-C", s.targetPath, "reset", "--hard")
 	if err != nil {
 		return fmt.Errorf("failed to reset: %s %w", out, err)
 	}
 
-	out, err = s.exec(ctx, "git", "-C", s.targetPath, "clean", "-fd")
+	out, err = s.runner.Run(ctx, "git", "-C", s.targetPath, "clean", "-fd")
 	if err != nil {
 		return fmt.Errorf("failed to clean: %s %w", out, err)
 	}
 
 	return nil
 }
 
-func (s *svc) exec(ctx context.Context, name string, args ...string) (string, error) {
-	cmd := exec.CommandContext(ctx, name, args...)
-	cmd.Env = append(os.Environ(), "GIT_TERMINAL_PROMPT=0")
+// gitConfigHint returns a helpful message suggesting git URL rewrite configuration.
+func gitConfigHint(u string) string {
+	if strings.HasPrefix(u, "https://") || strings.HasPrefix(u, "http://") {
+		host := strings.TrimPrefix(strings.TrimPrefix(u, "https://"), "http://")
+		if idx := strings.Index(host, "/"); idx > 0 {
+			host = host[:idx]
+		}
+		return fmt.Sprintf(`Tip: If you use SSH keys instead of HTTPS, configure git:
+  git config --global url."git@%s:".insteadOf "https://%s/"`, host, host)
+	}
 
-	out, err := cmd.CombinedOutput()
-	if err != nil {
-		return string(out), err
+	if strings.HasPrefix(u, "git@") {
+		host := strings.TrimPrefix(u, "git@")
+		if idx := strings.Index(host, ":"); idx > 0 {
+			host = host[:idx]
+		}
+		return fmt.Sprintf(`Tip: If you use HTTPS tokens instead of SSH, configure git:
+  git config --global url."https://%s/".insteadOf "git@%s:"`, host, host)
 	}
 
-	return string(out), nil
+	return ""
 }