git: refactor into interface

Convert the standalone functions in 'git.go' into a 'GitHelper' interface.
Like 'CronScheduler' & 'CronHelper', 'GitHelper's use of 'CommandExecutor'
sets it up to be unit testable in the future.

Signed-off-by: Victoria Dye <[email protected]>

Author: vdye

cmd/git-bundle-server/init.go

@@ -43,33 +43,20 @@ func (i *initCmd) Run(ctx context.Context, args []string) error {
 
 	repoProvider := utils.GetDependency[core.RepositoryProvider](ctx, i.container)
 	bundleProvider := utils.GetDependency[bundles.BundleProvider](ctx, i.container)
+	gitHelper := utils.GetDependency[git.GitHelper](ctx, i.container)
 
 	repo, err := repoProvider.CreateRepository(ctx, *route)
 	if err != nil {
 		return i.logger.Error(ctx, err)
 	}
 
 	fmt.Printf("Cloning repository from %s\n", *url)
-	gitErr := git.GitCommand("clone", "--bare", *url, repo.RepoDir)
-
-	if gitErr != nil {
-		return i.logger.Errorf(ctx, "failed to clone repository: %w", gitErr)
-	}
-
-	gitErr = git.GitCommand("-C", repo.RepoDir, "config", "remote.origin.fetch", "+refs/heads/*:refs/heads/*")
-	if gitErr != nil {
-		return i.logger.Errorf(ctx, "failed to configure refspec: %w", gitErr)
-	}
-
-	gitErr = git.GitCommand("-C", repo.RepoDir, "fetch", "origin")
-	if gitErr != nil {
-		return i.logger.Errorf(ctx, "failed to fetch latest refs: %w", gitErr)
-	}
+	gitHelper.CloneBareRepo(ctx, *url, repo.RepoDir)
 
 	bundle := bundleProvider.CreateInitialBundle(ctx, repo)
 	fmt.Printf("Constructing base bundle file at %s\n", bundle.Filename)
 
-	written, gitErr := git.CreateBundle(repo.RepoDir, bundle.Filename)
+	written, gitErr := gitHelper.CreateBundle(ctx, repo.RepoDir, bundle.Filename)
 	if gitErr != nil {
 		return i.logger.Errorf(ctx, "failed to create bundle: %w", gitErr)
 	}

cmd/utils/container-helpers.go

@@ -8,6 +8,7 @@ import (
 	"github.com/github/git-bundle-server/internal/common"
 	"github.com/github/git-bundle-server/internal/core"
 	"github.com/github/git-bundle-server/internal/daemon"
+	"github.com/github/git-bundle-server/internal/git"
 	"github.com/github/git-bundle-server/internal/log"
 )
 
@@ -30,7 +31,10 @@ func BuildGitBundleServerContainer(logger log.TraceLogger) *DependencyContainer
 		)
 	})
 	registerDependency(container, func(ctx context.Context) bundles.BundleProvider {
-		return bundles.NewBundleProvider(logger)
+		return bundles.NewBundleProvider(
+			logger,
+			GetDependency[git.GitHelper](ctx, container),
+		)
 	})
 	registerDependency(container, func(ctx context.Context) core.CronScheduler {
 		return core.NewCronScheduler(
@@ -47,6 +51,12 @@ func BuildGitBundleServerContainer(logger log.TraceLogger) *DependencyContainer
 			GetDependency[core.CronScheduler](ctx, container),
 		)
 	})
