diff --git a/pkg/git/libgit2/checkout.go b/pkg/git/libgit2/checkout.go index 1f6bb72d94..7fcfe45128 100644 --- a/pkg/git/libgit2/checkout.go +++ b/pkg/git/libgit2/checkout.go @@ -76,6 +76,7 @@ func (c *CheckoutBranch) Checkout(ctx context.Context, path, url string, auth *g if err != nil { return nil, "", fmt.Errorf("git resolve HEAD error: %w", err) } + defer head.Free() commit, err := repo.LookupCommit(head.Target()) if err != nil { return nil, "", fmt.Errorf("git commit '%s' not found: %w", head.Target(), err) @@ -98,31 +99,12 @@ func (c *CheckoutTag) Checkout(ctx context.Context, path, url string, auth *git. }, }) if err != nil { - return nil, "", fmt.Errorf("unable to clone '%s', error: %w", url, err) - } - ref, err := repo.References.Dwim(c.tag) - if err != nil { - return nil, "", fmt.Errorf("unable to find tag '%s': %w", c.tag, err) - } - err = repo.SetHeadDetached(ref.Target()) - if err != nil { - return nil, "", fmt.Errorf("git checkout error: %w", err) - } - head, err := repo.Head() - if err != nil { - return nil, "", fmt.Errorf("git resolve HEAD error: %w", err) + return nil, "", fmt.Errorf("unable to clone '%s', error: %w", url, gitutil.LibGit2Error(err)) } - commit, err := repo.LookupCommit(head.Target()) + commit, err := checkoutDetachedDwim(repo, c.tag) if err != nil { - return nil, "", fmt.Errorf("git commit '%s' not found: %w", head.Target(), err) - } - err = repo.CheckoutHead(&git2go.CheckoutOptions{ - Strategy: git2go.CheckoutForce, - }) - if err != nil { - return nil, "", fmt.Errorf("git checkout error: %w", err) + return nil, "", err } - return &Commit{commit}, fmt.Sprintf("%s/%s", c.tag, commit.Id().String()), nil } @@ -140,30 +122,19 @@ func (c *CheckoutCommit) Checkout(ctx context.Context, path, url string, auth *g CertificateCheckCallback: auth.CertCallback, }, }, - CheckoutBranch: c.branch, }) if err != nil { - return nil, "", fmt.Errorf("unable to clone '%s', error: %w", url, err) + return nil, "", fmt.Errorf("unable to clone '%s', error: %w", url, gitutil.LibGit2Error(err)) } + oid, err := git2go.NewOid(c.commit) if err != nil { - return nil, "", fmt.Errorf("git commit '%s' could not be parsed", c.commit) - } - commit, err := repo.LookupCommit(oid) - if err != nil { - return nil, "", fmt.Errorf("git commit '%s' not found: %w", c.commit, err) - } - tree, err := repo.LookupTree(commit.TreeId()) - if err != nil { - return nil, "", fmt.Errorf("git worktree error: %w", err) + return nil, "", fmt.Errorf("could not create oid for '%s': %w", c.commit, err) } - err = repo.CheckoutTree(tree, &git2go.CheckoutOptions{ - Strategy: git2go.CheckoutForce, - }) + commit, err := checkoutDetachedHEAD(repo, oid) if err != nil { return nil, "", fmt.Errorf("git checkout error: %w", err) } - return &Commit{commit}, fmt.Sprintf("%s/%s", c.branch, commit.Id().String()), nil } @@ -187,7 +158,7 @@ func (c *CheckoutSemVer) Checkout(ctx context.Context, path, url string, auth *g }, }) if err != nil { - return nil, "", fmt.Errorf("unable to clone '%s', error: %w", url, err) + return nil, "", fmt.Errorf("unable to clone '%s', error: %w", url, gitutil.LibGit2Error(err)) } tags := make(map[string]string) @@ -198,6 +169,7 @@ func (c *CheckoutSemVer) Checkout(ctx context.Context, path, url string, auth *g // Due to this, first attempt to resolve it as a simple tag (commit), but fallback to attempting to // resolve it as an annotated tag in case this results in an error. if c, err := repo.LookupCommit(id); err == nil { + defer c.Free() // Use the commit metadata as the decisive timestamp. tagTimestamps[cleanName] = c.Committer().When tags[cleanName] = name @@ -207,14 +179,17 @@ func (c *CheckoutSemVer) Checkout(ctx context.Context, path, url string, auth *g if err != nil { return fmt.Errorf("could not lookup '%s' as simple or annotated tag: %w", cleanName, err) } + defer t.Free() commit, err := t.Peel(git2go.ObjectCommit) if err != nil { return fmt.Errorf("could not get commit for tag '%s': %w", t.Name(), err) } + defer commit.Free() c, err := commit.AsCommit() if err != nil { return fmt.Errorf("could not get commit object for tag '%s': %w", t.Name(), err) } + defer c.Free() tagTimestamps[t.Name()] = c.Committer().When tags[t.Name()] = name return nil @@ -255,28 +230,62 @@ func (c *CheckoutSemVer) Checkout(ctx context.Context, path, url string, auth *g v := matchedVersions[len(matchedVersions)-1] t := v.Original() - ref, err := repo.References.Dwim(t) + commit, err := checkoutDetachedDwim(repo, t) + return &Commit{commit}, fmt.Sprintf("%s/%s", t, commit.Id().String()), nil +} + +// checkoutDetachedDwim attempts to perform a detached HEAD checkout by first DWIMing the short name +// to get a concrete reference, and then calling checkoutDetachedHEAD. +func checkoutDetachedDwim(repo *git2go.Repository, name string) (*git2go.Commit, error) { + ref, err := repo.References.Dwim(name) if err != nil { - return nil, "", fmt.Errorf("unable to find tag '%s': %w", t, err) + return nil, fmt.Errorf("unable to find '%s': %w", name, err) } - err = repo.SetHeadDetached(ref.Target()) + defer ref.Free() + c, err := ref.Peel(git2go.ObjectCommit) if err != nil { - return nil, "", fmt.Errorf("git checkout error: %w", err) + return nil, fmt.Errorf("could not get commit for ref '%s': %w", ref.Name(), err) } - head, err := repo.Head() + defer c.Free() + commit, err := c.AsCommit() if err != nil { - return nil, "", fmt.Errorf("git resolve HEAD error: %w", err) + return nil, fmt.Errorf("could not get commit object for ref '%s': %w", ref.Name(), err) } - commit, err := repo.LookupCommit(head.Target()) + defer commit.Free() + return checkoutDetachedHEAD(repo, commit.Id()) +} + +// checkoutDetachedHEAD attempts to perform a detached HEAD checkout for the given commit. +func checkoutDetachedHEAD(repo *git2go.Repository, oid *git2go.Oid) (*git2go.Commit, error) { + commit, err := repo.LookupCommit(oid) if err != nil { - return nil, "", fmt.Errorf("git commit '%s' not found: %w", head.Target().String(), err) + return nil, fmt.Errorf("git commit '%s' not found: %w", oid.String(), err) } - err = repo.CheckoutHead(&git2go.CheckoutOptions{ + if err = repo.SetHeadDetached(commit.Id()); err != nil { + commit.Free() + return nil, fmt.Errorf("could not detach HEAD at '%s': %w", oid.String(), err) + } + if err = repo.CheckoutHead(&git2go.CheckoutOptions{ Strategy: git2go.CheckoutForce, - }) + }); err != nil { + commit.Free() + return nil, fmt.Errorf("git checkout error: %w", err) + } + return commit, nil +} + +// headCommit returns the current HEAD of the repository, or an error. +func headCommit(repo *git2go.Repository) (*git2go.Commit, error) { + head, err := repo.Head() if err != nil { - return nil, "", fmt.Errorf("git checkout error: %w", err) + return nil, err } + defer head.Free() - return &Commit{commit}, fmt.Sprintf("%s/%s", t, commit.Id().String()), nil + commit, err := repo.LookupCommit(head.Target()) + if err != nil { + return nil, err + } + + return commit, nil } diff --git a/pkg/git/libgit2/checkout_test.go b/pkg/git/libgit2/checkout_test.go index 9772b2d048..5ad2b25234 100644 --- a/pkg/git/libgit2/checkout_test.go +++ b/pkg/git/libgit2/checkout_test.go @@ -31,59 +31,239 @@ import ( "github.com/fluxcd/source-controller/pkg/git" ) +func TestCheckoutBranch_Checkout(t *testing.T) { + repo, err := initBareRepo() + if err != nil { + t.Fatal(err) + } + + firstCommit, err := commitFile(repo, "branch", "init", time.Now()) + if err != nil { + t.Fatal(err) + } + + if err = createBranch(repo, "test", nil); err != nil { + t.Fatal(err) + } + + secondCommit, err := commitFile(repo, "branch", "second", time.Now()) + if err != nil { + t.Fatal(err) + } + + tests := []struct { + name string + branch string + expectedCommit string + expectedErr string + }{ + { + name: "Default branch", + branch: "master", + expectedCommit: secondCommit.String(), + }, + { + name: "Other branch", + branch: "test", + expectedCommit: firstCommit.String(), + }, + { + name: "Non existing branch", + branch: "invalid", + expectedErr: "reference 'refs/remotes/origin/invalid' not found", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + g := NewWithT(t) + + branch := CheckoutBranch{ + branch: tt.branch, + } + tmpDir, _ := os.MkdirTemp("", "test") + defer os.RemoveAll(tmpDir) + + _, ref, err := branch.Checkout(context.TODO(), tmpDir, repo.Path(), &git.Auth{}) + if tt.expectedErr != "" { + g.Expect(err.Error()).To(ContainSubstring(tt.expectedErr)) + g.Expect(ref).To(BeEmpty()) + return + } + g.Expect(ref).To(Equal(tt.branch + "/" + tt.expectedCommit)) + g.Expect(err).To(BeNil()) + }) + } +} + +func TestCheckoutTag_Checkout(t *testing.T) { + tests := []struct { + name string + tag string + annotated bool + checkoutTag string + expectTag string + expectErr string + }{ + { + name: "Tag", + tag: "tag-1", + checkoutTag: "tag-1", + expectTag: "tag-1", + }, + { + name: "Annotated", + tag: "annotated", + annotated: true, + checkoutTag: "annotated", + expectTag: "annotated", + }, + { + name: "Non existing tag", + checkoutTag: "invalid", + expectErr: "unable to find 'invalid': no reference found for shorthand 'invalid'", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + g := NewWithT(t) + + repo, err := initBareRepo() + if err != nil { + t.Fatal(err) + } + + var commit *git2go.Commit + if tt.tag != "" { + c, err := commitFile(repo, "tag", tt.tag, time.Now()) + if err != nil { + t.Fatal(err) + } + if commit, err = repo.LookupCommit(c); err != nil { + t.Fatal(err) + } + _, err = tag(repo, c, !tt.annotated, tt.tag, time.Now()) + if err != nil { + t.Fatal(err) + } + } + + tag := CheckoutTag{ + tag: tt.checkoutTag, + } + tmpDir, _ := os.MkdirTemp("", "test") + defer os.RemoveAll(tmpDir) + + _, ref, err := tag.Checkout(context.TODO(), tmpDir, repo.Path(), &git.Auth{}) + if tt.expectErr != "" { + g.Expect(err.Error()).To(Equal(tt.expectErr)) + g.Expect(ref).To(BeEmpty()) + return + } + if tt.expectTag != "" { + g.Expect(ref).To(Equal(tt.expectTag + "/" + commit.Id().String())) + g.Expect(filepath.Join(tmpDir, "tag")).To(BeARegularFile()) + g.Expect(os.ReadFile(filepath.Join(tmpDir, "tag"))).To(BeEquivalentTo(tt.tag)) + } + }) + } +} + +func TestCheckoutCommit_Checkout(t *testing.T) { + g := NewWithT(t) + + repo, err := initBareRepo() + if err != nil { + t.Fatal(err) + } + defer repo.Free() + defer os.RemoveAll(repo.Path()) + + c, err := commitFile(repo, "commit", "init", time.Now()) + if err != nil { + t.Fatal(err) + } + if _, err = commitFile(repo, "commit", "second", time.Now()); err != nil { + t.Fatal(err) + } + + commit := CheckoutCommit{ + commit: c.String(), + branch: "main", + } + tmpDir, _ := os.MkdirTemp("", "git2go") + defer os.RemoveAll(tmpDir) + + _, ref, err := commit.Checkout(context.TODO(), tmpDir, repo.Path(), &git.Auth{}) + g.Expect(err).To(BeNil()) + g.Expect(ref).To(Equal("main/" + c.String())) + g.Expect(filepath.Join(tmpDir, "commit")).To(BeARegularFile()) + g.Expect(os.ReadFile(filepath.Join(tmpDir, "commit"))).To(BeEquivalentTo("init")) + + commit = CheckoutCommit{ + commit: "4dc3185c5fc94eb75048376edeb44571cece25f4", + } + tmpDir2, _ := os.MkdirTemp("", "git2go") + defer os.RemoveAll(tmpDir) + + _, ref, err = commit.Checkout(context.TODO(), tmpDir2, repo.Path(), &git.Auth{}) + g.Expect(err.Error()).To(HavePrefix("git checkout error: git commit '4dc3185c5fc94eb75048376edeb44571cece25f4' not found:")) + g.Expect(ref).To(BeEmpty()) +} + func TestCheckoutTagSemVer_Checkout(t *testing.T) { g := NewWithT(t) now := time.Now() - tags := []struct{ + tags := []struct { tag string - simple bool + annotated bool commitTime time.Time tagTime time.Time }{ { - tag: "v0.0.1", - simple: true, + tag: "v0.0.1", + annotated: false, commitTime: now, }, { - tag: "v0.1.0+build-1", - simple: false, + tag: "v0.1.0+build-1", + annotated: true, commitTime: now.Add(1 * time.Minute), - tagTime: now.Add(1 * time.Hour), // This should be ignored during TS comparisons + tagTime: now.Add(1 * time.Hour), // This should be ignored during TS comparisons }, { - tag: "v0.1.0+build-2", - simple: true, + tag: "v0.1.0+build-2", + annotated: false, commitTime: now.Add(2 * time.Minute), }, { - tag: "0.2.0", - simple: false, + tag: "0.2.0", + annotated: true, commitTime: now, - tagTime: now, + tagTime: now, }, } - tests := []struct{ - name string - constraint string - expectError error - expectTag string + tests := []struct { + name string + constraint string + expectErr error + expectTag string }{ { - name: "Orders by SemVer", + name: "Orders by SemVer", constraint: ">0.1.0", - expectTag: "0.2.0", + expectTag: "0.2.0", }, { - name: "Orders by SemVer and timestamp", + name: "Orders by SemVer and timestamp", constraint: "<0.2.0", - expectTag: "v0.1.0+build-2", + expectTag: "v0.1.0+build-2", }, { - name: "Errors without match", + name: "Errors without match", constraint: ">=1.0.0", - expectError: errors.New("no match found for semver: >=1.0.0"), + expectErr: errors.New("no match found for semver: >=1.0.0"), }, } @@ -94,12 +274,19 @@ func TestCheckoutTagSemVer_Checkout(t *testing.T) { defer repo.Free() defer os.RemoveAll(repo.Path()) + refs := make(map[string]string, len(tags)) for _, tt := range tags { - cId, err := commit(repo, "tag.txt", tt.tag, tt.commitTime) + ref, err := commitFile(repo, "tag", tt.tag, tt.commitTime) + if err != nil { + t.Fatal(err) + } + commit, err := repo.LookupCommit(ref) if err != nil { t.Fatal(err) } - _, err = tag(repo, cId, tt.simple, tt.tag, tt.tagTime) + defer commit.Free() + refs[tt.tag] = commit.Id().String() + _, err = tag(repo, ref, tt.annotated, tt.tag, tt.tagTime) if err != nil { t.Fatal(err) } @@ -111,6 +298,8 @@ func TestCheckoutTagSemVer_Checkout(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { + g := NewWithT(t) + semVer := CheckoutSemVer{ semVer: tt.constraint, } @@ -118,16 +307,15 @@ func TestCheckoutTagSemVer_Checkout(t *testing.T) { defer os.RemoveAll(tmpDir) _, ref, err := semVer.Checkout(context.TODO(), tmpDir, repo.Path(), &git.Auth{}) - if tt.expectError != nil { - g.Expect(err).To(Equal(tt.expectError)) + if tt.expectErr != nil { + g.Expect(err).To(Equal(tt.expectErr)) g.Expect(ref).To(BeEmpty()) return } g.Expect(err).ToNot(HaveOccurred()) - g.Expect(ref).To(HavePrefix(tt.expectTag + "/")) - content, err := os.ReadFile(filepath.Join(tmpDir, "tag.txt")) - g.Expect(err).ToNot(HaveOccurred()) - g.Expect(content).To(BeEquivalentTo(tt.expectTag)) + g.Expect(ref).To(Equal(tt.expectTag + "/" + refs[tt.expectTag])) + g.Expect(filepath.Join(tmpDir, "tag")).To(BeARegularFile()) + g.Expect(os.ReadFile(filepath.Join(tmpDir, "tag"))).To(BeEquivalentTo(tt.expectTag)) }) } } @@ -145,22 +333,20 @@ func initBareRepo() (*git2go.Repository, error) { return repo, nil } -func headCommit(repo *git2go.Repository) (*git2go.Commit, error) { - head, err := repo.Head() - if err != nil { - return nil, err - } - defer head.Free() - - commit, err := repo.LookupCommit(head.Target()) - if err != nil { - return nil, err +func createBranch(repo *git2go.Repository, branch string, commit *git2go.Commit) error { + if commit == nil { + var err error + commit, err = headCommit(repo) + if err != nil { + return err + } + defer commit.Free() } - - return commit, nil + _, err := repo.CreateBranch(branch, commit, false) + return err } -func commit(repo *git2go.Repository, path, content string, time time.Time) (*git2go.Oid, error) { +func commitFile(repo *git2go.Repository, path, content string, time time.Time) (*git2go.Oid, error) { var parentC []*git2go.Commit head, err := headCommit(repo) if err == nil { @@ -192,12 +378,12 @@ func commit(repo *git2go.Repository, path, content string, time time.Time) (*git return nil, err } - newTreeOID, err := index.WriteTree() + treeID, err := index.WriteTree() if err != nil { return nil, err } - tree, err := repo.LookupTree(newTreeOID) + tree, err := repo.LookupTree(treeID) if err != nil { return nil, err } @@ -207,19 +393,18 @@ func commit(repo *git2go.Repository, path, content string, time time.Time) (*git if err != nil { return nil, err } - return commit, nil } -func tag(repo *git2go.Repository, cId *git2go.Oid, simple bool, tag string, time time.Time) (*git2go.Oid, error) { +func tag(repo *git2go.Repository, cId *git2go.Oid, annotated bool, tag string, time time.Time) (*git2go.Oid, error) { commit, err := repo.LookupCommit(cId) if err != nil { return nil, err } - if simple { - return repo.Tags.CreateLightweight(tag, commit, false) + if annotated { + return repo.Tags.Create(tag, commit, signature(time), fmt.Sprintf("Annotated tag for %s", tag)) } - return repo.Tags.Create(tag, commit, signature(time), fmt.Sprintf("Annotated tag for %s", tag)) + return repo.Tags.CreateLightweight(tag, commit, false) } func signature(time time.Time) *git2go.Signature {