diff --git a/pkg/git/libgit2/checkout.go b/pkg/git/libgit2/checkout.go index 43a64f07..b2ab16c2 100644 --- a/pkg/git/libgit2/checkout.go +++ b/pkg/git/libgit2/checkout.go @@ -64,7 +64,9 @@ type CheckoutBranch struct { LastRevision string } -func (c *CheckoutBranch) Checkout(ctx context.Context, path, url string, opts *git.AuthOptions) (*git.Commit, error) { +func (c *CheckoutBranch) Checkout(ctx context.Context, path, url string, opts *git.AuthOptions) (_ *git.Commit, err error) { + defer recoverPanic(&err) + repo, remote, err := getBlankRepoAndRemote(ctx, path, url, opts) if err != nil { @@ -149,7 +151,9 @@ type CheckoutTag struct { LastRevision string } -func (c *CheckoutTag) Checkout(ctx context.Context, path, url string, opts *git.AuthOptions) (*git.Commit, error) { +func (c *CheckoutTag) Checkout(ctx context.Context, path, url string, opts *git.AuthOptions) (_ *git.Commit, err error) { + defer recoverPanic(&err) + repo, remote, err := getBlankRepoAndRemote(ctx, path, url, opts) if err != nil { @@ -210,8 +214,10 @@ type CheckoutCommit struct { Commit string } -func (c *CheckoutCommit) Checkout(ctx context.Context, path, url string, opts *git.AuthOptions) (*git.Commit, error) { - repo, err := safeClone(url, path, &git2go.CloneOptions{ +func (c *CheckoutCommit) Checkout(ctx context.Context, path, url string, opts *git.AuthOptions) (_ *git.Commit, err error) { + defer recoverPanic(&err) + + repo, err := git2go.Clone(url, path, &git2go.CloneOptions{ FetchOptions: git2go.FetchOptions{ DownloadTags: git2go.DownloadTagsNone, RemoteCallbacks: RemoteCallbacks(ctx, opts), @@ -237,13 +243,15 @@ type CheckoutSemVer struct { SemVer string } -func (c *CheckoutSemVer) Checkout(ctx context.Context, path, url string, opts *git.AuthOptions) (*git.Commit, error) { +func (c *CheckoutSemVer) Checkout(ctx context.Context, path, url string, opts *git.AuthOptions) (_ *git.Commit, err error) { + defer recoverPanic(&err) + verConstraint, err := semver.NewConstraint(c.SemVer) if err != nil { return nil, fmt.Errorf("semver parse error: %w", err) } - repo, err := safeClone(url, path, &git2go.CloneOptions{ + repo, err := git2go.Clone(url, path, &git2go.CloneOptions{ FetchOptions: git2go.FetchOptions{ DownloadTags: git2go.DownloadTagsAll, RemoteCallbacks: RemoteCallbacks(ctx, opts), @@ -332,19 +340,6 @@ func (c *CheckoutSemVer) Checkout(ctx context.Context, path, url string, opts *g return buildCommit(cc, "refs/tags/"+t), nil } -// safeClone wraps git2go calls with panic recovering logic, ensuring -// a predictable execution path for callers. -func safeClone(url, path string, cloneOpts *git2go.CloneOptions) (repo *git2go.Repository, err error) { - defer func() { - if r := recover(); r != nil { - err = fmt.Errorf("recovered from git2go panic: %v", r) - } - }() - - repo, err = git2go.Clone(url, path, cloneOpts) - return -} - // 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) { @@ -443,3 +438,9 @@ func getBlankRepoAndRemote(ctx context.Context, path, url string, opts *git.Auth } return repo, remote, nil } + +func recoverPanic(err *error) { + if r := recover(); r != nil { + *err = fmt.Errorf("recovered from git2go panic: %v", r) + } +} diff --git a/pkg/git/libgit2/checkout_test.go b/pkg/git/libgit2/checkout_test.go index ec605362..98b57b24 100644 --- a/pkg/git/libgit2/checkout_test.go +++ b/pkg/git/libgit2/checkout_test.go @@ -581,42 +581,3 @@ func TestCheckout_ED25519(t *testing.T) { _, err = branchCheckoutStrat.Checkout(ctx, tmpDir, repoURL, authOpts) g.Expect(err).ToNot(HaveOccurred()) } - -func TestSafeClone(t *testing.T) { - g := NewWithT(t) - - // Create a git test server. - server, err := gittestserver.NewTempGitServer() - g.Expect(err).ToNot(HaveOccurred()) - defer os.RemoveAll(server.Root()) - server.Auth("test-user", "test-pswd") - server.AutoCreate() - - server.KeyDir(filepath.Join(server.Root(), "keys")) - g.Expect(server.ListenSSH()).To(Succeed()) - - go func() { - server.StartSSH() - }() - defer server.StopSSH() - - sshURL := server.SSHAddress() - repoURL := sshURL + "/test.git" - - u, err := url.Parse(sshURL) - g.Expect(err).NotTo(HaveOccurred()) - g.Expect(u.Host).ToNot(BeEmpty()) - - repo, err := safeClone(repoURL, t.TempDir(), &git2go.CloneOptions{ - FetchOptions: git2go.FetchOptions{ - RemoteCallbacks: git2go.RemoteCallbacks{ - CertificateCheckCallback: func(cert *git2go.Certificate, valid bool, hostname string) error { - panic("Oops!") - }, - }, - }}) - - g.Expect(repo).To(BeNil()) - g.Expect(err).To(HaveOccurred()) - g.Expect(err.Error()).Should(ContainSubstring("recovered from git2go panic")) -}