X Tutup
Skip to content

Commit eaa64df

Browse files
mislavmislav
authored
Merge pull request cli#4942 from cli/repo-sync-merge-upstream
repo sync: Use the new merge-upstream API if available
2 parents 8272035 + 1260023 commit eaa64df

File tree

3 files changed

+128
-68
lines changed

3 files changed

+128
-68
lines changed

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

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,9 @@ package sync
33
import (
44
"bytes"
55
"encoding/json"
6+
"errors"
67
"fmt"
8+
"net/http"
79

810
"github.com/cli/cli/v2/api"
911
"github.com/cli/cli/v2/internal/ghrepo"
@@ -27,6 +29,39 @@ func latestCommit(client *api.Client, repo ghrepo.Interface, branch string) (com
2729
return response, err
2830
}
2931

32+
type upstreamMergeErr struct{ error }
33+
34+
var upstreamMergeUnavailableErr = upstreamMergeErr{errors.New("upstream merge API is unavailable")}
35+
36+
func triggerUpstreamMerge(client *api.Client, repo ghrepo.Interface, branch string) (string, error) {
37+
var payload bytes.Buffer
38+
if err := json.NewEncoder(&payload).Encode(map[string]interface{}{
39+
"branch": branch,
40+
}); err != nil {
41+
return "", err
42+
}
43+
44+
var response struct {
45+
Message string `json:"message"`
46+
MergeType string `json:"merge_type"`
47+
BaseBranch string `json:"base_branch"`
48+
}
49+
path := fmt.Sprintf("repos/%s/%s/merge-upstream", repo.RepoOwner(), repo.RepoName())
50+
var httpErr api.HTTPError
51+
if err := client.REST(repo.RepoHost(), "POST", path, &payload, &response); err != nil {
52+
if errors.As(err, &httpErr) {
53+
switch httpErr.StatusCode {
54+
case http.StatusUnprocessableEntity, http.StatusConflict:
55+
return "", upstreamMergeErr{errors.New(httpErr.Message)}
56+
case http.StatusNotFound:
57+
return "", upstreamMergeUnavailableErr
58+
}
59+
}
60+
return "", err
61+
}
62+
return response.BaseBranch, nil
63+
}
64+
3065
func syncFork(client *api.Client, repo ghrepo.Interface, branch, SHA string, force bool) error {
3166
path := fmt.Sprintf("repos/%s/%s/git/refs/heads/%s", repo.RepoOwner(), repo.RepoName(), branch)
3267
body := map[string]interface{}{

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

Lines changed: 45 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import (
55
"fmt"
66
"net/http"
77
"regexp"
8+
"strings"
89

910
"github.com/MakeNowJust/heredoc"
1011
"github.com/cli/cli/v2/api"
@@ -177,38 +178,19 @@ func syncRemoteRepo(opts *SyncOptions) error {
177178
return err
178179
}
179180

180-
if opts.SrcArg == "" {
181-
opts.IO.StartProgressIndicator()
182-
srcRepo, err = api.RepoParent(apiClient, destRepo)
183-
opts.IO.StopProgressIndicator()
184-
if err != nil {
185-
return err
186-
}
187-
if srcRepo == nil {
188-
return fmt.Errorf("can't determine source repository for %s because repository is not fork", ghrepo.FullName(destRepo))
189-
}
190-
} else {
181+
if opts.SrcArg != "" {
191182
srcRepo, err = ghrepo.FromFullName(opts.SrcArg)
192183
if err != nil {
193184
return err
194185
}
195186
}
196187

197-
if destRepo.RepoHost() != srcRepo.RepoHost() {
188+
if srcRepo != nil && destRepo.RepoHost() != srcRepo.RepoHost() {
198189
return fmt.Errorf("can't sync repositories from different hosts")
199190
}
200191

201-
if opts.Branch == "" {
202-
opts.IO.StartProgressIndicator()
203-
opts.Branch, err = api.RepoDefaultBranch(apiClient, srcRepo)
204-
opts.IO.StopProgressIndicator()
205-
if err != nil {
206-
return err
207-
}
208-
}
209-
210192
opts.IO.StartProgressIndicator()
211-
err = executeRemoteRepoSync(apiClient, destRepo, srcRepo, opts)
193+
baseBranchLabel, err := executeRemoteRepoSync(apiClient, destRepo, srcRepo, opts)
212194
opts.IO.StopProgressIndicator()
213195
if err != nil {
214196
if errors.Is(err, divergingError) {
@@ -219,11 +201,15 @@ func syncRemoteRepo(opts *SyncOptions) error {
219201

220202
if opts.IO.IsStdoutTTY() {
221203
cs := opts.IO.ColorScheme()
222-
fmt.Fprintf(opts.IO.Out, "%s Synced the \"%s\" branch from %s to %s\n",
204+
branchName := opts.Branch
205+
if idx := strings.Index(baseBranchLabel, ":"); idx >= 0 {
206+
branchName = baseBranchLabel[idx+1:]
207+
}
208+
fmt.Fprintf(opts.IO.Out, "%s Synced the \"%s:%s\" branch from \"%s\"\n",
223209
cs.SuccessIcon(),
224-
opts.Branch,
225-
ghrepo.FullName(srcRepo),
226-
ghrepo.FullName(destRepo))
210+
destRepo.RepoOwner(),
211+
branchName,
212+
baseBranchLabel)
227213
}
228214

229215
return nil
@@ -294,20 +280,47 @@ func executeLocalRepoSync(srcRepo ghrepo.Interface, remote string, opts *SyncOpt
294280
return nil
295281
}
296282

297-
func executeRemoteRepoSync(client *api.Client, destRepo, srcRepo ghrepo.Interface, opts *SyncOptions) error {
298-
commit, err := latestCommit(client, srcRepo, opts.Branch)
283+
func executeRemoteRepoSync(client *api.Client, destRepo, srcRepo ghrepo.Interface, opts *SyncOptions) (string, error) {
284+
branchName := opts.Branch
285+
if branchName == "" {
286+
var err error
287+
branchName, err = api.RepoDefaultBranch(client, destRepo)
288+
if err != nil {
289+
return "", err
290+
}
291+
}
292+
293+
var apiErr upstreamMergeErr
294+
if baseBranch, err := triggerUpstreamMerge(client, destRepo, branchName); err == nil {
295+
return baseBranch, nil
296+
} else if !errors.As(err, &apiErr) {
297+
return "", err
298+
}
299+
300+
if srcRepo == nil {
301+
var err error
302+
srcRepo, err = api.RepoParent(client, destRepo)
303+
if err != nil {
304+
return "", err
305+
}
306+
if srcRepo == nil {
307+
return "", fmt.Errorf("can't determine source repository for %s because repository is not fork", ghrepo.FullName(destRepo))
308+
}
309+
}
310+
311+
commit, err := latestCommit(client, srcRepo, branchName)
299312
if err != nil {
300-
return err
313+
return "", err
301314
}
302315

303316
// This is not a great way to detect the error returned by the API
304317
// Unfortunately API returns 422 for multiple reasons
305318
notFastForwardErrorMessage := regexp.MustCompile(`^Update is not a fast forward$`)
306-
err = syncFork(client, destRepo, opts.Branch, commit.Object.SHA, opts.Force)
319+
err = syncFork(client, destRepo, branchName, commit.Object.SHA, opts.Force)
307320
var httpErr api.HTTPError
308321
if err != nil && errors.As(err, &httpErr) && notFastForwardErrorMessage.MatchString(httpErr.Message) {
309-
return divergingError
322+
return "", divergingError
310323
}
311324

312-
return err
325+
return fmt.Sprintf("%s:%s", srcRepo.RepoOwner(), branchName), nil
313326
}

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

Lines changed: 48 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -262,10 +262,26 @@ func Test_SyncRun(t *testing.T) {
262262
wantStdout: "✓ Synced the \"trunk\" branch from OWNER/REPO to local repository\n",
263263
},
264264
{
265-
name: "sync remote fork with parent - tty",
265+
name: "sync remote fork with parent with new api - tty",
266266
tty: true,
267267
opts: &SyncOptions{
268-
DestArg: "OWNER/REPO-FORK",
268+
DestArg: "FORKOWNER/REPO-FORK",
269+
},
270+
httpStubs: func(reg *httpmock.Registry) {
271+
reg.Register(
272+
httpmock.GraphQL(`query RepositoryInfo\b`),
273+
httpmock.StringResponse(`{"data":{"repository":{"defaultBranchRef":{"name": "trunk"}}}}`))
274+
reg.Register(
275+
httpmock.REST("POST", "repos/FORKOWNER/REPO-FORK/merge-upstream"),
276+
httpmock.StatusStringResponse(200, `{"base_branch": "OWNER:trunk"}`))
277+
},
278+
wantStdout: "✓ Synced the \"FORKOWNER:trunk\" branch from \"OWNER:trunk\"\n",
279+
},
280+
{
281+
name: "sync remote fork with parent using api fallback - tty",
282+
tty: true,
283+
opts: &SyncOptions{
284+
DestArg: "FORKOWNER/REPO-FORK",
269285
},
270286
httpStubs: func(reg *httpmock.Registry) {
271287
reg.Register(
@@ -274,34 +290,31 @@ func Test_SyncRun(t *testing.T) {
274290
reg.Register(
275291
httpmock.GraphQL(`query RepositoryInfo\b`),
276292
httpmock.StringResponse(`{"data":{"repository":{"defaultBranchRef":{"name": "trunk"}}}}`))
293+
reg.Register(
294+
httpmock.REST("POST", "repos/FORKOWNER/REPO-FORK/merge-upstream"),
295+
httpmock.StatusStringResponse(404, `{}`))
277296
reg.Register(
278297
httpmock.REST("GET", "repos/OWNER/REPO/git/refs/heads/trunk"),
279298
httpmock.StringResponse(`{"object":{"sha":"0xDEADBEEF"}}`))
280299
reg.Register(
281-
httpmock.REST("PATCH", "repos/OWNER/REPO-FORK/git/refs/heads/trunk"),
300+
httpmock.REST("PATCH", "repos/FORKOWNER/REPO-FORK/git/refs/heads/trunk"),
282301
httpmock.StringResponse(`{}`))
283302
},
284-
wantStdout: "✓ Synced the \"trunk\" branch from OWNER/REPO to OWNER/REPO-FORK\n",
303+
wantStdout: "✓ Synced the \"FORKOWNER:trunk\" branch from \"OWNER:trunk\"\n",
285304
},
286305
{
287306
name: "sync remote fork with parent - notty",
288307
tty: false,
289308
opts: &SyncOptions{
290-
DestArg: "OWNER/REPO-FORK",
309+
DestArg: "FORKOWNER/REPO-FORK",
291310
},
292311
httpStubs: func(reg *httpmock.Registry) {
293-
reg.Register(
294-
httpmock.GraphQL(`query RepositoryFindParent\b`),
295-
httpmock.StringResponse(`{"data":{"repository":{"parent":{"name":"REPO","owner":{"login": "OWNER"}}}}}`))
296312
reg.Register(
297313
httpmock.GraphQL(`query RepositoryInfo\b`),
298314
httpmock.StringResponse(`{"data":{"repository":{"defaultBranchRef":{"name": "trunk"}}}}`))
299315
reg.Register(
300-
httpmock.REST("GET", "repos/OWNER/REPO/git/refs/heads/trunk"),
301-
httpmock.StringResponse(`{"object":{"sha":"0xDEADBEEF"}}`))
302-
reg.Register(
303-
httpmock.REST("PATCH", "repos/OWNER/REPO-FORK/git/refs/heads/trunk"),
304-
httpmock.StringResponse(`{}`))
316+
httpmock.REST("POST", "repos/FORKOWNER/REPO-FORK/merge-upstream"),
317+
httpmock.StatusStringResponse(200, `{"base_branch": "OWNER:trunk"}`))
305318
},
306319
wantStdout: "",
307320
},
@@ -310,11 +323,15 @@ func Test_SyncRun(t *testing.T) {
310323
tty: true,
311324
opts: &SyncOptions{
312325
DestArg: "OWNER/REPO",
326+
Branch: "trunk",
313327
},
314328
httpStubs: func(reg *httpmock.Registry) {
329+
reg.Register(
330+
httpmock.REST("POST", "repos/OWNER/REPO/merge-upstream"),
331+
httpmock.StatusStringResponse(422, `{"message": "Validation Failed"}`))
315332
reg.Register(
316333
httpmock.GraphQL(`query RepositoryFindParent\b`),
317-
httpmock.StringResponse(`{"data":{"repository":{}}}`))
334+
httpmock.StringResponse(`{"data":{"repository":{"parent":null}}}`))
318335
},
319336
wantErr: true,
320337
errMsg: "can't determine source repository for OWNER/REPO because repository is not fork",
@@ -325,19 +342,14 @@ func Test_SyncRun(t *testing.T) {
325342
opts: &SyncOptions{
326343
DestArg: "OWNER/REPO",
327344
SrcArg: "OWNER2/REPO2",
345+
Branch: "trunk",
328346
},
329347
httpStubs: func(reg *httpmock.Registry) {
330348
reg.Register(
331-
httpmock.GraphQL(`query RepositoryInfo\b`),
332-
httpmock.StringResponse(`{"data":{"repository":{"defaultBranchRef":{"name": "trunk"}}}}`))
333-
reg.Register(
334-
httpmock.REST("GET", "repos/OWNER2/REPO2/git/refs/heads/trunk"),
335-
httpmock.StringResponse(`{"object":{"sha":"0xDEADBEEF"}}`))
336-
reg.Register(
337-
httpmock.REST("PATCH", "repos/OWNER/REPO/git/refs/heads/trunk"),
338-
httpmock.StringResponse(`{}`))
349+
httpmock.REST("POST", "repos/OWNER/REPO/merge-upstream"),
350+
httpmock.StatusStringResponse(200, `{"base_branch": "OWNER2:trunk"}`))
339351
},
340-
wantStdout: "✓ Synced the \"trunk\" branch from OWNER2/REPO2 to OWNER/REPO\n",
352+
wantStdout: "✓ Synced the \"OWNER:trunk\" branch from \"OWNER2:trunk\"\n",
341353
},
342354
{
343355
name: "sync remote fork with parent and specified branch",
@@ -348,16 +360,10 @@ func Test_SyncRun(t *testing.T) {
348360
},
349361
httpStubs: func(reg *httpmock.Registry) {
350362
reg.Register(
351-
httpmock.GraphQL(`query RepositoryFindParent\b`),
352-
httpmock.StringResponse(`{"data":{"repository":{"parent":{"name":"REPO","owner":{"login": "OWNER"}}}}}`))
353-
reg.Register(
354-
httpmock.REST("GET", "repos/OWNER/REPO/git/refs/heads/test"),
355-
httpmock.StringResponse(`{"object":{"sha":"0xDEADBEEF"}}`))
356-
reg.Register(
357-
httpmock.REST("PATCH", "repos/OWNER/REPO-FORK/git/refs/heads/test"),
358-
httpmock.StringResponse(`{}`))
363+
httpmock.REST("POST", "repos/OWNER/REPO-FORK/merge-upstream"),
364+
httpmock.StatusStringResponse(200, `{"base_branch": "OWNER:test"}`))
359365
},
360-
wantStdout: "✓ Synced the \"test\" branch from OWNER/REPO to OWNER/REPO-FORK\n",
366+
wantStdout: "✓ Synced the \"OWNER:test\" branch from \"OWNER:test\"\n",
361367
},
362368
{
363369
name: "sync remote fork with parent and force specified",
@@ -373,14 +379,17 @@ func Test_SyncRun(t *testing.T) {
373379
reg.Register(
374380
httpmock.GraphQL(`query RepositoryInfo\b`),
375381
httpmock.StringResponse(`{"data":{"repository":{"defaultBranchRef":{"name": "trunk"}}}}`))
382+
reg.Register(
383+
httpmock.REST("POST", "repos/OWNER/REPO-FORK/merge-upstream"),
384+
httpmock.StatusStringResponse(409, `{"message": "Merge conflict"}`))
376385
reg.Register(
377386
httpmock.REST("GET", "repos/OWNER/REPO/git/refs/heads/trunk"),
378387
httpmock.StringResponse(`{"object":{"sha":"0xDEADBEEF"}}`))
379388
reg.Register(
380389
httpmock.REST("PATCH", "repos/OWNER/REPO-FORK/git/refs/heads/trunk"),
381390
httpmock.StringResponse(`{}`))
382391
},
383-
wantStdout: "✓ Synced the \"trunk\" branch from OWNER/REPO to OWNER/REPO-FORK\n",
392+
wantStdout: "✓ Synced the \"OWNER:trunk\" branch from \"OWNER:trunk\"\n",
384393
},
385394
{
386395
name: "sync remote fork with parent and not fast forward merge",
@@ -395,6 +404,9 @@ func Test_SyncRun(t *testing.T) {
395404
reg.Register(
396405
httpmock.GraphQL(`query RepositoryInfo\b`),
397406
httpmock.StringResponse(`{"data":{"repository":{"defaultBranchRef":{"name": "trunk"}}}}`))
407+
reg.Register(
408+
httpmock.REST("POST", "repos/OWNER/REPO-FORK/merge-upstream"),
409+
httpmock.StatusStringResponse(409, `{"message": "Merge conflict"}`))
398410
reg.Register(
399411
httpmock.REST("GET", "repos/OWNER/REPO/git/refs/heads/trunk"),
400412
httpmock.StringResponse(`{"object":{"sha":"0xDEADBEEF"}}`))
@@ -454,11 +466,11 @@ func Test_SyncRun(t *testing.T) {
454466
defer reg.Verify(t)
455467
err := syncRun(tt.opts)
456468
if tt.wantErr {
457-
assert.Error(t, err)
458-
assert.Equal(t, tt.errMsg, err.Error())
469+
assert.EqualError(t, err, tt.errMsg)
459470
return
471+
} else if err != nil {
472+
t.Fatalf("syncRun() unexpected error: %v", err)
460473
}
461-
assert.NoError(t, err)
462474
assert.Equal(t, tt.wantStdout, stdout.String())
463475
})
464476
}

0 commit comments

Comments
 (0)
X Tutup