Skip to content

Commit a1e1842

Browse files
samcoesamcoe
committed
Catch a couple more edge cases
1 parent 0b80c30 commit a1e1842

File tree

4 files changed

+73
-4
lines changed

4 files changed

+73
-4
lines changed

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

Lines changed: 24 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,14 @@
11
package sync
22

33
import (
4+
"fmt"
5+
"strings"
6+
47
"github.com/cli/cli/git"
58
)
69

710
type gitClient interface {
11+
BranchRemote(string) (string, error)
812
Checkout([]string) error
913
CurrentBranch() (string, error)
1014
Fetch([]string) error
@@ -17,8 +21,27 @@ type gitClient interface {
1721

1822
type gitExecuter struct{}
1923

24+
func (g *gitExecuter) BranchRemote(branch string) (string, error) {
25+
args := append([]string{"rev-parse", "--symbolic-full-name", "--abbrev-ref", fmt.Sprintf("%s@{u}", branch)})
26+
cmd, err := git.GitCommand(args...)
27+
if err != nil {
28+
return "", err
29+
}
30+
out, err := cmd.Output()
31+
if err != nil {
32+
return "", err
33+
}
34+
parts := strings.SplitN(string(out), "/", 2)
35+
return parts[0], nil
36+
}
37+
2038
func (g *gitExecuter) Checkout(args []string) error {
21-
return git.CheckoutBranch(args[0])
39+
args = append([]string{"checkout"}, args...)
40+
cmd, err := git.GitCommand(args...)
41+
if err != nil {
42+
return err
43+
}
44+
return cmd.Run()
2245
}
2346

2447
func (g *gitExecuter) CurrentBranch() (string, error) {

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

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,11 @@ type mockGitClient struct {
88
mock.Mock
99
}
1010

11+
func (g *mockGitClient) BranchRemote(a string) (string, error) {
12+
args := g.Called(a)
13+
return args.String(0), args.Error(1)
14+
}
15+
1116
func (g *mockGitClient) Checkout(a []string) error {
1217
args := g.Called(a)
1318
return args.Error(0)

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

Lines changed: 19 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -152,6 +152,9 @@ func syncLocalRepo(opts *SyncOptions) error {
152152
if errors.Is(err, divergingError) {
153153
return fmt.Errorf("can't sync because there are diverging changes, you can use `--force` to overwrite the changes")
154154
}
155+
if errors.Is(err, mismatchRemotesError) {
156+
return fmt.Errorf("can't sync because %s is not tracking %s", opts.Branch, ghrepo.FullName(srcRepo))
157+
}
155158
return err
156159
}
157160

@@ -232,6 +235,7 @@ func syncRemoteRepo(opts *SyncOptions) error {
232235
}
233236

234237
var divergingError = errors.New("diverging changes")
238+
var mismatchRemotesError = errors.New("branch remote does not match specified source")
235239

236240
func executeLocalRepoSync(srcRepo ghrepo.Interface, remote string, opts *SyncOptions) error {
237241
git := opts.Git
@@ -244,6 +248,11 @@ func executeLocalRepoSync(srcRepo ghrepo.Interface, remote string, opts *SyncOpt
244248

245249
hasLocalBranch := git.HasLocalBranch([]string{branch})
246250
if hasLocalBranch {
251+
branchRemote, err := git.BranchRemote(branch)
252+
if branchRemote != remote {
253+
return mismatchRemotesError
254+
}
255+
247256
fastForward, err := git.IsAncestor([]string{branch, fmt.Sprintf("%s/%s", remote, branch)})
248257
if err != nil {
249258
return err
@@ -259,9 +268,16 @@ func executeLocalRepoSync(srcRepo ghrepo.Interface, remote string, opts *SyncOpt
259268
return err
260269
}
261270
if startBranch != branch {
262-
err = git.Checkout([]string{branch})
263-
if err != nil {
264-
return err
271+
if hasLocalBranch {
272+
err = git.Checkout([]string{branch})
273+
if err != nil {
274+
return err
275+
}
276+
} else {
277+
err = git.Checkout([]string{"--track", fmt.Sprintf("%s/%s", remote, branch)})
278+
if err != nil {
279+
return err
280+
}
265281
}
266282
}
267283
if hasLocalBranch {

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

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -125,6 +125,7 @@ func Test_SyncRun(t *testing.T) {
125125
mgc.On("IsDirty").Return(false, nil).Once()
126126
mgc.On("Fetch", []string{"origin", "+refs/heads/trunk"}).Return(nil).Once()
127127
mgc.On("HasLocalBranch", []string{"trunk"}).Return(true).Once()
128+
mgc.On("BranchRemote", "trunk").Return("origin", nil).Once()
128129
mgc.On("IsAncestor", []string{"trunk", "origin/trunk"}).Return(true, nil).Once()
129130
mgc.On("CurrentBranch").Return("trunk", nil).Once()
130131
mgc.On("Merge", []string{"--ff-only", "refs/remotes/origin/trunk"}).Return(nil).Once()
@@ -144,6 +145,7 @@ func Test_SyncRun(t *testing.T) {
144145
mgc.On("IsDirty").Return(false, nil).Once()
145146
mgc.On("Fetch", []string{"origin", "+refs/heads/trunk"}).Return(nil).Once()
146147
mgc.On("HasLocalBranch", []string{"trunk"}).Return(true).Once()
148+
mgc.On("BranchRemote", "trunk").Return("origin", nil).Once()
147149
mgc.On("IsAncestor", []string{"trunk", "origin/trunk"}).Return(true, nil).Once()
148150
mgc.On("CurrentBranch").Return("trunk", nil).Once()
149151
mgc.On("Merge", []string{"--ff-only", "refs/remotes/origin/trunk"}).Return(nil).Once()
@@ -165,6 +167,7 @@ func Test_SyncRun(t *testing.T) {
165167
mgc.On("IsDirty").Return(false, nil).Once()
166168
mgc.On("Fetch", []string{"upstream", "+refs/heads/trunk"}).Return(nil).Once()
167169
mgc.On("HasLocalBranch", []string{"trunk"}).Return(true).Once()
170+
mgc.On("BranchRemote", "trunk").Return("upstream", nil).Once()
168171
mgc.On("IsAncestor", []string{"trunk", "upstream/trunk"}).Return(true, nil).Once()
169172
mgc.On("CurrentBranch").Return("trunk", nil).Once()
170173
mgc.On("Merge", []string{"--ff-only", "refs/remotes/upstream/trunk"}).Return(nil).Once()
@@ -181,6 +184,7 @@ func Test_SyncRun(t *testing.T) {
181184
mgc.On("IsDirty").Return(false, nil).Once()
182185
mgc.On("Fetch", []string{"origin", "+refs/heads/test"}).Return(nil).Once()
183186
mgc.On("HasLocalBranch", []string{"test"}).Return(true).Once()
187+
mgc.On("BranchRemote", "test").Return("origin", nil).Once()
184188
mgc.On("IsAncestor", []string{"test", "origin/test"}).Return(true, nil).Once()
185189
mgc.On("CurrentBranch").Return("test", nil).Once()
186190
mgc.On("Merge", []string{"--ff-only", "refs/remotes/origin/test"}).Return(nil).Once()
@@ -202,6 +206,7 @@ func Test_SyncRun(t *testing.T) {
202206
mgc.On("IsDirty").Return(false, nil).Once()
203207
mgc.On("Fetch", []string{"origin", "+refs/heads/trunk"}).Return(nil).Once()
204208
mgc.On("HasLocalBranch", []string{"trunk"}).Return(true).Once()
209+
mgc.On("BranchRemote", "trunk").Return("origin", nil).Once()
205210
mgc.On("IsAncestor", []string{"trunk", "origin/trunk"}).Return(false, nil).Once()
206211
mgc.On("CurrentBranch").Return("trunk", nil).Once()
207212
mgc.On("Reset", []string{"--hard", "refs/remotes/origin/trunk"}).Return(nil).Once()
@@ -221,11 +226,30 @@ func Test_SyncRun(t *testing.T) {
221226
mgc.On("IsDirty").Return(false, nil).Once()
222227
mgc.On("Fetch", []string{"origin", "+refs/heads/trunk"}).Return(nil).Once()
223228
mgc.On("HasLocalBranch", []string{"trunk"}).Return(true).Once()
229+
mgc.On("BranchRemote", "trunk").Return("origin", nil).Once()
224230
mgc.On("IsAncestor", []string{"trunk", "origin/trunk"}).Return(false, nil).Once()
225231
},
226232
wantErr: true,
227233
errMsg: "can't sync because there are diverging changes, you can use `--force` to overwrite the changes",
228234
},
235+
{
236+
name: "sync local repo with parent and mismatching branch remotes",
237+
tty: true,
238+
opts: &SyncOptions{},
239+
httpStubs: func(reg *httpmock.Registry) {
240+
reg.Register(
241+
httpmock.GraphQL(`query RepositoryInfo\b`),
242+
httpmock.StringResponse(`{"data":{"repository":{"defaultBranchRef":{"name": "trunk"}}}}`))
243+
},
244+
gitStubs: func(mgc *mockGitClient) {
245+
mgc.On("IsDirty").Return(false, nil).Once()
246+
mgc.On("Fetch", []string{"origin", "+refs/heads/trunk"}).Return(nil).Once()
247+
mgc.On("HasLocalBranch", []string{"trunk"}).Return(true).Once()
248+
mgc.On("BranchRemote", "trunk").Return("upstream", nil).Once()
249+
},
250+
wantErr: true,
251+
errMsg: "can't sync because trunk is not tracking OWNER/REPO",
252+
},
229253
{
230254
name: "sync local repo with parent and local changes",
231255
tty: true,
@@ -249,6 +273,7 @@ func Test_SyncRun(t *testing.T) {
249273
mgc.On("IsDirty").Return(false, nil).Once()
250274
mgc.On("Fetch", []string{"origin", "+refs/heads/trunk"}).Return(nil).Once()
251275
mgc.On("HasLocalBranch", []string{"trunk"}).Return(true).Once()
276+
mgc.On("BranchRemote", "trunk").Return("origin", nil).Once()
252277
mgc.On("IsAncestor", []string{"trunk", "origin/trunk"}).Return(true, nil).Once()
253278
mgc.On("CurrentBranch").Return("test", nil).Once()
254279
mgc.On("Checkout", []string{"trunk"}).Return(nil).Once()

0 commit comments

Comments
 (0)