Skip to content

Commit

Permalink
Merge pull request #685 from knutgoetz/chore/gogit/delete-gogiterror-…
Browse files Browse the repository at this point in the history
…function

Delete obsolete goGitError function
  • Loading branch information
stefanprodan authored Nov 28, 2023
2 parents 02723c3 + cbc2172 commit e6b6af7
Show file tree
Hide file tree
Showing 3 changed files with 9 additions and 66 deletions.
6 changes: 5 additions & 1 deletion git/gogit/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -418,7 +418,11 @@ func (g *Client) Push(ctx context.Context, cfg repository.PushConfig) error {
ProxyOptions: g.proxy,
Options: cfg.Options,
})
return goGitError(err)
if err != nil {
return fmt.Errorf("failed to push to remote: %w", err)
}

return nil
}

// SwitchBranch switches the current branch to the given branch name.
Expand Down
28 changes: 4 additions & 24 deletions git/gogit/clone.go
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ func (g *Client) cloneBranch(ctx context.Context, url, branch string, opts repos
}
}
if err != nil {
return nil, fmt.Errorf("unable to clone '%s': %w", url, goGitError(err))
return nil, fmt.Errorf("unable to clone '%s': %w", url, err)
}
}

Expand Down Expand Up @@ -179,7 +179,7 @@ func (g *Client) cloneTag(ctx context.Context, url, tag string, opts repository.
URL: url,
}
}
return nil, fmt.Errorf("unable to clone '%s': %w", url, goGitError(err))
return nil, fmt.Errorf("unable to clone '%s': %w", url, err)
}

head, err := repo.Head()
Expand Down Expand Up @@ -243,7 +243,7 @@ func (g *Client) cloneCommit(ctx context.Context, url, commit string, opts repos
URL: url,
}
}
return nil, fmt.Errorf("unable to clone '%s': %w", url, goGitError(err))
return nil, fmt.Errorf("unable to clone '%s': %w", url, err)
}

w, err := repo.Worktree()
Expand Down Expand Up @@ -305,7 +305,7 @@ func (g *Client) cloneSemVer(ctx context.Context, url, semverTag string, opts re
URL: url,
}
}
return nil, fmt.Errorf("unable to clone '%s': %w", url, goGitError(err))
return nil, fmt.Errorf("unable to clone '%s': %w", url, err)
}

repoTags, err := repo.Tags()
Expand Down Expand Up @@ -609,23 +609,3 @@ func buildCommitWithRef(c *object.Commit, t *object.Tag, ref plumbing.ReferenceN
func isRemoteBranchNotFoundErr(err error, ref string) bool {
return strings.Contains(err.Error(), fmt.Sprintf("couldn't find remote ref '%s'", ref))
}

// goGitError translates an error from the go-git library, or returns
// `nil` if the argument is `nil`.
func goGitError(err error) error {
if err == nil {
return nil
}
switch strings.TrimSpace(err.Error()) {
case "unknown error: remote:":
// this unhelpful error arises because go-git takes the first
// line of the output on stderr, and for some git providers
// (GitLab, at least) the output has a blank line at the
// start. The rest of stderr is thrown away, so we can't get
// the actual error; but at least we know what was being
// attempted, and the likely cause.
return fmt.Errorf("push rejected; check git secret has write access")
default:
return err
}
}
41 changes: 0 additions & 41 deletions git/gogit/clone_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1516,47 +1516,6 @@ func TestClone_CredentialsOverHttp(t *testing.T) {
}
}

func TestGoGitErrorReplace(t *testing.T) {
// this is what go-git uses as the error message is if the remote
// sends a blank first line
unknownMessage := `unknown error: remote: `
err := errors.New(unknownMessage)
err = goGitError(err)
reformattedMessage := err.Error()
if reformattedMessage == unknownMessage {
t.Errorf("expected rewritten error, got %q", reformattedMessage)
}
}

func TestGoGitErrorUnchanged(t *testing.T) {
// this is (roughly) what GitHub sends if the deploy key doesn't
// have write access; go-git passes this on verbatim
regularMessage := `remote: ERROR: deploy key does not have write access`
expectedReformat := regularMessage
err := errors.New(regularMessage)
err = goGitError(err)
reformattedMessage := err.Error()
// test that it's been rewritten, without checking the exact content
if len(reformattedMessage) > len(expectedReformat) {
t.Errorf("expected %q, got %q", expectedReformat, reformattedMessage)
}
}

func Fuzz_GoGitError(f *testing.F) {
f.Add("")
f.Add("unknown error: remote: ")
f.Add("some other error")

f.Fuzz(func(t *testing.T, msg string) {
var err error
if msg != "" {
err = errors.New(msg)
}

_ = goGitError(err)
})
}

func initRepo(tmpDir string) (*extgogit.Repository, string, error) {
sto := filesystem.NewStorage(osfs.New(tmpDir, osfs.WithBoundOS()), cache.NewObjectLRUDefault())
repo, err := extgogit.Init(sto, memfs.New())
Expand Down

0 comments on commit e6b6af7

Please sign in to comment.