diff --git a/git/client.go b/git/client.go index d2919ad22..f1b357634 100644 --- a/git/client.go +++ b/git/client.go @@ -95,14 +95,36 @@ func (c *Client) Command(ctx context.Context, args ...string) (*Command, error) return &Command{cmd}, nil } +// CredentialPattern is used to indicate to AuthenticatedCommand which patterns git should match +// against when trying to find credentials. It is a little overengineered as a type because we +// want AuthenticatedCommand to have a clear complication error when this is not provided, +// as opposed to using a string which might compile with `client.AuthenticatedCommand(ctx, "fetch")`. +// +// It is only usable when constructed by another function in the package because the empty pattern, +// without insecure set to true, will result in an error in AuthenticatedCommand. +// +// Callers can currently opt-in to an insecure mode for backwards compatability by using +// InsecureAllMatchingCredentialsPattern. type CredentialPattern struct { insecure bool // should only be constructable via InsecureAllMatchingCredentialPattern pattern string } +// InsecureAllMatchingCredentialsPattern allows for opting in to an insecure mode for backwards compatability. var InsecureAllMatchingCredentialsPattern = CredentialPattern{insecure: true, pattern: ""} var disallowedCredentialPattern = CredentialPattern{insecure: false, pattern: ""} +// WM-TODO: Should this handle command modifiers, e.g. RepoDir being set in Clone +// WM-TODO: Are there any funny remotes that might not resolve to a URL? +// WM-TODO: This should probably have its own tests +func CredentialPatternFromRemote(ctx context.Context, c *Client, remote string) (CredentialPattern, error) { + gitURL, err := c.GetRemoteURL(ctx, remote) + if err != nil { + return CredentialPattern{}, err + } + return CredentialPatternFromGitURL(gitURL) +} + // AuthenticatedCommand is a wrapper around Command that included configuration to use gh // as the credential helper for git. func (c *Client) AuthenticatedCommand(ctx context.Context, credentialPattern CredentialPattern, args ...string) (*Command, error) { @@ -652,17 +674,6 @@ func (c *Client) Clone(ctx context.Context, cloneURL string, args []string, mods return target, nil } -// WM-TODO: Bit of a weird method to hang off the client? -// WM-TODO: We need to make sure this handles command modifiers everywhere... -// WM-TODO: Are there any funny refspec usages that might not resolve via get-url -func (c *Client) CredentialPatternFromRemote(ctx context.Context, remote string) (CredentialPattern, error) { - gitURL, err := c.GetRemoteURL(ctx, remote) - if err != nil { - return CredentialPattern{}, err - } - return CredentialPatternFromGitURL(gitURL) -} - func resolveGitPath() (string, error) { path, err := safeexec.LookPath("git") if err != nil { diff --git a/git/client_test.go b/git/client_test.go index 614f05995..9cacf4e65 100644 --- a/git/client_test.go +++ b/git/client_test.go @@ -1499,17 +1499,16 @@ func TestCommandMocking(t *testing.T) { return } - // WM-TODO: maybe don't use 255, I only picked it because it was distinct jsonVar, ok := os.LookupEnv("GH_HELPER_PROCESS_RICH_COMMANDS") if !ok { fmt.Fprint(os.Stderr, "missing GH_HELPER_PROCESS_RICH_COMMANDS") - os.Exit(255) + os.Exit(1) } var commands mockedCommands if err := json.Unmarshal([]byte(jsonVar), &commands); err != nil { fmt.Fprint(os.Stderr, "failed to unmarshal GH_HELPER_PROCESS_RICH_COMMANDS") - os.Exit(255) + os.Exit(1) } // The discarded args are those for the go test binary itself, e.g. `-test.run=TestHelperProcessRich` @@ -1518,12 +1517,17 @@ func TestCommandMocking(t *testing.T) { commandResult, ok := commands[args(strings.Join(realArgs, " "))] if !ok { fmt.Fprintf(os.Stderr, "unexpected command: %s\n", strings.Join(realArgs, " ")) - os.Exit(255) + os.Exit(1) + } + + if commandResult.Stdout != "" { + fmt.Fprint(os.Stdout, commandResult.Stdout) + } + + if commandResult.Stderr != "" { + fmt.Fprint(os.Stderr, commandResult.Stderr) } - // WM-TODO: maybe pointer on these fields, or only print if not-empty - fmt.Fprint(os.Stdout, commandResult.Stdout) - fmt.Fprint(os.Stderr, commandResult.Stderr) os.Exit(commandResult.ExitStatus) } diff --git a/pkg/cmd/pr/checkout/checkout.go b/pkg/cmd/pr/checkout/checkout.go index a4cb405c1..d7243af60 100644 --- a/pkg/cmd/pr/checkout/checkout.go +++ b/pkg/cmd/pr/checkout/checkout.go @@ -133,7 +133,7 @@ func checkoutRun(opts *CheckoutOptions) error { // Note that although we will probably be fetching from the headRemote, in practice, PR checkout can only // ever point to one host, and we know baseRemote must be populated, where headRemote might be nil (e.g. when // it was deleted). - credentialPattern, err := opts.GitClient.CredentialPatternFromRemote(context.Background(), baseRemote.Name) + credentialPattern, err := git.CredentialPatternFromRemote(context.Background(), opts.GitClient, baseRemote.Name) if err != nil { return err }