Skip to content

Commit 129e2d6

Browse files
authored
fix default branch detection when no ref passed (#437)
1 parent c89412a commit 129e2d6

3 files changed

Lines changed: 77 additions & 33 deletions

File tree

analyze/analyze.go

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -175,7 +175,7 @@ func (a *Analyzer) AnalyzeOrg(ctx context.Context, org string, numberOfGoroutine
175175
defer reposWg.Done()
176176
repoNameWithOwner := repo.GetRepoIdentifier()
177177
obs.OnRepoStarted(repoNameWithOwner)
178-
repoKey, err := a.cloneRepo(ctx, repo.BuildGitURL(a.ScmClient.GetProviderBaseURL()), a.ScmClient.GetToken(), "HEAD")
178+
repoKey, err := a.cloneRepo(ctx, repo.BuildGitURL(a.ScmClient.GetProviderBaseURL()), a.ScmClient.GetToken(), defaultBranchCloneRef(repo))
179179
if err != nil {
180180
log.Error().Err(err).Str("repo", repoNameWithOwner).Msg("failed to clone repo")
181181
obs.OnRepoError(repoNameWithOwner, err)
@@ -450,7 +450,11 @@ func (a *Analyzer) AnalyzeRepo(ctx context.Context, repoString string, ref strin
450450
log.Debug().Msgf("Starting repository analysis for: %s/%s on %s", org, repoName, provider)
451451

452452
obs.OnRepoStarted(repoString)
453-
repoKey, err := a.cloneRepo(ctx, repo.BuildGitURL(a.ScmClient.GetProviderBaseURL()), a.ScmClient.GetToken(), ref)
453+
cloneRef := ref
454+
if ref == "" || ref == "HEAD" {
455+
cloneRef = defaultBranchCloneRef(repo)
456+
}
457+
repoKey, err := a.cloneRepo(ctx, repo.BuildGitURL(a.ScmClient.GetProviderBaseURL()), a.ScmClient.GetToken(), cloneRef)
454458
if err != nil {
455459
obs.OnRepoError(repoString, err)
456460
return nil, err
@@ -749,3 +753,15 @@ func (a *Analyzer) cloneRepo(ctx context.Context, gitURL string, token string, r
749753
}
750754
return key, nil
751755
}
756+
757+
// defaultBranchCloneRef returns an explicit refs/heads/<branch> ref derived
758+
// from the SCM-provided default branch. Passing this to Clone avoids both
759+
// the ls-remote in the HEAD discovery path and the ls-remote in the bare-ref
760+
// resolver. Falls back to "HEAD" when the SCM didn't provide a default,
761+
// which routes through Clone's discovery path for correctness.
762+
func defaultBranchCloneRef(repo Repository) string {
763+
if db := repo.GetDefaultBranch(); db != "" {
764+
return "refs/heads/" + db
765+
}
766+
return "HEAD"
767+
}

providers/gitops/gitops.go

Lines changed: 31 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -138,37 +138,47 @@ func (g *GitClient) Clone(ctx context.Context, clonePath string, url string, tok
138138

139139
switch {
140140
case ref == "HEAD":
141-
// Try "main" first (most common), then "master", then ls-remote as fallback
142-
for _, branch := range []string{"main", "master"} {
141+
// Discover the actual default branch via ls-remote HEAD symref.
142+
// Doing this first (rather than fast-pathing to main/master) avoids
143+
// analyzing the wrong branch on repositories whose default is not main.
144+
discovered := discoverDefaultBranch(repo, token)
145+
if discovered != "" {
143146
fetchOpts.RefSpecs = []config.RefSpec{
144-
config.RefSpec(fmt.Sprintf("+refs/heads/%s:refs/remotes/origin/%s", branch, branch)),
147+
config.RefSpec(fmt.Sprintf("+refs/heads/%s:refs/remotes/origin/%s", discovered, discovered)),
145148
}
146149
err = repo.FetchContext(ctx, fetchOpts)
147150
if err == nil {
148-
defaultBranch = branch
149-
resolved.localRef = plumbing.ReferenceName("refs/remotes/origin/" + branch)
150-
break
151-
}
152-
if classifyFetchError(err) != nil && !strings.Contains(err.Error(), "couldn't find remote ref") {
153-
return classifyFetchError(err)
151+
defaultBranch = discovered
152+
resolved.localRef = plumbing.ReferenceName("refs/remotes/origin/" + discovered)
153+
} else if cerr := classifyFetchError(err); cerr != nil && !strings.Contains(err.Error(), "couldn't find remote ref") {
154+
return cerr
154155
}
155156
}
156157
if defaultBranch == "" {
157-
// Neither main nor master — ls-remote to find actual default
158-
discovered := discoverDefaultBranchFromURL(url, token)
159-
if discovered != "" {
158+
// Discovery failed (e.g. server didn't advertise HEAD, or fetch of
159+
// discovered branch failed). Try common defaults before falling
160+
// back to fetching all branches.
161+
for _, branch := range []string{"main", "master"} {
160162
fetchOpts.RefSpecs = []config.RefSpec{
161-
config.RefSpec(fmt.Sprintf("+refs/heads/%s:refs/remotes/origin/%s", discovered, discovered)),
163+
config.RefSpec(fmt.Sprintf("+refs/heads/%s:refs/remotes/origin/%s", branch, branch)),
164+
}
165+
err = repo.FetchContext(ctx, fetchOpts)
166+
if err == nil {
167+
defaultBranch = branch
168+
resolved.localRef = plumbing.ReferenceName("refs/remotes/origin/" + branch)
169+
break
170+
}
171+
if cerr := classifyFetchError(err); cerr != nil && !strings.Contains(err.Error(), "couldn't find remote ref") {
172+
return cerr
162173
}
163-
resolved.localRef = plumbing.ReferenceName("refs/remotes/origin/" + discovered)
164-
} else {
165-
fetchOpts.RefSpecs = []config.RefSpec{config.RefSpec("+refs/heads/*:refs/remotes/origin/*")}
166174
}
175+
}
176+
if defaultBranch == "" {
177+
fetchOpts.RefSpecs = []config.RefSpec{config.RefSpec("+refs/heads/*:refs/remotes/origin/*")}
167178
err = repo.FetchContext(ctx, fetchOpts)
168179
if err := classifyFetchError(err); err != nil {
169180
return err
170181
}
171-
defaultBranch = discovered
172182
}
173183
default:
174184
resolved, err = resolveRemoteRef(repo, url, token, ref)
@@ -772,20 +782,10 @@ func peelToCommit(store storer.EncodedObjectStorer, hash plumbing.Hash) (plumbin
772782
}
773783
}
774784

775-
// looksLikeSHA returns true if s looks like a full-length git commit SHA.
776-
// discoverDefaultBranch uses remote.List to find the HEAD symref target.
777-
// Returns empty string if it can't be determined.
778-
// discoverDefaultBranchFromURL does a lightweight ls-remote to find the HEAD symref.
779-
func discoverDefaultBranchFromURL(url string, token string) string {
780-
store := memory.NewStorage()
781-
repo, err := gogit.Init(store, nil)
782-
if err != nil {
783-
return ""
784-
}
785-
_, err = repo.CreateRemote(&config.RemoteConfig{Name: "origin", URLs: []string{url}})
786-
if err != nil {
787-
return ""
788-
}
785+
// discoverDefaultBranch performs a lightweight ls-remote on the repo's origin
786+
// remote and returns the branch name that HEAD points to. Returns "" when the
787+
// server doesn't advertise a HEAD symref or the listing fails.
788+
func discoverDefaultBranch(repo *gogit.Repository, token string) string {
789789
remote, err := repo.Remote("origin")
790790
if err != nil {
791791
return ""

providers/gitops/gitops_test.go

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -157,6 +157,34 @@ func TestClassifyFetchError(t *testing.T) {
157157
assert.Error(t, err)
158158
}
159159

160+
// TestDiscoverDefaultBranchPrefersRemoteHEAD is the regression for #436:
161+
// when a repo has both `main` and another branch, and HEAD points to the
162+
// other branch, discoverDefaultBranch must report the HEAD-pointed branch
163+
// rather than fast-pathing to "main". Clone() consults this helper before
164+
// falling back to "main"/"master", so getting this right ensures HEAD
165+
// analysis follows the actual default branch.
166+
func TestDiscoverDefaultBranchPrefersRemoteHEAD(t *testing.T) {
167+
dir := t.TempDir()
168+
remote, err := gogit.PlainInit(dir, false)
169+
require.NoError(t, err)
170+
171+
writeRepoFile(t, dir, "action.yml", "name: initial\n")
172+
initialCommit := commitAll(t, remote, "initial commit")
173+
174+
require.NoError(t, remote.Storer.SetReference(
175+
plumbing.NewHashReference(plumbing.ReferenceName("refs/heads/main"), initialCommit),
176+
))
177+
require.NoError(t, remote.Storer.SetReference(
178+
plumbing.NewHashReference(plumbing.ReferenceName("refs/heads/zip-zip"), initialCommit),
179+
))
180+
require.NoError(t, remote.Storer.SetReference(
181+
plumbing.NewSymbolicReference(plumbing.HEAD, plumbing.ReferenceName("refs/heads/zip-zip")),
182+
))
183+
184+
client := createTestClientRepo(t, dir)
185+
assert.Equal(t, "zip-zip", discoverDefaultBranch(client, ""))
186+
}
187+
160188
func TestResolveRemoteRefBareTagPrefersTag(t *testing.T) {
161189
remotePath, _ := createTestRemoteRepo(t)
162190
repo := createTestClientRepo(t, remotePath)

0 commit comments

Comments
 (0)