Merge pull request #2177 from jonathanlloyd/use-canonical-capitalization-in-remotes

Clone repos using canonical username/repo name capitalization

api/queries_repo.go

@@ -88,16 +88,23 @@ func (r Repository) ViewerCanTriage() bool {
 
 func GitHubRepo(client *Client, repo ghrepo.Interface) (*Repository, error) {
 	query := `
+	fragment repo on Repository {
+		id
+		name
+		owner { login }
+		hasIssuesEnabled
+		description
+		viewerPermission
+		defaultBranchRef {
+			name
+		}
+	}
+
 	query RepositoryInfo($owner: String!, $name: String!) {
 		repository(owner: $owner, name: $name) {
-			id
-			name
-			owner { login }
-			hasIssuesEnabled
-			description
-			viewerPermission
-			defaultBranchRef {
-				name
+			...repo
+			parent {
+				...repo
 			}
 		}
 	}`

pkg/cmd/repo/clone/clone.go

@@ -82,54 +82,68 @@ func cloneRun(opts *CloneOptions) error {
 	}
 
 	apiClient := api.NewClientFromHTTP(httpClient)
-	cloneURL := opts.Repository
-	if !strings.Contains(cloneURL, ":") {
-		if !strings.Contains(cloneURL, "/") {
+
+	respositoryIsURL := strings.Contains(opts.Repository, ":")
+	repositoryIsFullName := !respositoryIsURL && strings.Contains(opts.Repository, "/")
+
+	var repo ghrepo.Interface
+	var protocol string
+	if respositoryIsURL {
+		repoURL, err := git.ParseURL(opts.Repository)
+		if err != nil {
+			return err
+		}
+		repo, err = ghrepo.FromURL(repoURL)
+		if err != nil {
+			return err
+		}
+		if repoURL.Scheme == "git+ssh" {
+			repoURL.Scheme = "ssh"
+		}
+		protocol = repoURL.Scheme
+	} else {
+		var fullName string
+		if repositoryIsFullName {
+			fullName = opts.Repository
+		} else {
 			currentUser, err := api.CurrentLoginName(apiClient, ghinstance.OverridableDefault())
 			if err != nil {
 				return err
 			}
-			cloneURL = currentUser + "/" + cloneURL
+			fullName = currentUser + "/" + opts.Repository
 		}
-		repo, err := ghrepo.FromFullName(cloneURL)
+
+		repo, err = ghrepo.FromFullName(fullName)
 		if err != nil {
 			return err
 		}
 
-		protocol, err := cfg.Get(repo.RepoHost(), "git_protocol")
-		if err != nil {
-			return err
-		}
-		cloneURL = ghrepo.FormatRemoteURL(repo, protocol)
-	}
-
-	var repo ghrepo.Interface
-	var parentRepo ghrepo.Interface
-
-	// TODO: consider caching and reusing `git.ParseSSHConfig().Translator()`
-	// here to handle hostname aliases in SSH remotes
-	if u, err := git.ParseURL(cloneURL); err == nil {
-		repo, _ = ghrepo.FromURL(u)
-	}
-
-	if repo != nil {
-		parentRepo, err = api.RepoParent(apiClient, repo)
+		protocol, err = cfg.Get(repo.RepoHost(), "git_protocol")
 		if err != nil {
 			return err
 		}
 	}
 
-	cloneDir, err := git.RunClone(cloneURL, opts.GitArgs)
+	// Load the repo from the API to get the username/repo name in its
+	// canonical capitalization
+	canonicalRepo, err := api.GitHubRepo(apiClient, repo)
+	if err != nil {
+		return err
+	}
+	canonicalCloneURL := ghrepo.FormatRemoteURL(canonicalRepo, protocol)
+
+	cloneDir, err := git.RunClone(canonicalCloneURL, opts.GitArgs)
 	if err != nil {
 		return err
 	}
 
-	if parentRepo != nil {
-		protocol, err := cfg.Get(parentRepo.RepoHost(), "git_protocol")
+	// If the repo is a fork, add the parent as an upstream
+	if canonicalRepo.Parent != nil {
+		protocol, err := cfg.Get(canonicalRepo.Parent.RepoHost(), "git_protocol")
 		if err != nil {
 			return err
 		}
-		upstreamURL := ghrepo.FormatRemoteURL(parentRepo, protocol)
+		upstreamURL := ghrepo.FormatRemoteURL(canonicalRepo.Parent, protocol)
 
 		err = git.AddUpstreamRemote(upstreamURL, cloneDir)
 		if err != nil {

pkg/cmd/repo/clone/clone_test.go

@@ -77,22 +77,30 @@ func Test_RepoClone(t *testing.T) {
 		{
 			name: "HTTPS URL",
 			args: "https://github.com/OWNER/REPO",
-			want: "git clone https://github.com/OWNER/REPO",
+			want: "git clone https://github.com/OWNER/REPO.git",
 		},
 		{
 			name: "SSH URL",
 			args: "git@github.com:OWNER/REPO.git",
 			want: "git clone git@github.com:OWNER/REPO.git",
 		},
+		{
+			name: "Non-canonical capitalization",
+			args: "Owner/Repo",
+			want: "git clone https://github.com/OWNER/REPO.git",
+		},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
 			reg := &httpmock.Registry{}
 			reg.Register(
-				httpmock.GraphQL(`query RepositoryFindParent\b`),
+				httpmock.GraphQL(`query RepositoryInfo\b`),
 				httpmock.StringResponse(`
 				{ "data": { "repository": {
-					"parent": null
+					"name": "REPO",
+					"owner": {
+						"login": "OWNER"
+					}
 				} } }
 				`))
 
@@ -120,15 +128,21 @@ func Test_RepoClone(t *testing.T) {
 func Test_RepoClone_hasParent(t *testing.T) {
 	reg := &httpmock.Registry{}
 	reg.Register(
-		httpmock.GraphQL(`query RepositoryFindParent\b`),
+		httpmock.GraphQL(`query RepositoryInfo\b`),
 		httpmock.StringResponse(`
-		{ "data": { "repository": {
-			"parent": {
-				"owner": {"login": "hubot"},
-				"name": "ORIG"
-			}
-		} } }
-		`))
+				{ "data": { "repository": {
+					"name": "REPO",
+					"owner": {
+						"login": "OWNER"
+					},
+					"parent": {
+						"name": "ORIG",
+						"owner": {
+							"login": "hubot"
+						}
+					}
+				} } }
+				`))
 
 	httpClient := &http.Client{Transport: reg}
 
@@ -155,6 +169,16 @@ func Test_RepoClone_withoutUsername(t *testing.T) {
 		{ "data": { "viewer": {
 			"login": "OWNER"
 		}}}`))
+	reg.Register(
+		httpmock.GraphQL(`query RepositoryInfo\b`),
+		httpmock.StringResponse(`
+				{ "data": { "repository": {
+					"name": "REPO",
+					"owner": {
+						"login": "OWNER"
+					}
+				} } }
+				`))
 	reg.Register(
 		httpmock.GraphQL(`query RepositoryFindParent\b`),
 		httpmock.StringResponse(`