Skip to content

Commit 8d2f455

Browse files
untitakeruntitaker
authored
Add git_protocol setting to defaults and per-domain configs (#103)
* Add git_protocol setting to defaults and per-domain configs Fix #99 * Fix ensureRemote to handle local paths on Windows * Address review feedback
1 parent 8eed915 commit 8d2f455

5 files changed

Lines changed: 275 additions & 32 deletions

File tree

‎internal/cli/pr.go‎

Lines changed: 26 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -567,7 +567,7 @@ For same-repo PRs, the branch is fetched and checked out.`,
567567
return fmt.Errorf("invalid PR number: %s", args[0])
568568
}
569569

570-
forge, owner, repoName, _, err := resolve.Repo(flagRepo, flagForgeType)
570+
forge, owner, repoName, domain, err := resolve.Repo(flagRepo, flagForgeType)
571571
if err != nil {
572572
return err
573573
}
@@ -589,7 +589,7 @@ For same-repo PRs, the branch is fetched and checked out.`,
589589
}
590590

591591
if pr.Head.Fork != nil {
592-
return checkoutForkPR(ctx, pr, remoteRef, localBranch, flagRemoteName, flagDetach, flagForce)
592+
return checkoutForkPR(ctx, domain, pr, remoteRef, localBranch, flagRemoteName, flagDetach, flagForce)
593593
}
594594

