Skip to content

Commit 8b5abc7

Browse files
samcoesamcoe
committed
Extract git interactions into interface
1 parent c6f89d3 commit 8b5abc7

File tree

4 files changed

+136
-58
lines changed

4 files changed

+136
-58
lines changed

‎git/git.go‎

Lines changed: 0 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -368,30 +368,3 @@ func getBranchShortName(output []byte) string {
368368
branch := firstLine(output)
369369
return strings.TrimPrefix(branch, "refs/heads/")
370370
}
371-
372-
func IsAncestor(ancestor, commit string) (bool, error) {
373-
cmd, err := GitCommand("merge-base", "--is-ancestor", ancestor, commit)
374-
if err != nil {
375-
return false, err
376-
}
377-
err = run.PrepareCmd(cmd).Run()
378-
return err == nil, nil
379-
}
380-
381-
func IsDirty() (bool, error) {
382-
cmd, err := GitCommand("status", "--untracked-files=no", "--porcelain")
383-
if err != nil {
384-
return false, err
385-
}
386-
387-
output, err := run.PrepareCmd(cmd).Output()
388-
if err != nil {
389-
return true, err
390-
}
391-
392-
if len(output) > 0 {
393-
return true, nil
394-
}
395-
396-
return false, nil
397-
}

‎pkg/cmd/repo/sync/git.go‎

Lines changed: 97 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,97 @@
1+
package sync
2+
3+
import (
4+
"os/exec"
5+
6+
"github.com/cli/cli/git"
7+
"github.com/cli/cli/internal/run"
8+
)
9+
10+
type gitClient interface {
11+
Checkout([]string) error
12+
CurrentBranch() (string, error)
13+
Fetch([]string) error
14+
HasLocalBranch([]string) bool
15+
IsAncestor([]string) (bool, error)
16+
IsDirty() (bool, error)
17+
Merge([]string) error
18+
Reset([]string) error
19+
Stash([]string) error
20+
}
21+
22+
type gitExecuter struct {
23+
gitCommand func(args ...string) (*exec.Cmd, error)
24+
}
25+
26+
func (g *gitExecuter) Checkout(args []string) error {
27+
return git.CheckoutBranch(args[0])
28+
}
29+
30+
func (g *gitExecuter) CurrentBranch() (string, error) {
31+
return git.CurrentBranch()
32+
}
33+
34+
func (g *gitExecuter) Fetch(args []string) error {
35+
args = append([]string{"fetch"}, args...)
36+
cmd, err := g.gitCommand(args...)
37+
if err != nil {
38+
return err
39+
}
40+
return run.PrepareCmd(cmd).Run()
41+
}
42+
43+
func (g *gitExecuter) HasLocalBranch(args []string) bool {
44+
return git.HasLocalBranch(args[0])
45+
}
46+
47+
func (g *gitExecuter) IsAncestor(args []string) (bool, error) {
48+
args = append([]string{"merge-base", "--is-ancestor"}, args...)
49+
cmd, err := g.gitCommand(args...)
50+
if err != nil {
51+
return false, err
52+
}
53+
err = run.PrepareCmd(cmd).Run()
54+
return err == nil, nil
55+
}
56+
57+
func (g *gitExecuter) IsDirty() (bool, error) {
58+
cmd, err := g.gitCommand("status", "--untracked-files=no", "--porcelain")
59+
if err != nil {
60+
return false, err
61+
}
62+
output, err := run.PrepareCmd(cmd).Output()
63+
if err != nil {
64+
return true, err
65+
}
66+
if len(output) > 0 {
67+
return true, nil
68+
}
69+
return false, nil
70+
}
71+
72+
func (g *gitExecuter) Merge(args []string) error {
73+
args = append([]string{"merge"}, args...)
74+
cmd, err := g.gitCommand(args...)
75+
if err != nil {
76+
return err
77+
}
78+
return run.PrepareCmd(cmd).Run()
79+
}
80+
81+
func (g *gitExecuter) Reset(args []string) error {
82+
args = append([]string{"reset"}, args...)
83+
cmd, err := g.gitCommand(args...)
84+
if err != nil {
85+
return err
86+
}
87+
return run.PrepareCmd(cmd).Run()
88+
}
89+
90+
func (g *gitExecuter) Stash(args []string) error {
91+
args = append([]string{"stash"}, args...)
92+
cmd, err := g.gitCommand(args...)
93+
if err != nil {
94+
return err
95+
}
96+
return run.PrepareCmd(cmd).Run()
97+
}

‎pkg/cmd/repo/sync/sync.go‎

