From 15c064abdf57887b25da688e4c0c00cc79cd275e Mon Sep 17 00:00:00 2001 From: Hidde Beydals Date: Tue, 22 Feb 2022 13:53:13 +0100 Subject: [PATCH 1/2] git/libgit2: set CheckoutForce on branch strategy In the recent update from libgit2 1.1.x to 1.3.x, something seems to have changed upstream. Resulting in the clone of a branch ending up with a semi-bare file system state (in other words: without any files present in the directory). This commit patches the clone behavior to set the `CheckoutForce` strategy as `CheckoutOption`, which mitigates the issue. In addition, test cases have been added to ensure we do not run into this again by asserting the state of the branch after cloning. Signed-off-by: Hidde Beydals --- pkg/git/gogit/checkout_test.go | 11 ++++++++++- pkg/git/libgit2/checkout.go | 5 ++++- pkg/git/libgit2/checkout_test.go | 8 ++++++++ 3 files changed, 22 insertions(+), 2 deletions(-) diff --git a/pkg/git/gogit/checkout_test.go b/pkg/git/gogit/checkout_test.go index 946dd9c5d..6307c7ecb 100644 --- a/pkg/git/gogit/checkout_test.go +++ b/pkg/git/gogit/checkout_test.go @@ -58,17 +58,20 @@ func TestCheckoutBranch_Checkout(t *testing.T) { tests := []struct { name string branch string + filesCreated map[string]string expectedCommit string expectedErr string }{ { name: "Default branch", branch: "master", + filesCreated: map[string]string{"branch": "init"}, expectedCommit: firstCommit.String(), }, { name: "Other branch", branch: "test", + filesCreated: map[string]string{"branch": "second"}, expectedCommit: secondCommit.String(), }, { @@ -90,12 +93,18 @@ func TestCheckoutBranch_Checkout(t *testing.T) { cc, err := branch.Checkout(context.TODO(), tmpDir, path, nil) if tt.expectedErr != "" { + g.Expect(err).To(HaveOccurred()) g.Expect(err.Error()).To(ContainSubstring(tt.expectedErr)) g.Expect(cc).To(BeNil()) return } - g.Expect(err).To(BeNil()) + g.Expect(err).ToNot(HaveOccurred()) g.Expect(cc.String()).To(Equal(tt.branch + "/" + tt.expectedCommit)) + + for k, v := range tt.filesCreated { + g.Expect(filepath.Join(tmpDir, k)).To(BeARegularFile()) + g.Expect(os.ReadFile(filepath.Join(tmpDir, k))).To(BeEquivalentTo(v)) + } }) } } diff --git a/pkg/git/libgit2/checkout.go b/pkg/git/libgit2/checkout.go index 9f8a874ae..6732aeb12 100644 --- a/pkg/git/libgit2/checkout.go +++ b/pkg/git/libgit2/checkout.go @@ -66,6 +66,9 @@ func (c *CheckoutBranch) Checkout(ctx context.Context, path, url string, opts *g RemoteCallbacks: RemoteCallbacks(ctx, opts), ProxyOptions: git2go.ProxyOptions{Type: git2go.ProxyTypeAuto}, }, + CheckoutOptions: git2go.CheckoutOptions{ + Strategy: git2go.CheckoutForce, + }, CheckoutBranch: c.Branch, }) if err != nil { @@ -79,7 +82,7 @@ func (c *CheckoutBranch) Checkout(ctx context.Context, path, url string, opts *g defer head.Free() cc, err := repo.LookupCommit(head.Target()) if err != nil { - return nil, fmt.Errorf("could not find commit '%s' in branch '%s': %w", head.Target(), c.Branch, err) + return nil, fmt.Errorf("failed to lookup HEAD commit '%s' for branch '%s': %w", head.Target(), c.Branch, err) } defer cc.Free() return buildCommit(cc, "refs/heads/"+c.Branch), nil diff --git a/pkg/git/libgit2/checkout_test.go b/pkg/git/libgit2/checkout_test.go index ff2f5ccd5..f648eae6f 100644 --- a/pkg/git/libgit2/checkout_test.go +++ b/pkg/git/libgit2/checkout_test.go @@ -73,17 +73,20 @@ func TestCheckoutBranch_Checkout(t *testing.T) { tests := []struct { name string branch string + filesCreated map[string]string expectedCommit string expectedErr string }{ { name: "Default branch", branch: defaultBranch, + filesCreated: map[string]string{"branch": "second"}, expectedCommit: secondCommit.String(), }, { name: "Other branch", branch: "test", + filesCreated: map[string]string{"branch": "init"}, expectedCommit: firstCommit.String(), }, { @@ -112,6 +115,11 @@ func TestCheckoutBranch_Checkout(t *testing.T) { } g.Expect(err).ToNot(HaveOccurred()) g.Expect(cc.String()).To(Equal(tt.branch + "/" + tt.expectedCommit)) + + for k, v := range tt.filesCreated { + g.Expect(filepath.Join(tmpDir, k)).To(BeARegularFile()) + g.Expect(os.ReadFile(filepath.Join(tmpDir, k))).To(BeEquivalentTo(v)) + } }) } } From eff40e22e9d3370fe15a4099bac3275243dd6502 Mon Sep 17 00:00:00 2001 From: Hidde Beydals Date: Tue, 22 Feb 2022 14:13:18 +0100 Subject: [PATCH 2/2] git/libgit2: assert proper test of default branch If there is no configuration set for `init.defaultBranch`, it does not return an error but an empty string. We now take this into account so we do not overwrite the default, and make the default `master` to match with libgit2 defaults. In addition, some comments have been added to not get confused about what commits we are checking against. Signed-off-by: Hidde Beydals --- pkg/git/libgit2/checkout_test.go | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/pkg/git/libgit2/checkout_test.go b/pkg/git/libgit2/checkout_test.go index f648eae6f..3f9e451db 100644 --- a/pkg/git/libgit2/checkout_test.go +++ b/pkg/git/libgit2/checkout_test.go @@ -51,8 +51,8 @@ func TestCheckoutBranch_Checkout(t *testing.T) { // ignores the error here because it can be defaulted // https://github.blog/2020-07-27-highlights-from-git-2-28/#introducing-init-defaultbranch - defaultBranch := "main" - if v, err := cfg.LookupString("init.defaultBranch"); err != nil { + defaultBranch := "master" + if v, err := cfg.LookupString("init.defaultBranch"); err != nil && v != "" { defaultBranch = v } @@ -61,10 +61,12 @@ func TestCheckoutBranch_Checkout(t *testing.T) { t.Fatal(err) } + // Branch off on first commit if err = createBranch(repo, "test", nil); err != nil { t.Fatal(err) } + // Create second commit on default branch secondCommit, err := commitFile(repo, "branch", "second", time.Now()) if err != nil { t.Fatal(err)