Skip to content

Commit

Permalink
Add ability to parse non-GitHub.com git remotes
Browse files Browse the repository at this point in the history
  • Loading branch information
mislav committed Jun 23, 2020
1 parent 6d70f31 commit 3ea71e6
Show file tree
Hide file tree
Showing 8 changed files with 114 additions and 45 deletions.
6 changes: 6 additions & 0 deletions api/queries_repo.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,12 @@ func (r Repository) RepoName() string {
return r.Name
}

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

// IsFork is true when this repository has a parent repository
func (r Repository) IsFork() bool {
return r.Parent != nil
Expand Down
3 changes: 1 addition & 2 deletions command/pr_create.go
Original file line number Diff line number Diff line change
Expand Up @@ -304,8 +304,7 @@ func prCreate(cmd *cobra.Command, _ []string) error {
}
headRemote = &context.Remote{
Remote: gitRemote,
Owner: headRepo.RepoOwner(),
Repo: headRepo.RepoName(),
Repo: headRepo,
}
}

Expand Down
16 changes: 6 additions & 10 deletions command/pr_create_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (

"github.com/cli/cli/context"
"github.com/cli/cli/git"
"github.com/cli/cli/internal/ghrepo"
"github.com/cli/cli/pkg/httpmock"
"github.com/cli/cli/test"
)
Expand Down Expand Up @@ -759,13 +760,11 @@ func Test_determineTrackingBranch_noMatch(t *testing.T) {
remotes := context.Remotes{
&context.Remote{
Remote: &git.Remote{Name: "origin"},
Owner: "hubot",
Repo: "Spoon-Knife",
Repo: ghrepo.New("hubot", "Spoon-Knife"),
},
&context.Remote{
Remote: &git.Remote{Name: "upstream"},
Owner: "octocat",
Repo: "Spoon-Knife",
Repo: ghrepo.New("octocat", "Spoon-Knife"),
},
}

Expand All @@ -786,13 +785,11 @@ func Test_determineTrackingBranch_hasMatch(t *testing.T) {
remotes := context.Remotes{
&context.Remote{
Remote: &git.Remote{Name: "origin"},
Owner: "hubot",
Repo: "Spoon-Knife",
Repo: ghrepo.New("hubot", "Spoon-Knife"),
},
&context.Remote{
Remote: &git.Remote{Name: "upstream"},
Owner: "octocat",
Repo: "Spoon-Knife",
Repo: ghrepo.New("octocat", "Spoon-Knife"),
},
}

Expand All @@ -819,8 +816,7 @@ func Test_determineTrackingBranch_respectTrackingConfig(t *testing.T) {
remotes := context.Remotes{
&context.Remote{
Remote: &git.Remote{Name: "origin"},
Owner: "hubot",
Repo: "Spoon-Knife",
Repo: ghrepo.New("hubot", "Spoon-Knife"),
},
}

Expand Down
3 changes: 1 addition & 2 deletions context/blank_context.go
Original file line number Diff line number Diff line change
Expand Up @@ -62,8 +62,7 @@ func (c *blankContext) SetRemotes(stubs map[string]string) {
ownerWithName := strings.SplitN(repo, "/", 2)
c.remotes = append(c.remotes, &Remote{
Remote: &git.Remote{Name: remoteName},
Owner: ownerWithName[0],
Repo: ownerWithName[1],
Repo: ghrepo.New(ownerWithName[0], ownerWithName[1]),
})
}
}
Expand Down
15 changes: 9 additions & 6 deletions context/remote.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,18 +57,22 @@ func (r Remotes) Less(i, j int) bool {
// Remote represents a git remote mapped to a GitHub repository
type Remote struct {
*git.Remote
Owner string
Repo string
Repo ghrepo.Interface
}

// RepoName is the name of the GitHub repository
func (r Remote) RepoName() string {
return r.Repo
return r.Repo.RepoName()
}

// RepoOwner is the name of the GitHub account that owns the repo
func (r Remote) RepoOwner() string {
return r.Owner
return r.Repo.RepoOwner()
}

// RepoHost is the GitHub hostname that the remote points to
func (r Remote) RepoHost() string {
return r.Repo.RepoHost()
}

// TODO: accept an interface instead of git.RemoteSet
Expand All @@ -86,8 +90,7 @@ func translateRemotes(gitRemotes git.RemoteSet, urlTranslate func(*url.URL) *url
}
remotes = append(remotes, &Remote{
Remote: r,
Owner: repo.RepoOwner(),
Repo: repo.RepoName(),
Repo: repo,
})
}
return
Expand Down
18 changes: 7 additions & 11 deletions context/remote_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,9 @@ func eq(t *testing.T, got interface{}, expected interface{}) {

func Test_Remotes_FindByName(t *testing.T) {
list := Remotes{
&Remote{Remote: &git.Remote{Name: "mona"}, Owner: "monalisa", Repo: "myfork"},
&Remote{Remote: &git.Remote{Name: "origin"}, Owner: "monalisa", Repo: "octo-cat"},
&Remote{Remote: &git.Remote{Name: "upstream"}, Owner: "hubot", Repo: "tools"},
&Remote{Remote: &git.Remote{Name: "mona"}, Repo: ghrepo.New("monalisa", "myfork")},
&Remote{Remote: &git.Remote{Name: "origin"}, Repo: ghrepo.New("monalisa", "octo-cat")},
&Remote{Remote: &git.Remote{Name: "upstream"}, Repo: ghrepo.New("hubot", "tools")},
}

r, err := list.FindByName("upstream", "origin")
Expand Down Expand Up @@ -84,13 +84,11 @@ func Test_resolvedRemotes_triangularSetup(t *testing.T) {
Remotes: Remotes{
&Remote{
Remote: &git.Remote{Name: "origin"},
Owner: "OWNER",
Repo: "REPO",
Repo: ghrepo.New("OWNER", "REPO"),
},
&Remote{
Remote: &git.Remote{Name: "fork"},
Owner: "MYSELF",
Repo: "REPO",
Repo: ghrepo.New("MYSELF", "REPO"),
},
},
Network: api.RepoNetworkResult{
Expand Down Expand Up @@ -157,8 +155,7 @@ func Test_resolvedRemotes_forkLookup(t *testing.T) {
Remotes: Remotes{
&Remote{
Remote: &git.Remote{Name: "origin"},
Owner: "OWNER",
Repo: "REPO",
Repo: ghrepo.New("OWNER", "REPO"),
},
},
Network: api.RepoNetworkResult{
Expand Down Expand Up @@ -190,8 +187,7 @@ func Test_resolvedRemotes_clonedFork(t *testing.T) {
Remotes: Remotes{
&Remote{
Remote: &git.Remote{Name: "origin"},
Owner: "OWNER",
Repo: "REPO",
Repo: ghrepo.New("OWNER", "REPO"),
},
},
Network: api.RepoNetworkResult{
Expand Down
40 changes: 30 additions & 10 deletions internal/ghrepo/repo.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,13 @@ import (
"strings"
)

// TODO these are sprinkled across command, context, config, and ghrepo
const defaultHostname = "github.com"

// Interface describes an object that represents a GitHub repository
type Interface interface {
RepoName() string
RepoOwner() string
RepoHost() string
}

// New instantiates a GitHub repository from owner and name arguments
Expand All @@ -39,32 +39,52 @@ func FromFullName(nwo string) (Interface, error) {
return &r, nil
}

// FromURL extracts the GitHub repository information from a URL
// FromURL extracts the GitHub repository information from a git remote URL
func FromURL(u *url.URL) (Interface, error) {
if !strings.EqualFold(u.Hostname(), defaultHostname) && !strings.EqualFold(u.Hostname(), "www."+defaultHostname) {
return nil, fmt.Errorf("unsupported hostname: %s", u.Hostname())
if u.Hostname() == "" {
return nil, fmt.Errorf("no hostname detected")
}
parts := strings.SplitN(strings.TrimPrefix(u.Path, "/"), "/", 3)
if len(parts) < 2 {

parts := strings.SplitN(strings.Trim(u.Path, "/"), "/", 3)
if len(parts) != 2 {
return nil, fmt.Errorf("invalid path: %s", u.Path)
}
return New(parts[0], strings.TrimSuffix(parts[1], ".git")), nil

return &ghRepo{
owner: parts[0],
name: strings.TrimSuffix(parts[1], ".git"),
hostname: normalizeHostname(u.Hostname()),
}, nil
}

func normalizeHostname(h string) string {
return strings.ToLower(strings.TrimPrefix(h, "www."))
}

// IsSame compares two GitHub repositories
func IsSame(a, b Interface) bool {
return strings.EqualFold(a.RepoOwner(), b.RepoOwner()) &&
strings.EqualFold(a.RepoName(), b.RepoName())
strings.EqualFold(a.RepoName(), b.RepoName()) &&
normalizeHostname(a.RepoHost()) == normalizeHostname(b.RepoHost())
}

type ghRepo struct {
owner string
name string
owner string
name string
hostname string
}

func (r ghRepo) RepoOwner() string {
return r.owner
}

func (r ghRepo) RepoName() string {
return r.name
}

func (r ghRepo) RepoHost() string {
if r.hostname != "" {
return r.hostname
}
return defaultHostname
}
58 changes: 54 additions & 4 deletions internal/ghrepo/repo_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,31 +12,78 @@ func Test_repoFromURL(t *testing.T) {
name string
input string
result string
host string
err error
}{
{
name: "github.com URL",
input: "https://github.com/monalisa/octo-cat.git",
result: "monalisa/octo-cat",
host: "github.com",
err: nil,
},
{
name: "github.com URL with trailing slash",
input: "https://github.com/monalisa/octo-cat/",
result: "monalisa/octo-cat",
host: "github.com",
err: nil,
},
{
name: "www.github.com URL",
input: "http://www.GITHUB.com/monalisa/octo-cat.git",
result: "monalisa/octo-cat",
host: "github.com",
err: nil,
},
{
name: "unsupported hostname",
input: "https://example.com/one/two",
name: "too many path components",
input: "https://github.com/monalisa/octo-cat/pulls",
result: "",
err: errors.New("unsupported hostname: example.com"),
host: "",
err: errors.New("invalid path: /monalisa/octo-cat/pulls"),
},
{
name: "non-GitHub hostname",
input: "https://example.com/one/two",
result: "one/two",
host: "example.com",
err: nil,
},
{
name: "filesystem path",
input: "/path/to/file",
result: "",
err: errors.New("unsupported hostname: "),
host: "",
err: errors.New("no hostname detected"),
},
{
name: "filesystem path with scheme",
input: "file:///path/to/file",
result: "",
host: "",
err: errors.New("no hostname detected"),
},
{
name: "github.com SSH URL",
input: "ssh://github.com/monalisa/octo-cat.git",
result: "monalisa/octo-cat",
host: "github.com",
err: nil,
},
{
name: "github.com HTTPS+SSH URL",
input: "https+ssh://github.com/monalisa/octo-cat.git",
result: "monalisa/octo-cat",
host: "github.com",
err: nil,
},
{
name: "github.com git URL",
input: "git://github.com/monalisa/octo-cat.git",
result: "monalisa/octo-cat",
host: "github.com",
err: nil,
},
}

Expand All @@ -61,6 +108,9 @@ func Test_repoFromURL(t *testing.T) {
if tt.result != got {
t.Errorf("expected %q, got %q", tt.result, got)
}
if tt.host != repo.RepoHost() {
t.Errorf("expected %q, got %q", tt.host, repo.RepoHost())
}
})
}
}

0 comments on commit 3ea71e6

Please sign in to comment.