Lines changed: 36 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@ import (
44
"errors"
55
"fmt"
66
"net/http"
7-
"os/exec"
87
"regexp"
98

109
"github.com/AlecAivazis/survey/v2"
@@ -13,11 +12,9 @@ import (
1312
"github.com/cli/cli/context"
1413
"github.com/cli/cli/git"
1514
"github.com/cli/cli/internal/ghrepo"
16-
"github.com/cli/cli/internal/run"
1715
"github.com/cli/cli/pkg/cmdutil"
1816
"github.com/cli/cli/pkg/iostreams"
1917
"github.com/cli/cli/pkg/prompt"
20-
"github.com/cli/safeexec"
2118
"github.com/spf13/cobra"
2219
)
2320

@@ -27,6 +24,7 @@ type SyncOptions struct {
2724
BaseRepo func() (ghrepo.Interface, error)
2825
Remotes func() (context.Remotes, error)
2926
CurrentBranch func() (string, error)
27+
Git gitClient
3028
DestArg string
3129
SrcArg string
3230
Branch string
@@ -41,6 +39,7 @@ func NewCmdSync(f *cmdutil.Factory, runF func(*SyncOptions) error) *cobra.Comman
4139
BaseRepo: f.BaseRepo,
4240
Remotes: f.Remotes,
4341
CurrentBranch: f.Branch,
42+
Git: &gitExecuter{gitCommand: git.GitCommand},
4443
}
4544

4645
cmd := &cobra.Command{
@@ -202,12 +201,16 @@ func syncLocalRepo(srcRepo ghrepo.Interface, opts *SyncOptions) error {
202201
}
203202
remote := remotes[0]
204203
branch := opts.Branch
204+
git := opts.Git
205205

206-
_ = executeCmds([][]string{{"git", "fetch", remote.Name, fmt.Sprintf("+refs/heads/%s", branch)}})
206+
err = git.Fetch([]string{remote.Name, fmt.Sprintf("+refs/heads/%s", branch)})
207+
if err != nil {
208+
return err
209+
}
207210

208-
hasLocalBranch := git.HasLocalBranch(branch)
211+
hasLocalBranch := git.HasLocalBranch([]string{branch})
209212
if hasLocalBranch {
210-
fastForward, err := git.IsAncestor(branch, fmt.Sprintf("%s/%s", remote.Name, branch))
213+
fastForward, err := git.IsAncestor([]string{branch, fmt.Sprintf("%s/%s", remote.Name, branch)})
211214
if err != nil {
212215
return err
213216
}
@@ -217,38 +220,54 @@ func syncLocalRepo(srcRepo ghrepo.Interface, opts *SyncOptions) error {
217220
}
218221
}
219222

220-
startBranch, err := opts.CurrentBranch()
223+
dirtyRepo, err := git.IsDirty()
221224
if err != nil {
222225
return err
223226
}
224-
225-
dirtyRepo, err := git.IsDirty()
227+
startBranch, err := git.CurrentBranch()
226228
if err != nil {
227229
return err
228230
}
229231

230-
var cmds [][]string
231232
if dirtyRepo {
232-
cmds = append(cmds, []string{"git", "stash", "push"})
233+
err = git.Stash([]string{"push"})
234+
if err != nil {
235+
return err
236+
}
233237
}
234238
if startBranch != branch {
235-
cmds = append(cmds, []string{"git", "checkout", branch})
239+
err = git.Checkout([]string{branch})
240+
if err != nil {
241+
return err
242+
}
236243
}
237244
if hasLocalBranch {
238245
if opts.Force {
239-
cmds = append(cmds, []string{"git", "reset", "--hard", fmt.Sprintf("refs/remotes/%s/%s", remote, branch)})
246+
err = git.Reset([]string{"--hard", fmt.Sprintf("refs/remotes/%s/%s", remote, branch)})
247+
if err != nil {
248+
return err
249+
}
240250
} else {
241-
cmds = append(cmds, []string{"git", "merge", "--ff-only", fmt.Sprintf("refs/remotes/%s/%s", remote, branch)})
251+
err = git.Merge([]string{"--ff-only", fmt.Sprintf("refs/remotes/%s/%s", remote, branch)})
252+
if err != nil {
253+
return err
254+
}
242255
}
243256
}
244257
if startBranch != branch {
245-
cmds = append(cmds, []string{"git", "checkout", startBranch})
258+
err = git.Checkout([]string{startBranch})
259+
if err != nil {
260+
return err
261+
}
246262
}
247263
if dirtyRepo {
248-
cmds = append(cmds, []string{"git", "stash", "pop"})
264+
err = git.Stash([]string{"pop"})
265+
if err != nil {
266+
return err
267+
}
249268
}
250269

251-
return executeCmds(cmds)
270+
return nil
252271
}
253272

254273
func syncRemoteRepo(client *api.Client, destRepo, srcRepo ghrepo.Interface, opts *SyncOptions) error {
@@ -270,17 +289,3 @@ func syncRemoteRepo(client *api.Client, destRepo, srcRepo ghrepo.Interface, opts
270289

271290
return err
272291
}
273-
274-
func executeCmds(cmdQueue [][]string) error {
275-
exe, err := safeexec.LookPath("git")
276-
if err != nil {
277-
return err
278-
}
279-
for _, args := range cmdQueue {
280-
cmd := exec.Command(exe, args[1:]...)
281-
if err := run.PrepareCmd(cmd).Run(); err != nil {
282-
return err
283-
}
284-
}
285-
return nil
286-
}

‎pkg/cmd/repo/sync/sync_test.go‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ func TestNewCmdSync(t *testing.T) {
1717
input string
1818
output SyncOptions
1919
wantsErr bool
20+
errMsg string
2021
}{
2122
{
2223
name: "no argument",
@@ -69,6 +70,7 @@ func TestNewCmdSync(t *testing.T) {
6970
tty: false,
7071
input: "",
7172
wantsErr: true,
73+
errMsg: "`--confirm` required when not running interactively",
7274
},
7375
}
7476
for _, tt := range tests {
@@ -94,6 +96,7 @@ func TestNewCmdSync(t *testing.T) {
9496
_, err = cmd.ExecuteC()
9597
if tt.wantsErr {
9698
assert.Error(t, err)
99+
assert.Equal(t, tt.errMsg, err.Error())
97100
return
98101
}
99102

0 commit comments

Comments
 (0)