Set 'gc.autoDetach' to false to be executed 'git gc' in the foregroun…

…d by some git commands (#4761)

* Set 'gc.autoDetach' to false to be executed 'git gc' in the foreground by some git commands

Signed-off-by: Yoshiki Fujikane <ffjlabo@gmail.com>

* Add log for setting gc.autoDetach

Signed-off-by: Yoshiki Fujikane <ffjlabo@gmail.com>

* Rename to setGCAutoDetach

Signed-off-by: Yoshiki Fujikane <ffjlabo@gmail.com>

* Add test for setGCAutoDetach

Signed-off-by: Yoshiki Fujikane <ffjlabo@gmail.com>

* Refactor

Signed-off-by: Yoshiki Fujikane <ffjlabo@gmail.com>

---------

Signed-off-by: Yoshiki Fujikane <ffjlabo@gmail.com>

pkg/git/client.go

@@ -42,12 +42,13 @@ type Client interface {
 }
 
 type client struct {
-	username  string
-	email     string
-	gitPath   string
-	cacheDir  string
-	mu        sync.Mutex
-	repoLocks map[string]*sync.Mutex
+	username     string
+	email        string
+	gcAutoDetach bool // whether to be executed `git gc`in the foreground when some git commands (e.g. merge, commit and so on) are executed.
+	gitPath      string
+	cacheDir     string
+	mu           sync.Mutex
+	repoLocks    map[string]*sync.Mutex
 
 	gitEnvs       []string
 	gitEnvsByRepo map[string][]string
@@ -106,6 +107,7 @@ func NewClient(opts ...Option) (Client, error) {
 	c := &client{
 		username:      defaultUsername,
 		email:         defaultEmail,
+		gcAutoDetach:  true, // Enable this by default. See issue #4760, discussion #4758.
 		gitPath:       gitPath,
 		cacheDir:      cacheDir,
 		repoLocks:     make(map[string]*sync.Mutex),
@@ -187,6 +189,11 @@ func (c *client) Clone(ctx context.Context, repoID, remote, branch, destination
 		args = append(args, "-b", branch)
 	}
 	args = append(args, repoCachePath, destination)
+
+	logger.Info("cloning a repo from cached one in local",
+		zap.String("src", repoCachePath),
+		zap.String("dst", destination),
+	)
 	if out, err := runGitCommand(ctx, c.gitPath, "", c.envsForRepo(remote), args...); err != nil {
 		logger.Error("failed to clone from local",
 			zap.String("out", string(out)),
@@ -204,6 +211,11 @@ func (c *client) Clone(ctx context.Context, repoID, remote, branch, destination
 		}
 	}
 
+	logger.Info("setting gc.autoDetach", zap.Bool("gc.autoDetach", c.gcAutoDetach))
+	if err := r.setGCAutoDetach(ctx, c.gcAutoDetach); err != nil {
+		return nil, fmt.Errorf("failed to set auto detach: %v", err)
+	}
+
 	// Because we did a local cloning so the remote url of origin
 	// is the path to the cache directory.
 	// We do this change to correct it.

pkg/git/repo.go

@@ -21,6 +21,7 @@ import (
 	"os"
 	"os/exec"
 	"path/filepath"
+	"strconv"
 	"strings"
 )
 
@@ -294,6 +295,14 @@ func (r *repo) setRemote(ctx context.Context, remote string) error {
 	return nil
 }
 
+func (r *repo) setGCAutoDetach(ctx context.Context, autoDetach bool) error {
+	out, err := r.runGitCommand(ctx, "config", "gc.autoDetach", strconv.FormatBool(autoDetach))
+	if err != nil {
+		return formatCommandError(err, out)
+	}
+	return nil
+}
+
 func (r *repo) runGitCommand(ctx context.Context, args ...string) ([]byte, error) {
 	cmd := exec.CommandContext(ctx, r.gitPath, args...)
 	cmd.Dir = r.dir

pkg/git/repo_test.go

@@ -17,8 +17,11 @@ package git
 import (
 	"context"
 	"os"
+	"os/exec"
 	"path/filepath"
 	"sort"
+	"strconv"
+	"strings"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
@@ -183,3 +186,59 @@ func TestCommitChanges(t *testing.T) {
 	require.NoError(t, err)
 	assert.Equal(t, string(changes["a/b/c/new.txt"]), string(bytes))
 }
+
+func Test_setGCAutoDetach(t *testing.T) {
+	faker, err := newFaker()
+	require.NoError(t, err)
+	defer faker.clean()
+
+	var (
+		org      = "test-repo-org"
+		repoName = "repo-set-gc-auto-detach"
+		ctx      = context.Background()
+	)
+
+	err = faker.makeRepo(org, repoName)
+	require.NoError(t, err)
+	r := &repo{
+		dir:     faker.repoDir(org, repoName),
+		gitPath: faker.gitPath,
+	}
+
+	getGCAutoDetach := func(ctx context.Context, repo *repo) (bool, error) {
+		cmd := exec.CommandContext(ctx, r.gitPath, "config", "--get", "gc.autoDetach")
+		cmd.Dir = r.dir
+		out, err := cmd.CombinedOutput()
+		if err != nil {
+			return false, err
+		}
+		v, err := strconv.ParseBool(strings.TrimSuffix(string(out), "\n"))
+		if err != nil {
+			return false, err
+		}
+
+		return v, nil
+	}
+
+	// set  as true firstly, and then set as false.
+	// set true
+	err = r.setGCAutoDetach(ctx, true)
+	require.NoError(t, err)
+
+	got, err := getGCAutoDetach(ctx, r)
+	if err != nil {
+		t.Fatal(err)
+	}
+	require.NoError(t, err)
+
+	assert.Equal(t, true, got)
+
+	// set false
+	err = r.setGCAutoDetach(ctx, false)
+	require.NoError(t, err)
+
+	got, err = getGCAutoDetach(ctx, r)
+	require.NoError(t, err)
+
+	assert.Equal(t, false, got)
+}