Skip to content

respect git_protocol setting #810

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
vilmibm merged 2 commits into from
Apr 22, 2020
Merged

respect git_protocol setting #810

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
bec58ed
respect ssh
vilmibm Apr 20, 2020
f1bee0c
use more clear name
vilmibm Apr 22, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions command/pr_checkout.go
View file
Open in desktop
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))

@probablycorey probablycorey Apr 22, 2020

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 this is nice

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
View file
Open in desktop
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
46 changes: 26 additions & 20 deletions command/repo.go
View file
Open in desktop
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)

@probablycorey probablycorey Apr 22, 2020

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍👍this is easier to understand now

if err != nil {
return err
}
Expand Down Expand Up @@ -338,15 +333,15 @@ func repoFork(cmd *cobra.Command, args []string) error {
return fmt.Errorf("unable to create client: %w", err)
}

var toFork ghrepo.Interface
var repoToFork ghrepo.Interface
inParent := false // whether or not we're forking the repo we're currently "in"
if len(args) == 0 {
baseRepo, err := determineBaseRepo(cmd, ctx)
if err != nil {
return fmt.Errorf("unable to determine base repository: %w", err)
}
inParent = true
toFork = baseRepo
repoToFork = baseRepo
} else {
repoArg := args[0]

Expand All @@ -356,14 +351,23 @@ func repoFork(cmd *cobra.Command, args []string) error {
return fmt.Errorf("did not understand argument: %w", err)
}

toFork, err = ghrepo.FromURL(parsedURL)
repoToFork, err = ghrepo.FromURL(parsedURL)
if err != nil {
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)
}
repoToFork, 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() == "" {
repoToFork = ghrepo.FromFullName(repoArg)
if repoToFork.RepoName() == "" || repoToFork.RepoOwner() == "" {
return fmt.Errorf("could not parse owner or repo name from %s", repoArg)
}
}
Expand All @@ -372,12 +376,12 @@ func repoFork(cmd *cobra.Command, args []string) error {
greenCheck := utils.Green("✓")
out := colorableOut(cmd)
s := utils.Spinner(out)
loading := utils.Gray("Forking ") + utils.Bold(utils.Gray(ghrepo.FullName(toFork))) + utils.Gray("...")
loading := utils.Gray("Forking ") + utils.Bold(utils.Gray(ghrepo.FullName(repoToFork))) + utils.Gray("...")
s.Suffix = " " + loading
s.FinalMSG = utils.Gray(fmt.Sprintf("- %s\n", loading))
s.Start()

forkedRepo, err := api.ForkRepo(apiClient, toFork)
forkedRepo, err := api.ForkRepo(apiClient, repoToFork)
if err != nil {
s.Stop()
return fmt.Errorf("failed to fork: %w", err)
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, repoToFork, cloneDir)
if err != nil {
return err
}
Expand Down
28 changes: 20 additions & 8 deletions command/repo_test.go
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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, "git@github.com:OWNER/REPO.git")
}
16 changes: 13 additions & 3 deletions git/remote.go
View file
Open in desktop
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