Merge branch 'remote_repo_head_reference' into 'master'

Use HeadReference instead of GetDefaultBranch

See merge request https://gitlab.com/gitlab-org/gitaly/-/merge_requests/5988

Merged-by: Justin Tobler <jtobler@gitlab.com>
Approved-by: Justin Tobler <jtobler@gitlab.com>
Reviewed-by: Patrick Steinhardt <psteinhardt@gitlab.com>
Reviewed-by: Justin Tobler <jtobler@gitlab.com>
Co-authored-by: James Fargher <jfargher@gitlab.com>

9 files changed, 89 insertions, 10 deletions

diff --git a/internal/git/gittest/repository_suite.go b/internal/git/gittest/repository_suite.go
index f9c64c293..d7310da5c 100644
--- a/internal/git/gittest/repository_suite.go
+++ b/internal/git/gittest/repository_suite.go
@@ -33,6 +33,10 @@ func TestRepository(t *testing.T, cfg config.Cfg, getRepository GetRepositoryFun
 			desc: "GetDefaultBranch",
 			test: testRepositoryGetDefaultBranch,
 		},
+		{
+			desc: "HeadReference",
+			test: testRepositoryHeadReference,
+		},
 	} {
 		t.Run(tc.desc, func(t *testing.T) {
 			tc.test(t, cfg, getRepository)
@@ -190,3 +194,60 @@ func testRepositoryGetDefaultBranch(t *testing.T, cfg config.Cfg, getRepository
 		})
 	}
 }
+
+func testRepositoryHeadReference(t *testing.T, cfg config.Cfg, getRepository GetRepositoryFunc) {
+	t.Parallel()
+	ctx := testhelper.Context(t)
+
+	for _, tc := range []struct {
+		desc         string
+		repo         func(t *testing.T) git.Repository
+		expectedName git.ReferenceName
+	}{
+		{
+			desc: "default unborn",
+			repo: func(t *testing.T) git.Repository {
+				repo, _ := getRepository(t, ctx)
+				return repo
+			},
+			expectedName: git.DefaultRef,
+		},
+		{
+			desc: "default exists",
+			repo: func(t *testing.T) git.Repository {
+				repo, repoPath := getRepository(t, ctx)
+				WriteCommit(t, cfg, repoPath, WithBranch(git.DefaultBranch))
+				return repo
+			},
+			expectedName: git.DefaultRef,
+		},
+		{
+			desc: "HEAD set nonexistent",
+			repo: func(t *testing.T) git.Repository {
+				repo, repoPath := getRepository(t, ctx)
+				Exec(t, cfg, "-C", repoPath, "symbolic-ref", "HEAD", "refs/heads/feature")
+				return repo
+			},
+			expectedName: git.NewReferenceNameFromBranchName("feature"),
+		},
+		{
+			desc: "HEAD set exists",
+			repo: func(t *testing.T) git.Repository {
+				repo, repoPath := getRepository(t, ctx)
+				WriteCommit(t, cfg, repoPath, WithBranch("feature"))
+				Exec(t, cfg, "-C", repoPath, "symbolic-ref", "HEAD", "refs/heads/feature")
+				return repo
+			},
+			expectedName: git.NewReferenceNameFromBranchName("feature"),
+		},
+	} {
+		tc := tc
+
+		t.Run(tc.desc, func(t *testing.T) {
+			t.Parallel()
+			name, err := tc.repo(t).HeadReference(ctx)
+			require.NoError(t, err)
+			require.Equal(t, tc.expectedName, name)
+		})
+	}
+}
diff --git a/internal/git/localrepo/bundle_test.go b/internal/git/localrepo/bundle_test.go
index 953054d55..bff66a5de 100644
--- a/internal/git/localrepo/bundle_test.go
+++ b/internal/git/localrepo/bundle_test.go
@@ -222,7 +222,7 @@ func TestRepo_FetchBundle(t *testing.T) {
 			}
 			require.NoError(t, err)
 
-			headRef, err := repo.GetDefaultBranch(ctx)
+			headRef, err := repo.HeadReference(ctx)
 			require.NoError(t, err)
 			require.Equal(t, data.expectedHeadRef, headRef)
 
