From 6fd00d4ab02e99a0319ec0a7d048ccbffe4fcf58 Mon Sep 17 00:00:00 2001 From: Jingwen Owen Ou Date: Tue, 1 Apr 2014 08:55:15 -0700 Subject: [PATCH 01/15] Move default case to the first line of FormatError --- github/client.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/github/client.go b/github/client.go index 6aa8662ba..e48f4dce6 100644 --- a/github/client.go +++ b/github/client.go @@ -474,6 +474,8 @@ func (client *Client) apiHost() string { func FormatError(action string, err error) (ee error) { switch e := err.(type) { + default: + ee = err case *octokit.ResponseError: statusCode := e.Response.StatusCode var reason string @@ -504,8 +506,6 @@ func FormatError(action string, err error) (ee error) { case *AuthError: errStr := fmt.Sprintf("Error %s: Unauthorized (HTTP 401)", action) ee = fmt.Errorf(errStr) - default: - ee = err } return From 0dc6b9f6ba3b807e2e03af2271af3c840fcbc51a Mon Sep 17 00:00:00 2001 From: Jingwen Owen Ou Date: Tue, 1 Apr 2014 09:07:20 -0700 Subject: [PATCH 02/15] Fix asyn issue when forking repo --- commands/fork.go | 7 ++++--- features/fork.feature | 4 ++-- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/commands/fork.go b/commands/fork.go index 9ce07d3de..d158c35d7 100644 --- a/commands/fork.go +++ b/commands/fork.go @@ -47,7 +47,6 @@ func fork(cmd *Command, args *Args) { } forkProject := github.NewProject(host.User, project.Name, project.Host) - client := github.NewClient(project.Host) existingRepo, err := client.Repository(forkProject) if err == nil { @@ -70,8 +69,10 @@ func fork(cmd *Command, args *Args) { if flagForkNoRemote { os.Exit(0) } else { - u := forkProject.GitURL("", "", true) - args.Replace("git", "remote", "add", "-f", forkProject.Owner, u) + originURL := project.GitURL("", "", false) + url := forkProject.GitURL("", "", true) + args.Replace("git", "remote", "add", "-f", forkProject.Owner, originURL) + args.After("git", "remote", "set-url", forkProject.Owner, url) args.After("echo", fmt.Sprintf("new remote: %s", forkProject.Owner)) } } diff --git a/features/fork.feature b/features/fork.feature index 6e6b85700..bb707a767 100644 --- a/features/fork.feature +++ b/features/fork.feature @@ -8,8 +8,8 @@ Feature: hub fork Given the GitHub API server: """ before { halt 401 unless request.env['HTTP_AUTHORIZATION'] == 'token OTOKEN' } - get('/repos/mislav/dotfiles', :host_name => 'api.github.com') { 404 } - post('/repos/evilchelu/dotfiles/forks', :host_name => 'api.github.com') { '' } + get('/repos/mislav/dotfiles') { halt 404 } + post('/repos/evilchelu/dotfiles/forks') { '' } """ When I successfully run `hub fork` Then the output should contain exactly "new remote: mislav\n" From 78389b525082a393b006bccc688f47a54be06fb4 Mon Sep 17 00:00:00 2001 From: Jingwen Owen Ou Date: Tue, 1 Apr 2014 09:54:37 -0700 Subject: [PATCH 03/15] Fix cuke for forking private repo --- commands/fork.go | 3 ++- features/fork.feature | 4 ++-- github/localrepo.go | 7 ++++++- 3 files changed, 10 insertions(+), 4 deletions(-) diff --git a/commands/fork.go b/commands/fork.go index d158c35d7..92e062c1f 100644 --- a/commands/fork.go +++ b/commands/fork.go @@ -69,7 +69,8 @@ func fork(cmd *Command, args *Args) { if flagForkNoRemote { os.Exit(0) } else { - originURL := project.GitURL("", "", false) + originRemote, _ := localRepo.OriginRemote() + originURL := originRemote.URL.String() url := forkProject.GitURL("", "", true) args.Replace("git", "remote", "add", "-f", forkProject.Owner, originURL) args.After("git", "remote", "set-url", forkProject.Owner, url) diff --git a/features/fork.feature b/features/fork.feature index bb707a767..fbf9ebb77 100644 --- a/features/fork.feature +++ b/features/fork.feature @@ -22,8 +22,8 @@ Feature: hub fork Given the GitHub API server: """ before { halt 401 unless request.env['HTTP_AUTHORIZATION'] == 'token OTOKEN' } - get('/repos/mislav/dotfiles', :host_name => 'api.github.com') { 404 } - post('/repos/evilchelu/dotfiles/forks', :host_name => 'api.github.com') { '' } + get('/repos/mislav/dotfiles') { 404 } + post('/repos/evilchelu/dotfiles/forks') { '' } """ When I successfully run `hub fork` Then the output should contain exactly "new remote: mislav\n" diff --git a/github/localrepo.go b/github/localrepo.go index f24c2fcf9..620d8c492 100644 --- a/github/localrepo.go +++ b/github/localrepo.go @@ -123,10 +123,15 @@ func (r *GitHubRepo) RemoteBranchAndProject(owner string) (branch *Branch, proje return } +func (r *GitHubRepo) OriginRemote() (*Remote, error) { + return r.RemoteByName("origin") +} + func (r *GitHubRepo) MainProject() (project *Project, err error) { - origin, err := r.RemoteByName("origin") + origin, err := r.OriginRemote() if err != nil { err = fmt.Errorf("Aborted: the origin remote doesn't point to a GitHub repository.") + return } From 50fce1d4e8f2d9a6c2d4703f91b88d0a9efd1e64 Mon Sep 17 00:00:00 2001 From: Jingwen Owen Ou Date: Tue, 1 Apr 2014 10:21:37 -0700 Subject: [PATCH 04/15] Rename GH_API_HOST to HUB_TEST_HOST --- features/steps.rb | 3 +-- features/support/env.rb | 1 - github/apihost.go | 2 +- 3 files changed, 2 insertions(+), 4 deletions(-) diff --git a/features/steps.rb b/features/steps.rb index 3419cfb65..86b7e7f0d 100644 --- a/features/steps.rb +++ b/features/steps.rb @@ -106,8 +106,7 @@ eval endpoints_str, binding end # hit our Sinatra server instead of github.com - set_env 'HUB_TEST_HOST', "127.0.0.1:#{@server.port}" - set_env 'GH_API_HOST', "http://127.0.0.1:#{@server.port}" + set_env 'HUB_TEST_HOST', "http://127.0.0.1:#{@server.port}" end Given(/^I use a debugging proxy(?: at "(.+?)")?$/) do |address| diff --git a/features/support/env.rb b/features/support/env.rb index b5fa09f95..7238946c3 100644 --- a/features/support/env.rb +++ b/features/support/env.rb @@ -30,7 +30,6 @@ set_env 'HUB_SYSTEM_GIT', system_git # ensure that api.github.com is actually never hit in tests set_env 'HUB_TEST_HOST', '127.0.0.1:0' - set_env 'GH_API_HOST', 'http://127.0.0.1:0' # ensure we use fakebin `open` to test browsing set_env 'BROWSER', 'open' # sabotage opening a commit message editor interactively diff --git a/github/apihost.go b/github/apihost.go index 35371d603..98908f7ad 100644 --- a/github/apihost.go +++ b/github/apihost.go @@ -10,7 +10,7 @@ type apiHost struct { } func (ah *apiHost) String() string { - host := os.Getenv("GH_API_HOST") + host := os.Getenv("HUB_TEST_HOST") if host == "" && ah.Host != "" { host = ah.Host } From 829e99cd557662ac7aca2383c0b3ab9fa00e6f9e Mon Sep 17 00:00:00 2001 From: Jingwen Owen Ou Date: Tue, 1 Apr 2014 12:52:11 -0700 Subject: [PATCH 05/15] Add charset to cuke request header expectation --- features/fork.feature | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/features/fork.feature b/features/fork.feature index fbf9ebb77..367390637 100644 --- a/features/fork.feature +++ b/features/fork.feature @@ -57,7 +57,7 @@ Feature: hub fork Given the GitHub API server: """ get('/repos/mislav/dotfiles') { - halt 406 unless request.env['HTTP_ACCEPT'] == 'application/vnd.github.v3+json' + halt 406 unless request.env['HTTP_ACCEPT'] == 'application/vnd.github.v3+json;charset=utf-8' json :parent => { :html_url => 'https://github.com/unrelated/dotfiles' } } """ From 0b207f8406ff7923681bb7dcb0f380e82e217556 Mon Sep 17 00:00:00 2001 From: Jingwen Owen Ou Date: Tue, 1 Apr 2014 12:52:45 -0700 Subject: [PATCH 06/15] Standardize error message according to cukes --- github/client.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/github/client.go b/github/client.go index e48f4dce6..65432c0fd 100644 --- a/github/client.go +++ b/github/client.go @@ -273,13 +273,13 @@ func (client *Client) ForkRepository(project *Project) (repo *octokit.Repository api, err := client.api() if err != nil { - err = FormatError("forking repository", err) + err = FormatError("creating fork", err) return } repo, result := api.Repositories(client.requestURL(url)).Create(nil) if result.HasError() { - err = FormatError("forking repository", result.Err) + err = FormatError("creating fork", result.Err) return } From 141cfb237dc41b059529a0c1a3ba78b0b6ed2930 Mon Sep 17 00:00:00 2001 From: Jingwen Owen Ou Date: Tue, 1 Apr 2014 12:59:56 -0700 Subject: [PATCH 07/15] Fix error message when checking main project in fork --- commands/fork.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/commands/fork.go b/commands/fork.go index 92e062c1f..1ff8bb60b 100644 --- a/commands/fork.go +++ b/commands/fork.go @@ -38,7 +38,9 @@ func fork(cmd *Command, args *Args) { localRepo := github.LocalRepo() project, err := localRepo.MainProject() - utils.Check(err) + if err != nil { + utils.Check(fmt.Errorf("Error: repository under 'origin' remote is not a GitHub project")) + } configs := github.CurrentConfigs() host, err := configs.PromptForHost(project.Host) From 3d7f4c0136cd4ee4218673acf4c3c137ca7ab64a Mon Sep 17 00:00:00 2001 From: Jingwen Owen Ou Date: Tue, 1 Apr 2014 13:00:54 -0700 Subject: [PATCH 08/15] Remove host_name since Go doesn't allow to set HOST in http request --- features/fork.feature | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/features/fork.feature b/features/fork.feature index 367390637..3361f6880 100644 --- a/features/fork.feature +++ b/features/fork.feature @@ -122,7 +122,7 @@ Scenario: Related fork already exists Given the GitHub API server: """ before { halt 401 unless request.env['HTTP_AUTHORIZATION'] == 'token FITOKEN' } - post('/api/v3/repos/evilchelu/dotfiles/forks', :host_name => 'git.my.org') { '' } + post('/api/v3/repos/evilchelu/dotfiles/forks') { '' } """ And the "origin" remote has url "git@git.my.org:evilchelu/dotfiles.git" And I am "mislav" on git.my.org with OAuth token "FITOKEN" From a7badc8c846088c73e0334d06bf0472ecbda3cf8 Mon Sep 17 00:00:00 2001 From: Jingwen Owen Ou Date: Tue, 1 Apr 2014 13:40:02 -0700 Subject: [PATCH 09/15] Validate git dir when getting local repo --- commands/browse.go | 2 +- commands/checkout.go | 4 +++- commands/cherry_pick.go | 8 ++++++-- commands/ci_status.go | 7 +++++-- commands/compare.go | 5 +++-- commands/fetch.go | 9 ++++++--- commands/fork.go | 4 ++-- commands/pull_request.go | 3 ++- commands/push.go | 8 ++++++-- commands/remote.go | 4 +++- commands/utils.go | 11 +++++++---- github/branch_test.go | 22 +++++++++++++++------- github/localrepo.go | 12 ++++++++++-- 13 files changed, 69 insertions(+), 30 deletions(-) diff --git a/commands/browse.go b/commands/browse.go index f43c1f001..0965620de 100644 --- a/commands/browse.go +++ b/commands/browse.go @@ -67,7 +67,7 @@ func browse(command *Command, args *Args) { subpage = args.RemoveParam(0) } - localRepo := github.LocalRepo() + localRepo, _ := github.LocalRepo() if dest != "" { project = github.NewProject("", dest, "") branch = localRepo.MasterBranch() diff --git a/commands/checkout.go b/commands/checkout.go index c9bbcd4bb..0feba5183 100644 --- a/commands/checkout.go +++ b/commands/checkout.go @@ -84,7 +84,9 @@ func transformCheckoutArgs(args *Args) error { newBranchName = fmt.Sprintf("%s-%s", user, branch) } - repo := github.LocalRepo() + repo, err := github.LocalRepo() + utils.Check(err) + _, err = repo.RemoteByName(user) if err == nil { args.Before("git", "remote", "set-branches", "--add", user, branch) diff --git a/commands/cherry_pick.go b/commands/cherry_pick.go index a4ed1cbad..d718904f2 100644 --- a/commands/cherry_pick.go +++ b/commands/cherry_pick.go @@ -1,9 +1,10 @@ package commands import ( + "regexp" + "github.com/github/hub/github" "github.com/github/hub/utils" - "regexp" ) var cmdCherryPick = &Command{ @@ -72,7 +73,10 @@ func parseCherryPickProjectAndSha(ref string) (project *github.Project, sha stri if ownerWithShaRegexp.MatchString(ref) { matches := ownerWithShaRegexp.FindStringSubmatch(ref) sha = matches[2] - project, err := github.LocalRepo().CurrentProject() + localRepo, err := github.LocalRepo() + utils.Check(err) + + project, err := localRepo.CurrentProject() utils.Check(err) project.Owner = matches[1] } diff --git a/commands/ci_status.go b/commands/ci_status.go index 375b26e41..932d5f705 100644 --- a/commands/ci_status.go +++ b/commands/ci_status.go @@ -2,10 +2,11 @@ package commands import ( "fmt" + "os" + "github.com/github/hub/git" "github.com/github/hub/github" "github.com/github/hub/utils" - "os" ) var cmdCiStatus = &Command{ @@ -51,7 +52,9 @@ func ciStatus(cmd *Command, args *Args) { ref = args.RemoveParam(0) } - localRepo := github.LocalRepo() + localRepo, err := github.LocalRepo() + utils.Check(err) + project, err := localRepo.MainProject() utils.Check(err) diff --git a/commands/compare.go b/commands/compare.go index 17a68a5bd..9644b885d 100644 --- a/commands/compare.go +++ b/commands/compare.go @@ -44,12 +44,13 @@ func init() { > open https://github.com/other-user/REPO/compare/patch */ func compare(command *Command, args *Args) { - localRepo := github.LocalRepo() + localRepo, err := github.LocalRepo() + utils.Check(err) + var ( branch *github.Branch project *github.Project r string - err error ) branch, project, err = localRepo.RemoteBranchAndProject("") diff --git a/commands/fetch.go b/commands/fetch.go index 47a6d3548..d4617b164 100644 --- a/commands/fetch.go +++ b/commands/fetch.go @@ -1,10 +1,11 @@ package commands import ( - "github.com/github/hub/github" - "github.com/github/hub/utils" "regexp" "strings" + + "github.com/github/hub/github" + "github.com/github/hub/utils" ) var cmdFetch = &Command{ @@ -45,7 +46,9 @@ func fetch(command *Command, args *Args) { func tranformFetchArgs(args *Args) error { names := parseRemoteNames(args) - localRepo := github.LocalRepo() + + localRepo, err := github.LocalRepo() + utils.Check(err) projects := make(map[*github.Project]bool) ownerRegexp := regexp.MustCompile(OwnerRe) diff --git a/commands/fork.go b/commands/fork.go index 1ff8bb60b..f6915020f 100644 --- a/commands/fork.go +++ b/commands/fork.go @@ -4,7 +4,6 @@ import ( "fmt" "os" "reflect" - "github.com/github/hub/github" "github.com/github/hub/utils" ) @@ -35,7 +34,8 @@ func init() { [ repo forked on GitHub ] */ func fork(cmd *Command, args *Args) { - localRepo := github.LocalRepo() + localRepo, err := github.LocalRepo() + utils.Check(err) project, err := localRepo.MainProject() if err != nil { diff --git a/commands/pull_request.go b/commands/pull_request.go index 796fcaa2f..c55b31afb 100644 --- a/commands/pull_request.go +++ b/commands/pull_request.go @@ -76,7 +76,8 @@ func init() { [ create pull request with title & body from FILE ] */ func pullRequest(cmd *Command, args *Args) { - localRepo := github.LocalRepo() + localRepo, err := github.LocalRepo() + utils.Check(err) currentBranch, err := localRepo.CurrentBranch() utils.Check(err) diff --git a/commands/push.go b/commands/push.go index df0710f59..a35e0c814 100644 --- a/commands/push.go +++ b/commands/push.go @@ -1,9 +1,10 @@ package commands import ( + "strings" + "github.com/github/hub/github" "github.com/github/hub/utils" - "strings" ) var cmdPush = &Command{ @@ -44,9 +45,12 @@ func transformPushArgs(args *Args) { args.ReplaceParam(0, remotes[0]) if len(refs) == 0 { - localRepo := github.LocalRepo() + localRepo, err := github.LocalRepo() + utils.Check(err) + head, err := localRepo.CurrentBranch() utils.Check(err) + refs = []string{head.ShortName()} args.AppendParams(refs...) } diff --git a/commands/remote.go b/commands/remote.go index 9a65a46d3..b802a59b0 100644 --- a/commands/remote.go +++ b/commands/remote.go @@ -48,7 +48,9 @@ func transformRemoteArgs(args *Args) { return } - localRepo := github.LocalRepo() + localRepo, err := github.LocalRepo() + utils.Check(err) + var repoName string if name == "" { project, err := localRepo.MainProject() diff --git a/commands/utils.go b/commands/utils.go index 9a0c16b4b..617cf9d9b 100644 --- a/commands/utils.go +++ b/commands/utils.go @@ -1,13 +1,14 @@ package commands import ( - "github.com/github/hub/github" - "github.com/github/hub/utils" - "github.com/octokit/go-octokit/octokit" "io/ioutil" "os" "path/filepath" "strings" + + "github.com/github/hub/github" + "github.com/github/hub/utils" + "github.com/octokit/go-octokit/octokit" ) type listFlag []string @@ -101,7 +102,9 @@ func readMsg(msg string) (title, body string) { } func runInLocalRepo(fn func(localRepo *github.GitHubRepo, project *github.Project, client *github.Client)) { - localRepo := github.LocalRepo() + localRepo, err := github.LocalRepo() + utils.Check(err) + project, err := localRepo.CurrentProject() utils.Check(err) diff --git a/github/branch_test.go b/github/branch_test.go index 7da8b6780..43e96e4e0 100644 --- a/github/branch_test.go +++ b/github/branch_test.go @@ -1,32 +1,40 @@ package github import ( - "github.com/bmizerany/assert" "testing" + + "github.com/bmizerany/assert" ) func TestBranch_ShortName(t *testing.T) { - b := Branch{LocalRepo(), "refs/heads/master"} + lp, _ := LocalRepo() + b := Branch{lp, "refs/heads/master"} assert.Equal(t, "master", b.ShortName()) } func TestBranch_LongName(t *testing.T) { - b := Branch{LocalRepo(), "refs/heads/master"} + lp, _ := LocalRepo() + + b := Branch{lp, "refs/heads/master"} assert.Equal(t, "heads/master", b.LongName()) - b = Branch{LocalRepo(), "refs/remotes/origin/master"} + b = Branch{lp, "refs/remotes/origin/master"} assert.Equal(t, "origin/master", b.LongName()) } func TestBranch_RemoteName(t *testing.T) { - b := Branch{LocalRepo(), "refs/remotes/origin/master"} + lp, _ := LocalRepo() + + b := Branch{lp, "refs/remotes/origin/master"} assert.Equal(t, "origin", b.RemoteName()) - b = Branch{LocalRepo(), "refs/head/master"} + b = Branch{lp, "refs/head/master"} assert.Equal(t, "", b.RemoteName()) } func TestBranch_IsRemote(t *testing.T) { - b := Branch{LocalRepo(), "refs/remotes/origin/master"} + lp, _ := LocalRepo() + + b := Branch{lp, "refs/remotes/origin/master"} assert.T(t, b.IsRemote()) } diff --git a/github/localrepo.go b/github/localrepo.go index 620d8c492..fc181cbc5 100644 --- a/github/localrepo.go +++ b/github/localrepo.go @@ -6,8 +6,16 @@ import ( "github.com/github/hub/git" ) -func LocalRepo() *GitHubRepo { - return &GitHubRepo{} +func LocalRepo() (repo *GitHubRepo, err error) { + repo = &GitHubRepo{} + + _, err = git.Dir() + if err != nil { + err = fmt.Errorf("fatal: Not a git repository") + return + } + + return } type GitHubRepo struct { From a4bcb989230cd32bcb3bc2b5dcfe202a630e056b Mon Sep 17 00:00:00 2001 From: Jingwen Owen Ou Date: Sat, 5 Apr 2014 20:00:40 -0700 Subject: [PATCH 10/15] Bump go-octokit with changes of adding global headers --- Godeps/Godeps.json | 4 +- .../octokit/go-octokit/octokit/client.go | 55 ++++++++++--------- .../octokit/go-octokit/octokit/client_test.go | 20 +++++++ .../octokit/go-octokit/octokit/request.go | 30 +++++++--- 4 files changed, 71 insertions(+), 38 deletions(-) diff --git a/Godeps/Godeps.json b/Godeps/Godeps.json index aef7359ce..510d6280f 100644 --- a/Godeps/Godeps.json +++ b/Godeps/Godeps.json @@ -60,8 +60,8 @@ }, { "ImportPath": "github.com/octokit/go-octokit/octokit", - "Comment": "v0.4.0-66-g9368061", - "Rev": "9368061969ae7fe6a1f478ffac4e4708f17c6c59" + "Comment": "v0.4.0-70-g709004f", + "Rev": "709004f97535024b15c5266f7bd4bd25d61ddb42" }, { "ImportPath": "github.com/ogier/pflag", diff --git a/Godeps/_workspace/src/github.com/octokit/go-octokit/octokit/client.go b/Godeps/_workspace/src/github.com/octokit/go-octokit/octokit/client.go index 30d9281b5..1aae77dea 100644 --- a/Godeps/_workspace/src/github.com/octokit/go-octokit/octokit/client.go +++ b/Godeps/_workspace/src/github.com/octokit/go-octokit/octokit/client.go @@ -15,23 +15,25 @@ func NewClient(authMethod AuthMethod) *Client { func NewClientWith(baseURL string, userAgent string, authMethod AuthMethod, httpClient *http.Client) *Client { client, _ := sawyer.NewFromString(baseURL, httpClient) - return &Client{sawyerClient: client, UserAgent: userAgent, AuthMethod: authMethod} + return &Client{Client: client, UserAgent: userAgent, AuthMethod: authMethod} } type Client struct { - UserAgent string - AuthMethod AuthMethod - sawyerClient *sawyer.Client - rootRels hypermedia.Relations + *sawyer.Client + + UserAgent string + AuthMethod AuthMethod + rootRels hypermedia.Relations } func (c *Client) NewRequest(urlStr string) (req *Request, err error) { - sawyerReq, err := c.newSawyerRequest(urlStr) + req, err = newRequest(c, urlStr) if err != nil { return } - req = &Request{sawyerReq: sawyerReq} + c.applyRequestHeaders(req) + return } @@ -72,32 +74,41 @@ func (c *Client) patch(url *url.URL, input interface{}, output interface{}) (res } func (c *Client) upload(uploadUrl *url.URL, asset io.ReadCloser, contentType string, contentLength int64) (result *Result) { - req, err := c.newSawyerRequest(uploadUrl.String()) + req, err := c.NewRequest(uploadUrl.String()) if err != nil { result = newResult(nil, err) return } - req.Header.Add("Content-Type", contentType) + req.Header.Set("Content-Type", contentType) req.ContentLength = contentLength req.Body = asset - sawyerResp := req.Post() + sawyerResp := req.Request.Post() resp, err := NewResponse(sawyerResp) return newResult(resp, err) } -func (c *Client) newSawyerRequest(urlStr string) (sawyerReq *sawyer.Request, err error) { - sawyerReq, err = c.sawyerClient.NewRequest(urlStr) - if err != nil { - return +func (c *Client) applyRequestHeaders(req *Request) { + req.Header.Set("Accept", defaultMediaType) + req.Header.Set("User-Agent", c.UserAgent) + + if c.AuthMethod != nil { + req.Header.Set("Authorization", c.AuthMethod.String()) } - sawyerReq.Header.Add("Accept", defaultMediaType) - sawyerReq.Header.Add("User-Agent", c.UserAgent) + if basicAuth, ok := c.AuthMethod.(BasicAuth); ok && basicAuth.OneTimePassword != "" { + req.Header.Set("X-GitHub-OTP", basicAuth.OneTimePassword) + } - c.addAuthenticationHeaders(sawyerReq.Header) + // Go doesn't apply `Host` on the header, instead it consults `Request.Host` + // Populate `Host` if it exists in `Client.Header` + // See Bug https://code.google.com/p/go/issues/detail?id=7682 + host := c.Header.Get("Host") + if host != "" { + req.Request.Host = host + } return } @@ -114,13 +125,3 @@ func sendRequest(c *Client, url *url.URL, fn func(r *Request) (*Response, error) return } - -func (c *Client) addAuthenticationHeaders(header http.Header) { - if c.AuthMethod != nil { - header.Add("Authorization", c.AuthMethod.String()) - } - - if basicAuth, ok := c.AuthMethod.(BasicAuth); ok && basicAuth.OneTimePassword != "" { - header.Add("X-GitHub-OTP", basicAuth.OneTimePassword) - } -} diff --git a/Godeps/_workspace/src/github.com/octokit/go-octokit/octokit/client_test.go b/Godeps/_workspace/src/github.com/octokit/go-octokit/octokit/client_test.go index 59d3d2069..b5b1ed50e 100644 --- a/Godeps/_workspace/src/github.com/octokit/go-octokit/octokit/client_test.go +++ b/Godeps/_workspace/src/github.com/octokit/go-octokit/octokit/client_test.go @@ -124,3 +124,23 @@ func TestSuccessfulPost(t *testing.T) { assert.Equal(t, nil, err) assert.Equal(t, "octokit", output["login"]) } + +func TestAddHeader(t *testing.T) { + setup() + defer tearDown() + + mux.HandleFunc("/foo", func(w http.ResponseWriter, r *http.Request) { + testMethod(t, r, "GET") + testHeader(t, r, "Foo", "Bar") + assert.Equal(t, "example.com", r.Host) + respondWithJSON(w, `{"login": "octokit"}`) + }) + + client.Header.Set("Host", "example.com") + client.Header.Set("Foo", "Bar") + req, err := client.NewRequest("foo") + assert.Equal(t, nil, err) + + _, err = req.Get(nil) + assert.Equal(t, nil, err) +} diff --git a/Godeps/_workspace/src/github.com/octokit/go-octokit/octokit/request.go b/Godeps/_workspace/src/github.com/octokit/go-octokit/octokit/request.go index 88bff04c8..a86d748b4 100644 --- a/Godeps/_workspace/src/github.com/octokit/go-octokit/octokit/request.go +++ b/Godeps/_workspace/src/github.com/octokit/go-octokit/octokit/request.go @@ -5,44 +5,56 @@ import ( "github.com/lostisland/go-sawyer/mediatype" ) +func newRequest(client *Client, urlStr string) (req *Request, err error) { + sawyerReq, err := client.Client.NewRequest(urlStr) + if err != nil { + return + } + + req = &Request{client: client, Request: sawyerReq} + + return +} + type Request struct { - sawyerReq *sawyer.Request + *sawyer.Request + client *Client } func (r *Request) Head(output interface{}) (*Response, error) { - return r.createResponse(r.sawyerReq.Head(), output) + return r.createResponse(r.Request.Head(), output) } func (r *Request) Get(output interface{}) (*Response, error) { - return r.createResponse(r.sawyerReq.Get(), output) + return r.createResponse(r.Request.Get(), output) } func (r *Request) Post(input interface{}, output interface{}) (*Response, error) { r.setBody(input) - return r.createResponse(r.sawyerReq.Post(), output) + return r.createResponse(r.Request.Post(), output) } func (r *Request) Put(input interface{}, output interface{}) (*Response, error) { r.setBody(input) - return r.createResponse(r.sawyerReq.Put(), output) + return r.createResponse(r.Request.Put(), output) } func (r *Request) Delete(output interface{}) (*Response, error) { - return r.createResponse(r.sawyerReq.Delete(), output) + return r.createResponse(r.Request.Delete(), output) } func (r *Request) Patch(input interface{}, output interface{}) (*Response, error) { r.setBody(input) - return r.createResponse(r.sawyerReq.Patch(), output) + return r.createResponse(r.Request.Patch(), output) } func (r *Request) Options(output interface{}) (*Response, error) { - return r.createResponse(r.sawyerReq.Options(), output) + return r.createResponse(r.Request.Options(), output) } func (r *Request) setBody(input interface{}) { mtype, _ := mediatype.Parse(defaultMediaType) - r.sawyerReq.SetBody(mtype, input) + r.Request.SetBody(mtype, input) } func (r *Request) createResponse(sawyerResp *sawyer.Response, output interface{}) (resp *Response, err error) { From 052ce47edde8a968bf01ae2bb5cf2c9eac7e51e3 Mon Sep 17 00:00:00 2001 From: Jingwen Owen Ou Date: Sat, 5 Apr 2014 20:22:18 -0700 Subject: [PATCH 11/15] Extract out into newOctokitClient --- commands/fork.go | 4 ++-- github/client.go | 14 +++++++++----- 2 files changed, 11 insertions(+), 7 deletions(-) diff --git a/commands/fork.go b/commands/fork.go index f6915020f..6350ba01e 100644 --- a/commands/fork.go +++ b/commands/fork.go @@ -2,10 +2,10 @@ package commands import ( "fmt" - "os" - "reflect" "github.com/github/hub/github" "github.com/github/hub/utils" + "os" + "reflect" ) var cmdFork = &Command{ diff --git a/github/client.go b/github/client.go index 65432c0fd..3d3b01893 100644 --- a/github/client.go +++ b/github/client.go @@ -339,7 +339,7 @@ func (client *Client) GhLatestTagName() (tagName string, err error) { return } - c := octokit.NewClientWith(client.apiHost(), UserAgent, nil, nil) + c := client.newOctokitClient(nil) releases, result := c.Releases(client.requestURL(url)).All() if result.HasError() { err = fmt.Errorf("Error getting gh release: %s", result.Err) @@ -385,7 +385,7 @@ func (client *Client) FindOrCreateToken(user, password, twoFactorCode string) (t } basicAuth := octokit.BasicAuth{Login: user, Password: password, OneTimePassword: twoFactorCode} - c := octokit.NewClientWith(client.apiHost(), UserAgent, basicAuth, nil) + c := client.newOctokitClient(basicAuth) authsService := c.Authorizations(client.requestURL(url)) auths, result := authsService.All() @@ -451,13 +451,17 @@ func (client *Client) api() (c *octokit.Client, err error) { } tokenAuth := octokit.TokenAuth{AccessToken: client.Host.AccessToken} - tr := &http.Transport{Proxy: proxyFromEnvironment} - httpClient := &http.Client{Transport: tr} - c = octokit.NewClientWith(client.apiHost(), UserAgent, tokenAuth, httpClient) + c = client.newOctokitClient(tokenAuth) return } +func (client *Client) newOctokitClient(auth octokit.AuthMethod) *octokit.Client { + tr := &http.Transport{Proxy: proxyFromEnvironment} + httpClient := &http.Client{Transport: tr} + return octokit.NewClientWith(client.apiHost(), UserAgent, auth, httpClient) +} + func (client *Client) requestURL(u *url.URL) (uu *url.URL) { uu = u if client.Host != nil && client.Host.Host != GitHubHost { From 6ab839f2a8efa7ffc7c3b943b3c402fadbee434a Mon Sep 17 00:00:00 2001 From: Jingwen Owen Ou Date: Sat, 5 Apr 2014 22:13:39 -0700 Subject: [PATCH 12/15] Set host name it's in test with HUB_TEST_HOST --- github/apihost.go | 32 ------------------------------- github/apihost_test.go | 18 ------------------ github/client.go | 43 ++++++++++++++++++++++++++++++++++++------ github/client_test.go | 22 +++++++++++++++++++++ 4 files changed, 59 insertions(+), 56 deletions(-) delete mode 100644 github/apihost.go delete mode 100644 github/apihost_test.go diff --git a/github/apihost.go b/github/apihost.go deleted file mode 100644 index 98908f7ad..000000000 --- a/github/apihost.go +++ /dev/null @@ -1,32 +0,0 @@ -package github - -import ( - "net/url" - "os" -) - -type apiHost struct { - Host string -} - -func (ah *apiHost) String() string { - host := os.Getenv("HUB_TEST_HOST") - if host == "" && ah.Host != "" { - host = ah.Host - } - - if host == GitHubHost { - host = GitHubApiHost - } - - return absolute(host) -} - -func absolute(endpoint string) string { - u, _ := url.Parse(endpoint) - if u.Scheme == "" { - u.Scheme = "https" - } - - return u.String() -} diff --git a/github/apihost_test.go b/github/apihost_test.go deleted file mode 100644 index 1d6bdff87..000000000 --- a/github/apihost_test.go +++ /dev/null @@ -1,18 +0,0 @@ -package github - -import ( - "testing" - - "github.com/bmizerany/assert" -) - -func TestApiHost_String(t *testing.T) { - ah := &apiHost{"github.com"} - assert.Equal(t, "https://api.github.com", ah.String()) - - ah = &apiHost{"github.corporate.com"} - assert.Equal(t, "https://github.corporate.com", ah.String()) - - ah = &apiHost{"http://github.corporate.com"} - assert.Equal(t, "http://github.corporate.com", ah.String()) -} diff --git a/github/client.go b/github/client.go index 3d3b01893..143d741dd 100644 --- a/github/client.go +++ b/github/client.go @@ -457,9 +457,36 @@ func (client *Client) api() (c *octokit.Client, err error) { } func (client *Client) newOctokitClient(auth octokit.AuthMethod) *octokit.Client { + var host string + if client.Host != nil { + host = client.Host.Host + } + + if host == "" { + host = GitHubHost + } + + if host == GitHubHost { + host = GitHubApiHost + } + + apiHost := host + hubTestHost := os.Getenv("HUB_TEST_HOST") + if hubTestHost != "" { + apiHost = hubTestHost + } + + apiHost = absolute(apiHost) + tr := &http.Transport{Proxy: proxyFromEnvironment} httpClient := &http.Client{Transport: tr} - return octokit.NewClientWith(client.apiHost(), UserAgent, auth, httpClient) + c := octokit.NewClientWith(apiHost, UserAgent, auth, httpClient) + if hubTestHost != "" { + // if it's in test, make sure host name is in the header + c.Header.Set("Host", host) + } + + return c } func (client *Client) requestURL(u *url.URL) (uu *url.URL) { @@ -471,11 +498,6 @@ func (client *Client) requestURL(u *url.URL) (uu *url.URL) { return } -func (client *Client) apiHost() string { - ah := &apiHost{client.Host.Host} - return ah.String() -} - func FormatError(action string, err error) (ee error) { switch e := err.(type) { default: @@ -528,3 +550,12 @@ func warnExistenceOfRepo(project *Project, ee error) (err error) { return } + +func absolute(endpoint string) string { + u, _ := url.Parse(endpoint) + if u.Scheme == "" { + u.Scheme = "https" + } + + return u.String() +} diff --git a/github/client_test.go b/github/client_test.go index 38235cdcc..dfd13c2d9 100644 --- a/github/client_test.go +++ b/github/client_test.go @@ -3,12 +3,34 @@ package github import ( "fmt" "net/http" + "os" "testing" "github.com/bmizerany/assert" "github.com/octokit/go-octokit/octokit" ) +func TestClient_newOctokitClient(t *testing.T) { + c := NewClient("github.com") + cc := c.newOctokitClient(nil) + assert.Equal(t, "https://api.github.com", cc.Endpoint.String()) + + c = NewClient("github.corporate.com") + cc = c.newOctokitClient(nil) + assert.Equal(t, "https://github.corporate.com", cc.Endpoint.String()) + + c = NewClient("http://github.corporate.com") + cc = c.newOctokitClient(nil) + assert.Equal(t, "http://github.corporate.com", cc.Endpoint.String()) + + os.Setenv("HUB_TEST_HOST", "http://127.0.0.1") + defer os.Setenv("HUB_TEST_HOST", "") + c = NewClient("github.corporate.com") + cc = c.newOctokitClient(nil) + assert.Equal(t, "http://127.0.0.1", cc.Endpoint.String()) + assert.Equal(t, "github.corporate.com", cc.Header.Get("Host")) +} + func TestClient_FormatError(t *testing.T) { e := &octokit.ResponseError{ Response: &http.Response{ From 3c7580800393aea97557a68ba97ca8d6033f14c8 Mon Sep 17 00:00:00 2001 From: Jingwen Owen Ou Date: Sat, 5 Apr 2014 22:16:10 -0700 Subject: [PATCH 13/15] Put back filtering by host name --- features/fork.feature | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/features/fork.feature b/features/fork.feature index 3361f6880..10e1266ce 100644 --- a/features/fork.feature +++ b/features/fork.feature @@ -8,8 +8,8 @@ Feature: hub fork Given the GitHub API server: """ before { halt 401 unless request.env['HTTP_AUTHORIZATION'] == 'token OTOKEN' } - get('/repos/mislav/dotfiles') { halt 404 } - post('/repos/evilchelu/dotfiles/forks') { '' } + get('/repos/mislav/dotfiles', :host_name => 'api.github.com') { halt 404 } + post('/repos/evilchelu/dotfiles/forks', :host_name => 'api.github.com') { '' } """ When I successfully run `hub fork` Then the output should contain exactly "new remote: mislav\n" @@ -22,8 +22,8 @@ Feature: hub fork Given the GitHub API server: """ before { halt 401 unless request.env['HTTP_AUTHORIZATION'] == 'token OTOKEN' } - get('/repos/mislav/dotfiles') { 404 } - post('/repos/evilchelu/dotfiles/forks') { '' } + get('/repos/mislav/dotfiles', :host_name => 'api.github.com') { 404 } + post('/repos/evilchelu/dotfiles/forks', :host_name => 'api.github.com') { '' } """ When I successfully run `hub fork` Then the output should contain exactly "new remote: mislav\n" @@ -122,7 +122,7 @@ Scenario: Related fork already exists Given the GitHub API server: """ before { halt 401 unless request.env['HTTP_AUTHORIZATION'] == 'token FITOKEN' } - post('/api/v3/repos/evilchelu/dotfiles/forks') { '' } + post('/api/v3/repos/evilchelu/dotfiles/forks', :host_name => 'git.my.org') { '' } """ And the "origin" remote has url "git@git.my.org:evilchelu/dotfiles.git" And I am "mislav" on git.my.org with OAuth token "FITOKEN" From 8eec6837d0b91fde31f0d957e7140fe203c694f1 Mon Sep 17 00:00:00 2001 From: Jingwen Owen Ou Date: Sat, 5 Apr 2014 22:18:24 -0700 Subject: [PATCH 14/15] Don't need to halt with 404 --- features/fork.feature | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/features/fork.feature b/features/fork.feature index 10e1266ce..cbe9da94d 100644 --- a/features/fork.feature +++ b/features/fork.feature @@ -8,7 +8,7 @@ Feature: hub fork Given the GitHub API server: """ before { halt 401 unless request.env['HTTP_AUTHORIZATION'] == 'token OTOKEN' } - get('/repos/mislav/dotfiles', :host_name => 'api.github.com') { halt 404 } + get('/repos/mislav/dotfiles', :host_name => 'api.github.com') { 404 } post('/repos/evilchelu/dotfiles/forks', :host_name => 'api.github.com') { '' } """ When I successfully run `hub fork` From 56c141c2811cf4b1ea77d2d26c6976ec0f8a1542 Mon Sep 17 00:00:00 2001 From: Jingwen Owen Ou Date: Sun, 6 Apr 2014 07:53:16 -0700 Subject: [PATCH 15/15] Update go-octokit ref --- Godeps/Godeps.json | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Godeps/Godeps.json b/Godeps/Godeps.json index 510d6280f..7965625ad 100644 --- a/Godeps/Godeps.json +++ b/Godeps/Godeps.json @@ -60,8 +60,8 @@ }, { "ImportPath": "github.com/octokit/go-octokit/octokit", - "Comment": "v0.4.0-70-g709004f", - "Rev": "709004f97535024b15c5266f7bd4bd25d61ddb42" + "Comment": "v0.4.0-71-g4fee5e3", + "Rev": "4fee5e3019ca20d12c93c1b7f18f20c4a6afff24" }, { "ImportPath": "github.com/ogier/pflag",