From 30162a297a79e944d756ce9b6c2c76cccfba9e39 Mon Sep 17 00:00:00 2001 From: Fabio Bonelli Date: Tue, 10 Nov 2026 08:27:26 +0000 Subject: [PATCH 1/5] fix: bound vitality repository clones --- git/clone_repository.go | 17 +++++++-- git/repo_activity_test.go | 73 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 88 insertions(+), 2 deletions(-) diff --git a/git/clone_repository.go b/git/clone_repository.go index 13b0e5a..d01c947 100644 --- a/git/clone_repository.go +++ b/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,8 +54,15 @@ func CloneRepository(hostname, name, gitURL, index string) error { args = append(args, gitURL, tmpDir) - cmd := exec.CommandContext(context.Background(), "git", args...) + ctx, cancel := context.WithTimeout(context.Background(), cloneTimeout) + defer cancel() + + cmd := exec.CommandContext(ctx, "git", args...) if out, err := cmd.CombinedOutput(); err != 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) } @@ -62,8 +71,12 @@ func CloneRepository(hostname, name, gitURL, index string) error { return fmt.Errorf("cannot remove incomplete clone of %s: %w", gitURL, err) } - cmd = exec.CommandContext(context.Background(), "git", "clone", "--filter=blob:none", "--bare", gitURL, tmpDir) + cmd = exec.CommandContext(ctx, "git", "clone", "--filter=blob:none", "--bare", gitURL, tmpDir) if out, err := cmd.CombinedOutput(); err != 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) } } diff --git a/git/repo_activity_test.go b/git/repo_activity_test.go index f7b3e7a..1317f19 100644 --- a/git/repo_activity_test.go +++ b/git/repo_activity_test.go @@ -1,10 +1,17 @@ package git import ( + "context" + "os" + "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 +24,69 @@ 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) + + 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 }) + + 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.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 + } + + 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 { + for _, arg := range os.Args { + if strings.HasPrefix(arg, "--shallow-since=") { + require.NoError(t, os.WriteFile(callsPath, []byte("1"), 0o600)) + os.Exit(128) + } + } + + os.Exit(1) + } + + require.NoError(t, os.WriteFile(callsPath, []byte("2"), 0o600)) + select {} +} + +func mustReadFile(t *testing.T, path string) []byte { + t.Helper() + + data, err := os.ReadFile(path) + require.NoError(t, err) + + return data +} From 7ef7a2745ad28003b02592fbcde2d3c8cc4bd5ad Mon Sep 17 00:00:00 2001 From: Fabio Bonelli Date: Tue, 10 Nov 2026 08:40:58 +0000 Subject: [PATCH 2/5] fix: stop clone wait after timeout --- git/clone_repository.go | 2 ++ git/repo_activity_test.go | 10 ++++++++++ 2 files changed, 12 insertions(+) diff --git a/git/clone_repository.go b/git/clone_repository.go index d01c947..684da61 100644 --- a/git/clone_repository.go +++ b/git/clone_repository.go @@ -58,6 +58,7 @@ func CloneRepository(hostname, name, gitURL, index string) error { defer cancel() cmd := exec.CommandContext(ctx, "git", args...) + cmd.WaitDelay = time.Second if out, err := cmd.CombinedOutput(); err != nil { if ctx.Err() != nil { return fmt.Errorf("clone %s timed out: %w", gitURL, ctx.Err()) @@ -72,6 +73,7 @@ func CloneRepository(hostname, name, gitURL, index string) error { } cmd = exec.CommandContext(ctx, "git", "clone", "--filter=blob:none", "--bare", gitURL, tmpDir) + cmd.WaitDelay = time.Second if out, err := cmd.CombinedOutput(); err != nil { if ctx.Err() != nil { return fmt.Errorf("clone %s timed out: %w", gitURL, ctx.Err()) diff --git a/git/repo_activity_test.go b/git/repo_activity_test.go index 1317f19..28883d4 100644 --- a/git/repo_activity_test.go +++ b/git/repo_activity_test.go @@ -3,6 +3,7 @@ package git import ( "context" "os" + "os/exec" "path/filepath" "strings" "testing" @@ -60,6 +61,10 @@ 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(2 * time.Second) + return + } callsPath := os.Getenv("GO_CLONE_HELPER_CALLS") calls, err := os.ReadFile(callsPath) @@ -79,6 +84,11 @@ func TestCloneRepositoryHelperProcess(t *testing.T) { } require.NoError(t, os.WriteFile(callsPath, []byte("2"), 0o600)) + 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 {} } From f5fad07d9679f0f8d1c389236f0ce5d6f5f3181e Mon Sep 17 00:00:00 2001 From: Fabio Bonelli Date: Tue, 10 Nov 2026 08:46:54 +0000 Subject: [PATCH 3/5] test: verify clone timeout wait limit --- git/clone_repository.go | 2 ++ git/repo_activity_test.go | 5 ++++- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/git/clone_repository.go b/git/clone_repository.go index 684da61..084a526 100644 --- a/git/clone_repository.go +++ b/git/clone_repository.go @@ -59,6 +59,7 @@ func CloneRepository(hostname, name, gitURL, index string) error { cmd := exec.CommandContext(ctx, "git", args...) cmd.WaitDelay = time.Second + if out, err := cmd.CombinedOutput(); err != nil { if ctx.Err() != nil { return fmt.Errorf("clone %s timed out: %w", gitURL, ctx.Err()) @@ -74,6 +75,7 @@ func CloneRepository(hostname, name, gitURL, index string) error { cmd = exec.CommandContext(ctx, "git", "clone", "--filter=blob:none", "--bare", gitURL, tmpDir) cmd.WaitDelay = time.Second + if out, err := cmd.CombinedOutput(); err != nil { if ctx.Err() != nil { return fmt.Errorf("clone %s timed out: %w", gitURL, ctx.Err()) diff --git a/git/repo_activity_test.go b/git/repo_activity_test.go index 28883d4..e1a1418 100644 --- a/git/repo_activity_test.go +++ b/git/repo_activity_test.go @@ -49,10 +49,13 @@ func TestCloneRepositoryTimeout(t *testing.T) { 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))) } @@ -62,7 +65,7 @@ func TestCloneRepositoryHelperProcess(t *testing.T) { return } if os.Getenv("GO_WANT_CLONE_HELPER_CHILD") == "1" { - time.Sleep(2 * time.Second) + time.Sleep(5 * time.Second) return } From ea95e70a2158b7726c8f5e09e6f0ddd805b1ba0e Mon Sep 17 00:00:00 2001 From: Fabio Bonelli Date: Tue, 10 Nov 2026 08:55:02 +0000 Subject: [PATCH 4/5] test: cover clone timeout waits --- git/repo_activity_test.go | 45 ++++++++++++++++++++++++++++++++++++++- 1 file changed, 44 insertions(+), 1 deletion(-) diff --git a/git/repo_activity_test.go b/git/repo_activity_test.go index e1a1418..96389be 100644 --- a/git/repo_activity_test.go +++ b/git/repo_activity_test.go @@ -36,6 +36,35 @@ func TestCloneRepositoryTimeout(t *testing.T) { 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) @@ -76,9 +105,13 @@ func TestCloneRepositoryHelperProcess(t *testing.T) { } 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=") { - require.NoError(t, os.WriteFile(callsPath, []byte("1"), 0o600)) os.Exit(128) } } @@ -87,6 +120,16 @@ func TestCloneRepositoryHelperProcess(t *testing.T) { } 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 From e118185796f7054f3fb1757d79f76c82de12b018 Mon Sep 17 00:00:00 2001 From: Fabio Bonelli Date: Tue, 10 Nov 2026 08:57:13 +0000 Subject: [PATCH 5/5] refactor: extract clone fallback handling --- git/clone_repository.go | 67 +++++++++++++++++++++++++---------------- 1 file changed, 41 insertions(+), 26 deletions(-) diff --git a/git/clone_repository.go b/git/clone_repository.go index 084a526..0f6a081 100644 --- a/git/clone_repository.go +++ b/git/clone_repository.go @@ -57,32 +57,8 @@ func CloneRepository(hostname, name, gitURL, index string) error { ctx, cancel := context.WithTimeout(context.Background(), cloneTimeout) defer cancel() - cmd := exec.CommandContext(ctx, "git", args...) - cmd.WaitDelay = time.Second - - if out, err := cmd.CombinedOutput(); err != 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) - } - - cmd = exec.CommandContext(ctx, "git", "clone", "--filter=blob:none", "--bare", gitURL, tmpDir) - cmd.WaitDelay = time.Second - - if out, err := cmd.CombinedOutput(); err != 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) - } + if err := cloneWithFallback(ctx, args, existing, gitURL, tmpDir); err != nil { + return err } repository, err := gogit.PlainOpen(tmpDir) @@ -109,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 {