From 57fbe4f317ca7d0849eeeedb16c1abc21a81913b Mon Sep 17 00:00:00 2001 From: Sam Coe Date: Wed, 19 Oct 2022 21:11:36 +0300 Subject: [PATCH] Refactor to use new git client (#6447) --- context/context.go | 4 +- git/client.go | 185 +++++++++++++++------ git/client_test.go | 2 +- git/git.go | 154 ----------------- internal/run/run.go | 4 + pkg/cmd/auth/login/login.go | 4 + pkg/cmd/auth/login/login_test.go | 3 + pkg/cmd/auth/refresh/refresh.go | 10 +- pkg/cmd/auth/refresh/refresh_test.go | 2 +- pkg/cmd/auth/setupgit/setupgit.go | 1 + pkg/cmd/auth/shared/git_credential.go | 30 ++-- pkg/cmd/auth/shared/git_credential_test.go | 4 + pkg/cmd/auth/shared/login_flow.go | 8 +- pkg/cmd/browse/browse.go | 21 ++- pkg/cmd/browse/browse_test.go | 2 +- pkg/cmd/extension/manager.go | 4 +- pkg/cmd/factory/default.go | 23 +-- pkg/cmd/gist/clone/clone.go | 5 +- pkg/cmd/gist/clone/clone_test.go | 2 + pkg/cmd/pr/checkout/checkout.go | 65 ++++---- pkg/cmd/pr/checkout/checkout_test.go | 3 + pkg/cmd/pr/close/close.go | 10 +- pkg/cmd/pr/close/close_test.go | 2 + pkg/cmd/pr/create/create.go | 49 +++--- pkg/cmd/pr/create/create_test.go | 4 +- pkg/cmd/pr/merge/merge.go | 25 +-- pkg/cmd/pr/merge/merge_test.go | 1 + pkg/cmd/pr/shared/finder.go | 14 +- pkg/cmd/pr/shared/survey.go | 17 -- pkg/cmd/pr/shared/templates.go | 4 +- pkg/cmd/pr/status/status.go | 13 +- pkg/cmd/pr/status/status_test.go | 4 +- pkg/cmd/release/create/create.go | 23 +-- pkg/cmd/release/create/create_test.go | 5 + pkg/cmd/repo/clone/clone.go | 9 +- pkg/cmd/repo/clone/clone_test.go | 2 + pkg/cmd/repo/create/create.go | 76 +++++---- pkg/cmd/repo/create/create_test.go | 3 + pkg/cmd/repo/fork/fork.go | 21 ++- pkg/cmd/repo/fork/fork_test.go | 2 + pkg/cmd/repo/rename/rename.go | 12 +- pkg/cmd/repo/rename/rename_test.go | 2 + pkg/cmd/repo/sync/git.go | 46 ++--- pkg/cmd/repo/sync/sync.go | 2 +- 44 files changed, 459 insertions(+), 423 deletions(-) delete mode 100644 git/git.go diff --git a/context/context.go b/context/context.go index 45dfcfd97..568e3e623 100644 --- a/context/context.go +++ b/context/context.go @@ -2,6 +2,7 @@ package context import ( + "context" "errors" "sort" @@ -138,7 +139,8 @@ func (r *ResolvedRemotes) BaseRepo(io *iostreams.IOStreams, p iprompter) (ghrepo } // cache the result to git config - err := git.SetRemoteResolution(remote.Name, resolution) + c := &git.Client{} + err := c.SetRemoteResolution(context.Background(), remote.Name, resolution) return selectedRepo, err } diff --git a/git/client.go b/git/client.go index c6bfc21ac..b91d0aacb 100644 --- a/git/client.go +++ b/git/client.go @@ -38,22 +38,16 @@ func (e *NotInstalled) Unwrap() error { } type GitError struct { - stderr string - err error + ExitCode int + Stderr string + err error } func (ge *GitError) Error() string { - stderr := ge.stderr - if stderr == "" { - var exitError *exec.ExitError - if errors.As(ge.err, &exitError) { - stderr = string(exitError.Stderr) - } - } - if stderr == "" { + if ge.Stderr == "" { return fmt.Sprintf("failed to run git: %v", ge.err) } - return fmt.Sprintf("failed to run git: %s", stderr) + return fmt.Sprintf("failed to run git: %s", ge.Stderr) } func (ge *GitError) Unwrap() error { @@ -64,16 +58,77 @@ type gitCommand struct { *exec.Cmd } -// This is a hack in order to not break the hundreds of -// existing tests that rely on `run.PrepareCmd` to be invoked. func (gc *gitCommand) Run() error { - return run.PrepareCmd(gc.Cmd).Run() + // This is a hack in order to not break the hundreds of + // existing tests that rely on `run.PrepareCmd` to be invoked. + err := run.PrepareCmd(gc.Cmd).Run() + if err != nil { + ge := GitError{err: err} + var exitError *exec.ExitError + if errors.As(err, &exitError) { + ge.Stderr = string(exitError.Stderr) + ge.ExitCode = exitError.ExitCode() + } + return &ge + } + return nil } -// This is a hack in order to not break the hundreds of -// existing tests that rely on `run.PrepareCmd` to be invoked. func (gc *gitCommand) Output() ([]byte, error) { - return run.PrepareCmd(gc.Cmd).Output() + gc.Stdout = nil + gc.Stderr = nil + // This is a hack in order to not break the hundreds of + // existing tests that rely on `run.PrepareCmd` to be invoked. + out, err := run.PrepareCmd(gc.Cmd).Output() + if err != nil { + ge := GitError{err: err} + var exitError *exec.ExitError + if errors.As(err, &exitError) { + ge.Stderr = string(exitError.Stderr) + ge.ExitCode = exitError.ExitCode() + } + return []byte{}, &ge + } + return out, nil +} + +func (gc *gitCommand) setRepoDir(repoDir string) { + for i, arg := range gc.Args { + if arg == "-C" { + gc.Args[i+1] = repoDir + return + } + } + gc.Args = append(gc.Args[:3], gc.Args[1:]...) + gc.Args[1] = "-C" + gc.Args[2] = repoDir +} + +// Allow individual commands to be modified from the default client options. +type CommandModifier func(*gitCommand) + +func WithStderr(stderr io.Writer) CommandModifier { + return func(gc *gitCommand) { + gc.Stderr = stderr + } +} + +func WithStdout(stdout io.Writer) CommandModifier { + return func(gc *gitCommand) { + gc.Stdout = stdout + } +} + +func WithStdin(stdin io.Reader) CommandModifier { + return func(gc *gitCommand) { + gc.Stdin = stdin + } +} + +func WithRepoDir(repoDir string) CommandModifier { + return func(gc *gitCommand) { + gc.setRepoDir(repoDir) + } } type Client struct { @@ -133,8 +188,7 @@ func resolveGitPath() (string, error) { // AuthenticatedCommand is a wrapper around Command that included configuration to use gh // as the credential helper for git. func (c *Client) AuthenticatedCommand(ctx context.Context, args ...string) (*gitCommand, error) { - preArgs := []string{} - preArgs = append(preArgs, "-c", "credential.helper=") + preArgs := []string{"-c", "credential.helper="} if c.GhPath == "" { // Assumes that gh is in PATH. c.GhPath = "gh" @@ -153,7 +207,7 @@ func (c *Client) Remotes(ctx context.Context) (RemoteSet, error) { } remoteOut, remoteErr := remoteCmd.Output() if remoteErr != nil { - return nil, &GitError{err: remoteErr} + return nil, remoteErr } configArgs := []string{"config", "--get-regexp", `^remote\..*\.gh-resolved$`} @@ -164,9 +218,9 @@ func (c *Client) Remotes(ctx context.Context) (RemoteSet, error) { configOut, configErr := configCmd.Output() if configErr != nil { // Ignore exit code 1 as it means there are no resolved remotes. - var exitErr *exec.ExitError - if errors.As(configErr, &exitErr) && exitErr.ExitCode() != 1 { - return nil, &GitError{err: configErr} + var gitErr *GitError + if ok := errors.As(configErr, &gitErr); ok && gitErr.ExitCode != 1 { + return nil, gitErr } } @@ -176,18 +230,20 @@ func (c *Client) Remotes(ctx context.Context) (RemoteSet, error) { return remotes, nil } -func (c *Client) AddRemote(ctx context.Context, name, urlStr string, trackingBranches []string) (*Remote, error) { +func (c *Client) AddRemote(ctx context.Context, name, urlStr string, trackingBranches []string, mods ...CommandModifier) (*Remote, error) { args := []string{"remote", "add"} for _, branch := range trackingBranches { args = append(args, "-t", branch) } args = append(args, "-f", name, urlStr) - //TODO: Use AuthenticatedCommand cmd, err := c.Command(ctx, args...) if err != nil { return nil, err } - if err := cmd.Run(); err != nil { + for _, mod := range mods { + mod(cmd) + } + if _, err := cmd.Output(); err != nil { return nil, err } var urlParsed *url.URL @@ -216,7 +272,11 @@ func (c *Client) UpdateRemoteURL(ctx context.Context, name, url string) error { if err != nil { return err } - return cmd.Run() + _, err = cmd.Output() + if err != nil { + return err + } + return nil } func (c *Client) SetRemoteResolution(ctx context.Context, name, resolution string) error { @@ -225,7 +285,11 @@ func (c *Client) SetRemoteResolution(ctx context.Context, name, resolution strin if err != nil { return err } - return cmd.Run() + _, err = cmd.Output() + if err != nil { + return err + } + return nil } // CurrentBranch reads the checked-out branch for the git repository. @@ -235,14 +299,14 @@ func (c *Client) CurrentBranch(ctx context.Context) (string, error) { if err != nil { return "", err } - errBuf := bytes.Buffer{} - cmd.Stderr = &errBuf out, err := cmd.Output() if err != nil { - if errBuf.Len() == 0 { - return "", &GitError{err: err, stderr: "not on any branch"} + var gitErr *GitError + if ok := errors.As(err, &gitErr); ok && len(gitErr.Stderr) == 0 { + gitErr.Stderr = "not on any branch" + return "", gitErr } - return "", &GitError{err: err, stderr: errBuf.String()} + return "", err } branch := firstLine(out) return strings.TrimPrefix(branch, "refs/heads/"), nil @@ -257,7 +321,7 @@ func (c *Client) ShowRefs(ctx context.Context, ref ...string) ([]Ref, error) { } out, err := cmd.Output() if err != nil { - return nil, &GitError{err: err} + return nil, err } var refs []Ref for _, line := range outputLines(out) { @@ -279,15 +343,14 @@ func (c *Client) Config(ctx context.Context, name string) (string, error) { if err != nil { return "", err } - errBuf := bytes.Buffer{} - cmd.Stderr = &errBuf out, err := cmd.Output() if err != nil { - var exitError *exec.ExitError - if ok := errors.As(err, &exitError); ok && exitError.Error() == "1" { - return "", &GitError{err: err, stderr: fmt.Sprintf("unknown config key %s", name)} + var gitErr *GitError + if ok := errors.As(err, &gitErr); ok && gitErr.ExitCode == 1 { + gitErr.Stderr = fmt.Sprintf("unknown config key %s", name) + return "", gitErr } - return "", &GitError{err: err, stderr: errBuf.String()} + return "", err } return firstLine(out), nil } @@ -300,7 +363,7 @@ func (c *Client) UncommittedChangeCount(ctx context.Context) (int, error) { } out, err := cmd.Output() if err != nil { - return 0, &GitError{err: err} + return 0, err } lines := strings.Split(string(out), "\n") count := 0 @@ -320,7 +383,7 @@ func (c *Client) Commits(ctx context.Context, baseRef, headRef string) ([]*Commi } out, err := cmd.Output() if err != nil { - return nil, &GitError{err: err} + return nil, err } commits := []*Commit{} sha := 0 @@ -349,7 +412,7 @@ func (c *Client) lookupCommit(ctx context.Context, sha, format string) ([]byte, } out, err := cmd.Output() if err != nil { - return nil, &GitError{err: err} + return nil, err } return out, nil } @@ -372,13 +435,16 @@ func (c *Client) CommitBody(ctx context.Context, sha string) (string, error) { } // Push publishes a git ref to a remote and sets up upstream configuration. -func (c *Client) Push(ctx context.Context, remote string, ref string) error { +func (c *Client) Push(ctx context.Context, remote string, ref string, mods ...CommandModifier) error { args := []string{"push", "--set-upstream", remote, ref} //TODO: Use AuthenticatedCommand cmd, err := c.Command(ctx, args...) if err != nil { return err } + for _, mod := range mods { + mod(cmd) + } return cmd.Run() } @@ -424,7 +490,11 @@ func (c *Client) DeleteLocalBranch(ctx context.Context, branch string) error { if err != nil { return err } - return cmd.Run() + _, err = cmd.Output() + if err != nil { + return err + } + return nil } func (c *Client) HasLocalBranch(ctx context.Context, branch string) bool { @@ -433,7 +503,7 @@ func (c *Client) HasLocalBranch(ctx context.Context, branch string) bool { if err != nil { return false } - err = cmd.Run() + _, err = cmd.Output() return err == nil } @@ -443,7 +513,11 @@ func (c *Client) CheckoutBranch(ctx context.Context, branch string) error { if err != nil { return err } - return cmd.Run() + _, err = cmd.Output() + if err != nil { + return err + } + return nil } func (c *Client) CheckoutNewBranch(ctx context.Context, remoteName, branch string) error { @@ -453,7 +527,11 @@ func (c *Client) CheckoutNewBranch(ctx context.Context, remoteName, branch strin if err != nil { return err } - return cmd.Run() + _, err = cmd.Output() + if err != nil { + return err + } + return nil } func (c *Client) Pull(ctx context.Context, remote, branch string) error { @@ -466,7 +544,7 @@ func (c *Client) Pull(ctx context.Context, remote, branch string) error { return cmd.Run() } -func (c *Client) Clone(ctx context.Context, cloneURL string, args []string) (target string, err error) { +func (c *Client) Clone(ctx context.Context, cloneURL string, args []string) (string, error) { cloneArgs, target := parseCloneArgs(args) cloneArgs = append(cloneArgs, cloneURL) // If the args contain an explicit target, pass it to clone @@ -483,7 +561,10 @@ func (c *Client) Clone(ctx context.Context, cloneURL string, args []string) (tar return "", err } err = cmd.Run() - return + if err != nil { + return "", err + } + return target, nil } // ToplevelDir returns the top-level directory path of the current repository. @@ -495,7 +576,7 @@ func (c *Client) ToplevelDir(ctx context.Context) (string, error) { } out, err := cmd.Output() if err != nil { - return "", &GitError{err: err} + return "", err } return firstLine(out), nil } @@ -508,7 +589,7 @@ func (c *Client) GitDir(ctx context.Context) (string, error) { } out, err := cmd.Output() if err != nil { - return "", &GitError{err: err} + return "", err } return firstLine(out), nil } diff --git a/git/client_test.go b/git/client_test.go index 121231d23..cff47358f 100644 --- a/git/client_test.go +++ b/git/client_test.go @@ -394,6 +394,6 @@ func initRepo(t *testing.T, dir string) { } cmd, err := client.Command(context.Background(), []string{"init", "--quiet"}...) assert.NoError(t, err) - err = cmd.Run() + _, err = cmd.Output() assert.NoError(t, err) } diff --git a/git/git.go b/git/git.go deleted file mode 100644 index 5b934175b..000000000 --- a/git/git.go +++ /dev/null @@ -1,154 +0,0 @@ -package git - -import ( - "context" - "io" - "os" -) - -func GitCommand(args ...string) (*gitCommand, error) { - c := &Client{} - return c.Command(context.Background(), args...) -} - -func ShowRefs(ref ...string) ([]Ref, error) { - c := &Client{} - return c.ShowRefs(context.Background(), ref...) -} - -func CurrentBranch() (string, error) { - c := &Client{} - return c.CurrentBranch(context.Background()) -} - -func Config(name string) (string, error) { - c := &Client{} - return c.Config(context.Background(), name) -} - -func UncommittedChangeCount() (int, error) { - c := &Client{} - return c.UncommittedChangeCount(context.Background()) -} - -func Commits(baseRef, headRef string) ([]*Commit, error) { - c := &Client{} - return c.Commits(context.Background(), baseRef, headRef) -} - -func LastCommit() (*Commit, error) { - c := &Client{} - return c.LastCommit(context.Background()) -} - -func CommitBody(sha string) (string, error) { - c := &Client{} - return c.CommitBody(context.Background(), sha) -} - -func Push(remote string, ref string, cmdIn io.ReadCloser, cmdOut, cmdErr io.Writer) error { - //TODO: Replace with factory GitClient and use AuthenticatedCommand - c := &Client{ - Stdin: cmdIn, - Stdout: cmdOut, - Stderr: cmdErr, - } - return c.Push(context.Background(), remote, ref) -} - -func ReadBranchConfig(branch string) (cfg BranchConfig) { - c := &Client{} - return c.ReadBranchConfig(context.Background(), branch) -} - -func DeleteLocalBranch(branch string) error { - c := &Client{} - return c.DeleteLocalBranch(context.Background(), branch) -} - -func HasLocalBranch(branch string) bool { - c := &Client{} - return c.HasLocalBranch(context.Background(), branch) -} - -func CheckoutBranch(branch string) error { - c := &Client{} - return c.CheckoutBranch(context.Background(), branch) -} - -func CheckoutNewBranch(remoteName, branch string) error { - c := &Client{} - return c.CheckoutNewBranch(context.Background(), remoteName, branch) -} - -func Pull(remote, branch string) error { - //TODO: Replace with factory GitClient and use AuthenticatedCommand - c := &Client{ - Stdin: os.Stdin, - Stdout: os.Stdout, - Stderr: os.Stderr, - } - return c.Pull(context.Background(), remote, branch) -} - -func RunClone(cloneURL string, args []string) (target string, err error) { - //TODO: Replace with factory GitClient and use AuthenticatedCommand - c := &Client{ - Stdin: os.Stdin, - Stdout: os.Stdout, - Stderr: os.Stderr, - } - return c.Clone(context.Background(), cloneURL, args) -} - -func ToplevelDir() (string, error) { - c := &Client{} - return c.ToplevelDir(context.Background()) -} - -func GetDirFromPath(repoDir string) (string, error) { - c := &Client{ - RepoDir: repoDir, - } - return c.GitDir(context.Background()) -} - -func PathFromRepoRoot() string { - c := &Client{} - return c.PathFromRoot(context.Background()) -} - -func Remotes() (RemoteSet, error) { - c := &Client{} - return c.Remotes(context.Background()) -} - -func RemotesForPath(repoDir string) (RemoteSet, error) { - c := &Client{ - RepoDir: repoDir, - } - return c.Remotes(context.Background()) -} - -func AddRemote(name, url string) (*Remote, error) { - c := &Client{} - return c.AddRemote(context.Background(), name, url, []string{}) -} - -func AddNamedRemote(url, name, repoDir string, branches []string) error { - c := &Client{ - RepoDir: repoDir, - } - _, err := c.AddRemote(context.Background(), name, url, branches) - return err -} - -func UpdateRemoteURL(name, url string) error { - c := &Client{} - return c.UpdateRemoteURL(context.Background(), name, url) -} - -func SetRemoteResolution(name, resolution string) error { - c := &Client{} - return c.SetRemoteResolution(context.Background(), name, resolution) -} diff --git a/internal/run/run.go b/internal/run/run.go index d482e04cc..a7260db14 100644 --- a/internal/run/run.go +++ b/internal/run/run.go @@ -76,6 +76,10 @@ func (e CmdError) Error() string { return fmt.Sprintf("%s%s: %s", msg, e.Args[0], e.Err) } +func (e CmdError) Unwrap() error { + return e.Err +} + func printArgs(w io.Writer, args []string) error { if len(args) > 0 { // print commands, but omit the full path to an executable diff --git a/pkg/cmd/auth/login/login.go b/pkg/cmd/auth/login/login.go index be12392d1..364abb9f0 100644 --- a/pkg/cmd/auth/login/login.go +++ b/pkg/cmd/auth/login/login.go @@ -7,6 +7,7 @@ import ( "strings" "github.com/MakeNowJust/heredoc" + "github.com/cli/cli/v2/git" "github.com/cli/cli/v2/internal/config" "github.com/cli/cli/v2/internal/ghinstance" "github.com/cli/cli/v2/pkg/cmd/auth/shared" @@ -20,6 +21,7 @@ type LoginOptions struct { IO *iostreams.IOStreams Config func() (config.Config, error) HttpClient func() (*http.Client, error) + GitClient *git.Client Prompter shared.Prompt MainExecutable string @@ -38,6 +40,7 @@ func NewCmdLogin(f *cmdutil.Factory, runF func(*LoginOptions) error) *cobra.Comm IO: f.IOStreams, Config: f.Config, HttpClient: f.HttpClient, + GitClient: f.GitClient, Prompter: f.Prompter, } @@ -183,6 +186,7 @@ func loginRun(opts *LoginOptions) error { Executable: opts.MainExecutable, GitProtocol: opts.GitProtocol, Prompter: opts.Prompter, + GitClient: opts.GitClient, }) } diff --git a/pkg/cmd/auth/login/login_test.go b/pkg/cmd/auth/login/login_test.go index 649fdb48d..85e133b2b 100644 --- a/pkg/cmd/auth/login/login_test.go +++ b/pkg/cmd/auth/login/login_test.go @@ -8,6 +8,7 @@ import ( "testing" "github.com/MakeNowJust/heredoc" + "github.com/cli/cli/v2/git" "github.com/cli/cli/v2/internal/config" "github.com/cli/cli/v2/internal/prompter" "github.com/cli/cli/v2/internal/run" @@ -597,6 +598,8 @@ func Test_loginRun_Survey(t *testing.T) { } tt.opts.Prompter = pm + tt.opts.GitClient = &git.Client{GitPath: "some/path/git"} + rs, restoreRun := run.Stub() defer restoreRun(t) if tt.runStubs != nil { diff --git a/pkg/cmd/auth/refresh/refresh.go b/pkg/cmd/auth/refresh/refresh.go index 7b1142e7e..c66fb5237 100644 --- a/pkg/cmd/auth/refresh/refresh.go +++ b/pkg/cmd/auth/refresh/refresh.go @@ -6,6 +6,7 @@ import ( "strings" "github.com/MakeNowJust/heredoc" + "github.com/cli/cli/v2/git" "github.com/cli/cli/v2/internal/authflow" "github.com/cli/cli/v2/internal/config" "github.com/cli/cli/v2/pkg/cmd/auth/shared" @@ -17,7 +18,8 @@ import ( type RefreshOptions struct { IO *iostreams.IOStreams Config func() (config.Config, error) - httpClient *http.Client + HttpClient *http.Client + GitClient *git.Client Prompter shared.Prompt MainExecutable string @@ -37,7 +39,8 @@ func NewCmdRefresh(f *cmdutil.Factory, runF func(*RefreshOptions) error) *cobra. _, err := authflow.AuthFlowWithConfig(cfg, io, hostname, "", scopes, interactive) return err }, - httpClient: &http.Client{}, + HttpClient: &http.Client{}, + GitClient: f.GitClient, Prompter: f.Prompter, } @@ -122,7 +125,7 @@ func refreshRun(opts *RefreshOptions) error { var additionalScopes []string if oldToken, _ := cfg.AuthToken(hostname); oldToken != "" { - if oldScopes, err := shared.GetScopes(opts.httpClient, hostname, oldToken); err == nil { + if oldScopes, err := shared.GetScopes(opts.HttpClient, hostname, oldToken); err == nil { for _, s := range strings.Split(oldScopes, ",") { s = strings.TrimSpace(s) if s != "" { @@ -135,6 +138,7 @@ func refreshRun(opts *RefreshOptions) error { credentialFlow := &shared.GitCredentialFlow{ Executable: opts.MainExecutable, Prompter: opts.Prompter, + GitClient: opts.GitClient, } gitProtocol, _ := cfg.GetOrDefault(hostname, "git_protocol") if opts.Interactive && gitProtocol == "https" { diff --git a/pkg/cmd/auth/refresh/refresh_test.go b/pkg/cmd/auth/refresh/refresh_test.go index ffacb22d9..d1d6042c0 100644 --- a/pkg/cmd/auth/refresh/refresh_test.go +++ b/pkg/cmd/auth/refresh/refresh_test.go @@ -272,7 +272,7 @@ func Test_refreshRun(t *testing.T) { }, nil }, ) - tt.opts.httpClient = &http.Client{Transport: httpReg} + tt.opts.HttpClient = &http.Client{Transport: httpReg} pm := &prompter.PrompterMock{} if tt.prompterStubs != nil { diff --git a/pkg/cmd/auth/setupgit/setupgit.go b/pkg/cmd/auth/setupgit/setupgit.go index b5caefe5b..edc531235 100644 --- a/pkg/cmd/auth/setupgit/setupgit.go +++ b/pkg/cmd/auth/setupgit/setupgit.go @@ -34,6 +34,7 @@ func NewCmdSetupGit(f *cmdutil.Factory, runF func(*SetupGitOptions) error) *cobr RunE: func(cmd *cobra.Command, args []string) error { opts.gitConfigure = &shared.GitCredentialFlow{ Executable: f.Executable(), + GitClient: f.GitClient, } if runF != nil { diff --git a/pkg/cmd/auth/shared/git_credential.go b/pkg/cmd/auth/shared/git_credential.go index 491e2c82f..5ddac14d5 100644 --- a/pkg/cmd/auth/shared/git_credential.go +++ b/pkg/cmd/auth/shared/git_credential.go @@ -2,6 +2,7 @@ package shared import ( "bytes" + "context" "errors" "fmt" "path/filepath" @@ -16,6 +17,7 @@ import ( type GitCredentialFlow struct { Executable string Prompter Prompt + GitClient *git.Client shouldSetup bool helper string @@ -24,7 +26,7 @@ type GitCredentialFlow struct { func (flow *GitCredentialFlow) Prompt(hostname string) error { var gitErr error - flow.helper, gitErr = gitCredentialHelper(hostname) + flow.helper, gitErr = gitCredentialHelper(flow.GitClient, hostname) if isOurCredentialHelper(flow.helper) { flow.scopes = append(flow.scopes, "workflow") return nil @@ -59,6 +61,9 @@ func (flow *GitCredentialFlow) Setup(hostname, username, authToken string) error } func (flow *GitCredentialFlow) gitCredentialSetup(hostname, username, password string) error { + gitClient := flow.GitClient + ctx := context.Background() + if flow.helper == "" { credHelperKeys := []string{ gitCredentialHelperKey(hostname), @@ -76,18 +81,18 @@ func (flow *GitCredentialFlow) gitCredentialSetup(hostname, username, password s break } // first use a blank value to indicate to git we want to sever the chain of credential helpers - preConfigureCmd, err := git.GitCommand("config", "--global", "--replace-all", credHelperKey, "") + preConfigureCmd, err := gitClient.Command(ctx, "config", "--global", "--replace-all", credHelperKey, "") if err != nil { configErr = err break } - if err = preConfigureCmd.Run(); err != nil { + if _, err = preConfigureCmd.Output(); err != nil { configErr = err break } // second configure the actual helper for this host - configureCmd, err := git.GitCommand( + configureCmd, err := gitClient.Command(ctx, "config", "--global", "--add", credHelperKey, fmt.Sprintf("!%s auth git-credential", shellQuote(flow.Executable)), @@ -95,7 +100,7 @@ func (flow *GitCredentialFlow) gitCredentialSetup(hostname, username, password s if err != nil { configErr = err } else { - configErr = configureCmd.Run() + _, configErr = configureCmd.Output() } } @@ -103,7 +108,7 @@ func (flow *GitCredentialFlow) gitCredentialSetup(hostname, username, password s } // clear previous cached credentials - rejectCmd, err := git.GitCommand("credential", "reject") + rejectCmd, err := gitClient.Command(ctx, "credential", "reject") if err != nil { return err } @@ -113,12 +118,12 @@ func (flow *GitCredentialFlow) gitCredentialSetup(hostname, username, password s host=%s `, hostname)) - err = rejectCmd.Run() + _, err = rejectCmd.Output() if err != nil { return err } - approveCmd, err := git.GitCommand("credential", "approve") + approveCmd, err := gitClient.Command(ctx, "credential", "approve") if err != nil { return err } @@ -130,7 +135,7 @@ func (flow *GitCredentialFlow) gitCredentialSetup(hostname, username, password s password=%s `, hostname, username, password)) - err = approveCmd.Run() + _, err = approveCmd.Output() if err != nil { return err } @@ -143,12 +148,13 @@ func gitCredentialHelperKey(hostname string) string { return fmt.Sprintf("credential.%s.helper", host) } -func gitCredentialHelper(hostname string) (helper string, err error) { - helper, err = git.Config(gitCredentialHelperKey(hostname)) +func gitCredentialHelper(gitClient *git.Client, hostname string) (helper string, err error) { + ctx := context.Background() + helper, err = gitClient.Config(ctx, gitCredentialHelperKey(hostname)) if helper != "" { return } - helper, err = git.Config("credential.helper") + helper, err = gitClient.Config(ctx, "credential.helper") return } diff --git a/pkg/cmd/auth/shared/git_credential_test.go b/pkg/cmd/auth/shared/git_credential_test.go index fe674e1d7..9d3e90cc4 100644 --- a/pkg/cmd/auth/shared/git_credential_test.go +++ b/pkg/cmd/auth/shared/git_credential_test.go @@ -3,6 +3,7 @@ package shared import ( "testing" + "github.com/cli/cli/v2/git" "github.com/cli/cli/v2/internal/run" ) @@ -15,6 +16,7 @@ func TestGitCredentialSetup_configureExisting(t *testing.T) { f := GitCredentialFlow{ Executable: "gh", helper: "osxkeychain", + GitClient: &git.Client{GitPath: "some/path/git"}, } if err := f.gitCredentialSetup("example.com", "monalisa", "PASSWD"); err != nil { @@ -61,6 +63,7 @@ func TestGitCredentialsSetup_setOurs_GH(t *testing.T) { f := GitCredentialFlow{ Executable: "/path/to/gh", helper: "", + GitClient: &git.Client{GitPath: "some/path/git"}, } if err := f.gitCredentialSetup("github.com", "monalisa", "PASSWD"); err != nil { @@ -92,6 +95,7 @@ func TestGitCredentialSetup_setOurs_nonGH(t *testing.T) { f := GitCredentialFlow{ Executable: "/path/to/gh", helper: "", + GitClient: &git.Client{GitPath: "some/path/git"}, } if err := f.gitCredentialSetup("example.com", "monalisa", "PASSWD"); err != nil { diff --git a/pkg/cmd/auth/shared/login_flow.go b/pkg/cmd/auth/shared/login_flow.go index 9cc9d9865..99d82b5f7 100644 --- a/pkg/cmd/auth/shared/login_flow.go +++ b/pkg/cmd/auth/shared/login_flow.go @@ -8,6 +8,7 @@ import ( "github.com/MakeNowJust/heredoc" "github.com/cli/cli/v2/api" + "github.com/cli/cli/v2/git" "github.com/cli/cli/v2/internal/authflow" "github.com/cli/cli/v2/internal/ghinstance" "github.com/cli/cli/v2/pkg/cmd/ssh-key/add" @@ -27,6 +28,7 @@ type LoginOptions struct { IO *iostreams.IOStreams Config iconfig HTTPClient *http.Client + GitClient *git.Client Hostname string Interactive bool Web bool @@ -63,7 +65,11 @@ func Login(opts *LoginOptions) error { var additionalScopes []string - credentialFlow := &GitCredentialFlow{Executable: opts.Executable, Prompter: opts.Prompter} + credentialFlow := &GitCredentialFlow{ + Executable: opts.Executable, + Prompter: opts.Prompter, + GitClient: opts.GitClient, + } if opts.Interactive && gitProtocol == "https" { if err := credentialFlow.Prompt(hostname); err != nil { return err diff --git a/pkg/cmd/browse/browse.go b/pkg/cmd/browse/browse.go index ddff8bd92..85572fdac 100644 --- a/pkg/cmd/browse/browse.go +++ b/pkg/cmd/browse/browse.go @@ -1,6 +1,7 @@ package browse import ( + "context" "fmt" "net/http" "net/url" @@ -41,11 +42,13 @@ type BrowseOptions struct { func NewCmdBrowse(f *cmdutil.Factory, runF func(*BrowseOptions) error) *cobra.Command { opts := &BrowseOptions{ - Browser: f.Browser, - HttpClient: f.HttpClient, - IO: f.IOStreams, - PathFromRepoRoot: git.PathFromRepoRoot, - GitClient: &localGitClient{}, + Browser: f.Browser, + HttpClient: f.HttpClient, + IO: f.IOStreams, + PathFromRepoRoot: func() string { + return f.GitClient.PathFromRoot(context.Background()) + }, + GitClient: &localGitClient{client: f.GitClient}, } cmd := &cobra.Command{ @@ -269,14 +272,18 @@ type gitClient interface { LastCommit() (*git.Commit, error) } -type localGitClient struct{} +type localGitClient struct { + client *git.Client +} type remoteGitClient struct { repo func() (ghrepo.Interface, error) httpClient func() (*http.Client, error) } -func (gc *localGitClient) LastCommit() (*git.Commit, error) { return git.LastCommit() } +func (gc *localGitClient) LastCommit() (*git.Commit, error) { + return gc.client.LastCommit(context.Background()) +} func (gc *remoteGitClient) LastCommit() (*git.Commit, error) { httpClient, err := gc.httpClient() diff --git a/pkg/cmd/browse/browse_test.go b/pkg/cmd/browse/browse_test.go index 91197cc96..75e615be0 100644 --- a/pkg/cmd/browse/browse_test.go +++ b/pkg/cmd/browse/browse_test.go @@ -466,7 +466,7 @@ func Test_runBrowse(t *testing.T) { } opts.Browser = &browser if opts.PathFromRepoRoot == nil { - opts.PathFromRepoRoot = git.PathFromRepoRoot + opts.PathFromRepoRoot = func() string { return "" } } err := runBrowse(&opts) diff --git a/pkg/cmd/extension/manager.go b/pkg/cmd/extension/manager.go index 289a9aefa..c9f638b87 100644 --- a/pkg/cmd/extension/manager.go +++ b/pkg/cmd/extension/manager.go @@ -2,6 +2,7 @@ package extension import ( "bytes" + "context" _ "embed" "errors" "fmt" @@ -789,7 +790,8 @@ func isBinExtension(client *http.Client, repo ghrepo.Interface) (isBin bool, err } func repoFromPath(path string) (ghrepo.Interface, error) { - remotes, err := git.RemotesForPath(path) + gitClient := &git.Client{RepoDir: path} + remotes, err := gitClient.Remotes(context.Background()) if err != nil { return nil, err } diff --git a/pkg/cmd/factory/default.go b/pkg/cmd/factory/default.go index 77552c977..10bf5d72b 100644 --- a/pkg/cmd/factory/default.go +++ b/pkg/cmd/factory/default.go @@ -1,6 +1,7 @@ package factory import ( + "context" "fmt" "net/http" "os" @@ -8,7 +9,7 @@ import ( "time" "github.com/cli/cli/v2/api" - "github.com/cli/cli/v2/context" + ghContext "github.com/cli/cli/v2/context" "github.com/cli/cli/v2/git" "github.com/cli/cli/v2/internal/browser" "github.com/cli/cli/v2/internal/config" @@ -25,18 +26,18 @@ var ssoURLRE = regexp.MustCompile(`\burl=([^;]+)`) func New(appVersion string) *cmdutil.Factory { f := &cmdutil.Factory{ Config: configFunc(), // No factory dependencies - Branch: branchFunc(), // No factory dependencies ExecutableName: "gh", } f.IOStreams = ioStreams(f) // Depends on Config f.HttpClient = httpClientFunc(f, appVersion) // Depends on Config, IOStreams, and appVersion - f.Remotes = remotesFunc(f) // Depends on Config + f.GitClient = newGitClient(f) // Depends on IOStreams, and Executable + f.Remotes = remotesFunc(f) // Depends on Config, and GitClient f.BaseRepo = BaseRepoFunc(f) // Depends on Remotes f.Prompter = newPrompter(f) // Depends on Config and IOStreams f.Browser = newBrowser(f) // Depends on Config, and IOStreams - f.GitClient = newGitClient(f) // Depends on IOStreams, and Executable f.ExtensionManager = extensionManager(f) // Depends on Config, HttpClient, and IOStreams + f.Branch = branchFunc(f) // Depends on GitClient return f } @@ -64,7 +65,7 @@ func SmartBaseRepoFunc(f *cmdutil.Factory) func() (ghrepo.Interface, error) { if err != nil { return nil, err } - repoContext, err := context.ResolveRemotesToRepos(remotes, apiClient, "") + repoContext, err := ghContext.ResolveRemotesToRepos(remotes, apiClient, "") if err != nil { return nil, err } @@ -77,10 +78,12 @@ func SmartBaseRepoFunc(f *cmdutil.Factory) func() (ghrepo.Interface, error) { } } -func remotesFunc(f *cmdutil.Factory) func() (context.Remotes, error) { +func remotesFunc(f *cmdutil.Factory) func() (ghContext.Remotes, error) { rr := &remoteResolver{ - readRemotes: git.Remotes, - getConfig: f.Config, + readRemotes: func() (git.RemoteSet, error) { + return f.GitClient.Remotes(context.Background()) + }, + getConfig: f.Config, } return rr.Resolver() } @@ -142,9 +145,9 @@ func configFunc() func() (config.Config, error) { } } -func branchFunc() func() (string, error) { +func branchFunc(f *cmdutil.Factory) func() (string, error) { return func() (string, error) { - currentBranch, err := git.CurrentBranch() + currentBranch, err := f.GitClient.CurrentBranch(context.Background()) if err != nil { return "", fmt.Errorf("could not determine current branch: %w", err) } diff --git a/pkg/cmd/gist/clone/clone.go b/pkg/cmd/gist/clone/clone.go index 41fa104fa..ed074e700 100644 --- a/pkg/cmd/gist/clone/clone.go +++ b/pkg/cmd/gist/clone/clone.go @@ -1,6 +1,7 @@ package clone import ( + "context" "fmt" "net/http" @@ -16,6 +17,7 @@ import ( type CloneOptions struct { HttpClient func() (*http.Client, error) + GitClient *git.Client Config func() (config.Config, error) IO *iostreams.IOStreams @@ -28,6 +30,7 @@ func NewCmdClone(f *cmdutil.Factory, runF func(*CloneOptions) error) *cobra.Comm opts := &CloneOptions{ IO: f.IOStreams, HttpClient: f.HttpClient, + GitClient: f.GitClient, Config: f.Config, } @@ -84,7 +87,7 @@ func cloneRun(opts *CloneOptions) error { gistURL = formatRemoteURL(hostname, gistURL, protocol) } - _, err := git.RunClone(gistURL, opts.GitArgs) + _, err := opts.GitClient.Clone(context.Background(), gistURL, opts.GitArgs) if err != nil { return err } diff --git a/pkg/cmd/gist/clone/clone_test.go b/pkg/cmd/gist/clone/clone_test.go index ccca76b25..cd6f71b6e 100644 --- a/pkg/cmd/gist/clone/clone_test.go +++ b/pkg/cmd/gist/clone/clone_test.go @@ -5,6 +5,7 @@ import ( "strings" "testing" + "github.com/cli/cli/v2/git" "github.com/cli/cli/v2/internal/config" "github.com/cli/cli/v2/internal/run" "github.com/cli/cli/v2/pkg/cmdutil" @@ -25,6 +26,7 @@ func runCloneCommand(httpClient *http.Client, cli string) (*test.CmdOut, error) Config: func() (config.Config, error) { return config.NewBlankConfig(), nil }, + GitClient: &git.Client{GitPath: "some/path/git"}, } cmd := NewCmdClone(fac, nil) diff --git a/pkg/cmd/pr/checkout/checkout.go b/pkg/cmd/pr/checkout/checkout.go index 73c8f40d1..8addfc4ae 100644 --- a/pkg/cmd/pr/checkout/checkout.go +++ b/pkg/cmd/pr/checkout/checkout.go @@ -19,6 +19,7 @@ import ( type CheckoutOptions struct { HttpClient func() (*http.Client, error) + GitClient *git.Client Config func() (config.Config, error) IO *iostreams.IOStreams Remotes func() (cliContext.Remotes, error) @@ -37,6 +38,7 @@ func NewCmdCheckout(f *cmdutil.Factory, runF func(*CheckoutOptions) error) *cobr opts := &CheckoutOptions{ IO: f.IOStreams, HttpClient: f.HttpClient, + GitClient: f.GitClient, Config: f.Config, Remotes: f.Remotes, Branch: f.Branch, @@ -124,11 +126,11 @@ func checkoutRun(opts *CheckoutOptions) error { } if opts.RecurseSubmodules { - cmdQueue = append(cmdQueue, []string{"git", "submodule", "sync", "--recursive"}) - cmdQueue = append(cmdQueue, []string{"git", "submodule", "update", "--init", "--recursive"}) + cmdQueue = append(cmdQueue, []string{"submodule", "sync", "--recursive"}) + cmdQueue = append(cmdQueue, []string{"submodule", "update", "--init", "--recursive"}) } - err = executeCmds(cmdQueue, opts.IO) + err = executeCmds(opts.GitClient, cmdQueue) if err != nil { return err } @@ -145,7 +147,7 @@ func cmdsForExistingRemote(remote *cliContext.Remote, pr *api.PullRequest, opts refSpec += fmt.Sprintf(":refs/remotes/%s", remoteBranch) } - cmds = append(cmds, []string{"git", "fetch", remote.Name, refSpec}) + cmds = append(cmds, []string{"fetch", remote.Name, refSpec}) localBranch := pr.HeadRefName if opts.BranchName != "" { @@ -154,17 +156,17 @@ func cmdsForExistingRemote(remote *cliContext.Remote, pr *api.PullRequest, opts switch { case opts.Detach: - cmds = append(cmds, []string{"git", "checkout", "--detach", "FETCH_HEAD"}) - case localBranchExists(localBranch): - cmds = append(cmds, []string{"git", "checkout", localBranch}) + cmds = append(cmds, []string{"checkout", "--detach", "FETCH_HEAD"}) + case localBranchExists(opts.GitClient, localBranch): + cmds = append(cmds, []string{"checkout", localBranch}) if opts.Force { - cmds = append(cmds, []string{"git", "reset", "--hard", fmt.Sprintf("refs/remotes/%s", remoteBranch)}) + cmds = append(cmds, []string{"reset", "--hard", fmt.Sprintf("refs/remotes/%s", remoteBranch)}) } else { // TODO: check if non-fast-forward and suggest to use `--force` - cmds = append(cmds, []string{"git", "merge", "--ff-only", fmt.Sprintf("refs/remotes/%s", remoteBranch)}) + cmds = append(cmds, []string{"merge", "--ff-only", fmt.Sprintf("refs/remotes/%s", remoteBranch)}) } default: - cmds = append(cmds, []string{"git", "checkout", "-b", localBranch, "--track", remoteBranch}) + cmds = append(cmds, []string{"checkout", "-b", localBranch, "--track", remoteBranch}) } return cmds @@ -175,8 +177,8 @@ func cmdsForMissingRemote(pr *api.PullRequest, baseURLOrName, repoHost, defaultB ref := fmt.Sprintf("refs/pull/%d/head", pr.Number) if opts.Detach { - cmds = append(cmds, []string{"git", "fetch", baseURLOrName, ref}) - cmds = append(cmds, []string{"git", "checkout", "--detach", "FETCH_HEAD"}) + cmds = append(cmds, []string{"fetch", baseURLOrName, ref}) + cmds = append(cmds, []string{"checkout", "--detach", "FETCH_HEAD"}) return cmds } @@ -191,22 +193,22 @@ func cmdsForMissingRemote(pr *api.PullRequest, baseURLOrName, repoHost, defaultB currentBranch, _ := opts.Branch() if localBranch == currentBranch { // PR head matches currently checked out branch - cmds = append(cmds, []string{"git", "fetch", baseURLOrName, ref}) + cmds = append(cmds, []string{"fetch", baseURLOrName, ref}) if opts.Force { - cmds = append(cmds, []string{"git", "reset", "--hard", "FETCH_HEAD"}) + cmds = append(cmds, []string{"reset", "--hard", "FETCH_HEAD"}) } else { // TODO: check if non-fast-forward and suggest to use `--force` - cmds = append(cmds, []string{"git", "merge", "--ff-only", "FETCH_HEAD"}) + cmds = append(cmds, []string{"merge", "--ff-only", "FETCH_HEAD"}) } } else { if opts.Force { - cmds = append(cmds, []string{"git", "fetch", baseURLOrName, fmt.Sprintf("%s:%s", ref, localBranch), "--force"}) + cmds = append(cmds, []string{"fetch", baseURLOrName, fmt.Sprintf("%s:%s", ref, localBranch), "--force"}) } else { // TODO: check if non-fast-forward and suggest to use `--force` - cmds = append(cmds, []string{"git", "fetch", baseURLOrName, fmt.Sprintf("%s:%s", ref, localBranch)}) + cmds = append(cmds, []string{"fetch", baseURLOrName, fmt.Sprintf("%s:%s", ref, localBranch)}) } - cmds = append(cmds, []string{"git", "checkout", localBranch}) + cmds = append(cmds, []string{"checkout", localBranch}) } remote := baseURLOrName @@ -216,37 +218,32 @@ func cmdsForMissingRemote(pr *api.PullRequest, baseURLOrName, repoHost, defaultB remote = ghrepo.FormatRemoteURL(headRepo, protocol) mergeRef = fmt.Sprintf("refs/heads/%s", pr.HeadRefName) } - if missingMergeConfigForBranch(localBranch) { + if missingMergeConfigForBranch(opts.GitClient, localBranch) { // .remote is needed for `git pull` to work // .pushRemote is needed for `git push` to work, if user has set `remote.pushDefault`. // see https://git-scm.com/docs/git-config#Documentation/git-config.txt-branchltnamegtremote - cmds = append(cmds, []string{"git", "config", fmt.Sprintf("branch.%s.remote", localBranch), remote}) - cmds = append(cmds, []string{"git", "config", fmt.Sprintf("branch.%s.pushRemote", localBranch), remote}) - cmds = append(cmds, []string{"git", "config", fmt.Sprintf("branch.%s.merge", localBranch), mergeRef}) + cmds = append(cmds, []string{"config", fmt.Sprintf("branch.%s.remote", localBranch), remote}) + cmds = append(cmds, []string{"config", fmt.Sprintf("branch.%s.pushRemote", localBranch), remote}) + cmds = append(cmds, []string{"config", fmt.Sprintf("branch.%s.merge", localBranch), mergeRef}) } return cmds } -func missingMergeConfigForBranch(b string) bool { - mc, err := git.Config(fmt.Sprintf("branch.%s.merge", b)) +func missingMergeConfigForBranch(client *git.Client, b string) bool { + mc, err := client.Config(context.Background(), fmt.Sprintf("branch.%s.merge", b)) return err != nil || mc == "" } -func localBranchExists(b string) bool { - _, err := git.ShowRefs("refs/heads/" + b) +func localBranchExists(client *git.Client, b string) bool { + _, err := client.ShowRefs(context.Background(), "refs/heads/"+b) return err == nil } -func executeCmds(cmdQueue [][]string, ios *iostreams.IOStreams) error { - //TODO: Replace with factory GitClient - //TODO: Use AuthenticatedCommand - client := git.Client{ - Stdout: ios.Out, - Stderr: ios.ErrOut, - } +func executeCmds(client *git.Client, cmdQueue [][]string) error { for _, args := range cmdQueue { - cmd, err := client.Command(context.Background(), args[1:]...) + //TODO: Use AuthenticatedCommand + cmd, err := client.Command(context.Background(), args...) if err != nil { return err } diff --git a/pkg/cmd/pr/checkout/checkout_test.go b/pkg/cmd/pr/checkout/checkout_test.go index df77e6676..7540bbacf 100644 --- a/pkg/cmd/pr/checkout/checkout_test.go +++ b/pkg/cmd/pr/checkout/checkout_test.go @@ -197,6 +197,8 @@ func Test_checkoutRun(t *testing.T) { return remotes, nil } + opts.GitClient = &git.Client{GitPath: "some/path/git"} + err := checkoutRun(opts) if (err != nil) != tt.wantErr { t.Errorf("want error: %v, got: %v", tt.wantErr, err) @@ -234,6 +236,7 @@ func runCommand(rt http.RoundTripper, remotes context.Remotes, branch string, cl Branch: func() (string, error) { return branch, nil }, + GitClient: &git.Client{GitPath: "some/path/git"}, } cmd := NewCmdCheckout(factory, nil) diff --git a/pkg/cmd/pr/close/close.go b/pkg/cmd/pr/close/close.go index 7d80199b6..7e39005bd 100644 --- a/pkg/cmd/pr/close/close.go +++ b/pkg/cmd/pr/close/close.go @@ -1,6 +1,7 @@ package close import ( + "context" "fmt" "net/http" @@ -15,6 +16,7 @@ import ( type CloseOptions struct { HttpClient func() (*http.Client, error) + GitClient *git.Client IO *iostreams.IOStreams Branch func() (string, error) @@ -30,6 +32,7 @@ func NewCmdClose(f *cmdutil.Factory, runF func(*CloseOptions) error) *cobra.Comm opts := &CloseOptions{ IO: f.IOStreams, HttpClient: f.HttpClient, + GitClient: f.GitClient, Branch: f.Branch, } @@ -108,9 +111,10 @@ func closeRun(opts *CloseOptions) error { fmt.Fprintf(opts.IO.ErrOut, "%s Closed pull request #%d (%s)\n", cs.SuccessIconWithColor(cs.Red), pr.Number, pr.Title) if opts.DeleteBranch { + ctx := context.Background() branchSwitchString := "" apiClient := api.NewClientFromHTTP(httpClient) - localBranchExists := git.HasLocalBranch(pr.HeadRefName) + localBranchExists := opts.GitClient.HasLocalBranch(ctx, pr.HeadRefName) if opts.DeleteLocalBranch { if localBranchExists { @@ -125,13 +129,13 @@ func closeRun(opts *CloseOptions) error { if err != nil { return err } - err = git.CheckoutBranch(branchToSwitchTo) + err = opts.GitClient.CheckoutBranch(ctx, branchToSwitchTo) if err != nil { return err } } - if err := git.DeleteLocalBranch(pr.HeadRefName); err != nil { + if err := opts.GitClient.DeleteLocalBranch(ctx, pr.HeadRefName); err != nil { return fmt.Errorf("failed to delete local branch %s: %w", cs.Cyan(pr.HeadRefName), err) } diff --git a/pkg/cmd/pr/close/close_test.go b/pkg/cmd/pr/close/close_test.go index df9585fbb..b435fb8aa 100644 --- a/pkg/cmd/pr/close/close_test.go +++ b/pkg/cmd/pr/close/close_test.go @@ -9,6 +9,7 @@ import ( "github.com/MakeNowJust/heredoc" "github.com/cli/cli/v2/api" + "github.com/cli/cli/v2/git" "github.com/cli/cli/v2/internal/ghrepo" "github.com/cli/cli/v2/internal/run" "github.com/cli/cli/v2/pkg/cmd/pr/shared" @@ -72,6 +73,7 @@ func runCommand(rt http.RoundTripper, isTTY bool, cli string) (*test.CmdOut, err Branch: func() (string, error) { return "trunk", nil }, + GitClient: &git.Client{GitPath: "some/path/git"}, } cmd := NewCmdClose(factory, nil) diff --git a/pkg/cmd/pr/create/create.go b/pkg/cmd/pr/create/create.go index e9c664b88..75d4b58f1 100644 --- a/pkg/cmd/pr/create/create.go +++ b/pkg/cmd/pr/create/create.go @@ -1,6 +1,7 @@ package create import ( + "context" "errors" "fmt" "net/http" @@ -12,7 +13,7 @@ import ( "github.com/AlecAivazis/survey/v2" "github.com/MakeNowJust/heredoc" "github.com/cli/cli/v2/api" - "github.com/cli/cli/v2/context" + ghContext "github.com/cli/cli/v2/context" "github.com/cli/cli/v2/git" "github.com/cli/cli/v2/internal/browser" "github.com/cli/cli/v2/internal/config" @@ -32,9 +33,10 @@ type iprompter interface { type CreateOptions struct { // This struct stores user input and factory functions HttpClient func() (*http.Client, error) + GitClient *git.Client Config func() (config.Config, error) IO *iostreams.IOStreams - Remotes func() (context.Remotes, error) + Remotes func() (ghContext.Remotes, error) Branch func() (string, error) Browser browser.Browser Prompter iprompter @@ -68,22 +70,24 @@ type CreateOptions struct { type CreateContext struct { // This struct stores contextual data about the creation process and is for building up enough // data to create a pull request - RepoContext *context.ResolvedRemotes + RepoContext *ghContext.ResolvedRemotes BaseRepo *api.Repository HeadRepo ghrepo.Interface BaseTrackingBranch string BaseBranch string HeadBranch string HeadBranchLabel string - HeadRemote *context.Remote + HeadRemote *ghContext.Remote IsPushEnabled bool Client *api.Client + GitClient *git.Client } func NewCmdCreate(f *cmdutil.Factory, runF func(*CreateOptions) error) *cobra.Command { opts := &CreateOptions{ IO: f.IOStreams, HttpClient: f.HttpClient, + GitClient: f.GitClient, Config: f.Config, Remotes: f.Remotes, Branch: f.Branch, @@ -369,15 +373,16 @@ func createRun(opts *CreateOptions) (err error) { func initDefaultTitleBody(ctx CreateContext, state *shared.IssueMetadataState) error { baseRef := ctx.BaseTrackingBranch headRef := ctx.HeadBranch + gitClient := ctx.GitClient - commits, err := git.Commits(baseRef, headRef) + commits, err := gitClient.Commits(context.Background(), baseRef, headRef) if err != nil { return err } if len(commits) == 1 { state.Title = commits[0].Title - body, err := git.CommitBody(commits[0].Sha) + body, err := gitClient.CommitBody(context.Background(), commits[0].Sha) if err != nil { return err } @@ -395,11 +400,11 @@ func initDefaultTitleBody(ctx CreateContext, state *shared.IssueMetadataState) e return nil } -func determineTrackingBranch(remotes context.Remotes, headBranch string) *git.TrackingRef { +func determineTrackingBranch(gitClient *git.Client, remotes ghContext.Remotes, headBranch string) *git.TrackingRef { refsForLookup := []string{"HEAD"} var trackingRefs []git.TrackingRef - headBranchConfig := git.ReadBranchConfig(headBranch) + headBranchConfig := gitClient.ReadBranchConfig(context.Background(), headBranch) if headBranchConfig.RemoteName != "" { tr := git.TrackingRef{ RemoteName: headBranchConfig.RemoteName, @@ -418,7 +423,7 @@ func determineTrackingBranch(remotes context.Remotes, headBranch string) *git.Tr refsForLookup = append(refsForLookup, tr.String()) } - resolvedRefs, _ := git.ShowRefs(refsForLookup...) + resolvedRefs, _ := gitClient.ShowRefs(context.Background(), refsForLookup...) if len(resolvedRefs) > 1 { for _, r := range resolvedRefs[1:] { if r.Hash != resolvedRefs[0].Hash { @@ -480,7 +485,7 @@ func NewCreateContext(opts *CreateOptions) (*CreateContext, error) { return nil, err } - repoContext, err := context.ResolveRemotesToRepos(remotes, client, opts.RepoOverride) + repoContext, err := ghContext.ResolveRemotesToRepos(remotes, client, opts.RepoOverride) if err != nil { return nil, err } @@ -515,16 +520,17 @@ func NewCreateContext(opts *CreateOptions) (*CreateContext, error) { headBranch = headBranch[idx+1:] } - if ucc, err := git.UncommittedChangeCount(); err == nil && ucc > 0 { + gitClient := opts.GitClient + if ucc, err := gitClient.UncommittedChangeCount(context.Background()); err == nil && ucc > 0 { fmt.Fprintf(opts.IO.ErrOut, "Warning: %s\n", text.Pluralize(ucc, "uncommitted change")) } var headRepo ghrepo.Interface - var headRemote *context.Remote + var headRemote *ghContext.Remote if isPushEnabled { // determine whether the head branch is already pushed to a remote - if pushedTo := determineTrackingBranch(remotes, headBranch); pushedTo != nil { + if pushedTo := determineTrackingBranch(gitClient, remotes, headBranch); pushedTo != nil { isPushEnabled = false if r, err := remotes.FindByName(pushedTo.RemoteName); err == nil { headRepo = r @@ -625,6 +631,7 @@ func NewCreateContext(opts *CreateOptions) (*CreateContext, error) { IsPushEnabled: isPushEnabled, RepoContext: repoContext, Client: client, + GitClient: gitClient, }, nil } @@ -713,11 +720,12 @@ func handlePush(opts CreateOptions, ctx CreateContext) error { headRepoURL := ghrepo.FormatRemoteURL(headRepo, cloneProtocol) // TODO: prevent clashes with another remote of a same name - gitRemote, err := git.AddRemote("fork", headRepoURL) + gitClient := ctx.GitClient + gitRemote, err := gitClient.AddRemote(context.Background(), "fork", headRepoURL, []string{}) if err != nil { return fmt.Errorf("error adding remote: %w", err) } - headRemote = &context.Remote{ + headRemote = &ghContext.Remote{ Remote: gitRemote, Repo: headRepo, } @@ -729,12 +737,11 @@ func handlePush(opts CreateOptions, ctx CreateContext) error { pushTries := 0 maxPushTries := 3 for { - r := NewRegexpWriter(opts.IO.ErrOut, gitPushRegexp, "") - defer r.Flush() - cmdErr := r - cmdIn := opts.IO.In - cmdOut := opts.IO.Out - if err := git.Push(headRemote.Name, fmt.Sprintf("HEAD:%s", ctx.HeadBranch), cmdIn, cmdOut, cmdErr); err != nil { + w := NewRegexpWriter(opts.IO.ErrOut, gitPushRegexp, "") + defer w.Flush() + gitClient := ctx.GitClient + ref := fmt.Sprintf("HEAD:%s", ctx.HeadBranch) + if err := gitClient.Push(context.Background(), headRemote.Name, ref, git.WithStderr(w)); err != nil { if didForkRepo && pushTries < maxPushTries { pushTries++ // first wait 2 seconds after forking, then 4s, then 6s diff --git a/pkg/cmd/pr/create/create_test.go b/pkg/cmd/pr/create/create_test.go index c83dd13a2..0764ffe3b 100644 --- a/pkg/cmd/pr/create/create_test.go +++ b/pkg/cmd/pr/create/create_test.go @@ -869,6 +869,7 @@ func Test_createRun(t *testing.T) { return branch, nil } opts.Finder = shared.NewMockFinder(branch, nil, nil) + opts.GitClient = &git.Client{GitPath: "some/path/git"} cleanSetup := func() {} if tt.setup != nil { cleanSetup = tt.setup(&opts, t) @@ -985,7 +986,8 @@ func Test_determineTrackingBranch(t *testing.T) { tt.cmdStubs(cs) - ref := determineTrackingBranch(tt.remotes, "feature") + gitClient := &git.Client{GitPath: "some/path/git"} + ref := determineTrackingBranch(gitClient, tt.remotes, "feature") tt.assert(ref, t) }) } diff --git a/pkg/cmd/pr/merge/merge.go b/pkg/cmd/pr/merge/merge.go index 2972c09f9..dbf0923f7 100644 --- a/pkg/cmd/pr/merge/merge.go +++ b/pkg/cmd/pr/merge/merge.go @@ -1,6 +1,7 @@ package merge import ( + "context" "errors" "fmt" "net/http" @@ -8,7 +9,7 @@ import ( "github.com/AlecAivazis/survey/v2" "github.com/MakeNowJust/heredoc" "github.com/cli/cli/v2/api" - "github.com/cli/cli/v2/context" + ghContext "github.com/cli/cli/v2/context" "github.com/cli/cli/v2/git" "github.com/cli/cli/v2/internal/config" "github.com/cli/cli/v2/internal/ghrepo" @@ -26,9 +27,10 @@ type editor interface { type MergeOptions struct { HttpClient func() (*http.Client, error) + GitClient *git.Client IO *iostreams.IOStreams Branch func() (string, error) - Remotes func() (context.Remotes, error) + Remotes func() (ghContext.Remotes, error) Finder shared.PRFinder @@ -60,6 +62,7 @@ func NewCmdMerge(f *cmdutil.Factory, runF func(*MergeOptions) error) *cobra.Comm opts := &MergeOptions{ IO: f.IOStreams, HttpClient: f.HttpClient, + GitClient: f.GitClient, Branch: f.Branch, Remotes: f.Remotes, } @@ -224,7 +227,7 @@ func (m *mergeContext) warnIfDiverged() { return } - localBranchLastCommit, err := git.LastCommit() + localBranchLastCommit, err := m.opts.GitClient.LastCommit(context.Background()) if err != nil { return } @@ -396,6 +399,8 @@ func (m *mergeContext) deleteLocalBranch() error { return err } + ctx := context.Background() + // branch the command was run on is the same as the pull request branch if currentBranch == m.pr.HeadRefName { remotes, err := m.opts.Remotes() @@ -409,24 +414,24 @@ func (m *mergeContext) deleteLocalBranch() error { } targetBranch := m.pr.BaseRefName - if git.HasLocalBranch(targetBranch) { - if err := git.CheckoutBranch(targetBranch); err != nil { + if m.opts.GitClient.HasLocalBranch(ctx, targetBranch) { + if err := m.opts.GitClient.CheckoutBranch(ctx, targetBranch); err != nil { return err } } else { - if err := git.CheckoutNewBranch(baseRemote.Name, targetBranch); err != nil { + if err := m.opts.GitClient.CheckoutNewBranch(ctx, baseRemote.Name, targetBranch); err != nil { return err } } - if err := git.Pull(baseRemote.Name, targetBranch); err != nil { + if err := m.opts.GitClient.Pull(ctx, baseRemote.Name, targetBranch); err != nil { _ = m.warnf(fmt.Sprintf("%s warning: not possible to fast-forward to: %q\n", m.cs.WarningIcon(), targetBranch)) } m.switchedToBranch = targetBranch } - if err := git.DeleteLocalBranch(m.pr.HeadRefName); err != nil { + if err := m.opts.GitClient.DeleteLocalBranch(ctx, m.pr.HeadRefName); err != nil { return fmt.Errorf("failed to delete local branch %s: %w", m.cs.Cyan(m.pr.HeadRefName), err) } @@ -503,7 +508,7 @@ func NewMergeContext(opts *MergeOptions) (*mergeContext, error) { deleteBranch: opts.DeleteBranch, crossRepoPR: pr.HeadRepositoryOwner.Login != baseRepo.RepoOwner(), autoMerge: opts.AutoMergeEnable && !isImmediatelyMergeable(pr.MergeStateStatus), - localBranchExists: opts.CanDeleteLocalBranch && git.HasLocalBranch(pr.HeadRefName), + localBranchExists: opts.CanDeleteLocalBranch && opts.GitClient.HasLocalBranch(context.Background(), pr.HeadRefName), mergeQueueRequired: pr.IsMergeQueueEnabled, }, nil } @@ -730,7 +735,7 @@ func allowsAdminOverride(status string) bool { } } -func remoteForMergeConflictResolution(baseRepo ghrepo.Interface, pr *api.PullRequest, opts *MergeOptions) *context.Remote { +func remoteForMergeConflictResolution(baseRepo ghrepo.Interface, pr *api.PullRequest, opts *MergeOptions) *ghContext.Remote { if !mergeConflictStatus(pr.MergeStateStatus) || !opts.CanDeleteLocalBranch { return nil } diff --git a/pkg/cmd/pr/merge/merge_test.go b/pkg/cmd/pr/merge/merge_test.go index d1bb21d8c..0e3ced350 100644 --- a/pkg/cmd/pr/merge/merge_test.go +++ b/pkg/cmd/pr/merge/merge_test.go @@ -270,6 +270,7 @@ func runCommand(rt http.RoundTripper, branch string, isTTY bool, cli string) (*t }, }, nil }, + GitClient: &git.Client{GitPath: "some/path/git"}, } cmd := NewCmdMerge(factory, nil) diff --git a/pkg/cmd/pr/shared/finder.go b/pkg/cmd/pr/shared/finder.go index b700a7501..ed04c4295 100644 --- a/pkg/cmd/pr/shared/finder.go +++ b/pkg/cmd/pr/shared/finder.go @@ -53,12 +53,14 @@ func NewFinder(factory *cmdutil.Factory) PRFinder { } return &finder{ - baseRepoFn: factory.BaseRepo, - branchFn: factory.Branch, - remotesFn: factory.Remotes, - httpClient: factory.HttpClient, - progress: factory.IOStreams, - branchConfig: git.ReadBranchConfig, + baseRepoFn: factory.BaseRepo, + branchFn: factory.Branch, + remotesFn: factory.Remotes, + httpClient: factory.HttpClient, + progress: factory.IOStreams, + branchConfig: func(s string) git.BranchConfig { + return factory.GitClient.ReadBranchConfig(context.Background(), s) + }, } } diff --git a/pkg/cmd/pr/shared/survey.go b/pkg/cmd/pr/shared/survey.go index 31942c6a0..dd4ba085d 100644 --- a/pkg/cmd/pr/shared/survey.go +++ b/pkg/cmd/pr/shared/survey.go @@ -6,9 +6,7 @@ import ( "github.com/AlecAivazis/survey/v2" "github.com/cli/cli/v2/api" - "github.com/cli/cli/v2/git" "github.com/cli/cli/v2/internal/ghrepo" - "github.com/cli/cli/v2/pkg/githubtemplate" "github.com/cli/cli/v2/pkg/iostreams" "github.com/cli/cli/v2/pkg/prompt" "github.com/cli/cli/v2/pkg/surveyext" @@ -369,18 +367,3 @@ func MetadataSurvey(io *iostreams.IOStreams, baseRepo ghrepo.Interface, fetcher return nil } - -func FindTemplates(dir, path string) ([]string, string) { - if dir == "" { - rootDir, err := git.ToplevelDir() - if err != nil { - return []string{}, "" - } - dir = rootDir - } - - templateFiles := githubtemplate.FindNonLegacy(dir, path) - legacyTemplate := githubtemplate.FindLegacy(dir, path) - - return templateFiles, legacyTemplate -} diff --git a/pkg/cmd/pr/shared/templates.go b/pkg/cmd/pr/shared/templates.go index 975bdd5da..7aab57978 100644 --- a/pkg/cmd/pr/shared/templates.go +++ b/pkg/cmd/pr/shared/templates.go @@ -1,6 +1,7 @@ package shared import ( + "context" "fmt" "net/http" "time" @@ -233,7 +234,8 @@ func (m *templateManager) fetch() error { dir := m.rootDir if dir == "" { var err error - dir, err = git.ToplevelDir() + gitClient := &git.Client{} + dir, err = gitClient.ToplevelDir(context.Background()) if err != nil { return nil // abort silently } diff --git a/pkg/cmd/pr/status/status.go b/pkg/cmd/pr/status/status.go index 784159564..e4434978b 100644 --- a/pkg/cmd/pr/status/status.go +++ b/pkg/cmd/pr/status/status.go @@ -1,6 +1,7 @@ package status import ( + "context" "errors" "fmt" "net/http" @@ -9,7 +10,7 @@ import ( "strings" "github.com/cli/cli/v2/api" - "github.com/cli/cli/v2/context" + ghContext "github.com/cli/cli/v2/context" "github.com/cli/cli/v2/git" "github.com/cli/cli/v2/internal/config" "github.com/cli/cli/v2/internal/ghrepo" @@ -22,10 +23,11 @@ import ( type StatusOptions struct { HttpClient func() (*http.Client, error) + GitClient *git.Client Config func() (config.Config, error) IO *iostreams.IOStreams BaseRepo func() (ghrepo.Interface, error) - Remotes func() (context.Remotes, error) + Remotes func() (ghContext.Remotes, error) Branch func() (string, error) HasRepoOverride bool @@ -37,6 +39,7 @@ func NewCmdStatus(f *cmdutil.Factory, runF func(*StatusOptions) error) *cobra.Co opts := &StatusOptions{ IO: f.IOStreams, HttpClient: f.HttpClient, + GitClient: f.GitClient, Config: f.Config, Remotes: f.Remotes, Branch: f.Branch, @@ -86,7 +89,7 @@ func statusRun(opts *StatusOptions) error { } remotes, _ := opts.Remotes() - currentPRNumber, currentPRHeadRef, err = prSelectorForCurrentBranch(baseRepo, currentBranch, remotes) + currentPRNumber, currentPRHeadRef, err = prSelectorForCurrentBranch(opts.GitClient, baseRepo, currentBranch, remotes) if err != nil { return fmt.Errorf("could not query for pull request for current branch: %w", err) } @@ -165,9 +168,9 @@ func statusRun(opts *StatusOptions) error { return nil } -func prSelectorForCurrentBranch(baseRepo ghrepo.Interface, prHeadRef string, rem context.Remotes) (prNumber int, selector string, err error) { +func prSelectorForCurrentBranch(gitClient *git.Client, baseRepo ghrepo.Interface, prHeadRef string, rem ghContext.Remotes) (prNumber int, selector string, err error) { selector = prHeadRef - branchConfig := git.ReadBranchConfig(prHeadRef) + branchConfig := gitClient.ReadBranchConfig(context.Background(), prHeadRef) // the branch is configured to merge a special PR head ref prHeadRE := regexp.MustCompile(`^refs/pull/(\d+)/head$`) diff --git a/pkg/cmd/pr/status/status_test.go b/pkg/cmd/pr/status/status_test.go index 31f28396e..c3187739d 100644 --- a/pkg/cmd/pr/status/status_test.go +++ b/pkg/cmd/pr/status/status_test.go @@ -52,6 +52,7 @@ func runCommand(rt http.RoundTripper, branch string, isTTY bool, cli string) (*t } return branch, nil }, + GitClient: &git.Client{GitPath: "some/path/git"}, } cmd := NewCmdStatus(factory, nil) @@ -328,7 +329,8 @@ func Test_prSelectorForCurrentBranch(t *testing.T) { Repo: repo, }, } - prNum, headRef, err := prSelectorForCurrentBranch(repo, "Frederick888/main", rem) + gitClient := &git.Client{GitPath: "some/path/git"} + prNum, headRef, err := prSelectorForCurrentBranch(gitClient, repo, "Frederick888/main", rem) if err != nil { t.Fatalf("prSelectorForCurrentBranch error: %v", err) } diff --git a/pkg/cmd/release/create/create.go b/pkg/cmd/release/create/create.go index e6f97dd9f..8c36218da 100644 --- a/pkg/cmd/release/create/create.go +++ b/pkg/cmd/release/create/create.go @@ -2,6 +2,7 @@ package create import ( "bytes" + "context" "errors" "fmt" "io" @@ -26,6 +27,7 @@ type CreateOptions struct { IO *iostreams.IOStreams Config func() (config.Config, error) HttpClient func() (*http.Client, error) + GitClient *git.Client BaseRepo func() (ghrepo.Interface, error) Edit func(string, string, string, io.Reader, io.Writer, io.Writer) (string, error) @@ -56,6 +58,7 @@ func NewCmdCreate(f *cmdutil.Factory, runF func(*CreateOptions) error) *cobra.Co opts := &CreateOptions{ IO: f.IOStreams, HttpClient: f.HttpClient, + GitClient: f.GitClient, Config: f.Config, Edit: surveyext.Edit, } @@ -221,7 +224,7 @@ func createRun(opts *CreateOptions) error { var tagDescription string if opts.RepoOverride == "" { - tagDescription, _ = gitTagInfo(opts.TagName) + tagDescription, _ = gitTagInfo(opts.GitClient, opts.TagName) // If there is a local tag with the same name as specified // the user may not want to create a new tag on the remote // as the local one might be annotated or signed. @@ -268,10 +271,10 @@ func createRun(opts *CreateOptions) error { } if generatedNotes == nil { if opts.NotesStartTag != "" { - commits, _ := changelogForRange(fmt.Sprintf("%s..%s", opts.NotesStartTag, headRef)) + commits, _ := changelogForRange(opts.GitClient, fmt.Sprintf("%s..%s", opts.NotesStartTag, headRef)) generatedChangelog = generateChangelog(commits) - } else if prevTag, err := detectPreviousTag(headRef); err == nil { - commits, _ := changelogForRange(fmt.Sprintf("%s..%s", prevTag, headRef)) + } else if prevTag, err := detectPreviousTag(opts.GitClient, headRef); err == nil { + commits, _ := changelogForRange(opts.GitClient, fmt.Sprintf("%s..%s", prevTag, headRef)) generatedChangelog = generateChangelog(commits) } } @@ -469,8 +472,8 @@ func createRun(opts *CreateOptions) error { return nil } -func gitTagInfo(tagName string) (string, error) { - cmd, err := git.GitCommand("tag", "--list", tagName, "--format=%(contents:subject)%0a%0a%(contents:body)") +func gitTagInfo(client *git.Client, tagName string) (string, error) { + cmd, err := client.Command(context.Background(), "tag", "--list", tagName, "--format=%(contents:subject)%0a%0a%(contents:body)") if err != nil { return "", err } @@ -478,8 +481,8 @@ func gitTagInfo(tagName string) (string, error) { return string(b), err } -func detectPreviousTag(headRef string) (string, error) { - cmd, err := git.GitCommand("describe", "--tags", "--abbrev=0", fmt.Sprintf("%s^", headRef)) +func detectPreviousTag(client *git.Client, headRef string) (string, error) { + cmd, err := client.Command(context.Background(), "describe", "--tags", "--abbrev=0", fmt.Sprintf("%s^", headRef)) if err != nil { return "", err } @@ -492,8 +495,8 @@ type logEntry struct { Body string } -func changelogForRange(refRange string) ([]logEntry, error) { - cmd, err := git.GitCommand("-c", "log.ShowSignature=false", "log", "--first-parent", "--reverse", "--pretty=format:%B%x00", refRange) +func changelogForRange(client *git.Client, refRange string) ([]logEntry, error) { + cmd, err := client.Command(context.Background(), "-c", "log.ShowSignature=false", "log", "--first-parent", "--reverse", "--pretty=format:%B%x00", refRange) if err != nil { return nil, err } diff --git a/pkg/cmd/release/create/create_test.go b/pkg/cmd/release/create/create_test.go index 7fb6711b2..a6424c0d6 100644 --- a/pkg/cmd/release/create/create_test.go +++ b/pkg/cmd/release/create/create_test.go @@ -10,6 +10,7 @@ import ( "path/filepath" "testing" + "github.com/cli/cli/v2/git" "github.com/cli/cli/v2/internal/config" "github.com/cli/cli/v2/internal/ghrepo" "github.com/cli/cli/v2/internal/run" @@ -688,6 +689,8 @@ func Test_createRun(t *testing.T) { return ghrepo.FromFullName("OWNER/REPO") } + tt.opts.GitClient = &git.Client{GitPath: "some/path/git"} + err := createRun(&tt.opts) if tt.wantErr != "" { require.EqualError(t, err, tt.wantErr) @@ -1050,6 +1053,8 @@ func Test_createRun_interactive(t *testing.T) { return val, nil } + tt.opts.GitClient = &git.Client{GitPath: "some/path/git"} + t.Run(tt.name, func(t *testing.T) { //nolint:staticcheck // SA1019: prompt.NewAskStubber is deprecated: use PrompterMock as := prompt.NewAskStubber(t) diff --git a/pkg/cmd/repo/clone/clone.go b/pkg/cmd/repo/clone/clone.go index ba289aa47..cb46d4572 100644 --- a/pkg/cmd/repo/clone/clone.go +++ b/pkg/cmd/repo/clone/clone.go @@ -1,6 +1,7 @@ package clone import ( + "context" "fmt" "net/http" "strings" @@ -18,6 +19,7 @@ import ( type CloneOptions struct { HttpClient func() (*http.Client, error) + GitClient *git.Client Config func() (config.Config, error) IO *iostreams.IOStreams @@ -30,6 +32,7 @@ func NewCmdClone(f *cmdutil.Factory, runF func(*CloneOptions) error) *cobra.Comm opts := &CloneOptions{ IO: f.IOStreams, HttpClient: f.HttpClient, + GitClient: f.GitClient, Config: f.Config, } @@ -152,7 +155,9 @@ func cloneRun(opts *CloneOptions) error { canonicalCloneURL = strings.TrimSuffix(canonicalCloneURL, ".git") + ".wiki.git" } - cloneDir, err := git.RunClone(canonicalCloneURL, opts.GitArgs) + gitClient := opts.GitClient + ctx := context.Background() + cloneDir, err := gitClient.Clone(ctx, canonicalCloneURL, opts.GitArgs) if err != nil { return err } @@ -170,7 +175,7 @@ func cloneRun(opts *CloneOptions) error { upstreamName = canonicalRepo.Parent.RepoOwner() } - err = git.AddNamedRemote(upstreamURL, upstreamName, cloneDir, []string{canonicalRepo.Parent.DefaultBranchRef.Name}) + _, err = gitClient.AddRemote(ctx, upstreamName, upstreamURL, []string{canonicalRepo.Parent.DefaultBranchRef.Name}, git.WithRepoDir(cloneDir)) if err != nil { return err } diff --git a/pkg/cmd/repo/clone/clone_test.go b/pkg/cmd/repo/clone/clone_test.go index 34f2fe431..c6ec872be 100644 --- a/pkg/cmd/repo/clone/clone_test.go +++ b/pkg/cmd/repo/clone/clone_test.go @@ -5,6 +5,7 @@ import ( "strings" "testing" + "github.com/cli/cli/v2/git" "github.com/cli/cli/v2/internal/config" "github.com/cli/cli/v2/internal/run" "github.com/cli/cli/v2/pkg/cmdutil" @@ -104,6 +105,7 @@ func runCloneCommand(httpClient *http.Client, cli string) (*test.CmdOut, error) Config: func() (config.Config, error) { return config.NewBlankConfig(), nil }, + GitClient: &git.Client{GitPath: "some/path/git"}, } cmd := NewCmdClone(fac, nil) diff --git a/pkg/cmd/repo/create/create.go b/pkg/cmd/repo/create/create.go index e311ad727..931ffb926 100644 --- a/pkg/cmd/repo/create/create.go +++ b/pkg/cmd/repo/create/create.go @@ -1,6 +1,7 @@ package create import ( + "context" "errors" "fmt" "net/http" @@ -27,6 +28,7 @@ type iprompter interface { type CreateOptions struct { HttpClient func() (*http.Client, error) + GitClient *git.Client Config func() (config.Config, error) IO *iostreams.IOStreams Prompter iprompter @@ -57,6 +59,7 @@ func NewCmdCreate(f *cmdutil.Factory, runF func(*CreateOptions) error) *cobra.Co opts := &CreateOptions{ IO: f.IOStreams, HttpClient: f.HttpClient, + GitClient: f.GitClient, Config: f.Config, Prompter: f.Prompter, } @@ -390,10 +393,10 @@ func createFromScratch(opts *CreateOptions) error { // use the template's default branch checkoutBranch = templateRepoMainBranch } - if err := localInit(opts.IO, remoteURL, repo.RepoName(), checkoutBranch); err != nil { + if err := localInit(opts.GitClient, remoteURL, repo.RepoName(), checkoutBranch); err != nil { return err } - } else if _, err := git.RunClone(remoteURL, []string{}); err != nil { + } else if _, err := opts.GitClient.Clone(context.Background(), remoteURL, []string{}); err != nil { return err } } @@ -427,6 +430,7 @@ func createFromLocal(opts *CreateOptions) error { } repoPath := opts.Source + opts.GitClient.RepoDir = repoPath var baseRemote string if opts.Remote == "" { @@ -440,7 +444,7 @@ func createFromLocal(opts *CreateOptions) error { return err } - isRepo, err := isLocalRepo(repoPath) + isRepo, err := isLocalRepo(opts.GitClient) if err != nil { return err } @@ -451,7 +455,7 @@ func createFromLocal(opts *CreateOptions) error { return fmt.Errorf("%s is not a git repository. Run `git -C \"%s\" init` to initialize it", absPath, repoPath) } - committed, err := hasCommits(repoPath) + committed, err := hasCommits(opts.GitClient) if err != nil { return err } @@ -533,7 +537,7 @@ func createFromLocal(opts *CreateOptions) error { } } - if err := sourceInit(opts.IO, remoteURL, baseRemote, repoPath); err != nil { + if err := sourceInit(opts.GitClient, opts.IO, remoteURL, baseRemote); err != nil { return err } @@ -547,7 +551,7 @@ func createFromLocal(opts *CreateOptions) error { } if opts.Push { - repoPush, err := git.GitCommand("-C", repoPath, "push", "-u", baseRemote, "HEAD") + repoPush, err := opts.GitClient.Command(context.Background(), "push", "-u", baseRemote, "HEAD") if err != nil { return err } @@ -563,17 +567,17 @@ func createFromLocal(opts *CreateOptions) error { return nil } -func sourceInit(io *iostreams.IOStreams, remoteURL, baseRemote, repoPath string) error { +func sourceInit(gitClient *git.Client, io *iostreams.IOStreams, remoteURL, baseRemote string) error { cs := io.ColorScheme() isTTY := io.IsStdoutTTY() stdout := io.Out - remoteAdd, err := git.GitCommand("-C", repoPath, "remote", "add", baseRemote, remoteURL) + remoteAdd, err := gitClient.Command(context.Background(), "remote", "add", baseRemote, remoteURL) if err != nil { return err } - err = remoteAdd.Run() + _, err = remoteAdd.Output() if err != nil { return fmt.Errorf("%s Unable to add remote %q", cs.FailureIcon(), baseRemote) } @@ -584,12 +588,12 @@ func sourceInit(io *iostreams.IOStreams, remoteURL, baseRemote, repoPath string) } // check if local repository has committed changes -func hasCommits(repoPath string) (bool, error) { - hasCommitsCmd, err := git.GitCommand("-C", repoPath, "rev-parse", "HEAD") +func hasCommits(gitClient *git.Client) (bool, error) { + hasCommitsCmd, err := gitClient.Command(context.Background(), "rev-parse", "HEAD") if err != nil { return false, err } - err = hasCommitsCmd.Run() + _, err = hasCommitsCmd.Output() if err == nil { return true, nil } @@ -606,8 +610,8 @@ func hasCommits(repoPath string) (bool, error) { } // check if path is the top level directory of a git repo -func isLocalRepo(repoPath string) (bool, error) { - projectDir, projectDirErr := git.GetDirFromPath(repoPath) +func isLocalRepo(gitClient *git.Client) (bool, error) { + projectDir, projectDirErr := gitClient.GitDir(context.Background()) if projectDirErr != nil { var execError *exec.ExitError if errors.As(projectDirErr, &execError) { @@ -624,28 +628,26 @@ func isLocalRepo(repoPath string) (bool, error) { } // clone the checkout branch to specified path -func localInit(io *iostreams.IOStreams, remoteURL, path, checkoutBranch string) error { - gitInit, err := git.GitCommand("init", path) +func localInit(gitClient *git.Client, remoteURL, path, checkoutBranch string) error { + ctx := context.Background() + gitInit, err := gitClient.Command(ctx, "init", path) if err != nil { return err } - isTTY := io.IsStdoutTTY() - if isTTY { - gitInit.Stdout = io.Out - } - gitInit.Stderr = io.ErrOut - err = gitInit.Run() + _, err = gitInit.Output() if err != nil { return err } - gitRemoteAdd, err := git.GitCommand("-C", path, "remote", "add", "origin", remoteURL) + // Clone the client so we do not modify the original client's RepoDir. + gc := cloneGitClient(gitClient) + gc.RepoDir = path + + gitRemoteAdd, err := gc.Command(ctx, "remote", "add", "origin", remoteURL) if err != nil { return err } - gitRemoteAdd.Stdout = io.Out - gitRemoteAdd.Stderr = io.ErrOut - err = gitRemoteAdd.Run() + _, err = gitRemoteAdd.Output() if err != nil { return err } @@ -654,24 +656,21 @@ func localInit(io *iostreams.IOStreams, remoteURL, path, checkoutBranch string) return nil } - gitFetch, err := git.GitCommand("-C", path, "fetch", "origin", fmt.Sprintf("+refs/heads/%[1]s:refs/remotes/origin/%[1]s", checkoutBranch)) + gitFetch, err := gc.Command(ctx, "fetch", "origin", fmt.Sprintf("+refs/heads/%[1]s:refs/remotes/origin/%[1]s", checkoutBranch)) if err != nil { return err } - gitFetch.Stdout = io.Out - gitFetch.Stderr = io.ErrOut err = gitFetch.Run() if err != nil { return err } - gitCheckout, err := git.GitCommand("-C", path, "checkout", checkoutBranch) + gitCheckout, err := gc.Command(ctx, "checkout", checkoutBranch) if err != nil { return err } - gitCheckout.Stdout = io.Out - gitCheckout.Stderr = io.ErrOut - return gitCheckout.Run() + _, err = gitCheckout.Output() + return err } func interactiveGitIgnore(client *http.Client, hostname string, prompter iprompter) (string, error) { @@ -736,3 +735,14 @@ func interactiveRepoInfo(prompter iprompter, defaultName string) (string, string return name, description, strings.ToUpper(visibilityOptions[selected]), nil } + +func cloneGitClient(c *git.Client) *git.Client { + return &git.Client{ + GhPath: c.GhPath, + RepoDir: c.RepoDir, + GitPath: c.GitPath, + Stderr: c.Stderr, + Stdin: c.Stdin, + Stdout: c.Stdout, + } +} diff --git a/pkg/cmd/repo/create/create_test.go b/pkg/cmd/repo/create/create_test.go index f8849afd1..a1ce875f0 100644 --- a/pkg/cmd/repo/create/create_test.go +++ b/pkg/cmd/repo/create/create_test.go @@ -6,6 +6,7 @@ import ( "net/http" "testing" + "github.com/cli/cli/v2/git" "github.com/cli/cli/v2/internal/config" "github.com/cli/cli/v2/internal/prompter" "github.com/cli/cli/v2/internal/run" @@ -492,6 +493,8 @@ func Test_createRun(t *testing.T) { return config.NewBlankConfig(), nil } + tt.opts.GitClient = &git.Client{GitPath: "some/path/git"} + ios, _, stdout, stderr := iostreams.Test() ios.SetStdinTTY(tt.tty) ios.SetStdoutTTY(tt.tty) diff --git a/pkg/cmd/repo/fork/fork.go b/pkg/cmd/repo/fork/fork.go index 9416ba4b2..e17e4b34d 100644 --- a/pkg/cmd/repo/fork/fork.go +++ b/pkg/cmd/repo/fork/fork.go @@ -1,6 +1,7 @@ package fork import ( + "context" "fmt" "net/http" "net/url" @@ -9,7 +10,7 @@ import ( "github.com/MakeNowJust/heredoc" "github.com/cli/cli/v2/api" - "github.com/cli/cli/v2/context" + ghContext "github.com/cli/cli/v2/context" "github.com/cli/cli/v2/git" "github.com/cli/cli/v2/internal/config" "github.com/cli/cli/v2/internal/ghrepo" @@ -25,10 +26,11 @@ const defaultRemoteName = "origin" type ForkOptions struct { HttpClient func() (*http.Client, error) + GitClient *git.Client Config func() (config.Config, error) IO *iostreams.IOStreams BaseRepo func() (ghrepo.Interface, error) - Remotes func() (context.Remotes, error) + Remotes func() (ghContext.Remotes, error) Since func(time.Time) time.Duration GitArgs []string @@ -51,6 +53,7 @@ func NewCmdFork(f *cmdutil.Factory, runF func(*ForkOptions) error) *cobra.Comman opts := &ForkOptions{ IO: f.IOStreams, HttpClient: f.HttpClient, + GitClient: f.GitClient, Config: f.Config, BaseRepo: f.BaseRepo, Remotes: f.Remotes, @@ -226,6 +229,9 @@ func forkRun(opts *ForkOptions) error { } protocol, _ := cfg.Get(repoToFork.RepoHost(), "git_protocol") + gitClient := opts.GitClient + ctx := context.Background() + if inParent { remotes, err := opts.Remotes() if err != nil { @@ -264,6 +270,7 @@ func forkRun(opts *ForkOptions) error { return fmt.Errorf("failed to prompt: %w", err) } } + if remoteDesired { remoteName := opts.RemoteName remotes, err := opts.Remotes() @@ -274,11 +281,11 @@ func forkRun(opts *ForkOptions) error { if _, err := remotes.FindByName(remoteName); err == nil { if opts.Rename { renameTarget := "upstream" - renameCmd, err := git.GitCommand("remote", "rename", remoteName, renameTarget) + renameCmd, err := gitClient.Command(ctx, "remote", "rename", remoteName, renameTarget) if err != nil { return err } - err = renameCmd.Run() + _, err = renameCmd.Output() if err != nil { return err } @@ -289,7 +296,7 @@ func forkRun(opts *ForkOptions) error { forkedRepoCloneURL := ghrepo.FormatRemoteURL(forkedRepo, protocol) - _, err = git.AddRemote(remoteName, forkedRepoCloneURL) + _, err = gitClient.AddRemote(ctx, remoteName, forkedRepoCloneURL, []string{}) if err != nil { return fmt.Errorf("failed to add remote: %w", err) } @@ -309,13 +316,13 @@ func forkRun(opts *ForkOptions) error { } if cloneDesired { forkedRepoURL := ghrepo.FormatRemoteURL(forkedRepo, protocol) - cloneDir, err := git.RunClone(forkedRepoURL, opts.GitArgs) + cloneDir, err := gitClient.Clone(ctx, forkedRepoURL, opts.GitArgs) if err != nil { return fmt.Errorf("failed to clone fork: %w", err) } upstreamURL := ghrepo.FormatRemoteURL(repoToFork, protocol) - err = git.AddNamedRemote(upstreamURL, "upstream", cloneDir, []string{}) + _, err = gitClient.AddRemote(ctx, "upstream", upstreamURL, []string{}, git.WithRepoDir(cloneDir)) if err != nil { return err } diff --git a/pkg/cmd/repo/fork/fork_test.go b/pkg/cmd/repo/fork/fork_test.go index 111eb98f6..e5ffda36b 100644 --- a/pkg/cmd/repo/fork/fork_test.go +++ b/pkg/cmd/repo/fork/fork_test.go @@ -700,6 +700,8 @@ func TestRepoFork(t *testing.T) { return tt.remotes, nil } + tt.opts.GitClient = &git.Client{GitPath: "some/path/git"} + //nolint:staticcheck // SA1019: prompt.InitAskStubber is deprecated: use NewAskStubber as, teardown := prompt.InitAskStubber() defer teardown() diff --git a/pkg/cmd/repo/rename/rename.go b/pkg/cmd/repo/rename/rename.go index 0b256d1d3..2d07cd8b0 100644 --- a/pkg/cmd/repo/rename/rename.go +++ b/pkg/cmd/repo/rename/rename.go @@ -1,13 +1,14 @@ package rename import ( + "context" "fmt" "net/http" "github.com/AlecAivazis/survey/v2" "github.com/MakeNowJust/heredoc" "github.com/cli/cli/v2/api" - "github.com/cli/cli/v2/context" + ghContext "github.com/cli/cli/v2/context" "github.com/cli/cli/v2/git" "github.com/cli/cli/v2/internal/config" "github.com/cli/cli/v2/internal/ghrepo" @@ -19,10 +20,11 @@ import ( type RenameOptions struct { HttpClient func() (*http.Client, error) + GitClient *git.Client IO *iostreams.IOStreams Config func() (config.Config, error) BaseRepo func() (ghrepo.Interface, error) - Remotes func() (context.Remotes, error) + Remotes func() (ghContext.Remotes, error) DoConfirm bool HasRepoOverride bool newRepoSelector string @@ -32,6 +34,7 @@ func NewCmdRename(f *cmdutil.Factory, runf func(*RenameOptions) error) *cobra.Co opts := &RenameOptions{ IO: f.IOStreams, HttpClient: f.HttpClient, + GitClient: f.GitClient, Remotes: f.Remotes, Config: f.Config, } @@ -145,7 +148,7 @@ func renameRun(opts *RenameOptions) error { return nil } -func updateRemote(repo ghrepo.Interface, renamed ghrepo.Interface, opts *RenameOptions) (*context.Remote, error) { +func updateRemote(repo ghrepo.Interface, renamed ghrepo.Interface, opts *RenameOptions) (*ghContext.Remote, error) { cfg, err := opts.Config() if err != nil { return nil, err @@ -167,6 +170,7 @@ func updateRemote(repo ghrepo.Interface, renamed ghrepo.Interface, opts *RenameO } remoteURL := ghrepo.FormatRemoteURL(renamed, protocol) - err = git.UpdateRemoteURL(remote.Name, remoteURL) + err = opts.GitClient.UpdateRemoteURL(context.Background(), remote.Name, remoteURL) + return remote, err } diff --git a/pkg/cmd/repo/rename/rename_test.go b/pkg/cmd/repo/rename/rename_test.go index 97e1aebb7..523b2ba4a 100644 --- a/pkg/cmd/repo/rename/rename_test.go +++ b/pkg/cmd/repo/rename/rename_test.go @@ -259,6 +259,8 @@ func TestRenameRun(t *testing.T) { ios.SetStdoutTTY(tt.tty) tt.opts.IO = ios + tt.opts.GitClient = &git.Client{GitPath: "some/path/git"} + t.Run(tt.name, func(t *testing.T) { defer reg.Verify(t) err := renameRun(&tt.opts) diff --git a/pkg/cmd/repo/sync/git.go b/pkg/cmd/repo/sync/git.go index 1ec86b053..fbb63a2ec 100644 --- a/pkg/cmd/repo/sync/git.go +++ b/pkg/cmd/repo/sync/git.go @@ -1,11 +1,11 @@ package sync import ( + "context" "fmt" "strings" "github.com/cli/cli/v2/git" - "github.com/cli/cli/v2/pkg/iostreams" ) type gitClient interface { @@ -22,12 +22,12 @@ type gitClient interface { } type gitExecuter struct { - io *iostreams.IOStreams + client *git.Client } func (g *gitExecuter) BranchRemote(branch string) (string, error) { args := []string{"rev-parse", "--symbolic-full-name", "--abbrev-ref", fmt.Sprintf("%s@{u}", branch)} - cmd, err := git.GitCommand(args...) + cmd, err := g.client.Command(context.Background(), args...) if err != nil { return "", err } @@ -40,60 +40,60 @@ func (g *gitExecuter) BranchRemote(branch string) (string, error) { } func (g *gitExecuter) UpdateBranch(branch, ref string) error { - cmd, err := git.GitCommand("update-ref", fmt.Sprintf("refs/heads/%s", branch), ref) + cmd, err := g.client.Command(context.Background(), "update-ref", fmt.Sprintf("refs/heads/%s", branch), ref) if err != nil { return err } - return cmd.Run() + _, err = cmd.Output() + return err } func (g *gitExecuter) CreateBranch(branch, ref, upstream string) error { - cmd, err := git.GitCommand("branch", branch, ref) + ctx := context.Background() + cmd, err := g.client.Command(ctx, "branch", branch, ref) if err != nil { return err } - if err := cmd.Run(); err != nil { + if _, err := cmd.Output(); err != nil { return err } - cmd, err = git.GitCommand("branch", "--set-upstream-to", upstream, branch) + cmd, err = g.client.Command(ctx, "branch", "--set-upstream-to", upstream, branch) if err != nil { return err } - return cmd.Run() + _, err = cmd.Output() + return err } func (g *gitExecuter) CurrentBranch() (string, error) { - return git.CurrentBranch() + return g.client.CurrentBranch(context.Background()) } func (g *gitExecuter) Fetch(remote, ref string) error { args := []string{"fetch", "-q", remote, ref} - cmd, err := git.GitCommand(args...) + cmd, err := g.client.Command(context.Background(), args...) if err != nil { return err } - cmd.Stdin = g.io.In - cmd.Stdout = g.io.Out - cmd.Stderr = g.io.ErrOut return cmd.Run() } func (g *gitExecuter) HasLocalBranch(branch string) bool { - return git.HasLocalBranch(branch) + return g.client.HasLocalBranch(context.Background(), branch) } func (g *gitExecuter) IsAncestor(ancestor, progeny string) (bool, error) { args := []string{"merge-base", "--is-ancestor", ancestor, progeny} - cmd, err := git.GitCommand(args...) + cmd, err := g.client.Command(context.Background(), args...) if err != nil { return false, err } - err = cmd.Run() + _, err = cmd.Output() return err == nil, nil } func (g *gitExecuter) IsDirty() (bool, error) { - cmd, err := git.GitCommand("status", "--untracked-files=no", "--porcelain") + cmd, err := g.client.Command(context.Background(), "status", "--untracked-files=no", "--porcelain") if err != nil { return false, err } @@ -109,18 +109,20 @@ func (g *gitExecuter) IsDirty() (bool, error) { func (g *gitExecuter) MergeFastForward(ref string) error { args := []string{"merge", "--ff-only", "--quiet", ref} - cmd, err := git.GitCommand(args...) + cmd, err := g.client.Command(context.Background(), args...) if err != nil { return err } - return cmd.Run() + _, err = cmd.Output() + return err } func (g *gitExecuter) ResetHard(ref string) error { args := []string{"reset", "--hard", ref} - cmd, err := git.GitCommand(args...) + cmd, err := g.client.Command(context.Background(), args...) if err != nil { return err } - return cmd.Run() + _, err = cmd.Output() + return err } diff --git a/pkg/cmd/repo/sync/sync.go b/pkg/cmd/repo/sync/sync.go index ba2fdbafb..eebec4389 100644 --- a/pkg/cmd/repo/sync/sync.go +++ b/pkg/cmd/repo/sync/sync.go @@ -39,7 +39,7 @@ func NewCmdSync(f *cmdutil.Factory, runF func(*SyncOptions) error) *cobra.Comman IO: f.IOStreams, BaseRepo: f.BaseRepo, Remotes: f.Remotes, - Git: &gitExecuter{io: f.IOStreams}, + Git: &gitExecuter{client: f.GitClient}, } cmd := &cobra.Command{