Skip to content

Commit

Permalink
Merge pull request #79 from Songmu/fix
Browse files Browse the repository at this point in the history
adjust internal interfaces of commander
  • Loading branch information
Songmu authored Sep 4, 2022
2 parents 788a7eb + 30e3339 commit bf581dd
Show file tree
Hide file tree
Showing 3 changed files with 44 additions and 48 deletions.
21 changes: 3 additions & 18 deletions git.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,6 @@ import (
type commander struct {
outStream, errStream io.Writer
gitPath, dir string

err error
}

func (c *commander) getGitPath() string {
Expand All @@ -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 (
Expand All @@ -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...)
}
19 changes: 11 additions & 8 deletions tag.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand All @@ -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])
Expand All @@ -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
}
Expand All @@ -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
}
Expand Down
52 changes: 30 additions & 22 deletions tagpr.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand All @@ -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
}
}
Expand Down Expand Up @@ -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)
}
Expand All @@ -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,
Expand All @@ -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,
Expand Down Expand Up @@ -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] != "" {
Expand All @@ -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.
Expand All @@ -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
Expand All @@ -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")
}
}
}
Expand Down Expand Up @@ -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
}

Expand Down Expand Up @@ -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)
}
Expand All @@ -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)
}
Expand Down

0 comments on commit bf581dd

Please sign in to comment.