595595
return checkoutSameRepoPR(ctx, remoteRef, localBranch, flagDetach, flagForce)
@@ -603,7 +603,7 @@ For same-repo PRs, the branch is fetched and checked out.`,
603603
return cmd
604604
}
605605

606-
func checkoutForkPR(ctx context.Context, pr *forges.PullRequest, remoteRef, localBranch, flagRemoteName string, detach, force bool) error {
606+
func checkoutForkPR(ctx context.Context, domain string, pr *forges.PullRequest, remoteRef, localBranch, flagRemoteName string, detach, force bool) error {
607607
fork := pr.Head.Fork
608608
remoteName := flagRemoteName
609609
if remoteName == "" {
@@ -613,15 +613,12 @@ func checkoutForkPR(ctx context.Context, pr *forges.PullRequest, remoteRef, loca
613613
remoteName = "fork"
614614
}
615615

616-
cloneURL := fork.CloneURL
617-
if cloneURL == "" {
618-
cloneURL = fork.SSHURL
619-
}
620-
if cloneURL == "" {
616+
url := cloneURL(domain, fork.CloneURL, fork.SSHURL)
617+
if url == "" {
621618
return fmt.Errorf("no clone URL available for fork repository")
622619
}
623620

624-
remoteName, err := ensureRemote(ctx, remoteName, cloneURL)
621+
remoteName, err := ensureRemote(ctx, remoteName, url)
625622
if err != nil {
626623
return err
627624
}
@@ -638,7 +635,7 @@ func ensureRemote(ctx context.Context, preferredName, cloneURL string) (string,
638635
if err == nil {
639636
for _, line := range strings.Split(string(remotes), "\n") {
640637
fields := strings.Fields(line)
641-
if len(fields) >= 2 && fields[1] == cloneURL {
638+
if len(fields) >= 2 && remoteMatches(fields[1], cloneURL) {
642639
return fields[0], nil
643640
}
644641
}
@@ -655,13 +652,31 @@ func ensureRemote(ctx context.Context, preferredName, cloneURL string) (string,
655652
return preferredName, nil
656653
}
657654

658-
if strings.TrimSpace(string(existingURL)) == cloneURL {
655+
if remoteMatches(strings.TrimSpace(string(existingURL)), cloneURL) {
659656
return preferredName, nil
660657
}
661658

662659
return "", fmt.Errorf("remote %q already exists with a different URL; use --remote-name to specify a different name", preferredName)
663660
}
664661

662+
// remoteMatches reports whether existingURL points to the same repo as wantURL.
663+
// It first checks for an exact match, then falls back to comparing domain/owner/repo
664+
// so that SSH and HTTPS URLs for the same repository are treated as equivalent.
665+
func remoteMatches(existingURL, wantURL string) bool {
666+
if existingURL == wantURL {
667+
return true
668+
}
669+
wantDomain, wantOwner, wantRepo, err := forges.ParseRepoURL(wantURL)
670+
if err != nil {
671+
return false
672+
}
673+
domain, owner, repo, err := forges.ParseRepoURL(existingURL)
674+
if err != nil {
675+
return false
676+
}
677+
return domain == wantDomain && owner == wantOwner && repo == wantRepo
678+
}
679+
665680
func gitCheckout(ctx context.Context, remote, remoteRef, localBranch string, detach, force bool) error {
666681
refspec := fmt.Sprintf("+refs/heads/%s:refs/remotes/%s/%s", remoteRef, remote, remoteRef)
667682
fetchCmd := exec.CommandContext(ctx, "git", "fetch", "--", remote, refspec)

‎internal/cli/pr_checkout_test.go‎

Lines changed: 101 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -179,6 +179,107 @@ func pushBranchToRemote(t *testing.T, repoDir, remoteName, branchName string) {
179179
}
180180
}
181181

182+
func TestEnsureRemote(t *testing.T) {
183+
tests := []struct {
184+
name string
185+
existingURL string
186+
cloneURL string
187+
preferredName string
188+
wantRemoteName string
189+
wantErr string
190+
}{
191+
{
192+
name: "exact URL match",
193+
existingURL: "https://github.com/owner/repo.git",
194+
cloneURL: "https://github.com/owner/repo.git",
195+
preferredName: "fork",
196+
wantRemoteName: "origin",
197+
},
198+
{
199+
name: "SSH to HTTPS same repo",
200+
existingURL: "git@github.com:owner/repo.git",
201+
cloneURL: "https://github.com/owner/repo.git",
202+
preferredName: "fork",
203+
wantRemoteName: "origin",
204+
},
205+
{
206+
name: "HTTPS to SSH same repo",
207+
existingURL: "https://github.com/owner/repo.git",
208+
cloneURL: "git@github.com:owner/repo.git",
209+
preferredName: "fork",
210+
wantRemoteName: "origin",
211+
},
212+
{
213+
name: "preferred name remote matches with different URL format",
214+
existingURL: "git@github.com:contributor/repo.git",
215+
cloneURL: "https://github.com/contributor/repo.git",
216+
preferredName: "origin",
217+
wantRemoteName: "origin",
218+
},
219+
{
220+
name: "different repo adds new remote",
221+
existingURL: "https://github.com/owner/repo.git",
222+
cloneURL: "https://github.com/other/repo.git",
223+
preferredName: "other",
224+
wantRemoteName: "other",
225+
},
226+
{
227+
name: "preferred name exists with different repo",
228+
existingURL: "https://github.com/owner/repo.git",
229+
cloneURL: "https://github.com/other/repo.git",
230+
preferredName: "origin",
231+
wantErr: "already exists with a different URL",
232+
},
233+
}
234+
235+
if testing.Short() {
236+
t.Skip("skipping git integration test in short mode")
237+
}
238+
239+
for _, tt := range tests {
240+
t.Run(tt.name, func(t *testing.T) {
241+
dir := t.TempDir()
242+
243+
commands := [][]string{
244+
{"git", "init"},
245+
{"git", "config", "user.email", "test@test.com"},
246+
{"git", "config", "user.name", "Test User"},
247+
{"git", "remote", "add", "origin", tt.existingURL},
248+
}
249+
250+
for _, args := range commands {
251+
cmd := exec.Command(args[0], args[1:]...)
252+
cmd.Dir = dir
253+
if out, err := cmd.CombinedOutput(); err != nil {
254+
t.Fatalf("git command %v failed: %v\n%s", args, err, out)
255+
}
256+
}
257+
258+
t.Chdir(dir)
259+
260+
gotRemote, err := ensureRemote(context.Background(), tt.preferredName, tt.cloneURL)
261+
262+
if tt.wantErr != "" {
263+
if err == nil {
264+
t.Fatalf("expected error containing %q, got nil", tt.wantErr)
265+
}
266+
if !strings.Contains(err.Error(), tt.wantErr) {
267+
t.Errorf("expected error containing %q, got %q", tt.wantErr, err.Error())
268+
}
269+
return
270+
}
271+
272+
if err != nil {
273+
t.Fatalf("unexpected error: %v", err)
274+
}
275+
276+
if gotRemote != tt.wantRemoteName {
277+
t.Errorf("remote name: want %q, got %q", tt.wantRemoteName, gotRemote)
278+
}
279+
})
280+
}
281+
}
282+
182283
func TestPRCheckout(t *testing.T) {
183284
tests := []struct {
184285
name string

‎internal/cli/repo.go‎

Lines changed: 36 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import (
88
"strings"
99

1010
"github.com/git-pkgs/forge"
11+
"github.com/git-pkgs/forge/internal/config"
1112
"github.com/git-pkgs/forge/internal/output"
1213
"github.com/git-pkgs/forge/internal/resolve"
1314
"github.com/spf13/cobra"
@@ -38,6 +39,21 @@ func gitCloneArgs(url string, dest string, gitFlags []string) []string {
3839
return args
3940
}
4041

42+
// cloneURL returns the appropriate clone URL based on the configured git protocol.
43+
// Falls back to the other URL if the preferred one is empty.
44+
func cloneURL(domain, httpsURL, sshURL string) string {
45+
if config.GitProtocolFor(domain) == "ssh" {
46+
if sshURL != "" {
47+
return sshURL
48+
}
49+
return httpsURL
50+
}
51+
if httpsURL != "" {
52+
return httpsURL
53+
}
54+
return sshURL
55+
}
56+
4157
func init() {
4258
rootCmd.AddCommand(repoCmd)
4359
repoCmd.AddCommand(repoViewCmd())
@@ -278,11 +294,13 @@ func repoCreateCmd() *cobra.Command {
278294

279295
_, _ = fmt.Fprintf(os.Stdout, "%s\n", repo.HTMLURL)
280296

281-
if flagClone && repo.CloneURL != "" {
282-
cloneCmd := exec.CommandContext(cmd.Context(), "git", gitCloneArgs(repo.CloneURL, "", nil)...)
283-
cloneCmd.Stdout = os.Stdout
284-
cloneCmd.Stderr = os.Stderr
285-
return cloneCmd.Run()
297+
if flagClone {
298+
if url := cloneURL(domain, repo.CloneURL, repo.SSHURL); url != "" {
299+
cloneCmd := exec.CommandContext(cmd.Context(), "git", gitCloneArgs(url, "", nil)...)
300+
cloneCmd.Stdout = os.Stdout
301+
cloneCmd.Stderr = os.Stderr
302+
return cloneCmd.Run()
303+
}
286304
}
287305

288306
return nil
@@ -424,7 +442,7 @@ func repoForkCmd() *cobra.Command {
424442
repo = args[0]
425443
}
426444

427-
forge, owner, repoName, _, err := resolve.Repo(repo, flagForgeType)
445+
forge, owner, repoName, domain, err := resolve.Repo(repo, flagForgeType)
428446
if err != nil {
429447
return err
430448
}
@@ -446,11 +464,13 @@ func repoForkCmd() *cobra.Command {
446464

447465
_, _ = fmt.Fprintf(os.Stdout, "%s\n", r.HTMLURL)
448466

449-
if flagClone && r.CloneURL != "" {
450-
cloneCmd := exec.CommandContext(cmd.Context(), "git", gitCloneArgs(r.CloneURL, "", nil)...)
451-
cloneCmd.Stdout = os.Stdout
452-
cloneCmd.Stderr = os.Stderr
453-
return cloneCmd.Run()
467+
if flagClone {
468+
if url := cloneURL(domain, r.CloneURL, r.SSHURL); url != "" {
469+
cloneCmd := exec.CommandContext(cmd.Context(), "git", gitCloneArgs(url, "", nil)...)
470+
cloneCmd.Stdout = os.Stdout
471+
cloneCmd.Stderr = os.Stderr
472+
return cloneCmd.Run()
473+
}
454474
}
455475

456476
return nil
@@ -490,7 +510,7 @@ func repoCloneCmd() *cobra.Command {
490510
gitFlags = args[dashIdx:]
491511
}
492512

493-
forge, owner, repoName, _, err := resolve.Repo(repoArg, flagForgeType)
513+
forge, owner, repoName, domain, err := resolve.Repo(repoArg, flagForgeType)
494514
if err != nil {
495515
return err
496516
}
@@ -500,12 +520,12 @@ func repoCloneCmd() *cobra.Command {
500520
return err
501521
}
502522

503-
cloneURL := r.CloneURL
504-
if cloneURL == "" {
505-
cloneURL = r.HTMLURL + ".git"
523+
url := cloneURL(domain, r.CloneURL, r.SSHURL)
524+
if url == "" {
525+
url = r.HTMLURL + ".git"
506526
}
507527

508-
cloneCmd := exec.CommandContext(cmd.Context(), "git", gitCloneArgs(cloneURL, dest, gitFlags)...)
528+
cloneCmd := exec.CommandContext(cmd.Context(), "git", gitCloneArgs(url, dest, gitFlags)...)
509529
cloneCmd.Stdout = os.Stdout
510530
cloneCmd.Stderr = os.Stderr
511531
return cloneCmd.Run()

‎internal/config/config.go‎

Lines changed: 48 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -23,14 +23,16 @@ type Config struct {
2323
}
2424

2525
type DefaultSection struct {
26-
Output string // table, json, plain
27-
ForgeType string // default forge type
26+
Output string // table, json, plain
27+
ForgeType string // default forge type
28+
GitProtocol string // https or ssh
2829
}
2930

3031
type DomainSection struct {
31-
Type string // github, gitlab, gitea, forgejo, bitbucket
32-
Token string // only from user config, never .forge
33-
SSHHost string // alternate host for git-over-ssh; the section name remains the API host
32+
Type string // github, gitlab, gitea, forgejo, bitbucket
33+
Token string // only from user config, never .forge
34+
SSHHost string // alternate host for git-over-ssh; the section name remains the API host
35+
GitProtocol string // https or ssh; overrides default
3436
}
3537

3638
// DomainForSSHHost returns the API domain (the section name) whose ssh_host
@@ -65,6 +67,33 @@ func Load() (*Config, error) {
6567
return cached, cacheErr
6668
}
6769

70+
// GitProtocolFor returns the configured git protocol ("https" or "ssh") for the given domain.
71+
// Falls back to the default if not set for the domain. Returns "https" if not configured.
72+
func GitProtocolFor(domain string) string {
73+
cfg, err := Load()
74+
if err != nil || cfg == nil {
75+
return "https"
76+
}
77+
if ds, ok := cfg.Domains[domain]; ok && ds.GitProtocol != "" {
78+
return ds.GitProtocol
79+
}
80+
if cfg.Default.GitProtocol != "" {
81+
return cfg.Default.GitProtocol
82+
}
83+
return "https"
84+
}
85+
86+
func parseGitProtocol(v string) (string, error) {
87+
switch strings.ToLower(v) {
88+
case "ssh":
89+
return "ssh", nil
90+
case "https":
91+
return "https", nil
92+
default:
93+
return "", fmt.Errorf("invalid git_protocol %q: must be \"https\" or \"ssh\"", v)
94+
}
95+
}
96+
6897
// ResetCache clears the cached config. Only useful in tests.
6998
func ResetCache() {
7099
once = sync.Once{}
@@ -116,6 +145,13 @@ func loadFile(cfg *Config, path string, allowTokens bool) error {
116145
if v, ok := def["forge-type"]; ok {
117146
cfg.Default.ForgeType = v
118147
}
148+
if v, ok := def["git_protocol"]; ok {
149+
p, err := parseGitProtocol(v)
150+
if err != nil {
151+
return fmt.Errorf("%s: [default] %w", path, err)
152+
}
153+
cfg.Default.GitProtocol = p
154+
}
119155
}
120156

121157
for name, kv := range sections {
@@ -126,6 +162,13 @@ func loadFile(cfg *Config, path string, allowTokens bool) error {
126162
if v, ok := kv["type"]; ok {
127163
ds.Type = v
128164
}
165+
if v, ok := kv["git_protocol"]; ok {
166+
p, err := parseGitProtocol(v)
167+
if err != nil {
168+
return fmt.Errorf("%s: [%s] %w", path, name, err)
169+
}
170+
ds.GitProtocol = p
171+
}
129172
if allowTokens {
130173
if v, ok := kv["ssh_host"]; ok {
131174
ds.SSHHost = v

0 commit comments

Comments
 (0)