fix: add a timeout to vitality repository clones

git/clone_repository.go

@@ -21,6 +21,8 @@ import (
 	"github.com/spf13/viper"
 )
 
+var cloneTimeout = 30 * time.Second
+
 func vitalityCachePath(hostname, vendor, repo string) string {
 	return filepath.Join(viper.GetString("DATADIR"), "repos", hostname, vendor, repo, "vitality.json")
 }
@@ -52,20 +54,11 @@ func CloneRepository(hostname, name, gitURL, index string) error {
 
 	args = append(args, gitURL, tmpDir)
 
-	cmd := exec.CommandContext(context.Background(), "git", args...)
-	if out, err := cmd.CombinedOutput(); err != nil {
-		if existing == nil {
-			return fmt.Errorf("cannot clone %s: %w: %s", gitURL, err, out)
-		}
-
-		if err := os.RemoveAll(tmpDir); err != nil {
-			return fmt.Errorf("cannot remove incomplete clone of %s: %w", gitURL, err)
-		}
+	ctx, cancel := context.WithTimeout(context.Background(), cloneTimeout)
+	defer cancel()
 
-		cmd = exec.CommandContext(context.Background(), "git", "clone", "--filter=blob:none", "--bare", gitURL, tmpDir)
-		if out, err := cmd.CombinedOutput(); err != nil {
-			return fmt.Errorf("cannot clone %s: %w: %s", gitURL, err, out)
-		}
+	if err := cloneWithFallback(ctx, args, existing, gitURL, tmpDir); err != nil {
+		return err
 	}
 
 	repository, err := gogit.PlainOpen(tmpDir)
@@ -92,6 +85,45 @@ func CloneRepository(hostname, name, gitURL, index string) error {
 	return os.WriteFile(cachePath, data, 0o600)
 }
 
+func cloneWithFallback(
+	ctx context.Context, args []string, existing *vitality.Cache, gitURL, tmpDir string,
+) error {
+	out, err := runClone(ctx, args...)
+	if err == nil {
+		return nil
+	}
+
+	if ctx.Err() != nil {
+		return fmt.Errorf("clone %s timed out: %w", gitURL, ctx.Err())
+	}
+
+	if existing == nil {
+		return fmt.Errorf("cannot clone %s: %w: %s", gitURL, err, out)
+	}
+
+	if err := os.RemoveAll(tmpDir); err != nil {
+		return fmt.Errorf("cannot remove incomplete clone of %s: %w", gitURL, err)
+	}
+
+	out, err = runClone(ctx, "clone", "--filter=blob:none", "--bare", gitURL, tmpDir)
+	if err == nil {
+		return nil
+	}
+
+	if ctx.Err() != nil {
+		return fmt.Errorf("clone %s timed out: %w", gitURL, ctx.Err())
+	}
+
+	return fmt.Errorf("cannot clone %s: %w: %s", gitURL, err, out)
+}
+
+func runClone(ctx context.Context, args ...string) ([]byte, error) {
+	cmd := exec.CommandContext(ctx, "git", args...)
+	cmd.WaitDelay = time.Second
+
+	return cmd.CombinedOutput()
+}
+
 func loadExistingCache(path string) *vitality.Cache {
 	data, err := os.ReadFile(path)
 	if err != nil {

git/repo_activity_test.go

@@ -1,10 +1,18 @@
 package git
 
 import (
+	"context"
+	"os"
+	"os/exec"
+	"path/filepath"
+	"strings"
 	"testing"
 	"time"
 
+	"github.com/italia/publiccode-crawler/v4/git/vitality"
+	"github.com/spf13/viper"
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 )
 
 // fixedNow is a fixed point in time used to make all tests deterministic.
@@ -17,3 +25,124 @@ func TestMeanActivity(t *testing.T) {
 func TestMeanActivitySingle(t *testing.T) {
 	assert.Equal(t, float64(42), meanActivity(map[int]float64{0: 42}))
 }
+
+func TestCloneRepositoryTimeout(t *testing.T) {
+	dir := t.TempDir()
+	gitPath := filepath.Join(dir, "git")
+	callsPath := filepath.Join(dir, "calls")
+	script := "#!/bin/sh\nexec \"$GO_TEST_BINARY\" -test.run='^TestCloneRepositoryHelperProcess$' -- \"$@\"\n"
+	require.NoError(t, os.WriteFile(gitPath, []byte(script), 0o755))
+	t.Setenv("PATH", dir)
+	t.Setenv("GO_TEST_BINARY", os.Args[0])
+	t.Setenv("GO_WANT_CLONE_HELPER", "1")
+	t.Setenv("GO_CLONE_HELPER_CALLS", callsPath)
+	t.Setenv("GO_CLONE_HELPER_HANG_ON", "first")
+	viper.Set("DATADIR", t.TempDir())
+
+	previousTimeout := cloneTimeout
+	cloneTimeout = 100 * time.Millisecond
+	t.Cleanup(func() { cloneTimeout = previousTimeout })
+
+	started := time.Now()
+	err := CloneRepository("example.com", "owner/repository", "https://example.com/repository.git", "test")
+
+	require.Error(t, err)
+	assert.ErrorIs(t, err, context.DeadlineExceeded)
+	assert.ErrorContains(t, err, "https://example.com/repository.git")
+	assert.Less(t, time.Since(started), 3*time.Second)
+	assert.FileExists(t, callsPath)
+	assert.Equal(t, "1", string(mustReadFile(t, callsPath)))
+}
+
+func TestCloneRepositoryFallbackTimeout(t *testing.T) {
+	dir := t.TempDir()
+	gitPath := filepath.Join(dir, "git")
+	callsPath := filepath.Join(dir, "calls")
+	script := "#!/bin/sh\nexec \"$GO_TEST_BINARY\" -test.run='^TestCloneRepositoryHelperProcess$' -- \"$@\"\n"
+	require.NoError(t, os.WriteFile(gitPath, []byte(script), 0o755))
+	t.Setenv("PATH", dir)
+	t.Setenv("GO_TEST_BINARY", os.Args[0])
+	t.Setenv("GO_WANT_CLONE_HELPER", "1")
+	t.Setenv("GO_CLONE_HELPER_CALLS", callsPath)
+	t.Setenv("GO_CLONE_HELPER_HANG_ON", "fallback")
+
+	dataDir := t.TempDir()
+	viper.Set("DATADIR", dataDir)
+	cache, err := vitality.Marshal(vitality.Cache{LastUpdated: time.Now()})
+	require.NoError(t, err)
+	cachePath := vitalityCachePath("example.com", "owner", "repository")
+	require.NoError(t, os.MkdirAll(filepath.Dir(cachePath), 0o755))
+	require.NoError(t, os.WriteFile(cachePath, cache, 0o600))
+
+	previousTimeout := cloneTimeout
+	cloneTimeout = 100 * time.Millisecond
+	t.Cleanup(func() { cloneTimeout = previousTimeout })
+
+	started := time.Now()
+	err = CloneRepository("example.com", "owner/repository", "https://example.com/repository.git", "test")
+
+	require.Error(t, err)
+	assert.ErrorIs(t, err, context.DeadlineExceeded)
+	assert.ErrorContains(t, err, "https://example.com/repository.git")
+	assert.Less(t, time.Since(started), 3*time.Second)
+	assert.FileExists(t, callsPath)
+	assert.Equal(t, "2", string(mustReadFile(t, callsPath)))
+}
+
+func TestCloneRepositoryHelperProcess(t *testing.T) {
+	if os.Getenv("GO_WANT_CLONE_HELPER") != "1" {
+		return
+	}
+	if os.Getenv("GO_WANT_CLONE_HELPER_CHILD") == "1" {
+		time.Sleep(5 * time.Second)
+		return
+	}
+
+	callsPath := os.Getenv("GO_CLONE_HELPER_CALLS")
+	calls, err := os.ReadFile(callsPath)
+	if err != nil && !os.IsNotExist(err) {
+		os.Exit(1)
+	}
+
+	if len(calls) == 0 {
+		require.NoError(t, os.WriteFile(callsPath, []byte("1"), 0o600))
+		if os.Getenv("GO_CLONE_HELPER_HANG_ON") == "first" {
+			hangCloneHelper(t)
+		}
+
+		for _, arg := range os.Args {
+			if strings.HasPrefix(arg, "--shallow-since=") {
+				os.Exit(128)
+			}
+		}
+
+		os.Exit(1)
+	}
+
+	require.NoError(t, os.WriteFile(callsPath, []byte("2"), 0o600))
+	if os.Getenv("GO_CLONE_HELPER_HANG_ON") == "fallback" {
+		hangCloneHelper(t)
+	}
+
+	os.Exit(1)
+}
+
+func hangCloneHelper(t *testing.T) {
+	t.Helper()
+
+	child := exec.Command(os.Args[0], "-test.run=^TestCloneRepositoryHelperProcess$", "--")
+	child.Env = append(os.Environ(), "GO_WANT_CLONE_HELPER_CHILD=1")
+	child.Stdout = os.Stdout
+	child.Stderr = os.Stderr
+	require.NoError(t, child.Start())
+	select {}
+}
+
+func mustReadFile(t *testing.T, path string) []byte {
+	t.Helper()
+
+	data, err := os.ReadFile(path)
+	require.NoError(t, err)
+
+	return data
+}