fix(git): tolerate concurrent repacks in go-git storage (#39536)

Since `transfer.fsckObjects` makes fetches keep a pack, git 2.54+
background maintenance repacks a mirror right after its sync fetch, and
go-git then misses objects mid-repack. Fixes these flakes:

-
https://github.com/go-gitea/gitea/actions/runs/36875305717/job/110419770196
-
https://github.com/go-gitea/gitea/actions/runs/36900439473/job/110498845882

Changes:

- Keep reindexing while the pack set changes instead of retrying once
- List packs only once their `.idx` exists and don't fail the listing on
files removed mid-repack
- Look up large objects again when their file is gone before reading

modules/git/repo_base_gogit.go

@@ -8,9 +8,13 @@ package git
 
 import (
 	"errors"
+	"io"
+	"os"
 	"path/filepath"
 	"slices"
+	"strings"
 
+	"gitea.dev/modules/container"
 	"gitea.dev/modules/git/gitrepo"
 	"gitea.dev/modules/setting"
 
@@ -34,23 +38,96 @@ type Repository struct {
 
 // reindexingStorage reloads the pack index when git added or removed packs after go-git loaded it
 // https://github.com/go-git/go-git/issues/2439 https://github.com/go-git/go-git/issues/1623
+// FIXME: gogit workaround, remove with the gogit build
 type reindexingStorage struct {
 	*filesystem.Storage
 	packs []plumbing.Hash
 }
 
-func (s *reindexingStorage) EncodedObject(t plumbing.ObjectType, h plumbing.Hash) (plumbing.EncodedObject, error) {
-	obj, err := s.Storage.EncodedObject(t, h)
-	if !errors.Is(err, plumbing.ErrObjectNotFound) && !errors.Is(err, dotgit.ErrPackfileNotFound) {
-		return obj, err
+func isRepackError(err error) bool {
+	return errors.Is(err, plumbing.ErrObjectNotFound) || errors.Is(err, dotgit.ErrPackfileNotFound) || errors.Is(err, os.ErrNotExist)
+}
+
+// retry reruns fn while a concurrent repack keeps changing the packs
+func (s *reindexingStorage) retry(fn func() error) error {
+	for {
+		err := fn()
+		if !isRepackError(err) {
+			return err
+		}
+		packs, _ := s.ObjectPacks()
+		if slices.Equal(packs, s.packs) {
+			return err
+		}
+		s.packs = packs
+		s.Reindex()
 	}
-	packs, _ := s.ObjectPacks()
-	if slices.Equal(packs, s.packs) {
-		return obj, err
+}
+
+func (s *reindexingStorage) EncodedObject(t plumbing.ObjectType, h plumbing.Hash) (obj plumbing.EncodedObject, err error) {
+	err = s.retry(func() (err error) {
+		obj, err = s.Storage.EncodedObject(t, h)
+		return err
+	})
+	if err != nil {
+		return nil, err
 	}
-	s.packs = packs
-	s.Reindex()
-	return s.Storage.EncodedObject(t, h)
+	if _, ok := obj.(*plumbing.MemoryObject); ok {
+		return obj, nil
+	}
+	return &lazyObject{EncodedObject: obj, storage: s}, nil
+}
+
+// lazyObject looks up a large object again when its file got removed before Reader reopened it
+// FIXME: gogit workaround, remove with the gogit build
+type lazyObject struct {
+	plumbing.EncodedObject
+	storage *reindexingStorage
+}
+
+func (o *lazyObject) Reader() (rc io.ReadCloser, err error) {
+	rc, err = o.EncodedObject.Reader()
+	if !isRepackError(err) {
+		return rc, err
+	}
+	err = o.storage.retry(func() error {
+		obj, err := o.storage.Storage.EncodedObject(o.Type(), o.Hash())
+		if err == nil {
+			o.EncodedObject = obj
+			rc, err = obj.Reader()
+		}
+		return err
+	})
+	return rc, err
+}
+
+// packIdxFS lists packs like git, only while their .idx exists
+// FIXME: gogit workaround, remove with the gogit build
+type packIdxFS struct {
+	billy.Filesystem
+}
+
+func (f packIdxFS) ReadDir(dir string) ([]os.FileInfo, error) {
+	if dir != f.Join("objects", "pack") {
+		return f.Filesystem.ReadDir(dir)
+	}
+	dirFile, err := os.Open(filepath.Join(f.Root(), dir))
+	if err != nil {
+		return nil, err
+	}
+	defer dirFile.Close()
+	infos, err := dirFile.Readdir(-1) // skips files removed before their lstat, unlike billy's ReadDir
+	if err != nil {
+		return nil, err
+	}
+	names := make(container.Set[string], len(infos))
+	for _, info := range infos {
+		names.Add(info.Name())
+	}
+	return slices.DeleteFunc(infos, func(info os.FileInfo) bool {
+		base, isPack := strings.CutSuffix(info.Name(), ".pack")
+		return isPack && !names.Contains(base+".idx")
+	}), nil
 }
 
 func openRepositoryInternal(gitRepo *Repository) error {
@@ -72,7 +149,7 @@ func openRepositoryInternal(gitRepo *Repository) error {
 		altFs = osfs.New("/")
 	}
 	gitRepo.objectFormatCache = ParseGogitHash(plumbing.ZeroHash).Type()
-	storage := filesystem.NewStorageWithOptions(fs, cache.NewObjectLRUDefault(), filesystem.Options{KeepDescriptors: true, LargeObjectThreshold: setting.Git.LargeObjectThreshold, AlternatesFS: altFs})
+	storage := filesystem.NewStorageWithOptions(packIdxFS{fs}, cache.NewObjectLRUDefault(), filesystem.Options{KeepDescriptors: true, LargeObjectThreshold: setting.Git.LargeObjectThreshold, AlternatesFS: altFs})
 	packs, _ := storage.ObjectPacks()
 	gitRepo.gogitStorage = &reindexingStorage{Storage: storage, packs: packs}
 	gitRepo.gogitRepo, err = gogit.Open(gitRepo.gogitStorage, fs)

modules/git/repo_branch_test.go

@@ -4,10 +4,14 @@
 package git
 
 import (
+	"fmt"
+	"os"
 	"path/filepath"
+	"strings"
 	"testing"
 
 	"gitea.dev/modules/git/gitcmd"
+	"gitea.dev/modules/setting"
 
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
@@ -41,11 +45,13 @@ func TestRepository_GetBranches(t *testing.T) {
 	assert.ElementsMatch(t, []string{}, branches)
 }
 
-func TestGetBranchNamesAfterRepack(t *testing.T) {
+// FIXME: covers the gogit workarounds in repo_base_gogit.go, remove with the gogit build
+func TestReadsAfterConcurrentRepack(t *testing.T) {
 	repoDir := t.TempDir()
 	require.NoError(t, gitcmd.NewCommand("init", "--bare").AddDynamicArguments(repoDir).Run(t.Context()))
+	content := strings.Repeat("a", int(setting.Git.LargeObjectThreshold)+1)
 	for _, from := range []string{"", "from refs/heads/main^0\n"} {
-		stdin := "commit refs/heads/main\ncommitter a <a@a> 0 +0000\ndata 0\n" + from
+		stdin := fmt.Sprintf("commit refs/heads/main\ncommitter a <a@a> 0 +0000\ndata 0\n%sM 100644 inline f\ndata %d\n%s\n", from, len(content), content)
 		require.NoError(t, gitcmd.NewCommand("fast-import").WithDir(repoDir).WithStdinBytes([]byte(stdin)).Run(t.Context()))
 		require.NoError(t, gitcmd.NewCommand("repack", "-d").WithDir(repoDir).Run(t.Context()))
 	}
@@ -54,11 +60,24 @@ func TestGetBranchNamesAfterRepack(t *testing.T) {
 	require.NoError(t, err)
 	defer repo.Close()
 	require.False(t, repo.IsObjectExist(t.Context(), "0000000000000000000000000000000000000001"))
+	blobRepo, err := OpenRepositoryLocal(t.Context(), repoDir)
+	require.NoError(t, err)
+	defer blobRepo.Close()
+	commit, err := blobRepo.GetBranchCommit(t.Context(), "main")
+	require.NoError(t, err)
+	readBlob := func() string {
+		data, err := commit.GetFileContent(t.Context(), blobRepo, "f", len(content))
+		require.NoError(t, err)
+		return data
+	}
+	require.Equal(t, content, readBlob())
 	require.NoError(t, gitcmd.NewCommand("repack", "-a", "-d").WithDir(repoDir).Run(t.Context()))
+	require.NoError(t, os.WriteFile(filepath.Join(repoDir, "objects", "pack", "pack-"+strings.Repeat("1", 40)+".pack"), nil, 0o644))
 
 	branches, _, err := repo.GetBranchNames(t.Context(), 0, 0)
 	require.NoError(t, err)
 	assert.Equal(t, []string{"main"}, branches)
+	assert.Equal(t, content, readBlob())
 }
 
 func BenchmarkRepository_GetBranches(b *testing.B) {