diff --git a/context/context.go b/context/context.go index 568e3e623..45dfcfd97 100644 --- a/context/context.go +++ b/context/context.go @@ -2,7 +2,6 @@ package context import ( - "context" "errors" "sort" @@ -139,8 +138,7 @@ func (r *ResolvedRemotes) BaseRepo(io *iostreams.IOStreams, p iprompter) (ghrepo } // cache the result to git config - c := &git.Client{} - err := c.SetRemoteResolution(context.Background(), remote.Name, resolution) + err := git.SetRemoteResolution(remote.Name, resolution) return selectedRepo, err } diff --git a/git/client.go b/git/client.go index b91d0aacb..bada86265 100644 --- a/git/client.go +++ b/git/client.go @@ -38,16 +38,22 @@ func (e *NotInstalled) Unwrap() error { } type GitError struct { - ExitCode int - Stderr string - err error + stderr string + err error } func (ge *GitError) Error() string { - if ge.Stderr == "" { + stderr := ge.stderr + if stderr == "" { + var exitError *exec.ExitError + if errors.As(ge.err, &exitError) { + stderr = string(exitError.Stderr) + } + } + if stderr == "" { return fmt.Sprintf("failed to run git: %v", ge.err) } - return fmt.Sprintf("failed to run git: %s", ge.Stderr) + return fmt.Sprintf("failed to run git: %s", stderr) } func (ge *GitError) Unwrap() error { @@ -58,77 +64,16 @@ 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 { - // 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 + 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. func (gc *gitCommand) Output() ([]byte, error) { - 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) - } + return run.PrepareCmd(gc.Cmd).Output() } type Client struct { @@ -188,7 +133,8 @@ 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{"-c", "credential.helper="} + preArgs := []string{} + preArgs = append(preArgs, "-c", "credential.helper=") if c.GhPath == "" { // Assumes that gh is in PATH. c.GhPath = "gh" @@ -207,7 +153,7 @@ func (c *Client) Remotes(ctx context.Context) (RemoteSet, error) { } remoteOut, remoteErr := remoteCmd.Output() if remoteErr != nil { - return nil, remoteErr + return nil, &GitError{err: remoteErr} } configArgs := []string{"config", "--get-regexp", `^remote\..*\.gh-resolved$`} @@ -218,9 +164,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 gitErr *GitError - if ok := errors.As(configErr, &gitErr); ok && gitErr.ExitCode != 1 { - return nil, gitErr + var exitErr *exec.ExitError + if errors.As(configErr, &exitErr) && exitErr.ExitCode() != 1 { + return nil, &GitError{err: configErr} } } @@ -230,20 +176,18 @@ func (c *Client) Remotes(ctx context.Context) (RemoteSet, error) { return remotes, nil } -func (c *Client) AddRemote(ctx context.Context, name, urlStr string, trackingBranches []string, mods ...CommandModifier) (*Remote, error) { +func (c *Client) AddRemote(ctx context.Context, name, urlStr string, trackingBranches []string) (*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 } - for _, mod := range mods { - mod(cmd) - } - if _, err := cmd.Output(); err != nil { + if err := cmd.Run(); err != nil { return nil, err } var urlParsed *url.URL @@ -272,11 +216,7 @@ func (c *Client) UpdateRemoteURL(ctx context.Context, name, url string) error { if err != nil { return err } - _, err = cmd.Output() - if err != nil { - return err - } - return nil + return cmd.Run() } func (c *Client) SetRemoteResolution(ctx context.Context, name, resolution string) error { @@ -285,11 +225,7 @@ func (c *Client) SetRemoteResolution(ctx context.Context, name, resolution strin if err != nil { return err } - _, err = cmd.Output() - if err != nil { - return err - } - return nil + return cmd.Run() } // CurrentBranch reads the checked-out branch for the git repository. @@ -299,14 +235,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 { - var gitErr *GitError - if ok := errors.As(err, &gitErr); ok && len(gitErr.Stderr) == 0 { - gitErr.Stderr = "not on any branch" - return "", gitErr + if errBuf.Len() == 0 { + return "", &GitError{err: err, stderr: "not on any branch"} } - return "", err + return "", &GitError{err: err, stderr: errBuf.String()} } branch := firstLine(out) return strings.TrimPrefix(branch, "refs/heads/"), nil @@ -319,10 +255,9 @@ func (c *Client) ShowRefs(ctx context.Context, ref ...string) ([]Ref, error) { if err != nil { return nil, err } + // This functionality relies on parsing output from the git command despite + // an error status being returned from git. out, err := cmd.Output() - if err != nil { - return nil, err - } var refs []Ref for _, line := range outputLines(out) { parts := strings.SplitN(line, " ", 2) @@ -334,7 +269,7 @@ func (c *Client) ShowRefs(ctx context.Context, ref ...string) ([]Ref, error) { Name: parts[1], }) } - return refs, nil + return refs, err } func (c *Client) Config(ctx context.Context, name string) (string, error) { @@ -343,14 +278,15 @@ 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 gitErr *GitError - if ok := errors.As(err, &gitErr); ok && gitErr.ExitCode == 1 { - gitErr.Stderr = fmt.Sprintf("unknown config key %s", name) - return "", gitErr + 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)} } - return "", err + return "", &GitError{err: err, stderr: errBuf.String()} } return firstLine(out), nil } @@ -363,7 +299,7 @@ func (c *Client) UncommittedChangeCount(ctx context.Context) (int, error) { } out, err := cmd.Output() if err != nil { - return 0, err + return 0, &GitError{err: err} } lines := strings.Split(string(out), "\n") count := 0 @@ -383,7 +319,7 @@ func (c *Client) Commits(ctx context.Context, baseRef, headRef string) ([]*Commi } out, err := cmd.Output() if err != nil { - return nil, err + return nil, &GitError{err: err} } commits := []*Commit{} sha := 0 @@ -412,7 +348,7 @@ func (c *Client) lookupCommit(ctx context.Context, sha, format string) ([]byte, } out, err := cmd.Output() if err != nil { - return nil, err + return nil, &GitError{err: err} } return out, nil } @@ -435,16 +371,13 @@ 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, mods ...CommandModifier) error { +func (c *Client) Push(ctx context.Context, remote string, ref string) 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() } @@ -490,11 +423,7 @@ func (c *Client) DeleteLocalBranch(ctx context.Context, branch string) error { if err != nil { return err } - _, err = cmd.Output() - if err != nil { - return err - } - return nil + return cmd.Run() } func (c *Client) HasLocalBranch(ctx context.Context, branch string) bool { @@ -503,7 +432,7 @@ func (c *Client) HasLocalBranch(ctx context.Context, branch string) bool { if err != nil { return false } - _, err = cmd.Output() + err = cmd.Run() return err == nil } @@ -513,11 +442,7 @@ func (c *Client) CheckoutBranch(ctx context.Context, branch string) error { if err != nil { return err } - _, err = cmd.Output() - if err != nil { - return err - } - return nil + return cmd.Run() } func (c *Client) CheckoutNewBranch(ctx context.Context, remoteName, branch string) error { @@ -527,11 +452,7 @@ func (c *Client) CheckoutNewBranch(ctx context.Context, remoteName, branch strin if err != nil { return err } - _, err = cmd.Output() - if err != nil { - return err - } - return nil + return cmd.Run() } func (c *Client) Pull(ctx context.Context, remote, branch string) error { @@ -544,7 +465,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) (string, error) { +func (c *Client) Clone(ctx context.Context, cloneURL string, args []string) (target string, err error) { cloneArgs, target := parseCloneArgs(args) cloneArgs = append(cloneArgs, cloneURL) // If the args contain an explicit target, pass it to clone @@ -561,10 +482,7 @@ func (c *Client) Clone(ctx context.Context, cloneURL string, args []string) (str return "", err } err = cmd.Run() - if err != nil { - return "", err - } - return target, nil + return } // ToplevelDir returns the top-level directory path of the current repository. @@ -576,7 +494,7 @@ func (c *Client) ToplevelDir(ctx context.Context) (string, error) { } out, err := cmd.Output() if err != nil { - return "", err + return "", &GitError{err: err} } return firstLine(out), nil } @@ -589,7 +507,7 @@ func (c *Client) GitDir(ctx context.Context) (string, error) { } out, err := cmd.Output() if err != nil { - return "", err + return "", &GitError{err: err} } return firstLine(out), nil } diff --git a/git/client_test.go b/git/client_test.go index cff47358f..121231d23 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.Output() + err = cmd.Run() assert.NoError(t, err) } diff --git a/git/git.go b/git/git.go new file mode 100644 index 000000000..5b934175b --- /dev/null +++ b/git/git.go @@ -0,0 +1,154 @@ +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 a7260db14..d482e04cc 100644 --- a/internal/run/run.go +++ b/internal/run/run.go @@ -76,10 +76,6 @@ 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 364abb9f0..be12392d1 100644 --- a/pkg/cmd/auth/login/login.go +++ b/pkg/cmd/auth/login/login.go @@ -7,7 +7,6 @@ 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" @@ -21,7 +20,6 @@ type LoginOptions struct { IO *iostreams.IOStreams Config func() (config.Config, error) HttpClient func() (*http.Client, error) - GitClient *git.Client Prompter shared.Prompt MainExecutable string @@ -40,7 +38,6 @@ 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, } @@ -186,7 +183,6 @@ 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 85e133b2b..649fdb48d 100644 --- a/pkg/cmd/auth/login/login_test.go +++ b/pkg/cmd/auth/login/login_test.go @@ -8,7 +8,6 @@ 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" @@ -598,8 +597,6 @@ 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 c66fb5237..7b1142e7e 100644 --- a/pkg/cmd/auth/refresh/refresh.go +++ b/pkg/cmd/auth/refresh/refresh.go @@ -6,7 +6,6 @@ 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" @@ -18,8 +17,7 @@ import ( type RefreshOptions struct { IO *iostreams.IOStreams Config func() (config.Config, error) - HttpClient *http.Client - GitClient *git.Client + httpClient *http.Client Prompter shared.Prompt MainExecutable string @@ -39,8 +37,7 @@ func NewCmdRefresh(f *cmdutil.Factory, runF func(*RefreshOptions) error) *cobra. _, err := authflow.AuthFlowWithConfig(cfg, io, hostname, "", scopes, interactive) return err }, - HttpClient: &http.Client{}, - GitClient: f.GitClient, + httpClient: &http.Client{}, Prompter: f.Prompter, } @@ -125,7 +122,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 != "" { @@ -138,7 +135,6 @@ 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 d1d6042c0..ffacb22d9 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 edc531235..b5caefe5b 100644 --- a/pkg/cmd/auth/setupgit/setupgit.go +++ b/pkg/cmd/auth/setupgit/setupgit.go @@ -34,7 +34,6 @@ 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 5ddac14d5..491e2c82f 100644 --- a/pkg/cmd/auth/shared/git_credential.go +++ b/pkg/cmd/auth/shared/git_credential.go @@ -2,7 +2,6 @@ package shared import ( "bytes" - "context" "errors" "fmt" "path/filepath" @@ -17,7 +16,6 @@ import ( type GitCredentialFlow struct { Executable string Prompter Prompt - GitClient *git.Client shouldSetup bool helper string @@ -26,7 +24,7 @@ type GitCredentialFlow struct { func (flow *GitCredentialFlow) Prompt(hostname string) error { var gitErr error - flow.helper, gitErr = gitCredentialHelper(flow.GitClient, hostname) + flow.helper, gitErr = gitCredentialHelper(hostname) if isOurCredentialHelper(flow.helper) { flow.scopes = append(flow.scopes, "workflow") return nil @@ -61,9 +59,6 @@ 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), @@ -81,18 +76,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 := gitClient.Command(ctx, "config", "--global", "--replace-all", credHelperKey, "") + preConfigureCmd, err := git.GitCommand("config", "--global", "--replace-all", credHelperKey, "") if err != nil { configErr = err break } - if _, err = preConfigureCmd.Output(); err != nil { + if err = preConfigureCmd.Run(); err != nil { configErr = err break } // second configure the actual helper for this host - configureCmd, err := gitClient.Command(ctx, + configureCmd, err := git.GitCommand( "config", "--global", "--add", credHelperKey, fmt.Sprintf("!%s auth git-credential", shellQuote(flow.Executable)), @@ -100,7 +95,7 @@ func (flow *GitCredentialFlow) gitCredentialSetup(hostname, username, password s if err != nil { configErr = err } else { - _, configErr = configureCmd.Output() + configErr = configureCmd.Run() } } @@ -108,7 +103,7 @@ func (flow *GitCredentialFlow) gitCredentialSetup(hostname, username, password s } // clear previous cached credentials - rejectCmd, err := gitClient.Command(ctx, "credential", "reject") + rejectCmd, err := git.GitCommand("credential", "reject") if err != nil { return err } @@ -118,12 +113,12 @@ func (flow *GitCredentialFlow) gitCredentialSetup(hostname, username, password s host=%s `, hostname)) - _, err = rejectCmd.Output() + err = rejectCmd.Run() if err != nil { return err } - approveCmd, err := gitClient.Command(ctx, "credential", "approve") + approveCmd, err := git.GitCommand("credential", "approve") if err != nil { return err } @@ -135,7 +130,7 @@ func (flow *GitCredentialFlow) gitCredentialSetup(hostname, username, password s password=%s `, hostname, username, password)) - _, err = approveCmd.Output() + err = approveCmd.Run() if err != nil { return err } @@ -148,13 +143,12 @@ func gitCredentialHelperKey(hostname string) string { return fmt.Sprintf("credential.%s.helper", host) } -func gitCredentialHelper(gitClient *git.Client, hostname string) (helper string, err error) { - ctx := context.Background() - helper, err = gitClient.Config(ctx, gitCredentialHelperKey(hostname)) +func gitCredentialHelper(hostname string) (helper string, err error) { + helper, err = git.Config(gitCredentialHelperKey(hostname)) if helper != "" { return } - helper, err = gitClient.Config(ctx, "credential.helper") + helper, err = git.Config("credential.helper") return } diff --git a/pkg/cmd/auth/shared/git_credential_test.go b/pkg/cmd/auth/shared/git_credential_test.go index 9d3e90cc4..fe674e1d7 100644 --- a/pkg/cmd/auth/shared/git_credential_test.go +++ b/pkg/cmd/auth/shared/git_credential_test.go @@ -3,7 +3,6 @@ package shared import ( "testing" - "github.com/cli/cli/v2/git" "github.com/cli/cli/v2/internal/run" ) @@ -16,7 +15,6 @@ 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 { @@ -63,7 +61,6 @@ 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 { @@ -95,7 +92,6 @@ 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 99d82b5f7..9cc9d9865 100644 --- a/pkg/cmd/auth/shared/login_flow.go +++ b/pkg/cmd/auth/shared/login_flow.go @@ -8,7 +8,6 @@ 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" @@ -28,7 +27,6 @@ type LoginOptions struct { IO *iostreams.IOStreams Config iconfig HTTPClient *http.Client - GitClient *git.Client Hostname string Interactive bool Web bool @@ -65,11 +63,7 @@ func Login(opts *LoginOptions) error { var additionalScopes []string - credentialFlow := &GitCredentialFlow{ - Executable: opts.Executable, - Prompter: opts.Prompter, - GitClient: opts.GitClient, - } + credentialFlow := &GitCredentialFlow{Executable: opts.Executable, Prompter: opts.Prompter} 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 85572fdac..ddff8bd92 100644 --- a/pkg/cmd/browse/browse.go +++ b/pkg/cmd/browse/browse.go @@ -1,7 +1,6 @@ package browse import ( - "context" "fmt" "net/http" "net/url" @@ -42,13 +41,11 @@ 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: func() string { - return f.GitClient.PathFromRoot(context.Background()) - }, - GitClient: &localGitClient{client: f.GitClient}, + Browser: f.Browser, + HttpClient: f.HttpClient, + IO: f.IOStreams, + PathFromRepoRoot: git.PathFromRepoRoot, + GitClient: &localGitClient{}, } cmd := &cobra.Command{ @@ -272,18 +269,14 @@ type gitClient interface { LastCommit() (*git.Commit, error) } -type localGitClient struct { - client *git.Client -} +type localGitClient struct{} type remoteGitClient struct { repo func() (ghrepo.Interface, error) httpClient func() (*http.Client, error) } -func (gc *localGitClient) LastCommit() (*git.Commit, error) { - return gc.client.LastCommit(context.Background()) -} +func (gc *localGitClient) LastCommit() (*git.Commit, error) { return git.LastCommit() } 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 75e615be0..91197cc96 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 = func() string { return "" } + opts.PathFromRepoRoot = git.PathFromRepoRoot } err := runBrowse(&opts) diff --git a/pkg/cmd/extension/manager.go b/pkg/cmd/extension/manager.go index c9f638b87..289a9aefa 100644 --- a/pkg/cmd/extension/manager.go +++ b/pkg/cmd/extension/manager.go @@ -2,7 +2,6 @@ package extension import ( "bytes" - "context" _ "embed" "errors" "fmt" @@ -790,8 +789,7 @@ func isBinExtension(client *http.Client, repo ghrepo.Interface) (isBin bool, err } func repoFromPath(path string) (ghrepo.Interface, error) { - gitClient := &git.Client{RepoDir: path} - remotes, err := gitClient.Remotes(context.Background()) + remotes, err := git.RemotesForPath(path) if err != nil { return nil, err } diff --git a/pkg/cmd/factory/default.go b/pkg/cmd/factory/default.go index 10bf5d72b..77552c977 100644 --- a/pkg/cmd/factory/default.go +++ b/pkg/cmd/factory/default.go @@ -1,7 +1,6 @@ package factory import ( - "context" "fmt" "net/http" "os" @@ -9,7 +8,7 @@ import ( "time" "github.com/cli/cli/v2/api" - ghContext "github.com/cli/cli/v2/context" + "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" @@ -26,18 +25,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.GitClient = newGitClient(f) // Depends on IOStreams, and Executable - f.Remotes = remotesFunc(f) // Depends on Config, and GitClient + f.Remotes = remotesFunc(f) // Depends on Config 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 } @@ -65,7 +64,7 @@ func SmartBaseRepoFunc(f *cmdutil.Factory) func() (ghrepo.Interface, error) { if err != nil { return nil, err } - repoContext, err := ghContext.ResolveRemotesToRepos(remotes, apiClient, "") + repoContext, err := context.ResolveRemotesToRepos(remotes, apiClient, "") if err != nil { return nil, err } @@ -78,12 +77,10 @@ func SmartBaseRepoFunc(f *cmdutil.Factory) func() (ghrepo.Interface, error) { } } -func remotesFunc(f *cmdutil.Factory) func() (ghContext.Remotes, error) { +func remotesFunc(f *cmdutil.Factory) func() (context.Remotes, error) { rr := &remoteResolver{ - readRemotes: func() (git.RemoteSet, error) { - return f.GitClient.Remotes(context.Background()) - }, - getConfig: f.Config, + readRemotes: git.Remotes, + getConfig: f.Config, } return rr.Resolver() } @@ -145,9 +142,9 @@ func configFunc() func() (config.Config, error) { } } -func branchFunc(f *cmdutil.Factory) func() (string, error) { +func branchFunc() func() (string, error) { return func() (string, error) { - currentBranch, err := f.GitClient.CurrentBranch(context.Background()) + currentBranch, err := git.CurrentBranch() 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 ed074e700..41fa104fa 100644 --- a/pkg/cmd/gist/clone/clone.go +++ b/pkg/cmd/gist/clone/clone.go @@ -1,7 +1,6 @@ package clone import ( - "context" "fmt" "net/http" @@ -17,7 +16,6 @@ import ( type CloneOptions struct { HttpClient func() (*http.Client, error) - GitClient *git.Client Config func() (config.Config, error) IO *iostreams.IOStreams @@ -30,7 +28,6 @@ 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, } @@ -87,7 +84,7 @@ func cloneRun(opts *CloneOptions) error { gistURL = formatRemoteURL(hostname, gistURL, protocol) } - _, err := opts.GitClient.Clone(context.Background(), gistURL, opts.GitArgs) + _, err := git.RunClone(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 cd6f71b6e..ccca76b25 100644 --- a/pkg/cmd/gist/clone/clone_test.go +++ b/pkg/cmd/gist/clone/clone_test.go @@ -5,7 +5,6 @@ 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" @@ -26,7 +25,6 @@ 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 8addfc4ae..73c8f40d1 100644 --- a/pkg/cmd/pr/checkout/checkout.go +++ b/pkg/cmd/pr/checkout/checkout.go @@ -19,7 +19,6 @@ 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) @@ -38,7 +37,6 @@ 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, @@ -126,11 +124,11 @@ func checkoutRun(opts *CheckoutOptions) error { } if opts.RecurseSubmodules { - cmdQueue = append(cmdQueue, []string{"submodule", "sync", "--recursive"}) - cmdQueue = append(cmdQueue, []string{"submodule", "update", "--init", "--recursive"}) + cmdQueue = append(cmdQueue, []string{"git", "submodule", "sync", "--recursive"}) + cmdQueue = append(cmdQueue, []string{"git", "submodule", "update", "--init", "--recursive"}) } - err = executeCmds(opts.GitClient, cmdQueue) + err = executeCmds(cmdQueue, opts.IO) if err != nil { return err } @@ -147,7 +145,7 @@ func cmdsForExistingRemote(remote *cliContext.Remote, pr *api.PullRequest, opts refSpec += fmt.Sprintf(":refs/remotes/%s", remoteBranch) } - cmds = append(cmds, []string{"fetch", remote.Name, refSpec}) + cmds = append(cmds, []string{"git", "fetch", remote.Name, refSpec}) localBranch := pr.HeadRefName if opts.BranchName != "" { @@ -156,17 +154,17 @@ func cmdsForExistingRemote(remote *cliContext.Remote, pr *api.PullRequest, opts switch { case opts.Detach: - cmds = append(cmds, []string{"checkout", "--detach", "FETCH_HEAD"}) - case localBranchExists(opts.GitClient, localBranch): - cmds = append(cmds, []string{"checkout", localBranch}) + cmds = append(cmds, []string{"git", "checkout", "--detach", "FETCH_HEAD"}) + case localBranchExists(localBranch): + cmds = append(cmds, []string{"git", "checkout", localBranch}) if opts.Force { - cmds = append(cmds, []string{"reset", "--hard", fmt.Sprintf("refs/remotes/%s", remoteBranch)}) + cmds = append(cmds, []string{"git", "reset", "--hard", fmt.Sprintf("refs/remotes/%s", remoteBranch)}) } else { // TODO: check if non-fast-forward and suggest to use `--force` - cmds = append(cmds, []string{"merge", "--ff-only", fmt.Sprintf("refs/remotes/%s", remoteBranch)}) + cmds = append(cmds, []string{"git", "merge", "--ff-only", fmt.Sprintf("refs/remotes/%s", remoteBranch)}) } default: - cmds = append(cmds, []string{"checkout", "-b", localBranch, "--track", remoteBranch}) + cmds = append(cmds, []string{"git", "checkout", "-b", localBranch, "--track", remoteBranch}) } return cmds @@ -177,8 +175,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{"fetch", baseURLOrName, ref}) - cmds = append(cmds, []string{"checkout", "--detach", "FETCH_HEAD"}) + cmds = append(cmds, []string{"git", "fetch", baseURLOrName, ref}) + cmds = append(cmds, []string{"git", "checkout", "--detach", "FETCH_HEAD"}) return cmds } @@ -193,22 +191,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{"fetch", baseURLOrName, ref}) + cmds = append(cmds, []string{"git", "fetch", baseURLOrName, ref}) if opts.Force { - cmds = append(cmds, []string{"reset", "--hard", "FETCH_HEAD"}) + cmds = append(cmds, []string{"git", "reset", "--hard", "FETCH_HEAD"}) } else { // TODO: check if non-fast-forward and suggest to use `--force` - cmds = append(cmds, []string{"merge", "--ff-only", "FETCH_HEAD"}) + cmds = append(cmds, []string{"git", "merge", "--ff-only", "FETCH_HEAD"}) } } else { if opts.Force { - cmds = append(cmds, []string{"fetch", baseURLOrName, fmt.Sprintf("%s:%s", ref, localBranch), "--force"}) + cmds = append(cmds, []string{"git", "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{"fetch", baseURLOrName, fmt.Sprintf("%s:%s", ref, localBranch)}) + cmds = append(cmds, []string{"git", "fetch", baseURLOrName, fmt.Sprintf("%s:%s", ref, localBranch)}) } - cmds = append(cmds, []string{"checkout", localBranch}) + cmds = append(cmds, []string{"git", "checkout", localBranch}) } remote := baseURLOrName @@ -218,32 +216,37 @@ func cmdsForMissingRemote(pr *api.PullRequest, baseURLOrName, repoHost, defaultB remote = ghrepo.FormatRemoteURL(headRepo, protocol) mergeRef = fmt.Sprintf("refs/heads/%s", pr.HeadRefName) } - if missingMergeConfigForBranch(opts.GitClient, localBranch) { + if missingMergeConfigForBranch(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{"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}) + 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}) } return cmds } -func missingMergeConfigForBranch(client *git.Client, b string) bool { - mc, err := client.Config(context.Background(), fmt.Sprintf("branch.%s.merge", b)) +func missingMergeConfigForBranch(b string) bool { + mc, err := git.Config(fmt.Sprintf("branch.%s.merge", b)) return err != nil || mc == "" } -func localBranchExists(client *git.Client, b string) bool { - _, err := client.ShowRefs(context.Background(), "refs/heads/"+b) +func localBranchExists(b string) bool { + _, err := git.ShowRefs("refs/heads/" + b) return err == nil } -func executeCmds(client *git.Client, cmdQueue [][]string) error { +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, + } for _, args := range cmdQueue { - //TODO: Use AuthenticatedCommand - cmd, err := client.Command(context.Background(), args...) + cmd, err := client.Command(context.Background(), args[1:]...) if err != nil { return err } diff --git a/pkg/cmd/pr/checkout/checkout_test.go b/pkg/cmd/pr/checkout/checkout_test.go index 7540bbacf..df77e6676 100644 --- a/pkg/cmd/pr/checkout/checkout_test.go +++ b/pkg/cmd/pr/checkout/checkout_test.go @@ -197,8 +197,6 @@ 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) @@ -236,7 +234,6 @@ 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 7e39005bd..7d80199b6 100644 --- a/pkg/cmd/pr/close/close.go +++ b/pkg/cmd/pr/close/close.go @@ -1,7 +1,6 @@ package close import ( - "context" "fmt" "net/http" @@ -16,7 +15,6 @@ import ( type CloseOptions struct { HttpClient func() (*http.Client, error) - GitClient *git.Client IO *iostreams.IOStreams Branch func() (string, error) @@ -32,7 +30,6 @@ 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, } @@ -111,10 +108,9 @@ 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 := opts.GitClient.HasLocalBranch(ctx, pr.HeadRefName) + localBranchExists := git.HasLocalBranch(pr.HeadRefName) if opts.DeleteLocalBranch { if localBranchExists { @@ -129,13 +125,13 @@ func closeRun(opts *CloseOptions) error { if err != nil { return err } - err = opts.GitClient.CheckoutBranch(ctx, branchToSwitchTo) + err = git.CheckoutBranch(branchToSwitchTo) if err != nil { return err } } - if err := opts.GitClient.DeleteLocalBranch(ctx, pr.HeadRefName); err != nil { + if err := git.DeleteLocalBranch(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 b435fb8aa..df9585fbb 100644 --- a/pkg/cmd/pr/close/close_test.go +++ b/pkg/cmd/pr/close/close_test.go @@ -9,7 +9,6 @@ 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" @@ -73,7 +72,6 @@ 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 75d4b58f1..e9c664b88 100644 --- a/pkg/cmd/pr/create/create.go +++ b/pkg/cmd/pr/create/create.go @@ -1,7 +1,6 @@ package create import ( - "context" "errors" "fmt" "net/http" @@ -13,7 +12,7 @@ import ( "github.com/AlecAivazis/survey/v2" "github.com/MakeNowJust/heredoc" "github.com/cli/cli/v2/api" - ghContext "github.com/cli/cli/v2/context" + "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" @@ -33,10 +32,9 @@ 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() (ghContext.Remotes, error) + Remotes func() (context.Remotes, error) Branch func() (string, error) Browser browser.Browser Prompter iprompter @@ -70,24 +68,22 @@ 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 *ghContext.ResolvedRemotes + RepoContext *context.ResolvedRemotes BaseRepo *api.Repository HeadRepo ghrepo.Interface BaseTrackingBranch string BaseBranch string HeadBranch string HeadBranchLabel string - HeadRemote *ghContext.Remote + HeadRemote *context.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, @@ -373,16 +369,15 @@ 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 := gitClient.Commits(context.Background(), baseRef, headRef) + commits, err := git.Commits(baseRef, headRef) if err != nil { return err } if len(commits) == 1 { state.Title = commits[0].Title - body, err := gitClient.CommitBody(context.Background(), commits[0].Sha) + body, err := git.CommitBody(commits[0].Sha) if err != nil { return err } @@ -400,11 +395,11 @@ func initDefaultTitleBody(ctx CreateContext, state *shared.IssueMetadataState) e return nil } -func determineTrackingBranch(gitClient *git.Client, remotes ghContext.Remotes, headBranch string) *git.TrackingRef { +func determineTrackingBranch(remotes context.Remotes, headBranch string) *git.TrackingRef { refsForLookup := []string{"HEAD"} var trackingRefs []git.TrackingRef - headBranchConfig := gitClient.ReadBranchConfig(context.Background(), headBranch) + headBranchConfig := git.ReadBranchConfig(headBranch) if headBranchConfig.RemoteName != "" { tr := git.TrackingRef{ RemoteName: headBranchConfig.RemoteName, @@ -423,7 +418,7 @@ func determineTrackingBranch(gitClient *git.Client, remotes ghContext.Remotes, h refsForLookup = append(refsForLookup, tr.String()) } - resolvedRefs, _ := gitClient.ShowRefs(context.Background(), refsForLookup...) + resolvedRefs, _ := git.ShowRefs(refsForLookup...) if len(resolvedRefs) > 1 { for _, r := range resolvedRefs[1:] { if r.Hash != resolvedRefs[0].Hash { @@ -485,7 +480,7 @@ func NewCreateContext(opts *CreateOptions) (*CreateContext, error) { return nil, err } - repoContext, err := ghContext.ResolveRemotesToRepos(remotes, client, opts.RepoOverride) + repoContext, err := context.ResolveRemotesToRepos(remotes, client, opts.RepoOverride) if err != nil { return nil, err } @@ -520,17 +515,16 @@ func NewCreateContext(opts *CreateOptions) (*CreateContext, error) { headBranch = headBranch[idx+1:] } - gitClient := opts.GitClient - if ucc, err := gitClient.UncommittedChangeCount(context.Background()); err == nil && ucc > 0 { + if ucc, err := git.UncommittedChangeCount(); err == nil && ucc > 0 { fmt.Fprintf(opts.IO.ErrOut, "Warning: %s\n", text.Pluralize(ucc, "uncommitted change")) } var headRepo ghrepo.Interface - var headRemote *ghContext.Remote + var headRemote *context.Remote if isPushEnabled { // determine whether the head branch is already pushed to a remote - if pushedTo := determineTrackingBranch(gitClient, remotes, headBranch); pushedTo != nil { + if pushedTo := determineTrackingBranch(remotes, headBranch); pushedTo != nil { isPushEnabled = false if r, err := remotes.FindByName(pushedTo.RemoteName); err == nil { headRepo = r @@ -631,7 +625,6 @@ func NewCreateContext(opts *CreateOptions) (*CreateContext, error) { IsPushEnabled: isPushEnabled, RepoContext: repoContext, Client: client, - GitClient: gitClient, }, nil } @@ -720,12 +713,11 @@ func handlePush(opts CreateOptions, ctx CreateContext) error { headRepoURL := ghrepo.FormatRemoteURL(headRepo, cloneProtocol) // TODO: prevent clashes with another remote of a same name - gitClient := ctx.GitClient - gitRemote, err := gitClient.AddRemote(context.Background(), "fork", headRepoURL, []string{}) + gitRemote, err := git.AddRemote("fork", headRepoURL) if err != nil { return fmt.Errorf("error adding remote: %w", err) } - headRemote = &ghContext.Remote{ + headRemote = &context.Remote{ Remote: gitRemote, Repo: headRepo, } @@ -737,11 +729,12 @@ func handlePush(opts CreateOptions, ctx CreateContext) error { pushTries := 0 maxPushTries := 3 for { - 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 { + 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 { 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 0764ffe3b..c83dd13a2 100644 --- a/pkg/cmd/pr/create/create_test.go +++ b/pkg/cmd/pr/create/create_test.go @@ -869,7 +869,6 @@ 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) @@ -986,8 +985,7 @@ func Test_determineTrackingBranch(t *testing.T) { tt.cmdStubs(cs) - gitClient := &git.Client{GitPath: "some/path/git"} - ref := determineTrackingBranch(gitClient, tt.remotes, "feature") + ref := determineTrackingBranch(tt.remotes, "feature") tt.assert(ref, t) }) } diff --git a/pkg/cmd/pr/merge/merge.go b/pkg/cmd/pr/merge/merge.go index dbf0923f7..2972c09f9 100644 --- a/pkg/cmd/pr/merge/merge.go +++ b/pkg/cmd/pr/merge/merge.go @@ -1,7 +1,6 @@ package merge import ( - "context" "errors" "fmt" "net/http" @@ -9,7 +8,7 @@ import ( "github.com/AlecAivazis/survey/v2" "github.com/MakeNowJust/heredoc" "github.com/cli/cli/v2/api" - ghContext "github.com/cli/cli/v2/context" + "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" @@ -27,10 +26,9 @@ type editor interface { type MergeOptions struct { HttpClient func() (*http.Client, error) - GitClient *git.Client IO *iostreams.IOStreams Branch func() (string, error) - Remotes func() (ghContext.Remotes, error) + Remotes func() (context.Remotes, error) Finder shared.PRFinder @@ -62,7 +60,6 @@ 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, } @@ -227,7 +224,7 @@ func (m *mergeContext) warnIfDiverged() { return } - localBranchLastCommit, err := m.opts.GitClient.LastCommit(context.Background()) + localBranchLastCommit, err := git.LastCommit() if err != nil { return } @@ -399,8 +396,6 @@ 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() @@ -414,24 +409,24 @@ func (m *mergeContext) deleteLocalBranch() error { } targetBranch := m.pr.BaseRefName - if m.opts.GitClient.HasLocalBranch(ctx, targetBranch) { - if err := m.opts.GitClient.CheckoutBranch(ctx, targetBranch); err != nil { + if git.HasLocalBranch(targetBranch) { + if err := git.CheckoutBranch(targetBranch); err != nil { return err } } else { - if err := m.opts.GitClient.CheckoutNewBranch(ctx, baseRemote.Name, targetBranch); err != nil { + if err := git.CheckoutNewBranch(baseRemote.Name, targetBranch); err != nil { return err } } - if err := m.opts.GitClient.Pull(ctx, baseRemote.Name, targetBranch); err != nil { + if err := git.Pull(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 := m.opts.GitClient.DeleteLocalBranch(ctx, m.pr.HeadRefName); err != nil { + if err := git.DeleteLocalBranch(m.pr.HeadRefName); err != nil { return fmt.Errorf("failed to delete local branch %s: %w", m.cs.Cyan(m.pr.HeadRefName), err) } @@ -508,7 +503,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 && opts.GitClient.HasLocalBranch(context.Background(), pr.HeadRefName), + localBranchExists: opts.CanDeleteLocalBranch && git.HasLocalBranch(pr.HeadRefName), mergeQueueRequired: pr.IsMergeQueueEnabled, }, nil } @@ -735,7 +730,7 @@ func allowsAdminOverride(status string) bool { } } -func remoteForMergeConflictResolution(baseRepo ghrepo.Interface, pr *api.PullRequest, opts *MergeOptions) *ghContext.Remote { +func remoteForMergeConflictResolution(baseRepo ghrepo.Interface, pr *api.PullRequest, opts *MergeOptions) *context.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 0e3ced350..d1bb21d8c 100644 --- a/pkg/cmd/pr/merge/merge_test.go +++ b/pkg/cmd/pr/merge/merge_test.go @@ -270,7 +270,6 @@ 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 ed04c4295..b700a7501 100644 --- a/pkg/cmd/pr/shared/finder.go +++ b/pkg/cmd/pr/shared/finder.go @@ -53,14 +53,12 @@ func NewFinder(factory *cmdutil.Factory) PRFinder { } return &finder{ - 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) - }, + baseRepoFn: factory.BaseRepo, + branchFn: factory.Branch, + remotesFn: factory.Remotes, + httpClient: factory.HttpClient, + progress: factory.IOStreams, + branchConfig: git.ReadBranchConfig, } } diff --git a/pkg/cmd/pr/shared/survey.go b/pkg/cmd/pr/shared/survey.go index dd4ba085d..31942c6a0 100644 --- a/pkg/cmd/pr/shared/survey.go +++ b/pkg/cmd/pr/shared/survey.go @@ -6,7 +6,9 @@ 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" @@ -367,3 +369,18 @@ 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 7aab57978..975bdd5da 100644 --- a/pkg/cmd/pr/shared/templates.go +++ b/pkg/cmd/pr/shared/templates.go @@ -1,7 +1,6 @@ package shared import ( - "context" "fmt" "net/http" "time" @@ -234,8 +233,7 @@ func (m *templateManager) fetch() error { dir := m.rootDir if dir == "" { var err error - gitClient := &git.Client{} - dir, err = gitClient.ToplevelDir(context.Background()) + dir, err = git.ToplevelDir() if err != nil { return nil // abort silently } diff --git a/pkg/cmd/pr/status/status.go b/pkg/cmd/pr/status/status.go index e4434978b..784159564 100644 --- a/pkg/cmd/pr/status/status.go +++ b/pkg/cmd/pr/status/status.go @@ -1,7 +1,6 @@ package status import ( - "context" "errors" "fmt" "net/http" @@ -10,7 +9,7 @@ import ( "strings" "github.com/cli/cli/v2/api" - ghContext "github.com/cli/cli/v2/context" + "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" @@ -23,11 +22,10 @@ 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() (ghContext.Remotes, error) + Remotes func() (context.Remotes, error) Branch func() (string, error) HasRepoOverride bool @@ -39,7 +37,6 @@ 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, @@ -89,7 +86,7 @@ func statusRun(opts *StatusOptions) error { } remotes, _ := opts.Remotes() - currentPRNumber, currentPRHeadRef, err = prSelectorForCurrentBranch(opts.GitClient, baseRepo, currentBranch, remotes) + currentPRNumber, currentPRHeadRef, err = prSelectorForCurrentBranch(baseRepo, currentBranch, remotes) if err != nil { return fmt.Errorf("could not query for pull request for current branch: %w", err) } @@ -168,9 +165,9 @@ func statusRun(opts *StatusOptions) error { return nil } -func prSelectorForCurrentBranch(gitClient *git.Client, baseRepo ghrepo.Interface, prHeadRef string, rem ghContext.Remotes) (prNumber int, selector string, err error) { +func prSelectorForCurrentBranch(baseRepo ghrepo.Interface, prHeadRef string, rem context.Remotes) (prNumber int, selector string, err error) { selector = prHeadRef - branchConfig := gitClient.ReadBranchConfig(context.Background(), prHeadRef) + branchConfig := git.ReadBranchConfig(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 c3187739d..31f28396e 100644 --- a/pkg/cmd/pr/status/status_test.go +++ b/pkg/cmd/pr/status/status_test.go @@ -52,7 +52,6 @@ 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) @@ -329,8 +328,7 @@ func Test_prSelectorForCurrentBranch(t *testing.T) { Repo: repo, }, } - gitClient := &git.Client{GitPath: "some/path/git"} - prNum, headRef, err := prSelectorForCurrentBranch(gitClient, repo, "Frederick888/main", rem) + prNum, headRef, err := prSelectorForCurrentBranch(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 8c36218da..e6f97dd9f 100644 --- a/pkg/cmd/release/create/create.go +++ b/pkg/cmd/release/create/create.go @@ -2,7 +2,6 @@ package create import ( "bytes" - "context" "errors" "fmt" "io" @@ -27,7 +26,6 @@ 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) @@ -58,7 +56,6 @@ 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, } @@ -224,7 +221,7 @@ func createRun(opts *CreateOptions) error { var tagDescription string if opts.RepoOverride == "" { - tagDescription, _ = gitTagInfo(opts.GitClient, opts.TagName) + tagDescription, _ = gitTagInfo(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. @@ -271,10 +268,10 @@ func createRun(opts *CreateOptions) error { } if generatedNotes == nil { if opts.NotesStartTag != "" { - commits, _ := changelogForRange(opts.GitClient, fmt.Sprintf("%s..%s", opts.NotesStartTag, headRef)) + commits, _ := changelogForRange(fmt.Sprintf("%s..%s", opts.NotesStartTag, headRef)) generatedChangelog = generateChangelog(commits) - } else if prevTag, err := detectPreviousTag(opts.GitClient, headRef); err == nil { - commits, _ := changelogForRange(opts.GitClient, fmt.Sprintf("%s..%s", prevTag, headRef)) + } else if prevTag, err := detectPreviousTag(headRef); err == nil { + commits, _ := changelogForRange(fmt.Sprintf("%s..%s", prevTag, headRef)) generatedChangelog = generateChangelog(commits) } } @@ -472,8 +469,8 @@ func createRun(opts *CreateOptions) error { return nil } -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)") +func gitTagInfo(tagName string) (string, error) { + cmd, err := git.GitCommand("tag", "--list", tagName, "--format=%(contents:subject)%0a%0a%(contents:body)") if err != nil { return "", err } @@ -481,8 +478,8 @@ func gitTagInfo(client *git.Client, tagName string) (string, error) { return string(b), err } -func detectPreviousTag(client *git.Client, headRef string) (string, error) { - cmd, err := client.Command(context.Background(), "describe", "--tags", "--abbrev=0", fmt.Sprintf("%s^", headRef)) +func detectPreviousTag(headRef string) (string, error) { + cmd, err := git.GitCommand("describe", "--tags", "--abbrev=0", fmt.Sprintf("%s^", headRef)) if err != nil { return "", err } @@ -495,8 +492,8 @@ type logEntry struct { Body string } -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) +func changelogForRange(refRange string) ([]logEntry, error) { + cmd, err := git.GitCommand("-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 a6424c0d6..7fb6711b2 100644 --- a/pkg/cmd/release/create/create_test.go +++ b/pkg/cmd/release/create/create_test.go @@ -10,7 +10,6 @@ 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" @@ -689,8 +688,6 @@ 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) @@ -1053,8 +1050,6 @@ 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 cb46d4572..ba289aa47 100644 --- a/pkg/cmd/repo/clone/clone.go +++ b/pkg/cmd/repo/clone/clone.go @@ -1,7 +1,6 @@ package clone import ( - "context" "fmt" "net/http" "strings" @@ -19,7 +18,6 @@ import ( type CloneOptions struct { HttpClient func() (*http.Client, error) - GitClient *git.Client Config func() (config.Config, error) IO *iostreams.IOStreams @@ -32,7 +30,6 @@ 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, } @@ -155,9 +152,7 @@ func cloneRun(opts *CloneOptions) error { canonicalCloneURL = strings.TrimSuffix(canonicalCloneURL, ".git") + ".wiki.git" } - gitClient := opts.GitClient - ctx := context.Background() - cloneDir, err := gitClient.Clone(ctx, canonicalCloneURL, opts.GitArgs) + cloneDir, err := git.RunClone(canonicalCloneURL, opts.GitArgs) if err != nil { return err } @@ -175,7 +170,7 @@ func cloneRun(opts *CloneOptions) error { upstreamName = canonicalRepo.Parent.RepoOwner() } - _, err = gitClient.AddRemote(ctx, upstreamName, upstreamURL, []string{canonicalRepo.Parent.DefaultBranchRef.Name}, git.WithRepoDir(cloneDir)) + err = git.AddNamedRemote(upstreamURL, upstreamName, cloneDir, []string{canonicalRepo.Parent.DefaultBranchRef.Name}) if err != nil { return err } diff --git a/pkg/cmd/repo/clone/clone_test.go b/pkg/cmd/repo/clone/clone_test.go index c6ec872be..34f2fe431 100644 --- a/pkg/cmd/repo/clone/clone_test.go +++ b/pkg/cmd/repo/clone/clone_test.go @@ -5,7 +5,6 @@ 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" @@ -105,7 +104,6 @@ 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 931ffb926..e311ad727 100644 --- a/pkg/cmd/repo/create/create.go +++ b/pkg/cmd/repo/create/create.go @@ -1,7 +1,6 @@ package create import ( - "context" "errors" "fmt" "net/http" @@ -28,7 +27,6 @@ type iprompter interface { type CreateOptions struct { HttpClient func() (*http.Client, error) - GitClient *git.Client Config func() (config.Config, error) IO *iostreams.IOStreams Prompter iprompter @@ -59,7 +57,6 @@ 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, } @@ -393,10 +390,10 @@ func createFromScratch(opts *CreateOptions) error { // use the template's default branch checkoutBranch = templateRepoMainBranch } - if err := localInit(opts.GitClient, remoteURL, repo.RepoName(), checkoutBranch); err != nil { + if err := localInit(opts.IO, remoteURL, repo.RepoName(), checkoutBranch); err != nil { return err } - } else if _, err := opts.GitClient.Clone(context.Background(), remoteURL, []string{}); err != nil { + } else if _, err := git.RunClone(remoteURL, []string{}); err != nil { return err } } @@ -430,7 +427,6 @@ func createFromLocal(opts *CreateOptions) error { } repoPath := opts.Source - opts.GitClient.RepoDir = repoPath var baseRemote string if opts.Remote == "" { @@ -444,7 +440,7 @@ func createFromLocal(opts *CreateOptions) error { return err } - isRepo, err := isLocalRepo(opts.GitClient) + isRepo, err := isLocalRepo(repoPath) if err != nil { return err } @@ -455,7 +451,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(opts.GitClient) + committed, err := hasCommits(repoPath) if err != nil { return err } @@ -537,7 +533,7 @@ func createFromLocal(opts *CreateOptions) error { } } - if err := sourceInit(opts.GitClient, opts.IO, remoteURL, baseRemote); err != nil { + if err := sourceInit(opts.IO, remoteURL, baseRemote, repoPath); err != nil { return err } @@ -551,7 +547,7 @@ func createFromLocal(opts *CreateOptions) error { } if opts.Push { - repoPush, err := opts.GitClient.Command(context.Background(), "push", "-u", baseRemote, "HEAD") + repoPush, err := git.GitCommand("-C", repoPath, "push", "-u", baseRemote, "HEAD") if err != nil { return err } @@ -567,17 +563,17 @@ func createFromLocal(opts *CreateOptions) error { return nil } -func sourceInit(gitClient *git.Client, io *iostreams.IOStreams, remoteURL, baseRemote string) error { +func sourceInit(io *iostreams.IOStreams, remoteURL, baseRemote, repoPath string) error { cs := io.ColorScheme() isTTY := io.IsStdoutTTY() stdout := io.Out - remoteAdd, err := gitClient.Command(context.Background(), "remote", "add", baseRemote, remoteURL) + remoteAdd, err := git.GitCommand("-C", repoPath, "remote", "add", baseRemote, remoteURL) if err != nil { return err } - _, err = remoteAdd.Output() + err = remoteAdd.Run() if err != nil { return fmt.Errorf("%s Unable to add remote %q", cs.FailureIcon(), baseRemote) } @@ -588,12 +584,12 @@ func sourceInit(gitClient *git.Client, io *iostreams.IOStreams, remoteURL, baseR } // check if local repository has committed changes -func hasCommits(gitClient *git.Client) (bool, error) { - hasCommitsCmd, err := gitClient.Command(context.Background(), "rev-parse", "HEAD") +func hasCommits(repoPath string) (bool, error) { + hasCommitsCmd, err := git.GitCommand("-C", repoPath, "rev-parse", "HEAD") if err != nil { return false, err } - _, err = hasCommitsCmd.Output() + err = hasCommitsCmd.Run() if err == nil { return true, nil } @@ -610,8 +606,8 @@ func hasCommits(gitClient *git.Client) (bool, error) { } // check if path is the top level directory of a git repo -func isLocalRepo(gitClient *git.Client) (bool, error) { - projectDir, projectDirErr := gitClient.GitDir(context.Background()) +func isLocalRepo(repoPath string) (bool, error) { + projectDir, projectDirErr := git.GetDirFromPath(repoPath) if projectDirErr != nil { var execError *exec.ExitError if errors.As(projectDirErr, &execError) { @@ -628,26 +624,28 @@ func isLocalRepo(gitClient *git.Client) (bool, error) { } // clone the checkout branch to specified path -func localInit(gitClient *git.Client, remoteURL, path, checkoutBranch string) error { - ctx := context.Background() - gitInit, err := gitClient.Command(ctx, "init", path) +func localInit(io *iostreams.IOStreams, remoteURL, path, checkoutBranch string) error { + gitInit, err := git.GitCommand("init", path) if err != nil { return err } - _, err = gitInit.Output() + isTTY := io.IsStdoutTTY() + if isTTY { + gitInit.Stdout = io.Out + } + gitInit.Stderr = io.ErrOut + err = gitInit.Run() if err != nil { return err } - // 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) + gitRemoteAdd, err := git.GitCommand("-C", path, "remote", "add", "origin", remoteURL) if err != nil { return err } - _, err = gitRemoteAdd.Output() + gitRemoteAdd.Stdout = io.Out + gitRemoteAdd.Stderr = io.ErrOut + err = gitRemoteAdd.Run() if err != nil { return err } @@ -656,21 +654,24 @@ func localInit(gitClient *git.Client, remoteURL, path, checkoutBranch string) er return nil } - gitFetch, err := gc.Command(ctx, "fetch", "origin", fmt.Sprintf("+refs/heads/%[1]s:refs/remotes/origin/%[1]s", checkoutBranch)) + gitFetch, err := git.GitCommand("-C", path, "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 := gc.Command(ctx, "checkout", checkoutBranch) + gitCheckout, err := git.GitCommand("-C", path, "checkout", checkoutBranch) if err != nil { return err } - _, err = gitCheckout.Output() - return err + gitCheckout.Stdout = io.Out + gitCheckout.Stderr = io.ErrOut + return gitCheckout.Run() } func interactiveGitIgnore(client *http.Client, hostname string, prompter iprompter) (string, error) { @@ -735,14 +736,3 @@ 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 a1ce875f0..f8849afd1 100644 --- a/pkg/cmd/repo/create/create_test.go +++ b/pkg/cmd/repo/create/create_test.go @@ -6,7 +6,6 @@ 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" @@ -493,8 +492,6 @@ 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 e17e4b34d..9416ba4b2 100644 --- a/pkg/cmd/repo/fork/fork.go +++ b/pkg/cmd/repo/fork/fork.go @@ -1,7 +1,6 @@ package fork import ( - "context" "fmt" "net/http" "net/url" @@ -10,7 +9,7 @@ import ( "github.com/MakeNowJust/heredoc" "github.com/cli/cli/v2/api" - ghContext "github.com/cli/cli/v2/context" + "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,11 +25,10 @@ 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() (ghContext.Remotes, error) + Remotes func() (context.Remotes, error) Since func(time.Time) time.Duration GitArgs []string @@ -53,7 +51,6 @@ 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, @@ -229,9 +226,6 @@ 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 { @@ -270,7 +264,6 @@ func forkRun(opts *ForkOptions) error { return fmt.Errorf("failed to prompt: %w", err) } } - if remoteDesired { remoteName := opts.RemoteName remotes, err := opts.Remotes() @@ -281,11 +274,11 @@ func forkRun(opts *ForkOptions) error { if _, err := remotes.FindByName(remoteName); err == nil { if opts.Rename { renameTarget := "upstream" - renameCmd, err := gitClient.Command(ctx, "remote", "rename", remoteName, renameTarget) + renameCmd, err := git.GitCommand("remote", "rename", remoteName, renameTarget) if err != nil { return err } - _, err = renameCmd.Output() + err = renameCmd.Run() if err != nil { return err } @@ -296,7 +289,7 @@ func forkRun(opts *ForkOptions) error { forkedRepoCloneURL := ghrepo.FormatRemoteURL(forkedRepo, protocol) - _, err = gitClient.AddRemote(ctx, remoteName, forkedRepoCloneURL, []string{}) + _, err = git.AddRemote(remoteName, forkedRepoCloneURL) if err != nil { return fmt.Errorf("failed to add remote: %w", err) } @@ -316,13 +309,13 @@ func forkRun(opts *ForkOptions) error { } if cloneDesired { forkedRepoURL := ghrepo.FormatRemoteURL(forkedRepo, protocol) - cloneDir, err := gitClient.Clone(ctx, forkedRepoURL, opts.GitArgs) + cloneDir, err := git.RunClone(forkedRepoURL, opts.GitArgs) if err != nil { return fmt.Errorf("failed to clone fork: %w", err) } upstreamURL := ghrepo.FormatRemoteURL(repoToFork, protocol) - _, err = gitClient.AddRemote(ctx, "upstream", upstreamURL, []string{}, git.WithRepoDir(cloneDir)) + err = git.AddNamedRemote(upstreamURL, "upstream", cloneDir, []string{}) if err != nil { return err } diff --git a/pkg/cmd/repo/fork/fork_test.go b/pkg/cmd/repo/fork/fork_test.go index e5ffda36b..111eb98f6 100644 --- a/pkg/cmd/repo/fork/fork_test.go +++ b/pkg/cmd/repo/fork/fork_test.go @@ -700,8 +700,6 @@ 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 2d07cd8b0..0b256d1d3 100644 --- a/pkg/cmd/repo/rename/rename.go +++ b/pkg/cmd/repo/rename/rename.go @@ -1,14 +1,13 @@ package rename import ( - "context" "fmt" "net/http" "github.com/AlecAivazis/survey/v2" "github.com/MakeNowJust/heredoc" "github.com/cli/cli/v2/api" - ghContext "github.com/cli/cli/v2/context" + "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" @@ -20,11 +19,10 @@ 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() (ghContext.Remotes, error) + Remotes func() (context.Remotes, error) DoConfirm bool HasRepoOverride bool newRepoSelector string @@ -34,7 +32,6 @@ 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, } @@ -148,7 +145,7 @@ func renameRun(opts *RenameOptions) error { return nil } -func updateRemote(repo ghrepo.Interface, renamed ghrepo.Interface, opts *RenameOptions) (*ghContext.Remote, error) { +func updateRemote(repo ghrepo.Interface, renamed ghrepo.Interface, opts *RenameOptions) (*context.Remote, error) { cfg, err := opts.Config() if err != nil { return nil, err @@ -170,7 +167,6 @@ func updateRemote(repo ghrepo.Interface, renamed ghrepo.Interface, opts *RenameO } remoteURL := ghrepo.FormatRemoteURL(renamed, protocol) - err = opts.GitClient.UpdateRemoteURL(context.Background(), remote.Name, remoteURL) - + err = git.UpdateRemoteURL(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 523b2ba4a..97e1aebb7 100644 --- a/pkg/cmd/repo/rename/rename_test.go +++ b/pkg/cmd/repo/rename/rename_test.go @@ -259,8 +259,6 @@ 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 fbb63a2ec..1ec86b053 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 { - client *git.Client + io *iostreams.IOStreams } func (g *gitExecuter) BranchRemote(branch string) (string, error) { args := []string{"rev-parse", "--symbolic-full-name", "--abbrev-ref", fmt.Sprintf("%s@{u}", branch)} - cmd, err := g.client.Command(context.Background(), args...) + cmd, err := git.GitCommand(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 := g.client.Command(context.Background(), "update-ref", fmt.Sprintf("refs/heads/%s", branch), ref) - if err != nil { - return err - } - _, err = cmd.Output() - return err -} - -func (g *gitExecuter) CreateBranch(branch, ref, upstream string) error { - ctx := context.Background() - cmd, err := g.client.Command(ctx, "branch", branch, ref) - if err != nil { - return err - } - if _, err := cmd.Output(); err != nil { - return err - } - cmd, err = g.client.Command(ctx, "branch", "--set-upstream-to", upstream, branch) - if err != nil { - return err - } - _, err = cmd.Output() - return err -} - -func (g *gitExecuter) CurrentBranch() (string, error) { - return g.client.CurrentBranch(context.Background()) -} - -func (g *gitExecuter) Fetch(remote, ref string) error { - args := []string{"fetch", "-q", remote, ref} - cmd, err := g.client.Command(context.Background(), args...) + cmd, err := git.GitCommand("update-ref", fmt.Sprintf("refs/heads/%s", branch), ref) if err != nil { return err } return cmd.Run() } +func (g *gitExecuter) CreateBranch(branch, ref, upstream string) error { + cmd, err := git.GitCommand("branch", branch, ref) + if err != nil { + return err + } + if err := cmd.Run(); err != nil { + return err + } + cmd, err = git.GitCommand("branch", "--set-upstream-to", upstream, branch) + if err != nil { + return err + } + return cmd.Run() +} + +func (g *gitExecuter) CurrentBranch() (string, error) { + return git.CurrentBranch() +} + +func (g *gitExecuter) Fetch(remote, ref string) error { + args := []string{"fetch", "-q", remote, ref} + cmd, err := git.GitCommand(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 g.client.HasLocalBranch(context.Background(), branch) + return git.HasLocalBranch(branch) } func (g *gitExecuter) IsAncestor(ancestor, progeny string) (bool, error) { args := []string{"merge-base", "--is-ancestor", ancestor, progeny} - cmd, err := g.client.Command(context.Background(), args...) + cmd, err := git.GitCommand(args...) if err != nil { return false, err } - _, err = cmd.Output() + err = cmd.Run() return err == nil, nil } func (g *gitExecuter) IsDirty() (bool, error) { - cmd, err := g.client.Command(context.Background(), "status", "--untracked-files=no", "--porcelain") + cmd, err := git.GitCommand("status", "--untracked-files=no", "--porcelain") if err != nil { return false, err } @@ -109,20 +109,18 @@ func (g *gitExecuter) IsDirty() (bool, error) { func (g *gitExecuter) MergeFastForward(ref string) error { args := []string{"merge", "--ff-only", "--quiet", ref} - cmd, err := g.client.Command(context.Background(), args...) + cmd, err := git.GitCommand(args...) if err != nil { return err } - _, err = cmd.Output() - return err + return cmd.Run() } func (g *gitExecuter) ResetHard(ref string) error { args := []string{"reset", "--hard", ref} - cmd, err := g.client.Command(context.Background(), args...) + cmd, err := git.GitCommand(args...) if err != nil { return err } - _, err = cmd.Output() - return err + return cmd.Run() } diff --git a/pkg/cmd/repo/sync/sync.go b/pkg/cmd/repo/sync/sync.go index eebec4389..ba2fdbafb 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{client: f.GitClient}, + Git: &gitExecuter{io: f.IOStreams}, } cmd := &cobra.Command{