From 14a8e03bc3c8d7666940a23581d6ca674229f355 Mon Sep 17 00:00:00 2001 From: EBIBO Date: Mon, 20 Nov 2023 05:37:59 -0500 Subject: [PATCH] `gh repo sync` should be able to sync a local branch with an upstream remote (#8229) --- pkg/cmd/repo/sync/git.go | 16 ------------- pkg/cmd/repo/sync/mocks.go | 5 ---- pkg/cmd/repo/sync/sync.go | 13 ----------- pkg/cmd/repo/sync/sync_test.go | 42 ++++++++++++++++++++++------------ 4 files changed, 27 insertions(+), 49 deletions(-) diff --git a/pkg/cmd/repo/sync/git.go b/pkg/cmd/repo/sync/git.go index 48429f2bb..1f18d0fdb 100644 --- a/pkg/cmd/repo/sync/git.go +++ b/pkg/cmd/repo/sync/git.go @@ -3,13 +3,11 @@ package sync import ( "context" "fmt" - "strings" "github.com/cli/cli/v2/git" ) type gitClient interface { - BranchRemote(string) (string, error) CurrentBranch() (string, error) UpdateBranch(string, string) error CreateBranch(string, string, string) error @@ -25,20 +23,6 @@ type gitExecuter struct { client *git.Client } -func (g *gitExecuter) BranchRemote(branch string) (string, error) { - args := []string{"rev-parse", "--symbolic-full-name", "--abbrev-ref", fmt.Sprintf("%s@{u}", branch)} - cmd, err := g.client.Command(context.Background(), args...) - if err != nil { - return "", err - } - out, err := cmd.Output() - if err != nil { - return "", err - } - parts := strings.SplitN(string(out), "/", 2) - return parts[0], nil -} - 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 { diff --git a/pkg/cmd/repo/sync/mocks.go b/pkg/cmd/repo/sync/mocks.go index 718a89054..c852e03f2 100644 --- a/pkg/cmd/repo/sync/mocks.go +++ b/pkg/cmd/repo/sync/mocks.go @@ -8,11 +8,6 @@ type mockGitClient struct { mock.Mock } -func (g *mockGitClient) BranchRemote(a string) (string, error) { - args := g.Called(a) - return args.String(0), args.Error(1) -} - func (g *mockGitClient) UpdateBranch(b, r string) error { args := g.Called(b, r) return args.Error(0) diff --git a/pkg/cmd/repo/sync/sync.go b/pkg/cmd/repo/sync/sync.go index dd52868d1..3c0201e43 100644 --- a/pkg/cmd/repo/sync/sync.go +++ b/pkg/cmd/repo/sync/sync.go @@ -151,9 +151,6 @@ func syncLocalRepo(opts *SyncOptions) error { if errors.Is(err, divergingError) { return fmt.Errorf("can't sync because there are diverging changes; use `--force` to overwrite the destination branch") } - if errors.Is(err, mismatchRemotesError) { - return fmt.Errorf("can't sync because %s is not tracking %s", opts.Branch, ghrepo.FullName(srcRepo)) - } return err } @@ -220,7 +217,6 @@ func syncRemoteRepo(opts *SyncOptions) error { } var divergingError = errors.New("diverging changes") -var mismatchRemotesError = errors.New("branch remote does not match specified source") func executeLocalRepoSync(srcRepo ghrepo.Interface, remote string, opts *SyncOptions) error { git := opts.Git @@ -229,19 +225,10 @@ func executeLocalRepoSync(srcRepo ghrepo.Interface, remote string, opts *SyncOpt hasLocalBranch := git.HasLocalBranch(branch) if hasLocalBranch { - branchRemote, err := git.BranchRemote(branch) - if err != nil { - return err - } - if branchRemote != remote { - return mismatchRemotesError - } - fastForward, err := git.IsAncestor(branch, "FETCH_HEAD") if err != nil { return err } - if !fastForward && !useForce { return divergingError } diff --git a/pkg/cmd/repo/sync/sync_test.go b/pkg/cmd/repo/sync/sync_test.go index eafc0835e..8b9e4352f 100644 --- a/pkg/cmd/repo/sync/sync_test.go +++ b/pkg/cmd/repo/sync/sync_test.go @@ -122,12 +122,11 @@ func Test_SyncRun(t *testing.T) { httpmock.StringResponse(`{"data":{"repository":{"defaultBranchRef":{"name": "trunk"}}}}`)) }, gitStubs: func(mgc *mockGitClient) { - mgc.On("IsDirty").Return(false, nil).Once() mgc.On("Fetch", "origin", "refs/heads/trunk").Return(nil).Once() mgc.On("HasLocalBranch", "trunk").Return(true).Once() - mgc.On("BranchRemote", "trunk").Return("origin", nil).Once() mgc.On("IsAncestor", "trunk", "FETCH_HEAD").Return(true, nil).Once() mgc.On("CurrentBranch").Return("trunk", nil).Once() + mgc.On("IsDirty").Return(false, nil).Once() mgc.On("MergeFastForward", "FETCH_HEAD").Return(nil).Once() }, wantStdout: "✓ Synced the \"trunk\" branch from OWNER/REPO to local repository\n", @@ -139,12 +138,11 @@ func Test_SyncRun(t *testing.T) { Branch: "trunk", }, gitStubs: func(mgc *mockGitClient) { - mgc.On("IsDirty").Return(false, nil).Once() mgc.On("Fetch", "origin", "refs/heads/trunk").Return(nil).Once() mgc.On("HasLocalBranch", "trunk").Return(true).Once() - mgc.On("BranchRemote", "trunk").Return("origin", nil).Once() mgc.On("IsAncestor", "trunk", "FETCH_HEAD").Return(true, nil).Once() mgc.On("CurrentBranch").Return("trunk", nil).Once() + mgc.On("IsDirty").Return(false, nil).Once() mgc.On("MergeFastForward", "FETCH_HEAD").Return(nil).Once() }, wantStdout: "", @@ -157,12 +155,11 @@ func Test_SyncRun(t *testing.T) { SrcArg: "OWNER2/REPO2", }, gitStubs: func(mgc *mockGitClient) { - mgc.On("IsDirty").Return(false, nil).Once() mgc.On("Fetch", "upstream", "refs/heads/trunk").Return(nil).Once() mgc.On("HasLocalBranch", "trunk").Return(true).Once() - mgc.On("BranchRemote", "trunk").Return("upstream", nil).Once() mgc.On("IsAncestor", "trunk", "FETCH_HEAD").Return(true, nil).Once() mgc.On("CurrentBranch").Return("trunk", nil).Once() + mgc.On("IsDirty").Return(false, nil).Once() mgc.On("MergeFastForward", "FETCH_HEAD").Return(nil).Once() }, wantStdout: "✓ Synced the \"trunk\" branch from OWNER2/REPO2 to local repository\n", @@ -175,16 +172,33 @@ func Test_SyncRun(t *testing.T) { Force: true, }, gitStubs: func(mgc *mockGitClient) { - mgc.On("IsDirty").Return(false, nil).Once() mgc.On("Fetch", "origin", "refs/heads/trunk").Return(nil).Once() mgc.On("HasLocalBranch", "trunk").Return(true).Once() - mgc.On("BranchRemote", "trunk").Return("origin", nil).Once() mgc.On("IsAncestor", "trunk", "FETCH_HEAD").Return(false, nil).Once() mgc.On("CurrentBranch").Return("trunk", nil).Once() + mgc.On("IsDirty").Return(false, nil).Once() mgc.On("ResetHard", "FETCH_HEAD").Return(nil).Once() }, wantStdout: "✓ Synced the \"trunk\" branch from OWNER/REPO to local repository\n", }, + { + name: "sync local repo with specified source repo and force specified", + tty: true, + opts: &SyncOptions{ + Branch: "trunk", + SrcArg: "OWNER2/REPO2", + Force: true, + }, + gitStubs: func(mgc *mockGitClient) { + mgc.On("Fetch", "upstream", "refs/heads/trunk").Return(nil).Once() + mgc.On("HasLocalBranch", "trunk").Return(true).Once() + mgc.On("IsAncestor", "trunk", "FETCH_HEAD").Return(false, nil).Once() + mgc.On("CurrentBranch").Return("trunk", nil).Once() + mgc.On("IsDirty").Return(false, nil).Once() + mgc.On("ResetHard", "FETCH_HEAD").Return(nil).Once() + }, + wantStdout: "✓ Synced the \"trunk\" branch from OWNER2/REPO2 to local repository\n", + }, { name: "sync local repo with parent and not fast forward merge", tty: true, @@ -194,25 +208,25 @@ func Test_SyncRun(t *testing.T) { gitStubs: func(mgc *mockGitClient) { mgc.On("Fetch", "origin", "refs/heads/trunk").Return(nil).Once() mgc.On("HasLocalBranch", "trunk").Return(true).Once() - mgc.On("BranchRemote", "trunk").Return("origin", nil).Once() mgc.On("IsAncestor", "trunk", "FETCH_HEAD").Return(false, nil).Once() }, wantErr: true, errMsg: "can't sync because there are diverging changes; use `--force` to overwrite the destination branch", }, { - name: "sync local repo with parent and mismatching branch remotes", + name: "sync local repo with specified source repo and not fast forward merge", tty: true, opts: &SyncOptions{ Branch: "trunk", + SrcArg: "OWNER2/REPO2", }, gitStubs: func(mgc *mockGitClient) { - mgc.On("Fetch", "origin", "refs/heads/trunk").Return(nil).Once() + mgc.On("Fetch", "upstream", "refs/heads/trunk").Return(nil).Once() mgc.On("HasLocalBranch", "trunk").Return(true).Once() - mgc.On("BranchRemote", "trunk").Return("upstream", nil).Once() + mgc.On("IsAncestor", "trunk", "FETCH_HEAD").Return(false, nil).Once() }, wantErr: true, - errMsg: "can't sync because trunk is not tracking OWNER/REPO", + errMsg: "can't sync because there are diverging changes; use `--force` to overwrite the destination branch", }, { name: "sync local repo with parent and local changes", @@ -223,7 +237,6 @@ func Test_SyncRun(t *testing.T) { gitStubs: func(mgc *mockGitClient) { mgc.On("Fetch", "origin", "refs/heads/trunk").Return(nil).Once() mgc.On("HasLocalBranch", "trunk").Return(true).Once() - mgc.On("BranchRemote", "trunk").Return("origin", nil).Once() mgc.On("IsAncestor", "trunk", "FETCH_HEAD").Return(true, nil).Once() mgc.On("CurrentBranch").Return("trunk", nil).Once() mgc.On("IsDirty").Return(true, nil).Once() @@ -240,7 +253,6 @@ func Test_SyncRun(t *testing.T) { gitStubs: func(mgc *mockGitClient) { mgc.On("Fetch", "origin", "refs/heads/trunk").Return(nil).Once() mgc.On("HasLocalBranch", "trunk").Return(true).Once() - mgc.On("BranchRemote", "trunk").Return("origin", nil).Once() mgc.On("IsAncestor", "trunk", "FETCH_HEAD").Return(true, nil).Once() mgc.On("CurrentBranch").Return("test", nil).Once() mgc.On("UpdateBranch", "trunk", "FETCH_HEAD").Return(nil).Once()