From a1f79d8876f889ed65e951aadaf84117fc9fbaa9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Sat, 20 Aug 2016 22:49:56 +0200 Subject: [PATCH] Improve finding git remote to fetch from in new `checkout` The remote needs to match the base project from the pull request. --- commands/checkout.go | 29 +++++------- features/checkout.feature | 94 +++++++++++++++------------------------ github/localrepo.go | 22 +++++++++ 3 files changed, 67 insertions(+), 78 deletions(-) diff --git a/commands/checkout.go b/commands/checkout.go index 959d14a3..4cb4710a 100644 --- a/commands/checkout.go +++ b/commands/checkout.go @@ -78,31 +78,22 @@ func transformCheckoutArgs(args *Args) error { args.RemoveParam(idx) } - branch := pullRequest.Head.Ref - headRepo := pullRequest.Head.Repo - if headRepo == nil { - return fmt.Errorf("Error: that fork is not available anymore") + repo, err := github.LocalRepo() + if err != nil { + return err + } + + remote, err := repo.RemoteForRepo(pullRequest.Base.Repo) + if err != nil { + return err } - user := headRepo.Owner.Login if newBranchName == "" { - newBranchName = fmt.Sprintf("%s-%s", user, branch) - } - - repo, err := github.LocalRepo() - utils.Check(err) - - var remoteRepo string - - _, err = repo.RemoteByName(user) - if err == nil { - remoteRepo = user - } else { - remoteRepo = url.GitURL("", "", false) + newBranchName = fmt.Sprintf("%s-%s", pullRequest.Head.Repo.Owner.Login, pullRequest.Head.Ref) } branchMapping := fmt.Sprintf("pull/%s/head:%s", id, newBranchName) - args.Before("git", "fetch", remoteRepo, branchMapping) + args.Before("git", "fetch", remote.Name, branchMapping) replaceCheckoutParam(args, checkoutURL, newBranchName) return nil diff --git a/features/checkout.feature b/features/checkout.feature index dacf5cad..ba847667 100644 --- a/features/checkout.feature +++ b/features/checkout.feature @@ -19,30 +19,38 @@ Feature: hub checkout :name => "jekyll", :private => false } - } - } - """ - When I run `hub checkout -f https://github.com/mojombo/jekyll/pull/77 -q` - Then "git fetch git://github.com/mojombo/jekyll.git pull/77/head:mislav-fixes" should be run - And "git checkout -f mislav-fixes -q" should be run - - Scenario: Pull request from a renamed fork - Given the GitHub API server: - """ - get('/repos/mojombo/jekyll/pulls/77') { - json :head => { - :ref => "fixes", + }, :base => { :repo => { - :owner => { :login => "mislav" }, - :name => "jekyll-blog", - :private => false + :name => 'jekyll', + :html_url => 'https://github.com/mojombo/jekyll', + :owner => { :login => "mojombo" }, } } } """ - When I run `hub checkout https://github.com/mojombo/jekyll/pull/77` - Then "git fetch git://github.com/mojombo/jekyll.git pull/77/head:mislav-fixes" should be run - And "git checkout mislav-fixes" should be run + When I run `hub checkout -f https://github.com/mojombo/jekyll/pull/77 -q` + Then "git fetch origin pull/77/head:mislav-fixes" should be run + And "git checkout -f mislav-fixes -q" should be run + + Scenario: No matching remotes for pull request base + Given the GitHub API server: + """ + get('/repos/mislav/jekyll/pulls/77') { + json :base => { + :repo => { + :name => 'jekyll', + :html_url => 'https://github.com/mislav/jekyll', + :owner => { :login => "mislav" }, + } + } + } + """ + When I run `hub checkout -f https://github.com/mislav/jekyll/pull/77 -q` + Then the exit status should be 1 + And the stderr should contain exactly: + """ + could not find git remote for mislav/jekyll\n + """ Scenario: Custom name for new branch Given the GitHub API server: @@ -51,50 +59,18 @@ Feature: hub checkout json :head => { :ref => "fixes", :repo => { - :owner => { :login => "mislav" }, :name => "jekyll", - :private => false + :owner => { :login => "mislav" }, + } + }, :base => { + :repo => { + :name => 'jekyll', + :html_url => 'https://github.com/mojombo/jekyll', + :owner => { :login => "mojombo" }, } } } """ When I run `hub checkout https://github.com/mojombo/jekyll/pull/77 fixes-from-mislav` - Then "git fetch git://github.com/mojombo/jekyll.git pull/77/head:fixes-from-mislav" should be run + Then "git fetch origin pull/77/head:fixes-from-mislav" should be run And "git checkout fixes-from-mislav" should be run - - Scenario: Private pull request - Given the GitHub API server: - """ - get('/repos/mojombo/jekyll/pulls/77') { - json :head => { - :ref => "fixes", - :repo => { - :owner => { :login => "mislav" }, - :name => "jekyll", - :private => true - } - } - } - """ - When I run `hub checkout -f https://github.com/mojombo/jekyll/pull/77 -q` - Then "git fetch git://github.com/mojombo/jekyll.git pull/77/head:mislav-fixes" should be run - And "git checkout -f mislav-fixes -q" should be run - - Scenario: Remote for user already exists - Given the GitHub API server: - """ - get('/repos/mojombo/jekyll/pulls/77') { - json :head => { - :ref => "fixes", - :repo => { - :owner => { :login => "mislav" }, - :name => "jekyll", - :private => false - } - } - } - """ - And the "mislav" remote has url "git://github.com/mislav/jekyll.git" - When I run `hub checkout https://github.com/mojombo/jekyll/pull/77` - Then "git fetch mislav pull/77/head:mislav-fixes" should be run - And "git checkout mislav-fixes" should be run diff --git a/github/localrepo.go b/github/localrepo.go index f2291fa7..749ce6ad 100644 --- a/github/localrepo.go +++ b/github/localrepo.go @@ -2,6 +2,7 @@ package github import ( "fmt" + "net/url" "strings" "github.com/github/hub/git" @@ -148,6 +149,27 @@ func (r *GitHubRepo) RemoteBranchAndProject(owner string, preferUpstream bool) ( return } +func (r *GitHubRepo) RemoteForRepo(repo *Repository) (*Remote, error) { + r.loadRemotes() + + repoUrl, err := url.Parse(repo.HtmlUrl) + if err != nil { + return nil, err + } + + project := NewProject(repo.Owner.Login, repo.Name, repoUrl.Host) + + for _, remote := range r.remotes { + if rp, err := remote.Project(); err == nil { + if rp.SameAs(project) { + return &remote, nil + } + } + } + + return nil, fmt.Errorf("could not find git remote for %s/%s", repo.Owner.Login, repo.Name) +} + func (r *GitHubRepo) OriginRemote() (remote *Remote, err error) { return r.RemoteByName("origin") }