diff --git a/commands/checkout.go b/commands/checkout.go index 8ba51d2d..a4a89858 100644 --- a/commands/checkout.go +++ b/commands/checkout.go @@ -127,7 +127,12 @@ func transformCheckoutArgs(args *Args) error { remote := baseRemote.Name mergeRef := ref if pullRequest.MaintainerCanModify && pullRequest.Head.Repo != nil { - remote = pullRequest.Head.Repo.SshUrl + project, err := github.NewProjectFromRepo(pullRequest.Head.Repo) + if err != nil { + return err + } + + remote = project.GitURL("", "", true) mergeRef = fmt.Sprintf("refs/heads/%s", pullRequest.Head.Ref) } args.Before("git", "config", fmt.Sprintf("branch.%s.remote", newBranchName), remote) diff --git a/features/checkout.feature b/features/checkout.feature index 9271aafd..8411b358 100644 --- a/features/checkout.feature +++ b/features/checkout.feature @@ -30,9 +30,8 @@ Feature: hub checkout """ When I run `hub checkout -f https://github.com/mojombo/jekyll/pull/77 -q` Then "git fetch origin refs/pull/77/head:mislav-fixes" should be run - And "git config branch.mislav-fixes.remote origin" should be run - And "git config branch.mislav-fixes.merge refs/pull/77/head" should be run And "git checkout -f mislav-fixes -q" should be run + And "mislav-fixes" should merge "refs/pull/77/head" from remote "origin" Scenario: No matching remotes for pull request base Given the GitHub API server: @@ -75,9 +74,8 @@ Feature: hub checkout """ When I run `hub checkout https://github.com/mojombo/jekyll/pull/77 fixes-from-mislav` Then "git fetch origin refs/pull/77/head:fixes-from-mislav" should be run - And "git config branch.fixes-from-mislav.remote origin" should be run - And "git config branch.fixes-from-mislav.merge refs/pull/77/head" should be run And "git checkout fixes-from-mislav" should be run + And "fixes-from-mislav" should merge "refs/pull/77/head" from remote "origin" Scenario: Same-repo Given the GitHub API server: @@ -143,9 +141,8 @@ Feature: hub checkout """ When I run `hub checkout https://github.com/mojombo/jekyll/pull/77` Then "git fetch origin refs/pull/77/head:pr-77" should be run - And "git config branch.pr-77.remote origin" should be run - And "git config branch.pr-77.merge refs/pull/77/head" should be run And "git checkout pr-77" should be run + And "pr-77" should merge "refs/pull/77/head" from remote "origin" Scenario: Reuse existing remote for head branch Given the GitHub API server: @@ -209,7 +206,7 @@ Feature: hub checkout :repo => { :owner => { :login => "mislav" }, :name => "jekyll", - :ssh_url => "git@github.com:mislav/jekyll.git", + :html_url => "https://github.com/mislav/jekyll.git", :private => false }, }, :base => { @@ -223,6 +220,33 @@ Feature: hub checkout """ When I run `hub checkout -f https://github.com/mojombo/jekyll/pull/77 -q` Then "git fetch origin refs/pull/77/head:mislav-fixes" should be run - And "git config branch.mislav-fixes.remote git@github.com:mislav/jekyll.git" should be run - And "git config branch.mislav-fixes.merge refs/heads/fixes" should be run And "git checkout -f mislav-fixes -q" should be run + And "mislav-fixes" should merge "refs/heads/fixes" from remote "git@github.com:mislav/jekyll.git" + + Scenario: Modifiable fork with HTTPS + Given the GitHub API server: + """ + get('/repos/mojombo/jekyll/pulls/77') { + halt 406 unless request.env['HTTP_ACCEPT'] == 'application/vnd.github.v3+json;charset=utf-8' + json :head => { + :ref => "fixes", + :repo => { + :owner => { :login => "mislav" }, + :name => "jekyll", + :html_url => "https://github.com/mislav/jekyll.git", + :private => false + }, + }, :base => { + :repo => { + :name => 'jekyll', + :html_url => 'https://github.com/mojombo/jekyll', + :owner => { :login => "mojombo" }, + } + }, :maintainer_can_modify => true + } + """ + And HTTPS is preferred + When I run `hub checkout -f https://github.com/mojombo/jekyll/pull/77 -q` + Then "git fetch origin refs/pull/77/head:mislav-fixes" should be run + And "git checkout -f mislav-fixes -q" should be run + And "mislav-fixes" should merge "refs/heads/fixes" from remote "https://github.com/mislav/jekyll.git" diff --git a/features/steps.rb b/features/steps.rb index ba68d520..1b49d12f 100644 --- a/features/steps.rb +++ b/features/steps.rb @@ -208,6 +208,14 @@ Then(/^the "([^"]*)" submodule url should be "([^"]*)"$/) do |name, url| expect(found).to eql(url) end +Then(/^"([^"]*)" should merge "([^"]*)" from remote "([^"]*)"$/) do |name, merge, remote| + actual_remote = run_silent %(git config --get-all branch.#{name}.remote) + expect(remote).to eql(actual_remote) + + actual_merge = run_silent %(git config --get-all branch.#{name}.merge) + expect(merge).to eql(actual_merge) +end + Then(/^there should be no "([^"]*)" remote$/) do |remote_name| remotes = run_silent('git remote').split("\n") expect(remotes).to_not include(remote_name) diff --git a/features/support/fakebin/git b/features/support/fakebin/git index 8f3b6d5e..47c00248 100755 --- a/features/support/fakebin/git +++ b/features/support/fakebin/git @@ -4,7 +4,7 @@ # asserted that they ran. command="$1" -[ "$command" = "config" -a "${#@}" -le 2 ] || echo git "$@" >> "$HOME"/.history +[ "$command" = "config" ] || echo git "$@" >> "$HOME"/.history case "$command" in "clone" | "fetch" | "pull" | "push" ) diff --git a/github/client.go b/github/client.go index b45d7dd9..04e2bae6 100644 --- a/github/client.go +++ b/github/client.go @@ -411,7 +411,6 @@ type Repository struct { Private bool `json:"private"` Permissions *RepositoryPermissions `json:"permissions"` HtmlUrl string `json:"html_url"` - SshUrl string `json:"ssh_url"` } type RepositoryPermissions struct { diff --git a/github/project.go b/github/project.go index dfb519b9..6787679a 100644 --- a/github/project.go +++ b/github/project.go @@ -103,6 +103,16 @@ func preferredProtocol() string { return userProtocol } +func NewProjectFromRepo(repo *Repository) (p *Project, err error) { + url, err := url.Parse(repo.HtmlUrl) + if err != nil { + return + } + + p, err = NewProjectFromURL(url) + return +} + func NewProjectFromURL(url *url.URL) (p *Project, err error) { if !knownGitHubHostsInclude(url.Host) { err = &GithubHostError{url}