git: Demonstrate missing reverse indices for some Git commands

Packfiles we're writing to the repository should all have a reverse
index. As it turns out though, packfiles written by git-receive-pack(1)
and git-fetch(1) don't write this index because they're missing the Git
configuration for generating packfiles.

Add tests to demonstrate this bug for both Git commands.

2 files changed, 85 insertions, 0 deletions

diff --git a/internal/git/localrepo/remote_test.go b/internal/git/localrepo/remote_test.go
index 93b06058e..1a0f3762b 100644
--- a/internal/git/localrepo/remote_test.go
+++ b/internal/git/localrepo/remote_test.go
@@ -184,6 +184,33 @@ func TestRepo_FetchRemote(t *testing.T) {
 		require.Contains(t, err.Error(), "fatal: 'doesnotexist' does not appear to be a git repository")
 		require.IsType(t, err, ErrFetchFailed{})
 	})
+
+	t.Run("generates reverse index", func(t *testing.T) {
+		repo, repoPath := initBareWithRemote(t, "origin")
+
+		// The repository has no objects yet, so there shouldn't be any packfile either.
+		packfiles, err := filepath.Glob(filepath.Join(repoPath, "objects", "pack", "*.pack"))
+		require.NoError(t, err)
+		require.Empty(t, packfiles)
+
+		// Same goes for reverse indices, naturally.
+		reverseIndices, err := filepath.Glob(filepath.Join(repoPath, "objects", "pack", "*.rev"))
+		require.NoError(t, err)
+		require.Empty(t, reverseIndices)
+
+		require.NoError(t, repo.FetchRemote(ctx, "origin", FetchOpts{}))
+
+		// After the fetch we should end up with a single packfile.
+		packfiles, err = filepath.Glob(filepath.Join(repoPath, "objects", "pack", "*.pack"))
+		require.NoError(t, err)
+		require.Len(t, packfiles, 1)
+
+		// And furthermore, that packfile should have a reverse index. Due to a bug it
+		// doesn't though.
+		reverseIndices, err = filepath.Glob(filepath.Join(repoPath, "objects", "pack", "*.rev"))
+		require.NoError(t, err)
+		require.Empty(t, reverseIndices)
+	})
 }
 
 // captureGitSSHCommand creates a new intercepting command factory which captures the
diff --git a/internal/gitaly/service/smarthttp/receive_pack_test.go b/internal/gitaly/service/smarthttp/receive_pack_test.go
index eaa508709..7675ed5d0 100644
--- a/internal/gitaly/service/smarthttp/receive_pack_test.go
+++ b/internal/gitaly/service/smarthttp/receive_pack_test.go
@@ -226,6 +226,64 @@ func TestPostReceivePack_protocolV2(t *testing.T) {
 	gittest.Exec(t, cfg, "-C", repoPath, "show", newCommitID.String())
 }
 
+func TestPostReceivePack_packfiles(t *testing.T) {
+	t.Parallel()
+
+	ctx := testhelper.Context(t)
+
+	cfg := testcfg.Build(t)
+	cfg.SocketPath = startSmartHTTPServer(t, cfg).Address()
+	testcfg.BuildGitalyHooks(t, cfg)
+
+	client, conn := newSmartHTTPClient(t, cfg.SocketPath, cfg.Auth.Token)
+	defer conn.Close()
+
+	repo, repoPath := gittest.CreateRepository(ctx, t, cfg, gittest.CreateRepositoryConfig{
+		Seed: gittest.SeedGitLabTest,
+	})
+
+	stream, err := client.PostReceivePack(ctx)
+	require.NoError(t, err)
+
+	// Verify the before-state of the repository. It should have a single packfile, ...
+	packfiles, err := filepath.Glob(filepath.Join(repoPath, "objects", "pack", "*.pack"))
+	require.NoError(t, err)
+	require.Len(t, packfiles, 1)
+
+	// ... but no reverse index. `gittest.CreateRepository()` uses `CreateRepositoryFromURL()`
+	// with a file path that doesn't use `file://` as prefix. As a result, Git uses the local
+	// transport and just copies objects over without generating a reverse index. This is thus
+	// expected as we don't want to perform a "real" clone, which would be a lot more expensive.
+	reverseIndices, err := filepath.Glob(filepath.Join(repoPath, "objects", "pack", "*.rev"))
+	require.NoError(t, err)
+	require.Empty(t, reverseIndices)
+
+	_, _, request := createPushRequest(t, cfg)
+	performPush(t, stream, &gitalypb.PostReceivePackRequest{
+		Repository:   repo,
+		GlId:         "user-123",
+		GlRepository: "project-123",
+		GitProtocol:  git.ProtocolV2,
+		// By default, Git would unpack the received packfile if it has less than
+		// 100 objects. Decrease this limit so that we indeed end up with another
+		// new packfile even though we only push a small set of objects.
+		GitConfigOptions: []string{
+			"receive.unpackLimit=0",
+		},
+	}, request)
+
+	// We now should have two packfiles, ...
+	packfiles, err = filepath.Glob(filepath.Join(repoPath, "objects", "pack", "*.pack"))
+	require.NoError(t, err)
+	require.Len(t, packfiles, 2)
+
+	// ... and one reverse index for the newly added packfile. Due to a bug we don't generate
+	// the reverse index though.
+	reverseIndices, err = filepath.Glob(filepath.Join(repoPath, "objects", "pack", "*.rev"))
+	require.NoError(t, err)
+	require.Empty(t, reverseIndices)
+}
+
 func TestPostReceivePack_rejectViaGitConfigOptions(t *testing.T) {
 	t.Parallel()