repository: Fix commit-graphs referencing pruned commits after GC

When pruning objects we need to rewrite our commit-graphs so that they
don't reference any commits that have just been pruned. While this is
not typically an issue, git-fsck(1) will report any such inconsistencies
in the commit-graph. Furthermore, Git may return an error when being
asked to parse such a missing commit directly.

Fix `GarbageCollect()` to do so.

Changelog: fixed

2 files changed, 17 insertions, 15 deletions

diff --git a/internal/gitaly/service/repository/gc.go b/internal/gitaly/service/repository/gc.go
index 25db8f377..32240b767 100644
--- a/internal/gitaly/service/repository/gc.go
+++ b/internal/gitaly/service/repository/gc.go
@@ -51,12 +51,9 @@ func (s *server) GarbageCollect(ctx context.Context, in *gitalypb.GarbageCollect
 		return nil, err
 	}
 
-	writeCommitGraphCfg, err := housekeeping.WriteCommitGraphConfigForRepository(ctx, repo)
-	if err != nil {
-		return nil, helper.ErrInternalf("getting commit-graph config: %w", err)
-	}
-
-	if err := housekeeping.WriteCommitGraph(ctx, repo, writeCommitGraphCfg); err != nil {
+	if err := housekeeping.WriteCommitGraph(ctx, repo, housekeeping.WriteCommitGraphConfig{
+		ReplaceChain: true,
+	}); err != nil {
 		return nil, err
 	}
 
diff --git a/internal/gitaly/service/repository/gc_test.go b/internal/gitaly/service/repository/gc_test.go
index 1b83dd0bd..66755943f 100644
--- a/internal/gitaly/service/repository/gc_test.go
+++ b/internal/gitaly/service/repository/gc_test.go
@@ -273,8 +273,9 @@ func TestGarbageCollectDeletesFileLocks(t *testing.T) {
 	req := &gitalypb.GarbageCollectRequest{Repository: repo}
 
 	for _, tc := range []struct {
-		desc     string
-		lockfile string
+		desc                string
+		lockfile            string
+		expectedErrContains string
 	}{
 		{
 			desc:     "locked gitconfig",
@@ -287,6 +288,9 @@ func TestGarbageCollectDeletesFileLocks(t *testing.T) {
 		{
 			desc:     "locked commit-graph",
 			lockfile: "objects/info/commit-graphs/commit-graph-chain.lock",
+			// Writing commit-graphs fails if there is another, concurrent prorcess that
+			// has locked the commit-graph chain.
+			expectedErrContains: "Another git process seems to be running in this repository",
 		},
 	} {
 		t.Run(tc.desc, func(t *testing.T) {
@@ -308,7 +312,12 @@ func TestGarbageCollectDeletesFileLocks(t *testing.T) {
 
 				//nolint:staticcheck
 				_, err := client.GarbageCollect(ctx, req)
-				require.NoError(t, err)
+				if tc.expectedErrContains == "" {
+					require.NoError(t, err)
+				} else {
+					require.Error(t, err)
+					require.Contains(t, err.Error(), tc.expectedErrContains)
+				}
 				require.FileExists(t, lockPath)
 			})
 
@@ -566,10 +575,6 @@ func TestGarbageCollect_commitGraphsWithPrunedObjects(t *testing.T) {
 	_, err := client.GarbageCollect(ctx, &gitalypb.GarbageCollectRequest{Repository: repoProto})
 	require.NoError(t, err)
 
-	// ... but as it turns out it doesn't.
-	stderr.Reset()
-	verifyCmd = gittest.NewCommand(t, cfg, "-C", repoPath, "commit-graph", "verify")
-	verifyCmd.Stderr = &stderr
-	require.EqualError(t, verifyCmd.Run(), "exit status 1")
-	require.Equal(t, stderr.String(), fmt.Sprintf("error: Could not read %[1]s\nfailed to parse commit %[1]s from object database for commit-graph\n", unreachableCommitID))
+	// ... so that verification succeeds now.
+	gittest.Exec(t, cfg, "-C", repoPath, "commit-graph", "verify")
 }