Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
108 changes: 105 additions & 3 deletions go/kbfs/kbfsgit/runner.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 "<value> <name> [<attr>
Expand All @@ -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 {
Expand All @@ -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:
Expand All @@ -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)
Expand All @@ -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 {
Expand Down Expand Up @@ -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())
if targetErr == plumbing.ErrReferenceNotFound {
var bestBranch plumbing.ReferenceName
allRefs, refsErr := repo.References()
if refsErr == nil {
defer allRefs.Close()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

move to before the error check?

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
Expand Down
42 changes: 41 additions & 1 deletion go/kbfs/kbfsgit/runner_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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])
}
29 changes: 23 additions & 6 deletions go/kbfs/libgit/browser.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -73,6 +75,7 @@ func NewBrowser(
}

const masterBranch = "refs/heads/master"
branchWasEmpty := gitBranchName == ""
if gitBranchName == "" {
gitBranchName = masterBranch
} else if !strings.HasPrefix(string(gitBranchName), "refs/") {
Expand All @@ -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,
Expand Down
66 changes: 66 additions & 0 deletions go/kbfs/libgit/browser_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)

Expand Down Expand Up @@ -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)
}
Loading