diff --git a/git.go b/git.go index 428e619..b6a2655 100644 --- a/git.go +++ b/git.go @@ -11,8 +11,6 @@ import ( type commander struct { outStream, errStream io.Writer gitPath, dir string - - err error } func (c *commander) getGitPath() string { @@ -22,10 +20,7 @@ func (c *commander) getGitPath() string { return c.gitPath } -func (c *commander) cmdE(prog string, args ...string) (string, string, error) { - if c.err != nil { - return "", "", c.err - } +func (c *commander) Cmd(prog string, args ...string) (string, string, error) { log.Println(prog, args) var ( @@ -42,16 +37,6 @@ func (c *commander) cmdE(prog string, args ...string) (string, string, error) { return strings.TrimSpace(outBuf.String()), strings.TrimSpace(errBuf.String()), err } -func (c *commander) GitE(args ...string) (string, string, error) { - return c.cmdE(c.getGitPath(), args...) -} - -func (c *commander) Git(args ...string) (string, string) { - return c.cmd(c.getGitPath(), args...) -} - -func (c *commander) cmd(prog string, args ...string) (string, string) { - stdout, stderr, err := c.cmdE(prog, args...) - c.err = err - return stdout, stderr +func (c *commander) Git(args ...string) (string, string, error) { + return c.Cmd(c.getGitPath(), args...) } diff --git a/tag.go b/tag.go index d669e2e..a8dd0e5 100644 --- a/tag.go +++ b/tag.go @@ -9,7 +9,7 @@ import ( func (tp *tagpr) latestPullRequest(ctx context.Context) (*github.PullRequest, error) { // tag and exit if the HEAD is the merged tagpr - commitish, _, err := tp.c.GitE("rev-parse", "HEAD") + commitish, _, err := tp.c.Git("rev-parse", "HEAD") if err != nil { return nil, err } @@ -35,12 +35,16 @@ func (tp *tagpr) tagRelease(ctx context.Context, pr *github.PullRequest, currVer // "Rebase and merge" was used. However, we don't care about "Rebase and merge" and only support // "Create a merge commit" and "Squash and merge." if tp.cfg.versionFile == nil { - tp.c.Git("checkout", "HEAD~") + if _, _, err := tp.c.Git("checkout", "HEAD~"); err != nil { + return err + } vfile, err = detectVersionFile(".", currVer) if err != nil { return err } - tp.c.Git("checkout", releaseBranch) + if _, _, err := tp.c.Git("checkout", releaseBranch); err != nil { + return err + } } else { vfiles := strings.Split(tp.cfg.versionFile.String(), ",") vfile = strings.TrimSpace(vfiles[0]) @@ -65,7 +69,7 @@ func (tp *tagpr) tagRelease(ctx context.Context, pr *github.PullRequest, currVer // we generate release notes in advance. // Get the previous commitish to avoid picking up the merge of the pull // request made by tagpr. - targetCommitish, _, err := tp.c.GitE("rev-parse", "HEAD~") + targetCommitish, _, err := tp.c.Git("rev-parse", "HEAD~") if err != nil { return nil } @@ -79,11 +83,10 @@ func (tp *tagpr) tagRelease(ctx context.Context, pr *github.PullRequest, currVer return err } - tp.c.Git("tag", nextTag) - if tp.c.err != nil { - return tp.c.err + if _, _, err := tp.c.Git("tag", nextTag); err != nil { + return err } - _, _, err = tp.c.GitE("push", "--tags") + _, _, err = tp.c.Git("push", "--tags") if err != nil { return err } diff --git a/tagpr.go b/tagpr.go index a9ac858..31e2b02 100644 --- a/tagpr.go +++ b/tagpr.go @@ -51,7 +51,7 @@ func newTagPR(ctx context.Context, c *commander) (*tagpr, error) { if err != nil { return nil, err } - remoteURL, _, err := tp.c.GitE("config", "remote."+tp.remoteName+".url") + remoteURL, _, err := tp.c.Git("config", "remote."+tp.remoteName+".url") if err != nil { return nil, err } @@ -76,12 +76,12 @@ func newTagPR(ctx context.Context, c *commander) (*tagpr, error) { } tp.gh = cli - isShallow, _, err := tp.c.GitE("rev-parse", "--is-shallow-repository") + isShallow, _, err := tp.c.Git("rev-parse", "--is-shallow-repository") if err != nil { return nil, err } if isShallow == "true" { - if _, _, err := tp.c.GitE("fetch", "--unshallow"); err != nil { + if _, _, err := tp.c.Git("fetch", "--unshallow"); err != nil { return nil, err } } @@ -137,7 +137,7 @@ func (tp *tagpr) Run(ctx context.Context) error { } } - branch, _, err := tp.c.GitE("symbolic-ref", "--short", "HEAD") + branch, _, err := tp.c.Git("symbolic-ref", "--short", "HEAD") if err != nil { return fmt.Errorf("failed to git symbolic-ref: %w", err) } @@ -147,11 +147,15 @@ func (tp *tagpr) Run(ctx context.Context) error { } // XXX: should care GIT_*_NAME etc? - if _, _, err := tp.c.GitE("config", "user.email"); err != nil { - tp.c.Git("config", "--local", "user.email", gitEmail) + if _, _, err := tp.c.Git("config", "user.email"); err != nil { + if _, _, err := tp.c.Git("config", "--local", "user.email", gitEmail); err != nil { + return err + } } - if _, _, err := tp.c.GitE("config", "user.name"); err != nil { - tp.c.Git("config", "--local", "user.name", gitUser) + if _, _, err := tp.c.Git("config", "user.name"); err != nil { + if _, _, err := tp.c.Git("config", "--local", "user.name", gitUser); err != nil { + return err + } } // If the latest commit is a merge commit of the pull request by tagpr, @@ -164,8 +168,10 @@ func (tp *tagpr) Run(ctx context.Context) error { } rcBranch := fmt.Sprintf("tagpr-from-%s", currVer.Tag()) - tp.c.GitE("branch", "-D", rcBranch) - tp.c.Git("checkout", "-b", rcBranch) + tp.c.Git("branch", "-D", rcBranch) + if _, _, err := tp.c.Git("checkout", "-b", rcBranch); err != nil { + return err + } head := fmt.Sprintf("%s:%s", tp.owner, rcBranch) pulls, _, err := tp.gh.PullRequests.List(ctx, tp.owner, tp.repo, @@ -211,7 +217,7 @@ func (tp *tagpr) Run(ctx context.Context) error { prog = "sh" progArgs = []string{"-c", prog} } - tp.c.cmdE(prog, progArgs...) + tp.c.Cmd(prog, progArgs...) } if vfiles[0] != "" { @@ -221,7 +227,7 @@ func (tp *tagpr) Run(ctx context.Context) error { } } } - tp.c.GitE("add", "-f", tp.cfg.conf) // ignore any errors + tp.c.Git("add", "-f", tp.cfg.conf) // ignore any errors const releaseYml = ".github/release.yml" // TODO: It would be nice to be able to add an exclude setting even if release.yml already exists. @@ -236,15 +242,17 @@ func (tp *tagpr) Run(ctx context.Context) error { `), 0644); err != nil { return err } - tp.c.GitE("add", "-f", releaseYml) + tp.c.Git("add", "-f", releaseYml) } - tp.c.Git("commit", "--allow-empty", "-am", autoCommitMessage) + if _, _, err := tp.c.Git("commit", "--allow-empty", "-am", autoCommitMessage); err != nil { + return err + } // cherry-pick if the remote branch is exists and changed // XXX: Do I need to apply merge commits too? // (We ommited merge commits for now, because if we cherry-pick them, we need to add options like "-m 1". - out, _, err := tp.c.GitE( + out, _, err := tp.c.Git( "log", "--no-merges", "--pretty=format:%h %s", "main.."+tp.remoteName+"/"+rcBranch) if err == nil { var cherryPicks []string @@ -264,12 +272,12 @@ func (tp *tagpr) Run(ctx context.Context) error { // and apply it as much as possible. for i := len(cherryPicks) - 1; i >= 0; i-- { commitish := cherryPicks[i] - _, _, err := tp.c.GitE( + _, _, err := tp.c.Git( "cherry-pick", "--keep-redundant-commits", "--allow-empty", commitish) // conflict, etc. / Need error handling in case of non-conflict error? if err != nil { - tp.c.GitE("cherry-pick", "--abort") + tp.c.Git("cherry-pick", "--abort") } } } @@ -316,10 +324,10 @@ func (tp *tagpr) Run(ctx context.Context) error { return err } - tp.c.GitE("add", changelogMd) - tp.c.GitE("commit", "-m", autoChangelogMessage) + tp.c.Git("add", changelogMd) + tp.c.Git("commit", "-m", autoChangelogMessage) - if _, _, err := tp.c.GitE("push", "--force", tp.remoteName, rcBranch); err != nil { + if _, _, err := tp.c.Git("push", "--force", tp.remoteName, rcBranch); err != nil { return err } @@ -392,7 +400,7 @@ var headBranchReg = regexp.MustCompile(`(?m)^\s*HEAD branch: (.*)$`) func (tp *tagpr) defaultBranch() (string, error) { // `git symbolic-ref refs/remotes/origin/HEAD` sometimes doesn't work // So use `git remote show origin` for detecting default branch - show, _, err := tp.c.GitE("remote", "show", tp.remoteName) + show, _, err := tp.c.Git("remote", "show", tp.remoteName) if err != nil { return "", fmt.Errorf("failed to detect defaut branch: %w", err) } @@ -404,7 +412,7 @@ func (tp *tagpr) defaultBranch() (string, error) { } func (tp *tagpr) detectRemote() (string, error) { - remotesStr, _, err := tp.c.GitE("remote") + remotesStr, _, err := tp.c.Git("remote") if err != nil { return "", fmt.Errorf("failed to detect remote: %s", err) }