diff --git a/go/kbfs/kbfsgit/runner.go b/go/kbfs/kbfsgit/runner.go index f9a60c26d9f0..c4551957f393 100644 --- a/go/kbfs/kbfsgit/runner.go +++ b/go/kbfs/kbfsgit/runner.go @@ -682,6 +682,21 @@ func (r *runner) waitForJournal(ctx context.Context) error { r.printStageEndIfNeeded) } +// bestBranchFromCandidates selects the best branch for HEAD from a set of +// candidate branch names (prefer main > master > alphabetically first). +func bestBranchFromCandidates(best plumbing.ReferenceName, candidate plumbing.ReferenceName) plumbing.ReferenceName { + switch { + case candidate == "refs/heads/main": + return candidate + case candidate == "refs/heads/master" && best != "refs/heads/main": + return candidate + case best == "" || (best != "refs/heads/main" && best != "refs/heads/master" && candidate < best): + return candidate + default: + return best + } +} + // handleList: From https://git-scm.com/docs/git-remote-helpers // // Lists the refs, one per line, in the format " [ @@ -708,9 +723,18 @@ func (r *runner) handleList(ctx context.Context, args []string) (err error) { if err != nil { return err } + defer refs.Close() - var symRefs []string + type symRefInfo struct { + name plumbing.ReferenceName + target plumbing.ReferenceName + } + var symRefs []symRefInfo + hashRefNames := make(map[plumbing.ReferenceName]bool) hashesSeen := false + // Track the best fallback branch for HEAD in case its target + // doesn't exist (prefer main > master > alphabetically first). + var bestBranch plumbing.ReferenceName for { ref, err := refs.Next() if errors.Cause(err) == io.EOF { @@ -725,6 +749,11 @@ func (r *runner) handleList(ctx context.Context, args []string) (err error) { case plumbing.HashReference: value = ref.Hash().String() hashesSeen = true + hashRefNames[ref.Name()] = true + // Track best branch for fallback HEAD. + if strings.HasPrefix(ref.Name().String(), "refs/heads/") { + bestBranch = bestBranchFromCandidates(bestBranch, ref.Name()) + } case plumbing.SymbolicReference: value = "@" + ref.Target().String() default: @@ -737,7 +766,10 @@ func (r *runner) handleList(ctx context.Context, args []string) (err error) { // cloning an empty repo will result in an error because // the HEAD symbolic ref points to a ref that doesn't // exist. - symRefs = append(symRefs, refStr) + symRefs = append(symRefs, symRefInfo{ + name: ref.Name(), + target: ref.Target(), + }) continue } r.log.CDebugf(ctx, "Listing ref %s", refStr) @@ -748,7 +780,30 @@ func (r *runner) handleList(ctx context.Context, args []string) (err error) { } if hashesSeen && !forPush { - for _, refStr := range symRefs { + for _, sr := range symRefs { + target := sr.target + // If the symref target doesn't exist among hash refs, + // rewrite it to point to the best available branch, + // but only for HEAD. Other symrefs are emitted as-is. + if !hashRefNames[target] { + if sr.name == plumbing.HEAD { + if bestBranch == "" { + r.log.CDebugf(ctx, + "Skipping HEAD symref %s -> %s (no branches available)", + sr.name, target) + continue + } + r.log.CDebugf(ctx, + "Rewriting HEAD symref from %s to %s", + target, bestBranch) + target = bestBranch + } else { + r.log.CDebugf(ctx, + "Emitting non-HEAD symref %s -> %s with unknown target", + sr.name, target) + } + } + refStr := "@" + target.String() + " " + sr.name.String() + "\n" r.log.CDebugf(ctx, "Listing symbolic ref %s", refStr) _, err = r.output.Write([]byte(refStr)) if err != nil { @@ -1830,6 +1885,53 @@ func (r *runner) handlePushBatch(ctx context.Context, args [][]string) ( return nil, err } + // If HEAD points to a nonexistent ref, update it to point to the + // best available branch in the repo (prefer main > master > alphabetical). + // We intentionally repair HEAD even when the target was explicitly + // deleted in this batch, because a broken HEAD causes clone failures. + // This must happen before waitForJournal so the HEAD update is + // included in the same flush. + head, headErr := repo.Storer.Reference(plumbing.HEAD) + if headErr == nil && head.Type() == plumbing.SymbolicReference { + _, targetErr := repo.Storer.Reference(head.Target()) + var bestBranch plumbing.ReferenceName + if targetErr == plumbing.ErrReferenceNotFound { + allRefs, refsErr := repo.References() + if refsErr == nil { + defer allRefs.Close() + for { + ref, nextErr := allRefs.Next() + if errors.Cause(nextErr) == io.EOF { + break + } + if nextErr != nil { + break + } + if ref.Type() != plumbing.HashReference { + continue + } + if !strings.HasPrefix(ref.Name().String(), "refs/heads/") { + continue + } + bestBranch = bestBranchFromCandidates(bestBranch, ref.Name()) + } + } + } + if bestBranch != "" { + newHead := plumbing.NewSymbolicReference( + plumbing.HEAD, bestBranch) + if setErr := repo.Storer.SetReference( + newHead); setErr != nil { + r.log.CDebugf(ctx, + "Error updating HEAD to %s: %+v", + bestBranch, setErr) + } else { + r.log.CDebugf(ctx, + "Updated HEAD to point to %s", bestBranch) + } + } + } + err = r.waitForJournal(ctx) if err != nil { return nil, err diff --git a/go/kbfs/kbfsgit/runner_test.go b/go/kbfs/kbfsgit/runner_test.go index 982994338f1c..3648bb44dbb2 100644 --- a/go/kbfs/kbfsgit/runner_test.go +++ b/go/kbfs/kbfsgit/runner_test.go @@ -27,6 +27,7 @@ import ( "github.com/keybase/client/go/protocol/keybase1" "github.com/stretchr/testify/require" gogitcfg "gopkg.in/src-d/go-git.v4/config" + "gopkg.in/src-d/go-git.v4/plumbing" ) type testErrput struct { @@ -156,7 +157,7 @@ func makeLocalRepoWithOneFileCustomCommitMsg(t *testing.T, gitExec(t, dotgit, gitDir, "init") if branch != "" { - gitExec(t, dotgit, gitDir, "checkout", "-b", branch) + gitExec(t, dotgit, gitDir, "checkout", "-B", branch) } gitExec(t, dotgit, gitDir, "add", filename) @@ -1289,3 +1290,42 @@ func TestRunnerLFS(t *testing.T) { require.NoError(t, err) require.Equal(t, lfsData, buf) } + +// Test that when only a non-master branch (e.g. "main") is pushed, +// HEAD correctly points to that branch instead of the nonexistent +// "refs/heads/master". +func TestRunnerListNonMasterDefault(t *testing.T) { + ctx, config, tempdir := initConfigForRunner(t) + defer func() { _ = os.RemoveAll(tempdir) }() + defer libkbfs.CheckConfigAndShutdown(ctx, t, config) + + gitDir, err := os.MkdirTemp(os.TempDir(), "kbfsgittest") + require.NoError(t, err) + defer func() { _ = os.RemoveAll(gitDir) }() + + makeLocalRepoWithOneFile(t, gitDir, "foo", "hello", "main") + + h, err := tlfhandle.ParseHandle( + ctx, config.KBPKI(), config.MDOps(), config, "user1", tlf.Private) + require.NoError(t, err) + _, err = libgit.CreateRepoAndID(ctx, config, h, "test") + require.NoError(t, err) + + testPush(ctx, t, config, gitDir, + "refs/heads/main:refs/heads/main") + + // Verify the underlying KBFS repo HEAD symref was actually updated. + fs, _, err := libgit.GetRepoAndID(ctx, config, h, "test", "") + require.NoError(t, err) + storage, err := libgit.NewGitConfigWithoutRemotesStorer(fs) + require.NoError(t, err) + headRef, err := storage.Reference(plumbing.HEAD) + require.NoError(t, err) + require.Equal(t, plumbing.SymbolicReference, headRef.Type()) + require.Equal(t, plumbing.ReferenceName("refs/heads/main"), headRef.Target()) + + // List refs and verify HEAD points to refs/heads/main. + heads := testListAndGetHeads(ctx, t, config, gitDir, + []string{"refs/heads/main", "HEAD"}) + require.Equal(t, "@refs/heads/main", heads[1]) +} diff --git a/go/kbfs/libgit/browser.go b/go/kbfs/libgit/browser.go index 8f4c7dd7160c..4b3b9fb5bf22 100644 --- a/go/kbfs/libgit/browser.go +++ b/go/kbfs/libgit/browser.go @@ -57,10 +57,12 @@ type Browser struct { var _ billy.Filesystem = (*Browser)(nil) // NewBrowser makes a new Browser instance, browsing the given branch -// of the given repo. If `gitBranchName` is empty, -// "refs/heads/master" is used. If `gitBranchName` is not empty, but -// it doesn't begin with "refs/", then "refs/heads/" is prepended to -// it. +// of the given repo. If `gitBranchName` is empty, HEAD is resolved +// to determine the default branch; if HEAD is missing or points to a +// nonexistent ref, NewBrowser falls back to "refs/heads/master" if it +// exists, otherwise the repo is treated as empty. If `gitBranchName` +// is not empty but doesn't begin with "refs/", then "refs/heads/" is +// prepended to it. func NewBrowser( repoFS *libfs.FS, clock libkbfs.Clock, gitBranchName plumbing.ReferenceName, @@ -73,6 +75,7 @@ func NewBrowser( } const masterBranch = "refs/heads/master" + branchWasEmpty := gitBranchName == "" if gitBranchName == "" { gitBranchName = masterBranch } else if !strings.HasPrefix(string(gitBranchName), "refs/") { @@ -98,9 +101,23 @@ func NewBrowser( return nil, err } + // If no branch was specified, try to resolve HEAD to find the + // default branch instead of hardcoding master. + if branchWasEmpty { + headRef, headErr := repo.Reference(plumbing.HEAD, false) + if headErr == nil && headRef.Type() == plumbing.SymbolicReference { + gitBranchName = headRef.Target() + } + } + ref, err := repo.Reference(gitBranchName, true) - if err == plumbing.ErrReferenceNotFound && gitBranchName == masterBranch { - // This branch has no commits, so pretend it's empty. + if err == plumbing.ErrReferenceNotFound && branchWasEmpty && gitBranchName != masterBranch { + // HEAD points to a nonexistent ref; fall back to master. + gitBranchName = masterBranch + ref, err = repo.Reference(gitBranchName, true) + } + if err == plumbing.ErrReferenceNotFound && (gitBranchName == masterBranch || branchWasEmpty) { + // No commits on this branch, so pretend it's empty. return &Browser{ root: string(gitBranchName), sharedCache: sharedCache, diff --git a/go/kbfs/libgit/browser_test.go b/go/kbfs/libgit/browser_test.go index 480d9abd8841..5c2120f1333d 100644 --- a/go/kbfs/libgit/browser_test.go +++ b/go/kbfs/libgit/browser_test.go @@ -19,6 +19,7 @@ import ( "github.com/keybase/client/go/protocol/keybase1" "github.com/stretchr/testify/require" gogit "gopkg.in/src-d/go-git.v4" + "gopkg.in/src-d/go-git.v4/plumbing" "gopkg.in/src-d/go-git.v4/plumbing/object" ) @@ -176,3 +177,68 @@ func TestBrowserWithCache(t *testing.T) { require.NoError(t, err) testBrowser(t, cache) } + +func TestBrowserHeadResolution(t *testing.T) { + ctx, config, cancel, tempdir := initConfigForAutogit(t) + defer cancel() + defer func() { _ = os.RemoveAll(tempdir) }() + defer libkbfs.CheckConfigAndShutdown(ctx, t, config) + + h, err := tlfhandle.ParseHandle( + ctx, config.KBPKI(), config.MDOps(), nil, "user1", tlf.Private) + require.NoError(t, err) + rootFS, err := libfs.NewFS( + ctx, config, h, data.MasterBranch, "", "", keybase1.MDPriorityNormal) + require.NoError(t, err) + + t.Log("Init a new repo directly into KBFS.") + dotgitFS, _, err := GetOrCreateRepoAndID(ctx, config, h, "test-head", "") + require.NoError(t, err) + + err = rootFS.MkdirAll("worktree-head", 0o600) + require.NoError(t, err) + worktreeFS, err := rootFS.Chroot("worktree-head") + require.NoError(t, err) + dotgitStorage, err := NewGitConfigWithoutRemotesStorer(dotgitFS) + require.NoError(t, err) + repo, err := gogit.Init(dotgitStorage, worktreeFS) + require.NoError(t, err) + + t.Log("Set HEAD to point to refs/heads/main instead of master.") + newHead := plumbing.NewSymbolicReference( + plumbing.HEAD, "refs/heads/main") + err = repo.Storer.SetReference(newHead) + require.NoError(t, err) + + t.Log("Commit a file — go-git creates the main branch since HEAD points there.") + addFileToWorktreeAndCommit( + ctx, t, config, h, repo, worktreeFS, "hello.txt", "world") + + t.Log("NewBrowser with empty branch should resolve HEAD to main.") + b, err := NewBrowser(dotgitFS, config.Clock(), "", noopSharedInBrowserCache{}) + require.NoError(t, err) + require.NotNil(t, b.tree, "browser should have a non-nil tree") + fi, err := b.Stat("hello.txt") + require.NoError(t, err) + require.Equal(t, "hello.txt", fi.Name()) + + f, err := b.Open("hello.txt") + require.NoError(t, err) + defer func() { _ = f.Close() }() + fileData, err := io.ReadAll(f) + require.NoError(t, err) + require.Equal(t, "world", string(fileData)) + + t.Log("Test stale HEAD: point HEAD to a nonexistent ref.") + staleHead := plumbing.NewSymbolicReference( + plumbing.HEAD, "refs/heads/nonexistent") + err = repo.Storer.SetReference(staleHead) + require.NoError(t, err) + + t.Log("NewBrowser should return an empty browser without error.") + b, err = NewBrowser(dotgitFS, config.Clock(), "", noopSharedInBrowserCache{}) + require.NoError(t, err) + fis, err := b.ReadDir("") + require.NoError(t, err) + require.Len(t, fis, 0) +}