Change signature of GetDefaultBranch to only return a reference name

As a prerequisite to changing the implementation to no longer resolve to
a commit, we change the signature to only return a reference name.

Options were no longer required as if you already have the head
reference name, then you already have the default branch.

11 files changed, 43 insertions, 82 deletions

diff --git a/internal/git/localrepo/refs.go b/internal/git/localrepo/refs.go
index 3f6b87e89..7b9788eb3 100644
--- a/internal/git/localrepo/refs.go
+++ b/internal/git/localrepo/refs.go
@@ -267,63 +267,49 @@ func (repo *Repo) GetRemoteReferences(ctx context.Context, remote string, opts .
 	return refs, nil
 }
 
-// GetDefaultBranchOptions are options passed to GetDefaultBranch
-type GetDefaultBranchOptions struct {
-	// HeadReference allows specifying the HEAD symbolic reference instead of
-	// looking it up if it is already known
-	HeadReference git.ReferenceName
-}
-
 // GetDefaultBranch determines the default branch name
-func (repo *Repo) GetDefaultBranch(ctx context.Context, opts *GetDefaultBranchOptions) (git.Reference, error) {
-	if opts == nil {
-		opts = &GetDefaultBranchOptions{}
-	}
-
+func (repo *Repo) GetDefaultBranch(ctx context.Context) (git.ReferenceName, error) {
 	branches, err := repo.GetBranches(ctx)
 	if err != nil {
-		return git.Reference{}, err
+		return "", err
 	}
 	switch len(branches) {
 	case 0:
-		return git.Reference{}, nil
+		return "", nil
 	case 1:
-		return branches[0], nil
+		return branches[0].Name, nil
 	}
 
-	if len(opts.HeadReference) == 0 {
-		var err error
-		opts.HeadReference, err = repo.headReference(ctx)
-		if err != nil {
-			return git.Reference{}, err
-		}
+	headReference, err := repo.headReference(ctx)
+	if err != nil {
+		return "", err
 	}
 
-	var defaultRef, legacyDefaultRef git.Reference
+	var defaultRef, legacyDefaultRef git.ReferenceName
 	for _, branch := range branches {
-		if len(opts.HeadReference) != 0 && opts.HeadReference == branch.Name {
-			return branch, nil
+		if len(headReference) != 0 && headReference == branch.Name {
+			return branch.Name, nil
 		}
 
 		if string(git.DefaultRef) == branch.Name.String() {
-			defaultRef = branch
+			defaultRef = branch.Name
 		}
 
 		if string(git.LegacyDefaultRef) == branch.Name.String() {
-			legacyDefaultRef = branch
+			legacyDefaultRef = branch.Name
 		}
 	}
 
-	if len(defaultRef.Name) != 0 {
+	if len(defaultRef) != 0 {
 		return defaultRef, nil
 	}
 
-	if len(legacyDefaultRef.Name) != 0 {
+	if len(legacyDefaultRef) != 0 {
 		return legacyDefaultRef, nil
 	}
 
 	// If all else fails, return the first branch name
-	return branches[0], nil
+	return branches[0].Name, nil
 }
 
 func (repo *Repo) headReference(ctx context.Context) (git.ReferenceName, error) {
diff --git a/internal/git/localrepo/refs_test.go b/internal/git/localrepo/refs_test.go
index 8a4fbdc98..ebfde348d 100644
--- a/internal/git/localrepo/refs_test.go
+++ b/internal/git/localrepo/refs_test.go
@@ -459,11 +459,9 @@ func TestGetDefaultBranch(t *testing.T) {
 	const testOID = "1a0b36b3cdad1d2ee32457c102a8c0b7056fa863"
 
 	for _, tc := range []struct {
-		desc           string
-		repo           func(t *testing.T) *Repo
-		opts           *GetDefaultBranchOptions
-		expectedName   git.ReferenceName
-		expectedTarget string
+		desc         string
+		repo         func(t *testing.T) *Repo
+		expectedName git.ReferenceName
 	}{
 		{
 			desc: "default ref",
@@ -527,39 +525,16 @@ func TestGetDefaultBranch(t *testing.T) {
 				gittest.Exec(t, repo.cfg, "-C", repoPath, "symbolic-ref", "HEAD", "refs/heads/feature")
 				return repo
 			},
-			expectedName:   git.NewReferenceNameFromBranchName("feature"),
-			expectedTarget: testOID,
-		},
-		{
-			desc: "test repo HEAD provided",
-			repo: func(t *testing.T) *Repo {
-				repo, repoPath := setupRepo(t, false)
-				gittest.Exec(t, repo.cfg, "-C", repoPath, "update-ref", "refs/heads/feature", testOID)
-				return repo
-			},
-			opts: &GetDefaultBranchOptions{
-				HeadReference: "refs/heads/feature",
-			},
-			expectedName:   git.NewReferenceNameFromBranchName("feature"),
-			expectedTarget: testOID,
+			expectedName: git.NewReferenceNameFromBranchName("feature"),
 		},
 	} {
 		t.Run(tc.desc, func(t *testing.T) {
 			ctx, cancel := testhelper.Context()
 			defer cancel()
 
-			branch, err := tc.repo(t).GetDefaultBranch(ctx, tc.opts)
+			name, err := tc.repo(t).GetDefaultBranch(ctx)
 			require.NoError(t, err)
-			if len(tc.expectedName) != 0 {
-				require.Equal(t, tc.expectedName, branch.Name)
-			}
-			if len(tc.expectedTarget) != 0 {
-				require.Equal(t, tc.expectedTarget, branch.Target)
-			}
-			if len(tc.expectedName) == 0 && len(tc.expectedTarget) == 0 {
-				require.Empty(t, branch.Name)
-				require.Empty(t, branch.Target)
-			}
+			require.Equal(t, tc.expectedName, name)
 		})
 	}
 }
diff --git a/internal/gitaly/service/commit/commits_by_message.go b/internal/gitaly/service/commit/commits_by_message.go
index c21ad8a54..6353cf0ee 100644
--- a/internal/gitaly/service/commit/commits_by_message.go
+++ b/internal/gitaly/service/commit/commits_by_message.go
@@ -53,11 +53,11 @@ func (s *server) commitsByMessage(in *gitalypb.CommitsByMessageRequest, stream g
 
 	revision := in.GetRevision()
 	if len(revision) == 0 {
-		defaultBranch, err := repo.GetDefaultBranch(ctx, nil)
+		defaultBranch, err := repo.GetDefaultBranch(ctx)
 		if err != nil {
 			return err
 		}
-		revision = []byte(defaultBranch.Name)
+		revision = []byte(defaultBranch)
 	}
 
 	var paths []string
diff --git a/internal/gitaly/service/commit/find_commits.go b/internal/gitaly/service/commit/find_commits.go
index d0de9fced..6f6f63b4d 100644
--- a/internal/gitaly/service/commit/find_commits.go
+++ b/internal/gitaly/service/commit/find_commits.go
@@ -29,11 +29,11 @@ func (s *server) FindCommits(req *gitalypb.FindCommitsRequest, stream gitalypb.C
 	// Use Gitaly's default branch lookup function because that is already
 	// migrated.
 	if revision := req.Revision; len(revision) == 0 && !req.GetAll() {
-		defaultBranch, err := repo.GetDefaultBranch(ctx, nil)
+		defaultBranch, err := repo.GetDefaultBranch(ctx)
 		if err != nil {
 			return helper.ErrInternal(fmt.Errorf("defaultBranchName: %v", err))
 		}
-		req.Revision = []byte(defaultBranch.Name)
+		req.Revision = []byte(defaultBranch)
 	}
 	// Clients might send empty paths. That is an error
 	for _, path := range req.Paths {
diff --git a/internal/gitaly/service/commit/languages.go b/internal/gitaly/service/commit/languages.go
index fc77d75ef..b9ef474d4 100644
--- a/internal/gitaly/service/commit/languages.go
+++ b/internal/gitaly/service/commit/languages.go
@@ -26,11 +26,11 @@ func (s *server) CommitLanguages(ctx context.Context, req *gitalypb.CommitLangua
 
 	revision := string(req.Revision)
 	if revision == "" {
-		defaultBranch, err := repo.GetDefaultBranch(ctx, nil)
+		defaultBranch, err := repo.GetDefaultBranch(ctx)
 		if err != nil {
 			return nil, err
 		}
-		revision = defaultBranch.Name.String()
+		revision = defaultBranch.String()
 	}
 
 	commitID, err := s.lookupRevision(ctx, repo, revision)
diff --git a/internal/gitaly/service/commit/list_files.go b/internal/gitaly/service/commit/list_files.go
index f21224cee..4aa1fd448 100644
--- a/internal/gitaly/service/commit/list_files.go
+++ b/internal/gitaly/service/commit/list_files.go
@@ -29,16 +29,16 @@ func (s *server) ListFiles(in *gitalypb.ListFilesRequest, stream gitalypb.Commit
 
 	revision := string(in.GetRevision())
 	if len(revision) == 0 {
-		defaultBranch, err := repo.GetDefaultBranch(stream.Context(), nil)
+		defaultBranch, err := repo.GetDefaultBranch(stream.Context())
 		if err != nil {
 			return helper.ErrNotFoundf("revision not found %q", revision)
 		}
 
-		if len(defaultBranch.Name) == 0 {
+		if len(defaultBranch) == 0 {
 			return helper.ErrFailedPreconditionf("repository does not have a default branch")
 		}
 
-		revision = defaultBranch.Name.String()
+		revision = defaultBranch.String()
 	}
 
 	contained, err := s.localrepo(repo).HasRevision(stream.Context(), git.Revision(revision))
diff --git a/internal/gitaly/service/operations/apply_patch.go b/internal/gitaly/service/operations/apply_patch.go
index 960125682..506f4c66e 100644
--- a/internal/gitaly/service/operations/apply_patch.go
+++ b/internal/gitaly/service/operations/apply_patch.go
@@ -59,15 +59,15 @@ func (s *Server) userApplyPatch(ctx context.Context, header *gitalypb.UserApplyP
 			return fmt.Errorf("resolve target branch: %w", err)
 		}
 
-		defaultBranch, err := repo.GetDefaultBranch(ctx, nil)
+		defaultBranch, err := repo.GetDefaultBranch(ctx)
 		if err != nil {
 			return fmt.Errorf("default branch name: %w", err)
-		} else if len(defaultBranch.Name) == 0 {
+		} else if len(defaultBranch) == 0 {
 			return errNoDefaultBranch
 		}
 
 		branchCreated = true
-		parentCommitID, err = repo.ResolveRevision(ctx, defaultBranch.Name.Revision()+"^{commit}")
+		parentCommitID, err = repo.ResolveRevision(ctx, defaultBranch.Revision()+"^{commit}")
 		if err != nil {
 			return fmt.Errorf("resolve default branch commit: %w", err)
 		}
diff --git a/internal/gitaly/service/ref/refs.go b/internal/gitaly/service/ref/refs.go
index 4899367d6..d50b92de1 100644
--- a/internal/gitaly/service/ref/refs.go
+++ b/internal/gitaly/service/ref/refs.go
@@ -307,12 +307,12 @@ func SetDefaultBranchRef(ctx context.Context, repo git.RepositoryExecutor, ref s
 func (s *server) FindDefaultBranchName(ctx context.Context, in *gitalypb.FindDefaultBranchNameRequest) (*gitalypb.FindDefaultBranchNameResponse, error) {
 	repo := s.localrepo(in.GetRepository())
 
-	defaultBranch, err := repo.GetDefaultBranch(ctx, nil)
+	defaultBranch, err := repo.GetDefaultBranch(ctx)
 	if err != nil {
 		return nil, helper.ErrInternal(err)
 	}
 
-	return &gitalypb.FindDefaultBranchNameResponse{Name: []byte(defaultBranch.Name)}, nil
+	return &gitalypb.FindDefaultBranchNameResponse{Name: []byte(defaultBranch)}, nil
 }
 
 func parseSortKey(sortKey gitalypb.FindLocalBranchesRequest_SortBy) string {
@@ -376,12 +376,12 @@ func (s *server) findAllBranches(in *gitalypb.FindAllBranchesRequest, stream git
 	patterns := []string{"refs/heads", "refs/remotes"}
 
 	if in.MergedOnly {
-		defaultBranch, err := repo.GetDefaultBranch(stream.Context(), nil)
+		defaultBranch, err := repo.GetDefaultBranch(stream.Context())
 		if err != nil {
 			return err
 		}
 
-		args = append(args, git.Flag{Name: fmt.Sprintf("--merged=%s", defaultBranch.Name.String())})
+		args = append(args, git.Flag{Name: fmt.Sprintf("--merged=%s", defaultBranch.String())})
 
 		if len(in.MergedBranches) > 0 {
 			patterns = nil
diff --git a/internal/gitaly/service/ref/refs_test.go b/internal/gitaly/service/ref/refs_test.go
index 8a4858378..c3717957a 100644
--- a/internal/gitaly/service/ref/refs_test.go
+++ b/internal/gitaly/service/ref/refs_test.go
@@ -249,10 +249,10 @@ func TestSetDefaultBranchRef(t *testing.T) {
 
 			require.NoError(t, SetDefaultBranchRef(ctx, repo, tc.ref, cfg))
 
-			newRef, err := repo.GetDefaultBranch(ctx, nil)
+			newRef, err := repo.GetDefaultBranch(ctx)
 			require.NoError(t, err)
 
-			require.Equal(t, tc.expectedRef, newRef.Name.String())
+			require.Equal(t, tc.expectedRef, newRef.String())
 		})
 	}
 }
diff --git a/internal/gitaly/service/remote/fetch_internal_remote.go b/internal/gitaly/service/remote/fetch_internal_remote.go
index b43dde15e..b0eec8fbe 100644
--- a/internal/gitaly/service/remote/fetch_internal_remote.go
+++ b/internal/gitaly/service/remote/fetch_internal_remote.go
@@ -61,12 +61,12 @@ func FetchInternalRemote(
 		return status.Errorf(codes.Internal, "FetchInternalRemote: remote default branch: %v", err)
 	}
 
-	defaultBranch, err := repo.GetDefaultBranch(ctx, nil)
+	defaultBranch, err := repo.GetDefaultBranch(ctx)
 	if err != nil {
 		return status.Errorf(codes.Internal, "FetchInternalRemote: default branch: %v", err)
 	}
 
-	if defaultBranch.Name.String() != string(remoteDefaultBranch) {
+	if defaultBranch.String() != string(remoteDefaultBranch) {
 		if err := ref.SetDefaultBranchRef(ctx, repo, string(remoteDefaultBranch), cfg); err != nil {
 			return status.Errorf(codes.Internal, "FetchInternalRemote: set default branch: %v", err)
 		}
diff --git a/internal/gitaly/service/remote/update_remote_mirror.go b/internal/gitaly/service/remote/update_remote_mirror.go
index 69d0628d0..d94ac2065 100644
--- a/internal/gitaly/service/remote/update_remote_mirror.go
+++ b/internal/gitaly/service/remote/update_remote_mirror.go
@@ -102,7 +102,7 @@ func (s *server) updateRemoteMirror(stream gitalypb.RemoteService_UpdateRemoteMi
 		return fmt.Errorf("get local references: %w", err)
 	}
 
-	defaultBranch, err := repo.GetDefaultBranch(ctx, nil)
+	defaultBranch, err := repo.GetDefaultBranch(ctx)
 	if err != nil {
 		return fmt.Errorf("get default branch: %w", err)
 	}
@@ -194,7 +194,7 @@ func (s *server) updateRemoteMirror(stream gitalypb.RemoteService_UpdateRemoteMi
 			}
 
 			refspecs = append(refspecs, prefix+reference.String())
-			if reference == defaultBranch.Name {
+			if reference == defaultBranch {
 				// The default branch needs to be pushed in the first batch of refspecs as some features
 				// depend on it existing in the repository. The default branch may not exist in the repo
 				// yet if this is the first mirroring push.