diff --git a/internal/git/localrepo/refs_test.go b/internal/git/localrepo/refs_test.go
index df9cb9bca..ff092e640 100644
--- a/internal/git/localrepo/refs_test.go
+++ b/internal/git/localrepo/refs_test.go
@@ -509,7 +509,7 @@ func TestRepo_SetDefaultBranch(t *testing.T) {
 		{
 			desc:        "unknown ref",
 			ref:         "refs/heads/non_existent_ref",
-			expectedRef: git.LegacyDefaultRef,
+			expectedRef: "refs/heads/non_existent_ref",
 		},
 	}
 	for _, tc := range testCases {
@@ -525,7 +525,7 @@ func TestRepo_SetDefaultBranch(t *testing.T) {
 
 			require.NoError(t, repo.SetDefaultBranch(ctx, txManager, tc.ref))
 
-			newRef, err := repo.GetDefaultBranch(ctx)
+			newRef, err := repo.HeadReference(ctx)
 			require.NoError(t, err)
 
 			require.Equal(t, tc.expectedRef, newRef)
@@ -593,7 +593,7 @@ func TestRepo_SetDefaultBranch_errors(t *testing.T) {
 	t.Run("HEAD is locked by another process", func(t *testing.T) {
 		_, repo, _ := setupRepo(t)
 
-		ref, err := repo.GetDefaultBranch(ctx)
+		ref, err := repo.HeadReference(ctx)
 		require.NoError(t, err)
 
 		path, err := repo.Path()
@@ -604,7 +604,7 @@ func TestRepo_SetDefaultBranch_errors(t *testing.T) {
 		err = repo.SetDefaultBranch(ctx, &transaction.MockManager{}, "refs/heads/branch")
 		require.ErrorIs(t, err, safe.ErrFileAlreadyLocked)
 
-		refAfter, err := repo.GetDefaultBranch(ctx)
+		refAfter, err := repo.HeadReference(ctx)
 		require.NoError(t, err)
 		require.Equal(t, ref, refAfter)
 	})
diff --git a/internal/git/remoterepo/repository.go b/internal/git/remoterepo/repository.go
index a7d946cd8..4b1f2eb8b 100644
--- a/internal/git/remoterepo/repository.go
+++ b/internal/git/remoterepo/repository.go
@@ -113,3 +113,18 @@ func (rr *Repo) GetDefaultBranch(ctx context.Context) (git.ReferenceName, error)
 
 	return git.ReferenceName(resp.Name), nil
 }
+
+// HeadReference returns the reference that HEAD points to for the remote
+// repository.
+func (rr *Repo) HeadReference(ctx context.Context) (git.ReferenceName, error) {
+	resp, err := gitalypb.NewRefServiceClient(rr.conn).FindDefaultBranchName(
+		ctx, &gitalypb.FindDefaultBranchNameRequest{
+			Repository: rr.Repository,
+			HeadOnly:   true,
+		})
+	if err != nil {
+		return "", err
+	}
+
+	return git.ReferenceName(resp.Name), nil
+}
diff --git a/internal/git/remoterepo/repository_test.go b/internal/git/remoterepo/repository_test.go
index 61edd5f83..21fcab898 100644
--- a/internal/git/remoterepo/repository_test.go
+++ b/internal/git/remoterepo/repository_test.go
@@ -26,7 +26,7 @@ func TestRepository(t *testing.T) {
 	cfg := setupGitalyServer(t)
 
 	pool := client.NewPool()
-	defer pool.Close()
+	t.Cleanup(func() { testhelper.MustClose(t, pool) })
 
 	gittest.TestRepository(t, cfg, func(tb testing.TB, ctx context.Context) (git.Repository, string) {
 		tb.Helper()
diff --git a/internal/git/repository.go b/internal/git/repository.go
index b3cf9725f..fe999d9ac 100644
--- a/internal/git/repository.go
+++ b/internal/git/repository.go
@@ -49,6 +49,9 @@ type Repository interface {
 	HasBranches(ctx context.Context) (bool, error)
 	// GetDefaultBranch returns the default branch of the repository.
 	GetDefaultBranch(ctx context.Context) (ReferenceName, error)
+	// HeadReference returns the reference that HEAD points to for the
+	// repository.
+	HeadReference(ctx context.Context) (ReferenceName, error)
 }
 
 // RepositoryExecutor is an interface which allows execution of Git commands in a specific
diff --git a/internal/gitaly/service/remote/update_remote_mirror.go b/internal/gitaly/service/remote/update_remote_mirror.go
index bf2240927..cfbb0d49f 100644
--- a/internal/gitaly/service/remote/update_remote_mirror.go
+++ b/internal/gitaly/service/remote/update_remote_mirror.go
@@ -115,7 +115,7 @@ func (s *server) updateRemoteMirror(stream gitalypb.RemoteService_UpdateRemoteMi
 		return fmt.Errorf("get local references: %w", err)
 	}
 
-	defaultBranch, err := repo.GetDefaultBranch(ctx)
+	defaultBranch, err := repo.HeadReference(ctx)
 	if err != nil {
 		return fmt.Errorf("get default branch: %w", err)
 	}
diff --git a/internal/gitaly/service/repository/create_repository_from_bundle_test.go b/internal/gitaly/service/repository/create_repository_from_bundle_test.go
index 61ff967b9..c16253679 100644
--- a/internal/gitaly/service/repository/create_repository_from_bundle_test.go
+++ b/internal/gitaly/service/repository/create_repository_from_bundle_test.go
@@ -98,7 +98,7 @@ func TestCreateRepositoryFromBundle_successful(t *testing.T) {
 	require.NoError(t, err)
 	require.NotNil(t, commit)
 
-	gotDefaultBranch, err := importedRepo.GetDefaultBranch(ctx)
+	gotDefaultBranch, err := importedRepo.HeadReference(ctx)
 	require.NoError(t, err)
 	require.Equal(t, wantDefaultBranch, gotDefaultBranch.String())
 }
diff --git a/internal/gitaly/service/repository/replicate.go b/internal/gitaly/service/repository/replicate.go
index fa5303a52..bb9ddf3d1 100644
--- a/internal/gitaly/service/repository/replicate.go
+++ b/internal/gitaly/service/repository/replicate.go
@@ -263,12 +263,12 @@ func fetchInternalRemote(
 		return structerr.NewInternal("%w", err)
 	}
 
-	remoteDefaultBranch, err := remoteRepo.GetDefaultBranch(ctx)
+	remoteDefaultBranch, err := remoteRepo.HeadReference(ctx)
 	if err != nil {
 		return structerr.NewInternal("getting remote default branch: %w", err)
 	}
 
-	defaultBranch, err := repo.GetDefaultBranch(ctx)
+	defaultBranch, err := repo.HeadReference(ctx)
 	if err != nil {
 		return structerr.NewInternal("getting local default branch: %w", err)
 	}