+	registerDependency(container, func(ctx context.Context) git.GitHelper {
+		return git.NewGitHelper(
+			logger,
+			GetDependency[cmd.CommandExecutor](ctx, container),
+		)
+	})
 	registerDependency(container, func(ctx context.Context) daemon.DaemonProvider {
 		t, err := daemon.NewDaemonProvider(
 			logger,

internal/bundles/bundles.go

@@ -65,12 +65,17 @@ type BundleProvider interface {
 }
 
 type bundleProvider struct {
-	logger log.TraceLogger
+	logger    log.TraceLogger
+	gitHelper git.GitHelper
 }
 
-func NewBundleProvider(logger log.TraceLogger) BundleProvider {
+func NewBundleProvider(
+	l log.TraceLogger,
+	g git.GitHelper,
+) BundleProvider {
 	return &bundleProvider{
-		logger: logger,
+		logger:    l,
+		gitHelper: g,
 	}
 }
 
@@ -301,7 +306,7 @@ func (b *bundleProvider) CreateIncrementalBundle(ctx context.Context, repo *core
 		return nil, err
 	}
 
-	written, err := git.CreateIncrementalBundle(repo.RepoDir, bundle.Filename, lines)
+	written, err := b.gitHelper.CreateIncrementalBundle(ctx, repo.RepoDir, bundle.Filename, lines)
 	if err != nil {
 		return nil, fmt.Errorf("failed to create incremental bundle: %w", err)
 	}
@@ -366,7 +371,7 @@ func (b *bundleProvider) CollapseList(ctx context.Context, repo *core.Repository
 		URI:           fmt.Sprintf("./base-%d.bundle", maxTimestamp),
 	}
 
-	err := git.CreateBundleFromRefs(repo.RepoDir, bundle.Filename, refs)
+	err := b.gitHelper.CreateBundleFromRefs(ctx, repo.RepoDir, bundle.Filename, refs)
 	if err != nil {
 		return err
 	}

internal/git/git.go

@@ -2,78 +2,73 @@ package git
 
 import (
 	"bytes"
+	"context"
 	"fmt"
 	"os"
-	"os/exec"
 	"strings"
-)
 
-func GetExecCommand(args []string) (*exec.Cmd, error) {
-	git, err := exec.LookPath("git")
-	if err != nil {
-		return nil, fmt.Errorf("failed to find 'git' on the path: %w", err)
-	}
+	"github.com/github/git-bundle-server/internal/cmd"
+	"github.com/github/git-bundle-server/internal/log"
+)
 
-	cmd := exec.Command(git, args...)
-	cmd.Env = append(cmd.Env, "LC_CTYPE=C")
+type GitHelper interface {
+	CreateBundle(ctx context.Context, repoDir string, filename string) (bool, error)
+	CreateBundleFromRefs(ctx context.Context, repoDir string, filename string, refs map[string]string) error
+	CreateIncrementalBundle(ctx context.Context, repoDir string, filename string, prereqs []string) (bool, error)
+	CloneBareRepo(ctx context.Context, url string, destination string) error
+}
 
-	return cmd, nil
+type gitHelper struct {
+	logger  log.TraceLogger
+	cmdExec cmd.CommandExecutor
 }
 
-func GitCommand(args ...string) error {
-	cmd, err := GetExecCommand(args)
-	if err != nil {
-		return err
+func NewGitHelper(l log.TraceLogger, c cmd.CommandExecutor) GitHelper {
+	return &gitHelper{
+		logger:  l,
+		cmdExec: c,
 	}
+}
 
-	cmd.Stderr = os.Stderr
-	cmd.Stdout = os.Stdout
-
-	err = cmd.Start()
-	if err != nil {
-		return fmt.Errorf("git command failed to start: %w", err)
-	}
+func (g *gitHelper) gitCommand(ctx context.Context, args ...string) error {
+	exitCode, err := g.cmdExec.Run(ctx, "git", args,
+		cmd.Stdout(os.Stdout),
+		cmd.Stderr(os.Stderr),
+		cmd.Env([]string{"LC_CTYPE=C"}),
+	)
 
-	err = cmd.Wait()
 	if err != nil {
-		return fmt.Errorf("git command returned a failure: %w", err)
+		return g.logger.Error(ctx, err)
+	} else if exitCode != 0 {
+		return g.logger.Errorf(ctx, "'git' exited with status %d", exitCode)
 	}
 
-	return err
+	return nil
 }
 
-func GitCommandWithStdin(stdinLines []string, args ...string) error {
-	cmd, err := GetExecCommand(args)
-	if err != nil {
-		return err
-	}
-
+func (g *gitHelper) gitCommandWithStdin(ctx context.Context, stdinLines []string, args ...string) error {
 	buffer := bytes.Buffer{}
 	for line := range stdinLines {
 		buffer.Write([]byte(stdinLines[line] + "\n"))
 	}
+	exitCode, err := g.cmdExec.Run(ctx, "git", args,
+		cmd.Stdin(&buffer),
+		cmd.Stdout(os.Stdout),
+		cmd.Stderr(os.Stderr),
+		cmd.Env([]string{"LC_CTYPE=C"}),
+	)
 
-	cmd.Stdin = &buffer
-
-	errorBuffer := bytes.Buffer{}
-	cmd.Stderr = &errorBuffer
-	cmd.Stdout = os.Stdout
-
-	err = cmd.Start()
-	if err != nil {
-		return fmt.Errorf("git command failed to start: %w", err)
-	}
-
-	err = cmd.Wait()
 	if err != nil {
-		return fmt.Errorf("git command returned a failure: %w\nstderr: %s", err, errorBuffer.String())
+		return g.logger.Error(ctx, err)
+	} else if exitCode != 0 {
+		return g.logger.Errorf(ctx, "'git' exited with status %d", exitCode)
 	}
 
-	return err
+	return nil
 }
 
-func CreateBundle(repoDir string, filename string) (bool, error) {
-	err := GitCommand(
+func (g *gitHelper) CreateBundle(ctx context.Context, repoDir string, filename string) (bool, error) {
+	err := g.gitCommand(ctx,
 		"-C", repoDir, "bundle", "create",
 		filename, "--branches")
 	if err != nil {
@@ -86,19 +81,19 @@ func CreateBundle(repoDir string, filename string) (bool, error) {
 	return true, nil
 }
 
-func CreateBundleFromRefs(repoDir string, filename string, refs map[string]string) error {
+func (g *gitHelper) CreateBundleFromRefs(ctx context.Context, repoDir string, filename string, refs map[string]string) error {
 	refNames := []string{}
 
 	for ref, oid := range refs {
-		err := GitCommand("-C", repoDir, "branch", "-f", ref, oid)
+		err := g.gitCommand(ctx, "-C", repoDir, "branch", "-f", ref, oid)
 		if err != nil {
 			return fmt.Errorf("failed to create ref %s: %w", ref, err)
 		}
 
 		refNames = append(refNames, ref)
 	}
 
-	err := GitCommandWithStdin(
+	err := g.gitCommandWithStdin(ctx,
 		refNames,
 		"-C", repoDir, "bundle", "create",
 		filename, "--stdin")
@@ -109,8 +104,8 @@ func CreateBundleFromRefs(repoDir string, filename string, refs map[string]strin
 	return nil
 }
 
-func CreateIncrementalBundle(repoDir string, filename string, prereqs []string) (bool, error) {
-	err := GitCommandWithStdin(
+func (g *gitHelper) CreateIncrementalBundle(ctx context.Context, repoDir string, filename string, prereqs []string) (bool, error) {
+	err := g.gitCommandWithStdin(ctx,
 		prereqs, "-C", repoDir, "bundle", "create",
 		filename, "--stdin", "--branches")
 	if err != nil {
@@ -122,3 +117,23 @@ func CreateIncrementalBundle(repoDir string, filename string, prereqs []string)
 
 	return true, nil
 }
+
+func (g *gitHelper) CloneBareRepo(ctx context.Context, url string, destination string) error {
+	gitErr := g.gitCommand(ctx, "clone", "--bare", url, destination)
+
+	if gitErr != nil {
+		return g.logger.Errorf(ctx, "failed to clone repository: %w", gitErr)
+	}
+
+	gitErr = g.gitCommand(ctx, "-C", destination, "config", "remote.origin.fetch", "+refs/heads/*:refs/heads/*")
+	if gitErr != nil {
+		return g.logger.Errorf(ctx, "failed to configure refspec: %w", gitErr)
+	}
+
+	gitErr = g.gitCommand(ctx, "-C", destination, "fetch", "origin")
+	if gitErr != nil {
+		return g.logger.Errorf(ctx, "failed to fetch latest refs: %w", gitErr)
+	}
+
+	return nil
+}