Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

adjust internal interfaces of commander #79

Merged
merged 1 commit into from
Sep 4, 2022
Merged

adjust internal interfaces of commander #79

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 3 additions & 18 deletions git.go
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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