Skip to content

Commit

Permalink
respect ssh
Browse files Browse the repository at this point in the history
this adds recognition of the git_protocol setting when:

- creating a repo
- cloning a repo
- forking a repo
- forking/pushing during pr create
- checking out a PR

additionally, it:

- consolidates remote adding to use AddRemote; this introduces a fetch
where there previously hadn't been one
- changes repo clone to accept an ssh url
- changes repo fork to accept an ssh url

i just added basic unit tests; adding new test cases for all of the
above scenarios seemed like diminishing returns.
  • Loading branch information
vilmibm committed Apr 22, 2020
1 parent 80d7513 commit bec58ed
Show file tree
Hide file tree
Showing 7 changed files with 94 additions and 28 deletions.
4 changes: 2 additions & 2 deletions command/pr_checkout.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ func prCheckout(cmd *cobra.Command, args []string) error {

baseRemote, _ := remotes.FindByRepo(baseRepo.RepoOwner(), baseRepo.RepoName())
// baseRemoteSpec is a repository URL or a remote name to be used in git fetch
baseURLOrName := fmt.Sprintf("https://github.com/%s.git", ghrepo.FullName(baseRepo))
baseURLOrName := formatRemoteURL(cmd, ghrepo.FullName(baseRepo))
if baseRemote != nil {
baseURLOrName = baseRemote.Name
}
Expand Down Expand Up @@ -97,7 +97,7 @@ func prCheckout(cmd *cobra.Command, args []string) error {
remote := baseURLOrName
mergeRef := ref
if pr.MaintainerCanModify {
remote = fmt.Sprintf("https://github.com/%s/%s.git", pr.HeadRepositoryOwner.Login, pr.HeadRepository.Name)
remote = formatRemoteURL(cmd, fmt.Sprintf("%s/%s", pr.HeadRepositoryOwner.Login, pr.HeadRepository.Name))
mergeRef = fmt.Sprintf("refs/heads/%s", pr.HeadRefName)
}
if mc, err := git.Config(fmt.Sprintf("branch.%s.merge", newBranchName)); err != nil || mc == "" {
Expand Down
4 changes: 2 additions & 2 deletions command/pr_create.go
Original file line number Diff line number Diff line change
Expand Up @@ -250,8 +250,8 @@ func prCreate(cmd *cobra.Command, _ []string) error {
// In either case, we want to add the head repo as a new git remote so we
// can push to it.
if headRemote == nil {
// TODO: support non-HTTPS git remote URLs
headRepoURL := fmt.Sprintf("https://github.com/%s.git", ghrepo.FullName(headRepo))
headRepoURL := formatRemoteURL(cmd, ghrepo.FullName(headRepo))

// TODO: prevent clashes with another remote of a same name
gitRemote, err := git.AddRemote("fork", headRepoURL)
if err != nil {
Expand Down
32 changes: 19 additions & 13 deletions command/repo.go
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,7 @@ func runClone(cloneURL string, args []string) (target string, err error) {
func repoClone(cmd *cobra.Command, args []string) error {
cloneURL := args[0]
if !strings.Contains(cloneURL, ":") {
cloneURL = fmt.Sprintf("https://github.com/%s.git", cloneURL)
cloneURL = formatRemoteURL(cmd, cloneURL)
}

var repo ghrepo.Interface
Expand Down Expand Up @@ -158,7 +158,7 @@ func repoClone(cmd *cobra.Command, args []string) error {
}

if parentRepo != nil {
err := addUpstreamRemote(parentRepo, cloneDir)
err := addUpstreamRemote(cmd, parentRepo, cloneDir)
if err != nil {
return err
}
Expand All @@ -167,9 +167,8 @@ func repoClone(cmd *cobra.Command, args []string) error {
return nil
}

func addUpstreamRemote(parentRepo ghrepo.Interface, cloneDir string) error {
// TODO: support SSH remote URLs
upstreamURL := fmt.Sprintf("https://github.com/%s.git", ghrepo.FullName(parentRepo))
func addUpstreamRemote(cmd *cobra.Command, parentRepo ghrepo.Interface, cloneDir string) error {
upstreamURL := formatRemoteURL(cmd, ghrepo.FullName(parentRepo))

cloneCmd := git.GitCommand("-C", cloneDir, "remote", "add", "-f", "upstream", upstreamURL)
cloneCmd.Stdout = os.Stdout
Expand Down Expand Up @@ -267,14 +266,10 @@ func repoCreate(cmd *cobra.Command, args []string) error {
fmt.Fprintln(out, repo.URL)
}

remoteURL := repo.URL + ".git"
remoteURL := formatRemoteURL(cmd, ghrepo.FullName(repo))

if projectDirErr == nil {
// TODO: use git.AddRemote
remoteAdd := git.GitCommand("remote", "add", "origin", remoteURL)
remoteAdd.Stdout = os.Stdout
remoteAdd.Stderr = os.Stderr
err = run.PrepareCmd(remoteAdd).Run()
_, err = git.AddRemote("origin", remoteURL)
if err != nil {
return err
}
Expand Down Expand Up @@ -361,6 +356,15 @@ func repoFork(cmd *cobra.Command, args []string) error {
return fmt.Errorf("did not understand argument: %w", err)
}

} else if strings.HasPrefix(repoArg, "git@") {
parsedURL, err := git.ParseURL(repoArg)
if err != nil {
return fmt.Errorf("did not understand argument: %w", err)
}
toFork, err = ghrepo.FromURL(parsedURL)
if err != nil {
return fmt.Errorf("did not understand argument: %w", err)
}
} else {
toFork = ghrepo.FromFullName(repoArg)
if toFork.RepoName() == "" || toFork.RepoOwner() == "" {
Expand Down Expand Up @@ -437,7 +441,9 @@ func repoFork(cmd *cobra.Command, args []string) error {
fmt.Fprintf(out, "%s Renamed %s remote to %s\n", greenCheck, utils.Bold(remoteName), utils.Bold(renameTarget))
}

_, err = git.AddRemote(remoteName, forkedRepo.CloneURL)
forkedRepoCloneURL := formatRemoteURL(cmd, ghrepo.FullName(forkedRepo))

_, err = git.AddRemote(remoteName, forkedRepoCloneURL)
if err != nil {
return fmt.Errorf("failed to add remote: %w", err)
}
Expand All @@ -458,7 +464,7 @@ func repoFork(cmd *cobra.Command, args []string) error {
return fmt.Errorf("failed to clone fork: %w", err)
}

err = addUpstreamRemote(toFork, cloneDir)
err = addUpstreamRemote(cmd, toFork, cloneDir)
if err != nil {
return err
}
Expand Down
28 changes: 20 additions & 8 deletions command/repo_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,7 @@ func TestRepoFork_in_parent_yes(t *testing.T) {

expectedCmds := []string{
"git remote rename origin upstream",
"git remote add -f origin https://github.com/someone/repo.git",
"git remote add -f origin https://github.com/someone/REPO.git",
}

for x, cmd := range seenCmds {
Expand Down Expand Up @@ -287,7 +287,7 @@ func TestRepoFork_in_parent_survey_yes(t *testing.T) {

expectedCmds := []string{
"git remote rename origin upstream",
"git remote add -f origin https://github.com/someone/repo.git",
"git remote add -f origin https://github.com/someone/REPO.git",
}

for x, cmd := range seenCmds {
Expand Down Expand Up @@ -499,7 +499,11 @@ func TestRepoCreate(t *testing.T) {
{ "data": { "createRepository": {
"repository": {
"id": "REPOID",
"url": "https://github.com/OWNER/REPO"
"url": "https://github.com/OWNER/REPO",
"name": "REPO",
"owner": {
"login": "OWNER"
}
}
} } }
`))
Expand All @@ -522,7 +526,7 @@ func TestRepoCreate(t *testing.T) {
if seenCmd == nil {
t.Fatal("expected a command to run")
}
eq(t, strings.Join(seenCmd.Args, " "), "git remote add origin https://github.com/OWNER/REPO.git")
eq(t, strings.Join(seenCmd.Args, " "), "git remote add -f origin https://github.com/OWNER/REPO.git")

var reqBody struct {
Query string
Expand Down Expand Up @@ -564,7 +568,11 @@ func TestRepoCreate_org(t *testing.T) {
{ "data": { "createRepository": {
"repository": {
"id": "REPOID",
"url": "https://github.com/ORG/REPO"
"url": "https://github.com/ORG/REPO",
"name": "REPO",
"owner": {
"login": "ORG"
}
}
} } }
`))
Expand All @@ -587,7 +595,7 @@ func TestRepoCreate_org(t *testing.T) {
if seenCmd == nil {
t.Fatal("expected a command to run")
}
eq(t, strings.Join(seenCmd.Args, " "), "git remote add origin https://github.com/ORG/REPO.git")
eq(t, strings.Join(seenCmd.Args, " "), "git remote add -f origin https://github.com/ORG/REPO.git")

var reqBody struct {
Query string
Expand Down Expand Up @@ -629,7 +637,11 @@ func TestRepoCreate_orgWithTeam(t *testing.T) {
{ "data": { "createRepository": {
"repository": {
"id": "REPOID",
"url": "https://github.com/ORG/REPO"
"url": "https://github.com/ORG/REPO",
"name": "REPO",
"owner": {
"login": "ORG"
}
}
} } }
`))
Expand All @@ -652,7 +664,7 @@ func TestRepoCreate_orgWithTeam(t *testing.T) {
if seenCmd == nil {
t.Fatal("expected a command to run")
}
eq(t, strings.Join(seenCmd.Args, " "), "git remote add origin https://github.com/ORG/REPO.git")
eq(t, strings.Join(seenCmd.Args, " "), "git remote add -f origin https://github.com/ORG/REPO.git")

var reqBody struct {
Query string
Expand Down
19 changes: 19 additions & 0 deletions command/root.go
Original file line number Diff line number Diff line change
Expand Up @@ -222,3 +222,22 @@ func determineBaseRepo(cmd *cobra.Command, ctx context.Context) (ghrepo.Interfac

return baseRepo, nil
}

func formatRemoteURL(cmd *cobra.Command, fullRepoName string) string {
ctx := contextForCommand(cmd)

protocol := "https"
cfg, err := ctx.Config()
if err != nil {
fmt.Fprintf(colorableErr(cmd), "%s failed to load config: %s. using defaults\n", utils.Yellow("!"), err)
} else {
cfgProtocol, _ := cfg.Get(defaultHostname, "git_protocol")
protocol = cfgProtocol
}

if protocol == "ssh" {
return fmt.Sprintf("git@%s:%s.git", defaultHostname, fullRepoName)
}

return fmt.Sprintf("https://%s/%s.git", defaultHostname, fullRepoName)
}
19 changes: 19 additions & 0 deletions command/root_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,3 +40,22 @@ func TestChangelogURL(t *testing.T) {
t.Errorf("expected %s to create url %s but got %s", tag, url, result)
}
}

func TestRemoteURLFormatting_no_config(t *testing.T) {
initBlankContext("", "OWNER/REPO", "master")
result := formatRemoteURL(repoForkCmd, "OWNER/REPO")
eq(t, result, "https://github.com/OWNER/REPO.git")
}

func TestRemoteURLFormatting_ssh_config(t *testing.T) {
cfg := `---
hosts:
github.com:
user: OWNER
oauth_token: MUSTBEHIGHCUZIMATOKEN
git_protocol: ssh
`
initBlankContext(cfg, "OWNER/REPO", "master")
result := formatRemoteURL(repoForkCmd, "OWNER/REPO")
eq(t, result, "[email protected]:OWNER/REPO.git")
}
16 changes: 13 additions & 3 deletions git/remote.go
Original file line number Diff line number Diff line change
Expand Up @@ -79,9 +79,19 @@ func AddRemote(name, u string) (*Remote, error) {
return nil, err
}

urlParsed, err := url.Parse(u)
if err != nil {
return nil, err
var urlParsed *url.URL
if strings.HasPrefix(u, "https") {
urlParsed, err = url.Parse(u)
if err != nil {
return nil, err
}

} else {
urlParsed, err = ParseURL(u)
if err != nil {
return nil, err
}

}

return &Remote{
Expand Down

0 comments on commit bec58ed

Please sign in to comment.