Retry git clone on git clone failure in gh repo fork --clone (#6962)
This commit is contained in:
Josh Soref
GitHub
parent
1fbcdf52cc
commit
7541ee6aec
4 changed files with 123 additions and 65 deletions
2
go.mod
2
go.mod
|
|
@ -6,7 +6,7 @@ require (
|
|||
github.com/AlecAivazis/survey/v2 v2.3.6
|
||||
github.com/MakeNowJust/heredoc v1.0.0
|
||||
github.com/briandowns/spinner v1.18.1
|
||||
github.com/cenkalti/backoff/v4 v4.1.3
|
||||
github.com/cenkalti/backoff/v4 v4.2.0
|
||||
github.com/charmbracelet/glamour v0.5.1-0.20220727184942-e70ff2d969da
|
||||
github.com/charmbracelet/lipgloss v0.5.0
|
||||
github.com/cli/go-gh v1.0.0
|
||||
|
|
|
|||
4
go.sum
4
go.sum
|
|
@ -45,8 +45,8 @@ github.com/aymerick/douceur v0.2.0 h1:Mv+mAeH1Q+n9Fr+oyamOlAkUNPWPlA8PPGR0QAaYuP
|
|||
github.com/aymerick/douceur v0.2.0/go.mod h1:wlT5vV2O3h55X9m7iVYN0TBM0NH/MmbLnd30/FjWUq4=
|
||||
github.com/briandowns/spinner v1.18.1 h1:yhQmQtM1zsqFsouh09Bk/jCjd50pC3EOGsh28gLVvwY=
|
||||
github.com/briandowns/spinner v1.18.1/go.mod h1:mQak9GHqbspjC/5iUx3qMlIho8xBS/ppAL/hX5SmPJU=
|
||||
github.com/cenkalti/backoff/v4 v4.1.3 h1:cFAlzYUlVYDysBEH2T5hyJZMh3+5+WCBvSnK6Q8UtC4=
|
||||
github.com/cenkalti/backoff/v4 v4.1.3/go.mod h1:scbssz8iZGpm3xbr14ovlUdkxfGXNInqkPWOWmG2CLw=
|
||||
github.com/cenkalti/backoff/v4 v4.2.0 h1:HN5dHm3WBOgndBH6E8V0q2jIYIR3s9yglV8k/+MN3u4=
|
||||
github.com/cenkalti/backoff/v4 v4.2.0/go.mod h1:Y3VNntkOUPxTVeUxJ/G5vcM//AlwfmyYozVcomhLiZE=
|
||||
github.com/census-instrumentation/opencensus-proto v0.2.1/go.mod h1:f6KPmirojxKA12rnyqOA5BBL4O983OfeGPqjHWSTneU=
|
||||
github.com/charmbracelet/glamour v0.5.1-0.20220727184942-e70ff2d969da h1:FGz53GWQRiKQ/5xUsoCCkewSQIC7u81Scaxx2nUy3nM=
|
||||
github.com/charmbracelet/glamour v0.5.1-0.20220727184942-e70ff2d969da/go.mod h1:HXz79SMFnF9arKxqeoHWxmo1BhplAH7wehlRhKQIL94=
|
||||
|
|
|
|||
|
|
@ -2,6 +2,7 @@ package fork
|
|||
|
||||
import (
|
||||
"context"
|
||||
"errors"
|
||||
"fmt"
|
||||
"net/http"
|
||||
"net/url"
|
||||
|
|
@ -9,6 +10,7 @@ import (
|
|||
"time"
|
||||
|
||||
"github.com/MakeNowJust/heredoc"
|
||||
"github.com/cenkalti/backoff/v4"
|
||||
"github.com/cli/cli/v2/api"
|
||||
ghContext "github.com/cli/cli/v2/context"
|
||||
"github.com/cli/cli/v2/git"
|
||||
|
|
@ -32,6 +34,7 @@ type ForkOptions struct {
|
|||
BaseRepo func() (ghrepo.Interface, error)
|
||||
Remotes func() (ghContext.Remotes, error)
|
||||
Since func(time.Time) time.Duration
|
||||
BackOff backoff.BackOff
|
||||
|
||||
GitArgs []string
|
||||
Repository string
|
||||
|
|
@ -46,6 +49,10 @@ type ForkOptions struct {
|
|||
DefaultBranchOnly bool
|
||||
}
|
||||
|
||||
type errWithExitCode interface {
|
||||
ExitCode() int
|
||||
}
|
||||
|
||||
// TODO warn about useless flags (--remote, --remote-name) when running from outside a repository
|
||||
// TODO output over STDOUT not STDERR
|
||||
// TODO remote-name has no effect on its own; error that or change behavior
|
||||
|
|
@ -317,8 +324,25 @@ func forkRun(opts *ForkOptions) error {
|
|||
}
|
||||
}
|
||||
if cloneDesired {
|
||||
forkedRepoURL := ghrepo.FormatRemoteURL(forkedRepo, protocol)
|
||||
cloneDir, err := gitClient.Clone(ctx, forkedRepoURL, opts.GitArgs)
|
||||
// Allow injecting alternative BackOff in tests.
|
||||
if opts.BackOff == nil {
|
||||
bo := backoff.NewConstantBackOff(3 * time.Second)
|
||||
opts.BackOff = bo
|
||||
}
|
||||
|
||||
cloneDir, err := backoff.RetryWithData(func() (string, error) {
|
||||
forkedRepoURL := ghrepo.FormatRemoteURL(forkedRepo, protocol)
|
||||
dir, err := gitClient.Clone(ctx, forkedRepoURL, opts.GitArgs)
|
||||
if err == nil {
|
||||
return dir, err
|
||||
}
|
||||
var execError errWithExitCode
|
||||
if errors.As(err, &execError) && execError.ExitCode() == 128 {
|
||||
return "", err
|
||||
}
|
||||
return "", backoff.Permanent(err)
|
||||
}, backoff.WithContext(backoff.WithMaxRetries(opts.BackOff, 3), ctx))
|
||||
|
||||
if err != nil {
|
||||
return fmt.Errorf("failed to clone fork: %w", err)
|
||||
}
|
||||
|
|
|
|||
|
|
@ -9,6 +9,7 @@ import (
|
|||
"testing"
|
||||
"time"
|
||||
|
||||
"github.com/cenkalti/backoff/v4"
|
||||
"github.com/cli/cli/v2/context"
|
||||
"github.com/cli/cli/v2/git"
|
||||
"github.com/cli/cli/v2/internal/config"
|
||||
|
|
@ -663,78 +664,111 @@ func TestRepoFork(t *testing.T) {
|
|||
},
|
||||
wantErrOut: "✓ Created fork OWNER/REPO\n✓ Renamed fork to OWNER/NEW_REPO\n",
|
||||
},
|
||||
{
|
||||
name: "retries clone up to four times if necessary",
|
||||
opts: &ForkOptions{
|
||||
Repository: "OWNER/REPO",
|
||||
Clone: true,
|
||||
BackOff: &backoff.ZeroBackOff{},
|
||||
},
|
||||
httpStubs: forkPost,
|
||||
execStubs: func(cs *run.CommandStubber) {
|
||||
cs.Register(`git clone https://github.com/someone/REPO\.git`, 128, "")
|
||||
cs.Register(`git clone https://github.com/someone/REPO\.git`, 128, "")
|
||||
cs.Register(`git clone https://github.com/someone/REPO\.git`, 128, "")
|
||||
cs.Register(`git clone https://github.com/someone/REPO\.git`, 0, "")
|
||||
cs.Register(`git -C REPO remote add upstream https://github\.com/OWNER/REPO\.git`, 0, "")
|
||||
cs.Register(`git -C REPO fetch upstream`, 0, "")
|
||||
},
|
||||
},
|
||||
{
|
||||
name: "does not retry clone if error occurs and exit code is not 128",
|
||||
opts: &ForkOptions{
|
||||
Repository: "OWNER/REPO",
|
||||
Clone: true,
|
||||
BackOff: &backoff.ZeroBackOff{},
|
||||
},
|
||||
httpStubs: forkPost,
|
||||
execStubs: func(cs *run.CommandStubber) {
|
||||
cs.Register(`git clone https://github.com/someone/REPO\.git`, 128, "")
|
||||
cs.Register(`git clone https://github.com/someone/REPO\.git`, 65, "")
|
||||
},
|
||||
wantErr: true,
|
||||
errMsg: `failed to clone fork: failed to run git: git -c credential.helper= -c credential.helper=!"[^"]+" auth git-credential clone https://github.com/someone/REPO\.git exited with status 65`,
|
||||
},
|
||||
}
|
||||
|
||||
for _, tt := range tests {
|
||||
ios, _, stdout, stderr := iostreams.Test()
|
||||
ios.SetStdinTTY(tt.tty)
|
||||
ios.SetStdoutTTY(tt.tty)
|
||||
ios.SetStderrTTY(tt.tty)
|
||||
tt.opts.IO = ios
|
||||
|
||||
tt.opts.BaseRepo = func() (ghrepo.Interface, error) {
|
||||
return ghrepo.New("OWNER", "REPO"), nil
|
||||
}
|
||||
|
||||
reg := &httpmock.Registry{}
|
||||
if tt.httpStubs != nil {
|
||||
tt.httpStubs(reg)
|
||||
}
|
||||
tt.opts.HttpClient = func() (*http.Client, error) {
|
||||
return &http.Client{Transport: reg}, nil
|
||||
}
|
||||
|
||||
cfg := config.NewBlankConfig()
|
||||
if tt.cfgStubs != nil {
|
||||
tt.cfgStubs(cfg)
|
||||
}
|
||||
tt.opts.Config = func() (config.Config, error) {
|
||||
return cfg, nil
|
||||
}
|
||||
|
||||
tt.opts.Remotes = func() (context.Remotes, error) {
|
||||
if tt.remotes == nil {
|
||||
return []*context.Remote{
|
||||
{
|
||||
Remote: &git.Remote{
|
||||
Name: "origin",
|
||||
FetchURL: &url.URL{},
|
||||
},
|
||||
Repo: ghrepo.New("OWNER", "REPO"),
|
||||
},
|
||||
}, nil
|
||||
}
|
||||
return tt.remotes, nil
|
||||
}
|
||||
|
||||
tt.opts.GitClient = &git.Client{
|
||||
GhPath: "some/path/gh",
|
||||
GitPath: "some/path/git",
|
||||
}
|
||||
|
||||
//nolint:staticcheck // SA1019: prompt.InitAskStubber is deprecated: use NewAskStubber
|
||||
as, teardown := prompt.InitAskStubber()
|
||||
defer teardown()
|
||||
if tt.askStubs != nil {
|
||||
tt.askStubs(as)
|
||||
}
|
||||
cs, restoreRun := run.Stub()
|
||||
defer restoreRun(t)
|
||||
if tt.execStubs != nil {
|
||||
tt.execStubs(cs)
|
||||
}
|
||||
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
ios, _, stdout, stderr := iostreams.Test()
|
||||
ios.SetStdinTTY(tt.tty)
|
||||
ios.SetStdoutTTY(tt.tty)
|
||||
ios.SetStderrTTY(tt.tty)
|
||||
tt.opts.IO = ios
|
||||
|
||||
tt.opts.BaseRepo = func() (ghrepo.Interface, error) {
|
||||
return ghrepo.New("OWNER", "REPO"), nil
|
||||
}
|
||||
|
||||
reg := &httpmock.Registry{}
|
||||
if tt.httpStubs != nil {
|
||||
tt.httpStubs(reg)
|
||||
}
|
||||
tt.opts.HttpClient = func() (*http.Client, error) {
|
||||
return &http.Client{Transport: reg}, nil
|
||||
}
|
||||
|
||||
cfg := config.NewBlankConfig()
|
||||
if tt.cfgStubs != nil {
|
||||
tt.cfgStubs(cfg)
|
||||
}
|
||||
tt.opts.Config = func() (config.Config, error) {
|
||||
return cfg, nil
|
||||
}
|
||||
|
||||
tt.opts.Remotes = func() (context.Remotes, error) {
|
||||
if tt.remotes == nil {
|
||||
return []*context.Remote{
|
||||
{
|
||||
Remote: &git.Remote{
|
||||
Name: "origin",
|
||||
FetchURL: &url.URL{},
|
||||
},
|
||||
Repo: ghrepo.New("OWNER", "REPO"),
|
||||
},
|
||||
}, nil
|
||||
}
|
||||
return tt.remotes, nil
|
||||
}
|
||||
|
||||
tt.opts.GitClient = &git.Client{
|
||||
GhPath: "some/path/gh",
|
||||
GitPath: "some/path/git",
|
||||
}
|
||||
|
||||
//nolint:staticcheck // SA1019: prompt.InitAskStubber is deprecated: use NewAskStubber
|
||||
as, teardown := prompt.InitAskStubber()
|
||||
defer teardown()
|
||||
if tt.askStubs != nil {
|
||||
tt.askStubs(as)
|
||||
}
|
||||
|
||||
cs, restoreRun := run.Stub()
|
||||
defer restoreRun(t)
|
||||
if tt.execStubs != nil {
|
||||
tt.execStubs(cs)
|
||||
}
|
||||
|
||||
if tt.opts.Since == nil {
|
||||
tt.opts.Since = func(t time.Time) time.Duration {
|
||||
return 2 * time.Second
|
||||
}
|
||||
}
|
||||
|
||||
defer reg.Verify(t)
|
||||
err := forkRun(tt.opts)
|
||||
if tt.wantErr {
|
||||
assert.Error(t, err)
|
||||
assert.Equal(t, tt.errMsg, err.Error())
|
||||
assert.Error(t, err, tt.errMsg)
|
||||
return
|
||||
}
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue