Skip to content

Commit

Permalink
Respect hostnames when resolving git remotes and URL args to repos
Browse files Browse the repository at this point in the history
  • Loading branch information
mislav committed Jul 2, 2020
1 parent e4448c7 commit 446a411
Show file tree
Hide file tree
Showing 8 changed files with 143 additions and 78 deletions.
31 changes: 24 additions & 7 deletions api/queries_repo.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,9 @@ type Repository struct {
}

Parent *Repository

// pseudo-field that keeps track of host name of this repo
hostname string
}

// RepositoryOwner is the owner of a GitHub repository
Expand All @@ -53,8 +56,7 @@ func (r Repository) RepoName() string {

// RepoHost is the GitHub hostname of the repository
func (r Repository) RepoHost() string {
// FIXME: inherit hostname from the server
return "github.com"
return r.hostname
}

// IsFork is true when this repository has a parent repository
Expand Down Expand Up @@ -109,7 +111,7 @@ func GitHubRepo(client *Client, repo ghrepo.Interface) (*Repository, error) {
return nil, err
}

return &result.Repository, nil
return initRepoHostname(&result.Repository, repo.RepoHost()), nil
}

// RepoParent finds out the parent repository of a fork
Expand Down Expand Up @@ -139,7 +141,7 @@ func RepoParent(client *Client, repo ghrepo.Interface) (ghrepo.Interface, error)
return nil, nil
}

parent := ghrepo.New(query.Repository.Parent.Owner.Login, query.Repository.Parent.Name)
parent := ghrepo.NewWithHost(query.Repository.Parent.Owner.Login, query.Repository.Parent.Name, repo.RepoHost())
return parent, nil
}

Expand All @@ -151,6 +153,11 @@ type RepoNetworkResult struct {

// RepoNetwork inspects the relationship between multiple GitHub repositories
func RepoNetwork(client *Client, repos []ghrepo.Interface) (RepoNetworkResult, error) {
var hostname string
if len(repos) > 0 {
hostname = repos[0].RepoHost()
}

queries := make([]string, 0, len(repos))
for i, repo := range repos {
queries = append(queries, fmt.Sprintf(`
Expand Down Expand Up @@ -233,14 +240,22 @@ func RepoNetwork(client *Client, repos []ghrepo.Interface) (RepoNetworkResult, e
if err := decoder.Decode(&repo); err != nil {
return result, err
}
result.Repositories = append(result.Repositories, &repo)
result.Repositories = append(result.Repositories, initRepoHostname(&repo, hostname))
} else {
return result, fmt.Errorf("unknown GraphQL result key %q", name)
}
}
return result, nil
}

func initRepoHostname(repo *Repository, hostname string) *Repository {
repo.hostname = hostname
if repo.Parent != nil {
repo.Parent.hostname = hostname
}
return repo
}

// repositoryV3 is the repository result from GitHub API v3
type repositoryV3 struct {
NodeID string
Expand Down Expand Up @@ -271,6 +286,7 @@ func ForkRepo(client *Client, repo ghrepo.Interface) (*Repository, error) {
Login: result.Owner.Login,
},
ViewerPermission: "WRITE",
hostname: repo.RepoHost(),
}, nil
}

Expand Down Expand Up @@ -312,7 +328,7 @@ func RepoFindFork(client *Client, repo ghrepo.Interface) (*Repository, error) {
// `affiliations` condition, to guard against versions of GitHub with a
// faulty `affiliations` implementation
if len(forks) > 0 && forks[0].ViewerCanPush() {
return &forks[0], nil
return initRepoHostname(&forks[0], repo.RepoHost()), nil
}
return nil, &NotFoundError{errors.New("no fork found")}
}
Expand Down Expand Up @@ -374,7 +390,8 @@ func RepoCreate(client *Client, input RepoCreateInput) (*Repository, error) {
return nil, err
}

return &response.CreateRepository.Repository, nil
// FIXME: support Enterprise hosts
return initRepoHostname(&response.CreateRepository.Repository, "github.com"), nil
}

func RepositoryReadme(client *Client, fullName string) (string, error) {
Expand Down
48 changes: 5 additions & 43 deletions command/issue.go
Original file line number Diff line number Diff line change
@@ -1,11 +1,9 @@
package command

import (
"errors"
"fmt"
"io"
"net/url"
"regexp"
"strconv"
"strings"
"time"
Expand Down Expand Up @@ -244,12 +242,7 @@ func issueView(cmd *cobra.Command, args []string) error {
return err
}

baseRepo, err := determineBaseRepo(apiClient, cmd, ctx)
if err != nil {
return err
}

issue, err := issueFromArg(apiClient, baseRepo, args[0])
issue, _, err := issueFromArg(ctx, apiClient, cmd, args[0])
if err != nil {
return err
}
Expand Down Expand Up @@ -341,21 +334,6 @@ func printIssuePreview(out io.Writer, issue *api.Issue) error {
return nil
}

var issueURLRE = regexp.MustCompile(`^https://github\.com/([^/]+)/([^/]+)/issues/(\d+)`)

func issueFromArg(apiClient *api.Client, baseRepo ghrepo.Interface, arg string) (*api.Issue, error) {
if issueNumber, err := strconv.Atoi(strings.TrimPrefix(arg, "#")); err == nil {
return api.IssueByNumber(apiClient, baseRepo, issueNumber)
}

if m := issueURLRE.FindStringSubmatch(arg); m != nil {
issueNumber, _ := strconv.Atoi(m[3])
return api.IssueByNumber(apiClient, baseRepo, issueNumber)
}

return nil, fmt.Errorf("invalid issue format: %q", arg)
}

func issueCreate(cmd *cobra.Command, args []string) error {
ctx := contextForCommand(cmd)
apiClient, err := apiClientForContext(ctx)
Expand Down Expand Up @@ -685,27 +663,19 @@ func issueClose(cmd *cobra.Command, args []string) error {
return err
}

baseRepo, err := determineBaseRepo(apiClient, cmd, ctx)
issue, baseRepo, err := issueFromArg(ctx, apiClient, cmd, args[0])
if err != nil {
return err
}

issue, err := issueFromArg(apiClient, baseRepo, args[0])
var idErr *api.IssuesDisabledError
if errors.As(err, &idErr) {
return fmt.Errorf("issues disabled for %s", ghrepo.FullName(baseRepo))
} else if err != nil {
return fmt.Errorf("failed to find issue #%d: %w", issue.Number, err)
}

if issue.Closed {
fmt.Fprintf(colorableErr(cmd), "%s Issue #%d is already closed\n", utils.Yellow("!"), issue.Number)
return nil
}

err = api.IssueClose(apiClient, baseRepo, *issue)
if err != nil {
return fmt.Errorf("API call failed:%w", err)
return err
}

fmt.Fprintf(colorableErr(cmd), "%s Closed issue #%d\n", utils.Red("✔"), issue.Number)
Expand All @@ -720,27 +690,19 @@ func issueReopen(cmd *cobra.Command, args []string) error {
return err
}

baseRepo, err := determineBaseRepo(apiClient, cmd, ctx)
issue, baseRepo, err := issueFromArg(ctx, apiClient, cmd, args[0])
if err != nil {
return err
}

issue, err := issueFromArg(apiClient, baseRepo, args[0])
var idErr *api.IssuesDisabledError
if errors.As(err, &idErr) {
return fmt.Errorf("issues disabled for %s", ghrepo.FullName(baseRepo))
} else if err != nil {
return fmt.Errorf("failed to find issue #%d: %w", issue.Number, err)
}

if !issue.Closed {
fmt.Fprintf(colorableErr(cmd), "%s Issue #%d is already open\n", utils.Yellow("!"), issue.Number)
return nil
}

err = api.IssueReopen(apiClient, baseRepo, *issue)
if err != nil {
return fmt.Errorf("API call failed:%w", err)
return err
}

fmt.Fprintf(colorableErr(cmd), "%s Reopened issue #%d\n", utils.Green("✔"), issue.Number)
Expand Down
64 changes: 64 additions & 0 deletions command/issue_lookup.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
package command

import (
"fmt"
"net/url"
"regexp"
"strconv"
"strings"

"github.com/cli/cli/api"
"github.com/cli/cli/context"
"github.com/cli/cli/internal/ghrepo"
"github.com/spf13/cobra"
)

func issueFromArg(ctx context.Context, apiClient *api.Client, cmd *cobra.Command, arg string) (*api.Issue, ghrepo.Interface, error) {
issue, baseRepo, err := issueFromURL(apiClient, arg)
if err != nil {
return nil, nil, err
}
if issue != nil {
return issue, baseRepo, nil
}

baseRepo, err = determineBaseRepo(apiClient, cmd, ctx)
if err != nil {
return nil, nil, fmt.Errorf("could not determine base repo: %w", err)
}

issueNumber, err := strconv.Atoi(strings.TrimPrefix(arg, "#"))
if err != nil {
return nil, nil, fmt.Errorf("invalid issue format: %q", arg)
}

issue, err = issueFromNumber(apiClient, baseRepo, issueNumber)
return issue, baseRepo, err
}

var issueURLRE = regexp.MustCompile(`^/([^/]+)/([^/]+)/issues/(\d+)`)

func issueFromURL(apiClient *api.Client, s string) (*api.Issue, ghrepo.Interface, error) {
u, err := url.Parse(s)
if err != nil {
return nil, nil, nil
}

if u.Scheme != "https" && u.Scheme != "http" {
return nil, nil, nil
}

m := issueURLRE.FindStringSubmatch(u.Path)
if m == nil {
return nil, nil, nil
}

repo := ghrepo.NewWithHost(m[1], m[2], u.Hostname())
issueNumber, _ := strconv.Atoi(m[3])
issue, err := issueFromNumber(apiClient, repo, issueNumber)
return issue, repo, err
}

func issueFromNumber(apiClient *api.Client, repo ghrepo.Interface, issueNumber int) (*api.Issue, error) {
return api.IssueByNumber(apiClient, repo, issueNumber)
}
18 changes: 5 additions & 13 deletions command/issue_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -387,6 +387,7 @@ func TestIssueView_Preview(t *testing.T) {
func TestIssueView_web_notFound(t *testing.T) {
initBlankContext("", "OWNER/REPO", "master")
http := initFakeHTTP()
http.StubRepoResponse("OWNER", "REPO")

http.StubResponse(200, bytes.NewBufferString(`
{ "errors": [
Expand Down Expand Up @@ -432,7 +433,6 @@ func TestIssueView_disabledIssues(t *testing.T) {
func TestIssueView_web_urlArg(t *testing.T) {
initBlankContext("", "OWNER/REPO", "master")
http := initFakeHTTP()
http.StubRepoResponse("OWNER", "REPO")

http.StubResponse(200, bytes.NewBufferString(`
{ "data": { "repository": { "hasIssuesEnabled": true, "issue": {
Expand Down Expand Up @@ -857,12 +857,8 @@ func TestIssueClose_issuesDisabled(t *testing.T) {
`))

_, err := RunCommand("issue close 13")
if err == nil {
t.Fatalf("expected error when issues are disabled")
}

if !strings.Contains(err.Error(), "issues disabled") {
t.Fatalf("got unexpected error: %s", err)
if err == nil || err.Error() != "the 'OWNER/REPO' repository has disabled issues" {
t.Fatalf("got error: %v", err)
}
}

Expand Down Expand Up @@ -930,11 +926,7 @@ func TestIssueReopen_issuesDisabled(t *testing.T) {
`))

_, err := RunCommand("issue reopen 2")
if err == nil {
t.Fatalf("expected error when issues are disabled")
}

if !strings.Contains(err.Error(), "issues disabled") {
t.Fatalf("got unexpected error: %s", err)
if err == nil || err.Error() != "the 'OWNER/REPO' repository has disabled issues" {
t.Fatalf("got error: %v", err)
}
}
3 changes: 1 addition & 2 deletions command/pr_checkout.go
Original file line number Diff line number Diff line change
Expand Up @@ -84,8 +84,7 @@ func prCheckout(cmd *cobra.Command, args []string) error {
remote := baseURLOrName
mergeRef := ref
if pr.MaintainerCanModify {
// FIXME: inherit hostname from the server
headRepo := ghrepo.New(pr.HeadRepositoryOwner.Login, pr.HeadRepository.Name)
headRepo := ghrepo.NewWithHost(pr.HeadRepositoryOwner.Login, pr.HeadRepository.Name, baseRepo.RepoHost())
remote = formatRemoteURL(cmd, headRepo)
mergeRef = fmt.Sprintf("refs/heads/%s", pr.HeadRefName)
}
Expand Down
26 changes: 19 additions & 7 deletions command/pr_lookup.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package command

import (
"fmt"
"net/url"
"regexp"
"strconv"
"strings"
Expand Down Expand Up @@ -55,16 +56,27 @@ func prFromNumberString(ctx context.Context, apiClient *api.Client, repo ghrepo.
return nil, nil
}

var pullURLRE = regexp.MustCompile(`^/([^/]+)/([^/]+)/pull/(\d+)`)

func prFromURL(ctx context.Context, apiClient *api.Client, s string) (*api.PullRequest, ghrepo.Interface, error) {
r := regexp.MustCompile(`^https://github\.com/([^/]+)/([^/]+)/pull/(\d+)`)
if m := r.FindStringSubmatch(s); m != nil {
repo := ghrepo.New(m[1], m[2])
prNumberString := m[3]
pr, err := prFromNumberString(ctx, apiClient, repo, prNumberString)
return pr, repo, err
u, err := url.Parse(s)
if err != nil {
return nil, nil, nil
}

if u.Scheme != "https" && u.Scheme != "http" {
return nil, nil, nil
}

m := pullURLRE.FindStringSubmatch(u.Path)
if m == nil {
return nil, nil, nil
}

return nil, nil, nil
repo := ghrepo.NewWithHost(m[1], m[2], u.Hostname())
prNumberString := m[3]
pr, err := prFromNumberString(ctx, apiClient, repo, prNumberString)
return pr, repo, err
}

func prForCurrentBranch(ctx context.Context, apiClient *api.Client, repo ghrepo.Interface) (*api.PullRequest, error) {
Expand Down
Loading

0 comments on commit 446a411

Please sign in to comment.