Skip to content

Commit af1138c

Browse files
mislavmislav
committed
Update git client for pr create
1 parent a11709a commit af1138c

File tree

7 files changed

+252
-164
lines changed

7 files changed

+252
-164
lines changed

‎git/local_repo.go‎

Lines changed: 85 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,17 +1,25 @@
11
package git
22

33
import (
4+
"bufio"
45
"bytes"
56
"context"
7+
"io"
68
"os/exec"
9+
"regexp"
710
"strings"
811

912
"github.com/cli/safeexec"
1013
)
1114

1215
type LocalRepo struct {
13-
gitExePath string
14-
gitExeErr error
16+
Stdin io.Reader
17+
Stdout io.Writer
18+
Stderr io.Writer
19+
20+
gitCommandInit func(ctx context.Context, args ...string) (*exec.Cmd, error)
21+
gitExePath string
22+
gitExeErr error
1523
}
1624

1725
func (c *LocalRepo) PathPrefix(ctx context.Context) (string, error) {
@@ -40,6 +48,9 @@ func (c *LocalRepo) LastCommit(ctx context.Context) (*Commit, error) {
4048
}
4149

4250
func (c *LocalRepo) gitCommand(ctx context.Context, args ...string) (*exec.Cmd, error) {
51+
if c.gitCommandInit != nil {
52+
return c.gitCommandInit(ctx, args...)
53+
}
4354
gitExe, err := c.gitExe()
4455
if err != nil {
4556
return nil, err
@@ -54,3 +65,75 @@ func (c *LocalRepo) gitExe() (string, error) {
5465
}
5566
return c.gitExePath, c.gitExeErr
5667
}
68+
69+
type PushOptions struct {
70+
RemoteName string
71+
TargetBranch string
72+
ShowProgress bool
73+
SetUpstream bool
74+
}
75+
76+
var gitPushRegexp = regexp.MustCompile(`^remote: (Create a pull request.*by visiting|[[:space:]]*https://.*/pull/new/)`)
77+
78+
// Push publishes a branch to a git remote and filters out "Create a pull request by visiting <URL>" lines
79+
// before forwarding them to stderr.
80+
func (g *LocalRepo) Push(ctx context.Context, opts PushOptions) error {
81+
args := []string{"push"}
82+
if opts.SetUpstream {
83+
args = append(args, "--set-upstream")
84+
}
85+
if opts.ShowProgress {
86+
args = append(args, "--progress")
87+
}
88+
args = append(args, opts.RemoteName, "HEAD:"+opts.TargetBranch)
89+
90+
pushCmd, err := g.gitCommand(ctx, args...)
91+
if err != nil {
92+
return err
93+
}
94+
pushCmd.Stdin = g.Stdin
95+
pushCmd.Stdout = g.Stdout
96+
r, err := pushCmd.StderrPipe()
97+
if err != nil {
98+
return err
99+
}
100+
if err = pushCmd.Start(); err != nil {
101+
return err
102+
}
103+
if err = filterLines(g.Stderr, r, gitPushRegexp); err != nil {
104+
return err
105+
}
106+
return pushCmd.Wait()
107+
}
108+
109+
func filterLines(w io.Writer, r io.Reader, re *regexp.Regexp) error {
110+
s := bufio.NewScanner(r)
111+
s.Split(func(data []byte, atEOF bool) (advance int, token []byte, err error) {
112+
if atEOF && len(data) == 0 {
113+
return 0, nil, nil
114+
}
115+
i := bytes.IndexAny(data, "\r\n")
116+
if i >= 0 {
117+
// encompass both CR & LF characters if they appear together
118+
if data[i] == '\r' && len(data) > i+1 && data[i+1] == '\n' {
119+
return i + 2, data[0 : i+2], nil
120+
}
121+
return i + 1, data[0 : i+1], nil
122+
}
123+
if atEOF {
124+
return len(data), data, nil
125+
}
126+
// Request more data.
127+
return 0, nil, nil
128+
})
129+
for s.Scan() {
130+
line := s.Bytes()
131+
if !re.Match(line) {
132+
_, err := w.Write(line)
133+
if err != nil {
134+
return err
135+
}
136+
}
137+
}
138+
return s.Err()
139+
}

pkg/cmd/pr/create/git_test.go renamed to git/local_repo_test.go

Lines changed: 15 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,8 @@
1-
package create
1+
package git
22

33
import (
44
"bytes"
5+
"context"
56
"fmt"
67
"os"
78
"os/exec"
@@ -42,7 +43,7 @@ func TestHelperProcess(t *testing.T) {
4243
}
4344

4445
func Test_gitClientExec_Push(t *testing.T) {
45-
gitCommand := func(args ...string) (*exec.Cmd, error) {
46+
gitCommand := func(ctx context.Context, args ...string) (*exec.Cmd, error) {
4647
args = append([]string{"-test.run=TestHelperProcess", "--", "git"}, args...)
4748
cmd := exec.Command(os.Args[0], args...)
4849
cmd.Env = []string{"GH_WANT_HELPER_PROCESS=1"}
@@ -51,26 +52,26 @@ func Test_gitClientExec_Push(t *testing.T) {
5152

5253
tests := []struct {
5354
name string
54-
args []string
55+
args PushOptions
5556
wantStdout string
5657
wantStderr *logWriter
5758
wantErr bool
5859
}{
5960
{
6061
name: "no output",
61-
args: []string{"origin", "HEAD:no-output"},
62+
args: PushOptions{RemoteName: "origin", TargetBranch: "no-output"},
6263
wantStdout: "",
6364
wantStderr: &logWriter{},
6465
},
6566
{
6667
name: "some output",
67-
args: []string{"origin", "HEAD:some-output"},
68+
args: PushOptions{RemoteName: "origin", TargetBranch: "some-output"},
6869
wantStdout: "STDOUT\n",
6970
wantStderr: &logWriter{"STDERR\n"},
7071
},
7172
{
7273
name: "progress output",
73-
args: []string{"--progress", "origin", "HEAD:feature"},
74+
args: PushOptions{RemoteName: "origin", TargetBranch: "feature", ShowProgress: true},
7475
wantStdout: "",
7576
wantStderr: &logWriter{
7677
"progress: 0%\r",
@@ -85,23 +86,27 @@ func Test_gitClientExec_Push(t *testing.T) {
8586
},
8687
{
8788
name: "cr-lf",
88-
args: []string{"origin", "HEAD:crlf"},
89+
args: PushOptions{RemoteName: "origin", TargetBranch: "crlf"},
8990
wantStdout: "",
9091
wantStderr: &logWriter{"one\r\n", "two\r\n", "three\r", "four"},
9192
},
9293
{
9394
name: "failure",
94-
args: []string{"nonexist", "HEAD:feature"},
95+
args: PushOptions{RemoteName: "nonexist", TargetBranch: "feature"},
9596
wantErr: true,
9697
wantStderr: &logWriter{"unrecognized arguments: []string{\"git\", \"push\", \"nonexist\", \"HEAD:feature\"}\n"},
9798
},
9899
}
99100
for _, tt := range tests {
100101
t.Run(tt.name, func(t *testing.T) {
101-
g := &gitClientExec{gitCommand: gitCommand}
102102
stdout := &bytes.Buffer{}
103103
stderr := &logWriter{}
104-
if err := g.Push(tt.args, stdout, stderr); (err != nil) != tt.wantErr {
104+
g := &LocalRepo{
105+
gitCommandInit: gitCommand,
106+
Stdout: stdout,
107+
Stderr: stderr,
108+
}
109+
if err := g.Push(context.Background(), tt.args); (err != nil) != tt.wantErr {
105110
t.Errorf("gitClientExec.Push() error = %v, wantErr %v", err, tt.wantErr)
106111
return
107112
}

‎pkg/cmd/pr/create/create.go‎

Lines changed: 24 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,9 @@
11
package create
22

33
import (
4+
"context"
45
"errors"
56
"fmt"
6-
"io"
77
"net/http"
88
"net/url"
99
"strings"
@@ -12,7 +12,7 @@ import (
1212
"github.com/AlecAivazis/survey/v2"
1313
"github.com/MakeNowJust/heredoc"
1414
"github.com/cli/cli/v2/api"
15-
"github.com/cli/cli/v2/context"
15+
remotesContext "github.com/cli/cli/v2/context"
1616
"github.com/cli/cli/v2/git"
1717
"github.com/cli/cli/v2/internal/config"
1818
"github.com/cli/cli/v2/internal/ghrepo"
@@ -28,17 +28,17 @@ type browser interface {
2828
Browse(string) error
2929
}
3030

31-
//go:generate mockery --name gitClient --structname GitClient
31+
//go:generate moq -fmt goimports -rm -skip-ensure -out git_client_mock.go . gitClient
3232
type gitClient interface {
33-
Push(args []string, stdout io.Writer, stderr io.Writer) error
33+
Push(ctx context.Context, opts git.PushOptions) error
3434
}
3535

3636
type CreateOptions struct {
3737
// This struct stores user input and factory functions
3838
HttpClient func() (*http.Client, error)
3939
Config func() (config.Config, error)
4040
IO *iostreams.IOStreams
41-
Remotes func() (context.Remotes, error)
41+
Remotes func() (remotesContext.Remotes, error)
4242
Branch func() (string, error)
4343
Browser browser
4444
Git gitClient
@@ -72,14 +72,14 @@ type CreateOptions struct {
7272
type CreateContext struct {
7373
// This struct stores contextual data about the creation process and is for building up enough
7474
// data to create a pull request
75-
RepoContext *context.ResolvedRemotes
75+
RepoContext *remotesContext.ResolvedRemotes
7676
BaseRepo *api.Repository
7777
HeadRepo ghrepo.Interface
7878
BaseTrackingBranch string
7979
BaseBranch string
8080
HeadBranch string
8181
HeadBranchLabel string
82-
HeadRemote *context.Remote
82+
HeadRemote *remotesContext.Remote
8383
IsPushEnabled bool
8484
Client *api.Client
8585
}
@@ -92,7 +92,11 @@ func NewCmdCreate(f *cmdutil.Factory, runF func(*CreateOptions) error) *cobra.Co
9292
Remotes: f.Remotes,
9393
Branch: f.Branch,
9494
Browser: f.Browser,
95-
Git: &gitClientExec{gitCommand: git.GitCommand},
95+
Git: &git.LocalRepo{
96+
Stdin: f.IOStreams.In,
97+
Stdout: f.IOStreams.Out,
98+
Stderr: f.IOStreams.ErrOut,
99+
},
96100
}
97101

98102
var bodyFile string
@@ -393,7 +397,7 @@ func initDefaultTitleBody(ctx CreateContext, state *shared.IssueMetadataState) e
393397
return nil
394398
}
395399

396-
func determineTrackingBranch(remotes context.Remotes, headBranch string) *git.TrackingRef {
400+
func determineTrackingBranch(remotes remotesContext.Remotes, headBranch string) *git.TrackingRef {
397401
refsForLookup := []string{"HEAD"}
398402
var trackingRefs []git.TrackingRef
399403

@@ -478,7 +482,7 @@ func NewCreateContext(opts *CreateOptions) (*CreateContext, error) {
478482
return nil, err
479483
}
480484

481-
repoContext, err := context.ResolveRemotesToRepos(remotes, client, opts.RepoOverride)
485+
repoContext, err := remotesContext.ResolveRemotesToRepos(remotes, client, opts.RepoOverride)
482486
if err != nil {
483487
return nil, err
484488
}
@@ -518,7 +522,7 @@ func NewCreateContext(opts *CreateOptions) (*CreateContext, error) {
518522
}
519523

520524
var headRepo ghrepo.Interface
521-
var headRemote *context.Remote
525+
var headRemote *remotesContext.Remote
522526

523527
if isPushEnabled {
524528
// determine whether the head branch is already pushed to a remote
@@ -717,7 +721,7 @@ func handlePush(opts CreateOptions, ctx CreateContext) error {
717721
if err != nil {
718722
return fmt.Errorf("error adding remote: %w", err)
719723
}
720-
headRemote = &context.Remote{
724+
headRemote = &remotesContext.Remote{
721725
Remote: gitRemote,
722726
Repo: headRepo,
723727
}
@@ -727,14 +731,15 @@ func handlePush(opts CreateOptions, ctx CreateContext) error {
727731
if ctx.IsPushEnabled {
728732
pushTries := 0
729733
maxPushTries := 3
734+
ct := context.TODO()
730735
for {
731-
args := []string{"--set-upstream"}
732-
if opts.IO.IsStderrTTY() {
733-
args = append(args, "--progress")
734-
}
735-
args = append(args, headRemote.Name, fmt.Sprintf("HEAD:%s", ctx.HeadBranch))
736-
if err := opts.Git.Push(args, opts.IO.Out, opts.IO.ErrOut); err != nil {
737-
if didForkRepo && pushTries < maxPushTries {
736+
if err := opts.Git.Push(ct, git.PushOptions{
737+
RemoteName: headRemote.Name,
738+
TargetBranch: ctx.HeadBranch,
739+
SetUpstream: true,
740+
ShowProgress: opts.IO.IsStderrTTY(),
741+
}); err != nil {
742+
if ct.Err() == nil && didForkRepo && pushTries < maxPushTries {
738743
pushTries++
739744
// first wait 2 seconds after forking, then 4s, then 6s
740745
waitSeconds := 2 * pushTries

0 commit comments

Comments
 (0)