Skip to content

Commit becb316

Browse files
samcoesamcoe
authored
Merge pull request cli#2177 from jonathanlloyd/use-canonical-capitalization-in-remotes
Clone repos using canonical username/repo name capitalization
2 parents 869f511 + 8f44aee commit becb316

File tree

3 files changed

+89
-44
lines changed

3 files changed

+89
-44
lines changed

‎api/queries_repo.go‎

Lines changed: 15 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -88,16 +88,23 @@ func (r Repository) ViewerCanTriage() bool {
8888

8989
func GitHubRepo(client *Client, repo ghrepo.Interface) (*Repository, error) {
9090
query := `
91+
fragment repo on Repository {
92+
id
93+
name
94+
owner { login }
95+
hasIssuesEnabled
96+
description
97+
viewerPermission
98+
defaultBranchRef {
99+
name
100+
}
101+
}
102+
91103
query RepositoryInfo($owner: String!, $name: String!) {
92104
repository(owner: $owner, name: $name) {
93-
id
94-
name
95-
owner { login }
96-
hasIssuesEnabled
97-
description
98-
viewerPermission
99-
defaultBranchRef {
100-
name
105+
...repo
106+
parent {
107+
...repo
101108
}
102109
}
103110
}`

‎pkg/cmd/repo/clone/clone.go‎

Lines changed: 39 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -82,54 +82,68 @@ func cloneRun(opts *CloneOptions) error {
8282
}
8383

8484
apiClient := api.NewClientFromHTTP(httpClient)
85-
cloneURL := opts.Repository
86-
if !strings.Contains(cloneURL, ":") {
87-
if !strings.Contains(cloneURL, "/") {
85+
86+
respositoryIsURL := strings.Contains(opts.Repository, ":")
87+
repositoryIsFullName := !respositoryIsURL && strings.Contains(opts.Repository, "/")
88+
89+
var repo ghrepo.Interface
90+
var protocol string
91+
if respositoryIsURL {
92+
repoURL, err := git.ParseURL(opts.Repository)
93+
if err != nil {
94+
return err
95+
}
96+
repo, err = ghrepo.FromURL(repoURL)
97+
if err != nil {
98+
return err
99+
}
100+
if repoURL.Scheme == "git+ssh" {
101+
repoURL.Scheme = "ssh"
102+
}
103+
protocol = repoURL.Scheme
104+
} else {
105+
var fullName string
106+
if repositoryIsFullName {
107+
fullName = opts.Repository
108+
} else {
88109
currentUser, err := api.CurrentLoginName(apiClient, ghinstance.OverridableDefault())
89110
if err != nil {
90111
return err
91112
}
92-
cloneURL = currentUser + "/" + cloneURL
113+
fullName = currentUser + "/" + opts.Repository
93114
}
94-
repo, err := ghrepo.FromFullName(cloneURL)
115+
116+
repo, err = ghrepo.FromFullName(fullName)
95117
if err != nil {
96118
return err
97119
}
98120

99-
protocol, err := cfg.Get(repo.RepoHost(), "git_protocol")
121+
protocol, err = cfg.Get(repo.RepoHost(), "git_protocol")
100122
if err != nil {
101123
return err
102124
}
103-
cloneURL = ghrepo.FormatRemoteURL(repo, protocol)
104-
}
105-
106-
var repo ghrepo.Interface
107-
var parentRepo ghrepo.Interface
108-
109-
// TODO: consider caching and reusing `git.ParseSSHConfig().Translator()`
110-
// here to handle hostname aliases in SSH remotes
111-
if u, err := git.ParseURL(cloneURL); err == nil {
112-
repo, _ = ghrepo.FromURL(u)
113125
}
114126

115-
if repo != nil {
116-
parentRepo, err = api.RepoParent(apiClient, repo)
117-
if err != nil {
118-
return err
119-
}
127+
// Load the repo from the API to get the username/repo name in its
128+
// canonical capitalization
129+
canonicalRepo, err := api.GitHubRepo(apiClient, repo)
130+
if err != nil {
131+
return err
120132
}
133+
canonicalCloneURL := ghrepo.FormatRemoteURL(canonicalRepo, protocol)
121134

122-
cloneDir, err := git.RunClone(cloneURL, opts.GitArgs)
135+
cloneDir, err := git.RunClone(canonicalCloneURL, opts.GitArgs)
123136
if err != nil {
124137
return err
125138
}
126139

127-
if parentRepo != nil {
128-
protocol, err := cfg.Get(parentRepo.RepoHost(), "git_protocol")
140+
// If the repo is a fork, add the parent as an upstream
141+
if canonicalRepo.Parent != nil {
142+
protocol, err := cfg.Get(canonicalRepo.Parent.RepoHost(), "git_protocol")
129143
if err != nil {
130144
return err
131145
}
132-
upstreamURL := ghrepo.FormatRemoteURL(parentRepo, protocol)
146+
upstreamURL := ghrepo.FormatRemoteURL(canonicalRepo.Parent, protocol)
133147

134148
err = git.AddUpstreamRemote(upstreamURL, cloneDir)
135149
if err != nil {

‎pkg/cmd/repo/clone/clone_test.go‎

Lines changed: 35 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -77,22 +77,30 @@ func Test_RepoClone(t *testing.T) {
7777
{
7878
name: "HTTPS URL",
7979
args: "https://github.com/OWNER/REPO",
80-
want: "git clone https://github.com/OWNER/REPO",
80+
want: "git clone https://github.com/OWNER/REPO.git",
8181
},
8282
{
8383
name: "SSH URL",
8484
args: "git@github.com:OWNER/REPO.git",
8585
want: "git clone git@github.com:OWNER/REPO.git",
8686
},
87+
{
88+
name: "Non-canonical capitalization",
89+
args: "Owner/Repo",
90+
want: "git clone https://github.com/OWNER/REPO.git",
91+
},
8792
}
8893
for _, tt := range tests {
8994
t.Run(tt.name, func(t *testing.T) {
9095
reg := &httpmock.Registry{}
9196
reg.Register(
92-
httpmock.GraphQL(`query RepositoryFindParent\b`),
97+
httpmock.GraphQL(`query RepositoryInfo\b`),
9398
httpmock.StringResponse(`
9499
{ "data": { "repository": {
95-
"parent": null
100+
"name": "REPO",
101+
"owner": {
102+
"login": "OWNER"
103+
}
96104
} } }
97105
`))
98106

@@ -120,15 +128,21 @@ func Test_RepoClone(t *testing.T) {
120128
func Test_RepoClone_hasParent(t *testing.T) {
121129
reg := &httpmock.Registry{}
122130
reg.Register(
123-
httpmock.GraphQL(`query RepositoryFindParent\b`),
131+
httpmock.GraphQL(`query RepositoryInfo\b`),
124132
httpmock.StringResponse(`
125-
{ "data": { "repository": {
126-
"parent": {
127-
"owner": {"login": "hubot"},
128-
"name": "ORIG"
129-
}
130-
} } }
131-
`))
133+
{ "data": { "repository": {
134+
"name": "REPO",
135+
"owner": {
136+
"login": "OWNER"
137+
},
138+
"parent": {
139+
"name": "ORIG",
140+
"owner": {
141+
"login": "hubot"
142+
}
143+
}
144+
} } }
145+
`))
132146

133147
httpClient := &http.Client{Transport: reg}
134148

@@ -155,6 +169,16 @@ func Test_RepoClone_withoutUsername(t *testing.T) {
155169
{ "data": { "viewer": {
156170
"login": "OWNER"
157171
}}}`))
172+
reg.Register(
173+
httpmock.GraphQL(`query RepositoryInfo\b`),
174+
httpmock.StringResponse(`
175+
{ "data": { "repository": {
176+
"name": "REPO",
177+
"owner": {
178+
"login": "OWNER"
179+
}
180+
} } }
181+
`))
158182
reg.Register(
159183
httpmock.GraphQL(`query RepositoryFindParent\b`),
160184
httpmock.StringResponse(`

0 commit comments

Comments
 (0)