Skip to content

Commit

Permalink
gogit: Add WithSingleBranch
Browse files Browse the repository at this point in the history 
At present go-git does not support the MULTI_ACK capability, which
means that follow-up fetches on a given remote will fail.

To support Image Automation Controller use cases, the SwitchBranch
was initially short-circuited to avoid additional fetches. However,
this has the side effect of the controller pushing the same change
to the target repository multiple times. (fluxcd/flux2#3384)

In order to avoid this, a new WithSingleBranch option was created
to enable the download of all references at the initial clone.
From now on SwitchBranch has the single responsibility of switching
branches, and no longer pulling references.

The package git/gogit's primary goal is to support Flux use cases,
currently there is no need to expand the current API to expose ways
for users to refresh repository references outside the initial clone.

Signed-off-by: Paulo Gomes <[email protected]>
  • Loading branch information
Paulo Gomes committed Dec 16, 2022
1 parent 01e30ef commit 3d0a4a3
Show file tree
Hide file tree
Showing 3 changed files with 204 additions and 58 deletions.
120 changes: 79 additions & 41 deletions git/gogit/client.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ type Client struct {
forcePush bool
credentialsOverHTTP bool
useDefaultKnownHosts bool
singleBranch bool
}

var _ repository.Client = &Client{}
Expand All @@ -71,6 +72,8 @@ func NewClient(path string, authOpts *git.AuthOptions, clientOpts ...ClientOptio
g := &Client{
path: securePath,
authOpts: authOpts,
// Default to single branch as it is the most performant option.
singleBranch: true,
}

if len(clientOpts) == 0 {
Expand Down Expand Up @@ -107,6 +110,25 @@ func WithWorkTreeFS(wt billy.Filesystem) ClientOption {
}
}

// WithSingleBranch indicates whether only the references of a single
// branch will be fetched during cloning operations.
// For read-only clones, and for single branch write operations,
// a single branch is advised for performance reasons.
//
// For write operations that require multiple branches, for example,
// cloning from main and pushing into a feature branch, this should be
// disabled. Otherwise a second fetch will be required to get the state
// of the target branch, which won't work against some Git servers due
// to MULTI_ACK not being implemented in go-git.
//
// By default this is enabled.
func WithSingleBranch(singleBranch bool) ClientOption {
return func(c *Client) error {
c.singleBranch = singleBranch
return nil
}
}

func WithDiskStorage() ClientOption {
return func(c *Client) error {
wt := fs.New(c.path)
Expand Down Expand Up @@ -337,6 +359,32 @@ func (g *Client) Push(ctx context.Context) error {
})
}

// SwitchBranch switches the current branch to the given branch name.
//
// No new references are fetched from the remote during the process,
// this is to ensure that the same flow can be used across all Git
// servers, regardless of them requiring MULTI_ACK or not. Once MULTI_ACK
// is implemented in go-git, this can be revisited.
//
// If more than one remote branch state is required, create the gogit
// client using WithSingleBranch(false). This will fetch all remote
// branches as part of the initial clone. Note that this is fully
// compatible with shallow clones.
//
// The following cases are handled:
// - Branch does not exist results in one being created using HEAD
// of the worktree.
// - Branch exists only remotely, results in a local branch being
// created tracking the remote HEAD.
// - Branch exists only locally, results in a checkout to the
// existing branch.
// - Branch exists locally and remotely, the local branch will take
// precendece.
//
// To override a remote branch with the state from the current branch,
// (i.e. image automation controller), use WithForcePush(true) in
// combination with WithSingleBranch(true). This will ignore the
// remote branch's existence.
func (g *Client) SwitchBranch(ctx context.Context, branchName string) error {
if g.repository == nil {
return git.ErrNoGitRepository
Expand All @@ -346,59 +394,49 @@ func (g *Client) SwitchBranch(ctx context.Context, branchName string) error {
if err != nil {
return fmt.Errorf("failed to load worktree: %w", err)
}
authMethod, err := transportAuth(g.authOpts, g.useDefaultKnownHosts)
if err != nil {
return fmt.Errorf("failed to construct auth method with options: %w", err)
}

_, err = g.repository.Branch(branchName)
var create bool
if err == extgogit.ErrBranchNotFound {
create = true
// Assumes both local and remote branches exists until proven otherwise.
remote, local := true, true
remRefName := plumbing.NewRemoteReferenceName(extgogit.DefaultRemoteName, branchName)
remRef, err := g.repository.Reference(remRefName, true)
if errors.Is(err, plumbing.ErrReferenceNotFound) {
remote = false
} else if err != nil {
return err
}

err = wt.Checkout(&extgogit.CheckoutOptions{
Branch: plumbing.NewBranchReferenceName(branchName),
Create: create,
})
if err != nil {
return fmt.Errorf("could not checkout to branch '%s': %w", branchName, err)
return fmt.Errorf("could not fetch remote reference '%s': %w", branchName, err)
}

// When force push is enabled, we always override the push branch.
// No need to fetch additional refs from that branch.
if g.forcePush {
return nil
refName := plumbing.NewBranchReferenceName(branchName)
_, err = g.repository.Reference(refName, true)
if errors.Is(err, plumbing.ErrReferenceNotFound) {
local = false
} else if err != nil {
return fmt.Errorf("could not fetch local reference '%s': %w", branchName, err)
}

err = g.repository.FetchContext(ctx, &extgogit.FetchOptions{
RemoteName: extgogit.DefaultRemoteName,
RefSpecs: []config.RefSpec{
config.RefSpec(fmt.Sprintf("+refs/heads/%s:refs/remotes/%s/%[1]s", branchName, extgogit.DefaultRemoteName)),
},
Auth: authMethod,
})
if err != nil && !errors.Is(err, extgogit.NoErrAlreadyUpToDate) && !errors.Is(err, extgogit.NoMatchingRefSpecError{}) {
return fmt.Errorf("could not fetch context: %w", err)
}
ref, err := g.repository.Reference(plumbing.NewRemoteReferenceName(extgogit.DefaultRemoteName, branchName), true)
create := false
// If the remote branch exists, but not the local branch, create a local
// reference to the remote's HEAD.
if remote && !local {
branchRef := plumbing.NewHashReference(refName, remRef.Hash())

// If remote ref doesn't exist, no need to reset to remote target commit, exit early.
if err == plumbing.ErrReferenceNotFound {
return nil
} else if err != nil {
return fmt.Errorf("could not fetch remote reference '%s': %w", branchName, err)
err = g.repository.Storer.SetReference(branchRef)
if err != nil {
return fmt.Errorf("could not create reference to remote HEAD '%s': %w", branchRef.Hash().String(), err)
}
} else if !remote && !local {
// If the target branch does not exist locally or remotely, create a new
// branch using the current worktree HEAD.
create = true
}

err = wt.Reset(&extgogit.ResetOptions{
Commit: ref.Hash(),
Mode: extgogit.HardReset,
err = wt.Checkout(&extgogit.CheckoutOptions{
Branch: refName,
Create: create,
})
if err != nil {
return fmt.Errorf("could not reset branch to be at commit '%s': %w", ref.Hash().String(), err)
return fmt.Errorf("could not checkout to branch '%s': %w", branchName, err)
}

return nil
}

Expand Down
136 changes: 122 additions & 14 deletions git/gogit/client_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -307,10 +307,12 @@ func TestForcePush(t *testing.T) {

func TestSwitchBranch(t *testing.T) {
tests := []struct {
name string
setupFunc func(g *WithT, path string) string
branch string
forcePush bool
name string
setupFunc func(g *WithT, path string) string
changeRepo func(g *WithT, c *Client) string
branch string
forcePush bool
singleBranch bool
}{
{
name: "switch to a branch ahead of the current branch",
Expand All @@ -336,6 +338,79 @@ func TestSwitchBranch(t *testing.T) {
},
branch: "ahead",
},
{
name: "switch to a branch that exists locally and remotely",
setupFunc: func(g *WithT, repoURL string) string {
tmp := t.TempDir()
repo, err := extgogit.PlainClone(tmp, false, &extgogit.CloneOptions{
URL: repoURL,
ReferenceName: plumbing.NewBranchReferenceName(git.DefaultBranch),
RemoteName: git.DefaultRemote,
})
g.Expect(err).ToNot(HaveOccurred())

err = createBranch(repo, "ahead")
g.Expect(err).ToNot(HaveOccurred())

cc, err := commitFile(repo, "test", "I live in the remote branch", time.Now())
g.Expect(err).ToNot(HaveOccurred())
err = repo.Push(&extgogit.PushOptions{
RemoteName: git.DefaultRemote,
})
g.Expect(err).ToNot(HaveOccurred())
return cc.String()
},
changeRepo: func(g *WithT, c *Client) string {
wt, err := c.repository.Worktree()
g.Expect(err).ToNot(HaveOccurred())

err = wt.Checkout(&extgogit.CheckoutOptions{
Branch: plumbing.NewBranchReferenceName("ahead"),
Create: true,
})
g.Expect(err).ToNot(HaveOccurred())

cc, err := commitFile(c.repository, "new change", "local branch is warmer though", time.Now())
g.Expect(err).ToNot(HaveOccurred())

err = wt.Checkout(&extgogit.CheckoutOptions{
Branch: plumbing.Master,
})
g.Expect(err).ToNot(HaveOccurred())

return cc.String()
},
branch: "ahead",
},
{
name: "singlebranch: ignore a branch that exists in the remote",
setupFunc: func(g *WithT, repoURL string) string {
tmp := t.TempDir()
repo, err := extgogit.PlainClone(tmp, false, &extgogit.CloneOptions{
URL: repoURL,
ReferenceName: plumbing.NewBranchReferenceName(git.DefaultBranch),
RemoteName: git.DefaultRemote,
})
g.Expect(err).ToNot(HaveOccurred())

head, err := repo.Head()
g.Expect(err).ToNot(HaveOccurred())

err = createBranch(repo, "singlebranch-ahead")
g.Expect(err).ToNot(HaveOccurred())

_, err = commitFile(repo, "test", "I am going to be treated as stale", time.Now())
g.Expect(err).ToNot(HaveOccurred())
err = repo.Push(&extgogit.PushOptions{
RemoteName: git.DefaultRemote,
})
g.Expect(err).ToNot(HaveOccurred())

return head.Hash().String()
},
branch: "singlebranch-ahead",
singleBranch: true,
},
{
name: "switch to a branch behind the current branch",
setupFunc: func(g *WithT, repoURL string) string {
Expand Down Expand Up @@ -387,20 +462,16 @@ func TestSwitchBranch(t *testing.T) {
})
g.Expect(err).ToNot(HaveOccurred())

ref, err := repo.Head()
g.Expect(err).ToNot(HaveOccurred())
hash := ref.Hash().String()

err = createBranch(repo, "ahead")
g.Expect(err).ToNot(HaveOccurred())

_, err = commitFile(repo, "test", "testing gogit switch ahead branch", time.Now())
cc, err := commitFile(repo, "test", "testing gogit switch ahead branch", time.Now())
g.Expect(err).ToNot(HaveOccurred())
err = repo.Push(&extgogit.PushOptions{
RemoteName: git.DefaultRemote,
})
g.Expect(err).ToNot(HaveOccurred())
return hash
return cc.String()
},
branch: "ahead",
forcePush: true,
Expand Down Expand Up @@ -447,6 +518,36 @@ func TestSwitchBranch(t *testing.T) {
branch: "new",
forcePush: true,
},
{
name: "force: ignore a branch that exists in the remote",
setupFunc: func(g *WithT, repoURL string) string {
tmp := t.TempDir()
repo, err := extgogit.PlainClone(tmp, false, &extgogit.CloneOptions{
URL: repoURL,
ReferenceName: plumbing.NewBranchReferenceName(git.DefaultBranch),
RemoteName: git.DefaultRemote,
})
g.Expect(err).ToNot(HaveOccurred())

head, err := repo.Head()
g.Expect(err).ToNot(HaveOccurred())

err = createBranch(repo, "singlebranch-ahead")
g.Expect(err).ToNot(HaveOccurred())

_, err = commitFile(repo, "test", "remote change that will be overwritten", time.Now())
g.Expect(err).ToNot(HaveOccurred())
err = repo.Push(&extgogit.PushOptions{
RemoteName: git.DefaultRemote,
})
g.Expect(err).ToNot(HaveOccurred())

return head.Hash().String()
},
branch: "singlebranch-ahead",
singleBranch: true,
forcePush: true,
},
}

for _, tt := range tests {
Expand All @@ -464,19 +565,22 @@ func TestSwitchBranch(t *testing.T) {
g.Expect(err).ToNot(HaveOccurred())
defer server.StopHTTP()

var expectedHash string
if tt.setupFunc != nil {
expectedHash = tt.setupFunc(g, filepath.Join(server.Root(), "test.git"))
}

repoURL := server.HTTPAddressWithCredentials() + "/" + "test.git"
tmp := t.TempDir()
repo, err := extgogit.PlainClone(tmp, false, &extgogit.CloneOptions{
URL: repoURL,
ReferenceName: plumbing.NewBranchReferenceName(git.DefaultBranch),
RemoteName: git.DefaultRemote,
SingleBranch: tt.singleBranch,
})
g.Expect(err).ToNot(HaveOccurred())

var expectedHash string
if tt.setupFunc != nil {
expectedHash = tt.setupFunc(g, filepath.Join(server.Root(), "test.git"))
} else {
if tt.setupFunc == nil {
head, err := repo.Head()
g.Expect(err).ToNot(HaveOccurred())
expectedHash = head.Hash().String()
Expand All @@ -487,6 +591,10 @@ func TestSwitchBranch(t *testing.T) {
ggc.repository = repo
ggc.forcePush = tt.forcePush

if tt.changeRepo != nil {
expectedHash = tt.changeRepo(g, ggc)
}

err = ggc.SwitchBranch(context.TODO(), tt.branch)
g.Expect(err).ToNot(HaveOccurred())

Expand Down
6 changes: 3 additions & 3 deletions git/gogit/clone.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ func (g *Client) cloneBranch(ctx context.Context, url, branch string, opts repos
Auth: authMethod,
RemoteName: git.DefaultRemote,
ReferenceName: plumbing.NewBranchReferenceName(branch),
SingleBranch: true,
SingleBranch: g.singleBranch,
NoCheckout: false,
Depth: depth,
RecurseSubmodules: recurseSubmodules(opts.RecurseSubmodules),
Expand Down Expand Up @@ -173,7 +173,7 @@ func (g *Client) cloneTag(ctx context.Context, url, tag string, opts repository.
Auth: authMethod,
RemoteName: git.DefaultRemote,
ReferenceName: plumbing.NewTagReferenceName(tag),
SingleBranch: true,
SingleBranch: g.singleBranch,
NoCheckout: false,
Depth: depth,
RecurseSubmodules: recurseSubmodules(opts.RecurseSubmodules),
Expand Down Expand Up @@ -222,7 +222,7 @@ func (g *Client) cloneCommit(ctx context.Context, url, commit string, opts repos
CABundle: caBundle(g.authOpts),
}
if opts.Branch != "" {
cloneOpts.SingleBranch = true
cloneOpts.SingleBranch = g.singleBranch
cloneOpts.ReferenceName = plumbing.NewBranchReferenceName(opts.Branch)
}

Expand Down

0 comments on commit 3d0a4a3

Please sign in to comment.