diff --git a/acceptance/testdata/pr/pr-checkout-worktree-detach.txtar b/acceptance/testdata/pr/pr-checkout-worktree-detach.txtar new file mode 100644 index 00000000000..c0feec4e2e4 --- /dev/null +++ b/acceptance/testdata/pr/pr-checkout-worktree-detach.txtar @@ -0,0 +1,56 @@ +# Checkout a PR into a worktree with a detached HEAD, then reuse that worktree. + +# Set up env vars +env REPO=${SCRIPT_NAME}-${RANDOM_STRING} + +# Use gh as a credential helper +exec gh auth setup-git + +# Create a repository with a file so it has a default branch +exec gh repo create ${ORG}/${REPO} --add-readme --private + +# Defer repo cleanup +defer gh repo delete --yes ${ORG}/${REPO} + +# Clone the repo +exec gh repo clone ${ORG}/${REPO} + +# Prepare a branch to PR +cd ${REPO} +exec git checkout -b feature-branch +exec git commit --allow-empty -m 'Empty Commit' +exec git push -u origin feature-branch + +# Create the PR +exec gh pr create --title 'Feature Title' --body 'Feature Body' +stdout2env PR_URL + +# Return to the default branch +exec git checkout main + +# Checkout the PR into a fresh worktree with a detached HEAD +exec gh pr checkout ${PR_URL} --detach --worktree ../wt +exists ../wt + +# The worktree HEAD is detached, so it has no symbolic ref +! exec git -C ../wt symbolic-ref -q HEAD + +# The detached HEAD points at the PR head commit +exec git -C ../wt rev-parse HEAD +stdout2env WT_HEAD +exec git rev-parse origin/feature-branch +stdout ${WT_HEAD} + +# The main working copy is left untouched +exec git rev-parse --abbrev-ref HEAD +stdout '(?m)^main$' + +# Checking out again into the same worktree reuses it and stays detached +exec gh pr checkout ${PR_URL} --detach --worktree ../wt +! exec git -C ../wt symbolic-ref -q HEAD +exec git -C ../wt rev-parse HEAD +stdout ${WT_HEAD} + +# The main working copy is left untouched +exec git rev-parse --abbrev-ref HEAD +stdout '(?m)^main$' diff --git a/acceptance/testdata/pr/pr-checkout-worktree-from-fork.txtar b/acceptance/testdata/pr/pr-checkout-worktree-from-fork.txtar new file mode 100644 index 00000000000..8d9fc89cc2e --- /dev/null +++ b/acceptance/testdata/pr/pr-checkout-worktree-from-fork.txtar @@ -0,0 +1,55 @@ +# Checkout a fork PR whose head repository is not configured as a remote into a +# worktree, then reuse that worktree. + +# Set up env vars +env REPO=${SCRIPT_NAME}-${RANDOM_STRING} + +# Use gh as a credential helper +exec gh auth setup-git + +# Create a repository with a file so it has a default branch +exec gh repo create ${ORG}/${REPO} --add-readme --private + +# Defer upstream cleanup +defer gh repo delete --yes ${ORG}/${REPO} + +# Create a fork +exec gh repo fork ${ORG}/${REPO} --org ${ORG} --fork-name ${REPO}-fork +sleep 5 + +# Defer fork cleanup +defer gh repo delete --yes ${ORG}/${REPO}-fork + +# Clone both repos +exec gh repo clone ${ORG}/${REPO} +exec gh repo clone ${ORG}/${REPO}-fork + +# Prepare a branch to PR in the fork itself +cd ${REPO}-fork +exec git checkout -b feature-branch +exec git commit --allow-empty -m 'Empty Commit' +exec git push -u origin feature-branch + +exec gh repo set-default ${ORG}/${REPO}-fork +exec gh pr create --title 'Feature Title' --body 'Feature Body' +stdout2env PR_URL + +# From the upstream clone, where the fork is not a remote, check out the PR into a worktree +cd ${WORK}/${REPO} +exec gh pr checkout ${PR_URL} --worktree ../wt +exists ../wt +exec git -C ../wt rev-parse --abbrev-ref HEAD +stdout '(?m)^feature-branch$' + +# The main working copy is left untouched +exec git rev-parse --abbrev-ref HEAD +stdout '(?m)^main$' + +# Reusing the same worktree for the same fork PR works +exec gh pr checkout ${PR_URL} --worktree ../wt +exec git -C ../wt rev-parse --abbrev-ref HEAD +stdout '(?m)^feature-branch$' + +# The main working copy is left untouched +exec git rev-parse --abbrev-ref HEAD +stdout '(?m)^main$' diff --git a/acceptance/testdata/pr/pr-checkout-worktree.txtar b/acceptance/testdata/pr/pr-checkout-worktree.txtar new file mode 100644 index 00000000000..f16a2dd3373 --- /dev/null +++ b/acceptance/testdata/pr/pr-checkout-worktree.txtar @@ -0,0 +1,135 @@ +# Checkout a PR into a git worktree, then reuse that worktree, rename its branch, +# force sync it, and add a second worktree once the local branch already exists. + +# Set up env vars +env REPO=${SCRIPT_NAME}-${RANDOM_STRING} + +# Use gh as a credential helper +exec gh auth setup-git + +# Create a repository with a file so it has a default branch +exec gh repo create ${ORG}/${REPO} --add-readme --private + +# Defer repo cleanup +defer gh repo delete --yes ${ORG}/${REPO} + +# Clone the repo +exec gh repo clone ${ORG}/${REPO} + +# Prepare a branch to PR +cd ${REPO} +exec git checkout -b feature-branch +exec git commit --allow-empty -m 'Empty Commit' +exec git push -u origin feature-branch + +# Create the PR +exec gh pr create --title 'Feature Title' --body 'Feature Body' +stdout2env PR_URL + +# Remove the local branch so checkout has to create it from the remote +exec git checkout main +exec git branch -D feature-branch +stdout 'Deleted branch feature-branch' + +# Checkout the PR into a fresh worktree +exec gh pr checkout ${PR_URL} --worktree ../wt +exists ../wt + +# The worktree is on the PR branch +exec git -C ../wt rev-parse --abbrev-ref HEAD +stdout '(?m)^feature-branch$' + +# The main working copy stays on the default branch +exec git rev-parse --abbrev-ref HEAD +stdout '(?m)^main$' + +# Checking out the same PR into the same worktree again reuses it +exec gh pr checkout ${PR_URL} --worktree ../wt +exec git -C ../wt rev-parse --abbrev-ref HEAD +stdout '(?m)^feature-branch$' + +# The main working copy is left untouched +exec git rev-parse --abbrev-ref HEAD +stdout '(?m)^main$' + +# Checking out into the reused worktree with a new branch name creates that branch +exec gh pr checkout ${PR_URL} --worktree ../wt --branch renamed-branch +exec git -C ../wt rev-parse --abbrev-ref HEAD +stdout '(?m)^renamed-branch$' + +# The main working copy is left untouched +exec git rev-parse --abbrev-ref HEAD +stdout '(?m)^main$' + +# Give the worktree's PR branch a local commit so it diverges from the PR head. +# A plain reuse would fast-forward-only merge and keep this commit, so --force is +# required to discard it with a hard reset. +exec git -C ../wt checkout feature-branch +exec git -C ../wt commit --allow-empty -m 'Diverging local commit' + +# Force checking out the PR into the reused worktree hard resets it to the PR head +exec gh pr checkout ${PR_URL} --worktree ../wt --force +exec git -C ../wt rev-parse --abbrev-ref HEAD +stdout '(?m)^feature-branch$' + +# The diverging local commit was discarded by the hard reset +exec git -C ../wt log -1 --format=%s +! stdout 'Diverging local commit' + +# The worktree branch now matches the PR head +exec git -C ../wt rev-parse HEAD +stdout2env WT_HEAD +exec git rev-parse origin/feature-branch +stdout ${WT_HEAD} + +# The main working copy is left untouched +exec git rev-parse --abbrev-ref HEAD +stdout '(?m)^main$' + +# With the local branch now present, removing the worktree and checking out into a +# fresh path adds a new worktree for the existing branch +exec git worktree remove ../wt +exec gh pr checkout ${PR_URL} --worktree ../wt2 +exists ../wt2 +exec git -C ../wt2 rev-parse --abbrev-ref HEAD +stdout '(?m)^feature-branch$' + +# The main working copy is left untouched +exec git rev-parse --abbrev-ref HEAD +stdout '(?m)^main$' + +# Now create a two-sided divergence: advance the remote PR branch with a pushed +# commit the local branch will not have... +exec git -C ../wt2 commit --allow-empty -m 'Remote commit' +exec git -C ../wt2 push origin feature-branch + +# ...then rewind the local branch and give it a different, local-only commit, so +# neither branch is an ancestor of the other +exec git -C ../wt2 reset --hard HEAD~1 +exec git -C ../wt2 commit --allow-empty -m 'Local commit' + +# A non-force checkout cannot fast-forward across the divergence and fails +! exec gh pr checkout ${PR_URL} --worktree ../wt2 +stderr 'Not possible to fast-forward' + +# The local-only commit is still there because the failed sync changed nothing +exec git -C ../wt2 log -1 --format=%s +stdout 'Local commit' + +# Forcing the checkout hard resets the branch to the advanced PR head +exec gh pr checkout ${PR_URL} --worktree ../wt2 --force +exec git -C ../wt2 rev-parse --abbrev-ref HEAD +stdout '(?m)^feature-branch$' +exec git -C ../wt2 log -1 --format=%s +stdout 'Remote commit' +! stdout 'Local commit' + +# The worktree branch now matches the advanced PR head +exec git -C ../wt2 rev-parse HEAD +stdout2env WT2_HEAD +exec git rev-parse origin/feature-branch +stdout ${WT2_HEAD} + +# The main working copy is left untouched +exec git rev-parse --abbrev-ref HEAD +stdout '(?m)^main$' diff --git a/pkg/cmd/pr/checkout/checkout.go b/pkg/cmd/pr/checkout/checkout.go index a137f92f50d..962c17de9b8 100644 --- a/pkg/cmd/pr/checkout/checkout.go +++ b/pkg/cmd/pr/checkout/checkout.go @@ -4,6 +4,9 @@ import ( "context" "fmt" "net/http" + "os" + "path/filepath" + "slices" "strings" "github.com/MakeNowJust/heredoc" @@ -33,6 +36,7 @@ type CheckoutOptions struct { Force bool Detach bool BranchName string + Worktree string } func NewCmdCheckout(f *cmdutil.Factory, runF func(*CheckoutOptions) error) *cobra.Command { @@ -56,10 +60,15 @@ func NewCmdCheckout(f *cmdutil.Factory, runF func(*CheckoutOptions) error) *cobr $ gh pr checkout 32 $ gh pr checkout https://github.com/OWNER/REPO/pull/32 $ gh pr checkout feature + $ gh pr checkout 32 --branch feature --worktree /path/to/wt-feature `), Args: cobra.MaximumNArgs(1), Aliases: []string{"co"}, RunE: func(cmd *cobra.Command, args []string) error { + if cmd.Flags().Changed("worktree") && opts.Worktree == "" { + return cmdutil.FlagErrorf("--worktree cannot be blank") + } + if len(args) > 0 { opts.PRResolver = &specificPRResolver{ prFinder: shared.NewFinder(f), @@ -97,6 +106,7 @@ func NewCmdCheckout(f *cmdutil.Factory, runF func(*CheckoutOptions) error) *cobr cmd.Flags().BoolVarP(&opts.Force, "force", "f", false, "Reset the existing local branch to the latest state of the pull request") cmd.Flags().BoolVarP(&opts.Detach, "detach", "", false, "Checkout PR with a detached HEAD") cmd.Flags().StringVarP(&opts.BranchName, "branch", "b", "", "Local branch name to use (default [the name of the head branch])") + cmd.Flags().StringVar(&opts.Worktree, "worktree", "", "Check out the pull request into a worktree at the given `path`") return cmd } @@ -107,6 +117,17 @@ func checkoutRun(opts *CheckoutOptions) error { return err } + var reuseWorktree bool + if opts.Worktree != "" { + if err := ensureWorktreePathSafe(opts.Worktree); err != nil { + return err + } + reuseWorktree, err = resolveWorktreeTarget(opts.GitClient, opts.Worktree) + if err != nil { + return err + } + } + cfg, err := opts.Config() if err != nil { return err @@ -137,7 +158,7 @@ func checkoutRun(opts *CheckoutOptions) error { var cmdQueue [][]string if headRemote != nil { - cmdQueue = append(cmdQueue, cmdsForExistingRemote(headRemote, pr, opts)...) + cmdQueue = append(cmdQueue, cmdsForExistingRemote(headRemote, pr, opts, reuseWorktree)...) } else { httpClient, err := opts.HttpClient() if err != nil { @@ -149,12 +170,18 @@ func checkoutRun(opts *CheckoutOptions) error { if err != nil { return err } - cmdQueue = append(cmdQueue, cmdsForMissingRemote(pr, baseURLOrName, baseRepo.RepoHost(), defaultBranch, protocol, opts)...) + cmdQueue = append(cmdQueue, cmdsForMissingRemote(pr, baseURLOrName, baseRepo.RepoHost(), defaultBranch, protocol, opts, reuseWorktree)...) } if opts.RecurseSubmodules { - cmdQueue = append(cmdQueue, []string{"submodule", "sync", "--recursive"}) - cmdQueue = append(cmdQueue, []string{"submodule", "update", "--init", "--recursive"}) + // Run submodule commands inside the worktree when checking out into + // one, so its submodules (not the main worktree's) get initialized. + var prefix []string + if opts.Worktree != "" { + prefix = []string{"-C", opts.Worktree} + } + cmdQueue = append(cmdQueue, slices.Concat(prefix, []string{"submodule", "sync", "--recursive"})) + cmdQueue = append(cmdQueue, slices.Concat(prefix, []string{"submodule", "update", "--init", "--recursive"})) } // Note that although we will probably be fetching from the head, in practice, PR checkout can only @@ -164,10 +191,16 @@ func checkoutRun(opts *CheckoutOptions) error { return err } + if opts.Worktree != "" && opts.IO.IsStdoutTTY() { + cs := opts.IO.ColorScheme() + fmt.Fprintf(opts.IO.ErrOut, "%s Checked out PR #%d in worktree %s\n", cs.SuccessIcon(), pr.Number, opts.Worktree) + fmt.Fprintf(opts.IO.ErrOut, " To start working: cd %s\n", opts.Worktree) + } + return nil } -func cmdsForExistingRemote(remote *cliContext.Remote, pr *api.PullRequest, opts *CheckoutOptions) [][]string { +func cmdsForExistingRemote(remote *cliContext.Remote, pr *api.PullRequest, opts *CheckoutOptions, reuseWorktree bool) [][]string { var cmds [][]string remoteBranch := fmt.Sprintf("%s/%s", remote.Name, pr.HeadRefName) @@ -176,24 +209,38 @@ func cmdsForExistingRemote(remote *cliContext.Remote, pr *api.PullRequest, opts refSpec += fmt.Sprintf(":refs/remotes/%s", remoteBranch) } - cmds = append(cmds, []string{"fetch", remote.Name, refSpec, "--no-tags"}) - localBranch := pr.HeadRefName if opts.BranchName != "" { localBranch = opts.BranchName } + remoteBranchRef := fmt.Sprintf("refs/remotes/%s", remoteBranch) + fetchCmd := []string{"fetch", remote.Name, refSpec, "--no-tags"} + + if opts.Detach { + return append(cmds, detachCmds(fetchCmd, opts.Worktree, reuseWorktree)...) + } + + cmds = append(cmds, fetchCmd) + switch { - case opts.Detach: - cmds = append(cmds, []string{"checkout", "--detach", "FETCH_HEAD"}) - case localBranchExists(opts.GitClient, localBranch): - cmds = append(cmds, []string{"checkout", localBranch}) - if opts.Force { - cmds = append(cmds, []string{"reset", "--hard", fmt.Sprintf("refs/remotes/%s", remoteBranch)}) + case opts.Worktree != "": + if reuseWorktree { + if localBranchExists(opts.GitClient, localBranch) { + cmds = append(cmds, worktreeCheckoutCmds(opts.Worktree, localBranch, remoteBranchRef, opts.Force)...) + } else { + // New --branch name while reusing a worktree: create it tracking the remote. + cmds = append(cmds, []string{"-C", opts.Worktree, "checkout", "-b", localBranch, "--track", remoteBranch}) + } + } else if localBranchExists(opts.GitClient, localBranch) { + cmds = append(cmds, []string{"worktree", "add", "--", opts.Worktree, localBranch}) + cmds = append(cmds, syncBranchCmds(opts.Worktree, remoteBranchRef, opts.Force)...) } else { - // TODO: check if non-fast-forward and suggest to use `--force` - cmds = append(cmds, []string{"merge", "--ff-only", fmt.Sprintf("refs/remotes/%s", remoteBranch)}) + cmds = append(cmds, []string{"worktree", "add", "--track", "-b", localBranch, "--", opts.Worktree, remoteBranch}) } + case localBranchExists(opts.GitClient, localBranch): + cmds = append(cmds, []string{"checkout", localBranch}) + cmds = append(cmds, syncBranchCmds("", remoteBranchRef, opts.Force)...) default: cmds = append(cmds, []string{"checkout", "-b", localBranch, "--track", remoteBranch}) } @@ -201,14 +248,13 @@ func cmdsForExistingRemote(remote *cliContext.Remote, pr *api.PullRequest, opts return cmds } -func cmdsForMissingRemote(pr *api.PullRequest, baseURLOrName, repoHost, defaultBranch, protocol string, opts *CheckoutOptions) [][]string { +func cmdsForMissingRemote(pr *api.PullRequest, baseURLOrName, repoHost, defaultBranch, protocol string, opts *CheckoutOptions, reuseWorktree bool) [][]string { var cmds [][]string ref := fmt.Sprintf("refs/pull/%d/head", pr.Number) if opts.Detach { - cmds = append(cmds, []string{"fetch", baseURLOrName, ref, "--no-tags"}) - cmds = append(cmds, []string{"checkout", "--detach", "FETCH_HEAD"}) - return cmds + fetchCmd := []string{"fetch", baseURLOrName, ref, "--no-tags"} + return detachCmds(fetchCmd, opts.Worktree, reuseWorktree) } localBranch := pr.HeadRefName @@ -220,15 +266,29 @@ func cmdsForMissingRemote(pr *api.PullRequest, baseURLOrName, repoHost, defaultB } currentBranch, _ := opts.Branch() - if localBranch == currentBranch { - // PR head matches currently checked out branch - cmds = append(cmds, []string{"fetch", baseURLOrName, ref, "--no-tags"}) - if opts.Force { - cmds = append(cmds, []string{"reset", "--hard", "FETCH_HEAD"}) + if opts.Worktree != "" { + if reuseWorktree { + // FETCH_HEAD is per-worktree, and git refuses to update a branch via + // refspec while it is checked out, so fetch to FETCH_HEAD inside the worktree. + cmds = append(cmds, []string{"-C", opts.Worktree, "fetch", baseURLOrName, ref, "--no-tags"}) + if localBranchExists(opts.GitClient, localBranch) { + cmds = append(cmds, []string{"-C", opts.Worktree, "checkout", localBranch}) + cmds = append(cmds, syncBranchCmds(opts.Worktree, "FETCH_HEAD", opts.Force)...) + } else { + cmds = append(cmds, []string{"-C", opts.Worktree, "checkout", "-b", localBranch, "FETCH_HEAD"}) + } } else { - // TODO: check if non-fast-forward and suggest to use `--force` - cmds = append(cmds, []string{"merge", "--ff-only", "FETCH_HEAD"}) + fetchCmd := []string{"fetch", baseURLOrName, fmt.Sprintf("%s:%s", ref, localBranch), "--no-tags"} + if opts.Force { + fetchCmd = append(fetchCmd, "--force") + } + cmds = append(cmds, fetchCmd) + cmds = append(cmds, []string{"worktree", "add", "--", opts.Worktree, localBranch}) } + } else if localBranch == currentBranch { + // PR head matches currently checked out branch + cmds = append(cmds, []string{"fetch", baseURLOrName, ref, "--no-tags"}) + cmds = append(cmds, syncBranchCmds("", "FETCH_HEAD", opts.Force)...) } else { // TODO: check if non-fast-forward and suggest to use `--force` fetchCmd := []string{"fetch", baseURLOrName, fmt.Sprintf("%s:%s", ref, localBranch), "--no-tags"} @@ -268,15 +328,145 @@ func localBranchExists(client *git.Client, b string) bool { return err == nil } +// resolveWorktreeTarget asks git where path lives, letting git resolve symlinks, +// "..", case, and trailing slashes for us instead of comparing paths ourselves. +// It returns whether an existing linked worktree there should be reused, and +// errors when the path cannot host a new worktree: a path inside a different +// repository, a subdirectory of another worktree, or the worktree we are already +// running in. Detection is best-effort: if git cannot resolve the current or +// target worktree (e.g. the path does not exist yet), reuse is false so git +// worktree add handles the path. +func resolveWorktreeTarget(client *git.Client, path string) (reuseWorktree bool, err error) { + abs, err := filepath.Abs(path) + if err != nil { + return false, err + } + + // git emits one line per flag, so we expect exactly two lines here. + current, ok := revParseFacts(client, "", "--show-toplevel", "--git-common-dir") + if !ok || len(current) != 2 { + return false, nil + } + currentToplevel, currentCommonDir := current[0], current[1] + + // A non-existent or non-git target fails here: it is a fresh path for a new worktree. + target, ok := revParseFacts(client, abs, "--show-toplevel", "--show-prefix", "--git-common-dir") + if !ok || len(target) != 3 { + return false, nil + } + targetToplevel, targetPrefix, targetCommonDir := target[0], target[1], target[2] + + switch { + case targetCommonDir != currentCommonDir: + return false, fmt.Errorf("--worktree path is inside a different repository") + case targetToplevel == currentToplevel: + return false, fmt.Errorf("--worktree path points to the repository you're already in; omit --worktree to check out here") + case targetPrefix != "": + return false, fmt.Errorf("--worktree path is inside an existing worktree") + } + // The path is the root of another linked worktree of this repo; reuse it. + return true, nil +} + +// revParseFacts runs `git rev-parse --path-format=absolute ` and +// returns one absolute path per flag, in flag order (an empty --show-prefix +// yields an empty string), with ok=false if git fails. When dir is non-empty +// the query is scoped there with -C. +func revParseFacts(client *git.Client, dir string, flags ...string) (fields []string, ok bool) { + args := append([]string{"rev-parse", "--path-format=absolute"}, flags...) + if dir != "" { + args = append([]string{"-C", dir}, args...) + } + cmd, err := client.Command(context.Background(), args...) + if err != nil { + return nil, false + } + out, err := cmd.Output() + if err != nil { + return nil, false + } + return strings.Split(strings.TrimRight(string(out), "\n"), "\n"), true +} + +// detachCmds returns the commands for a detached checkout. When reusing an +// existing linked worktree, FETCH_HEAD must be written inside it (it is +// per-worktree), so the fetch runs with -C . +func detachCmds(fetchCmd []string, worktree string, reuseWorktree bool) [][]string { + if worktree == "" { + return [][]string{ + fetchCmd, + {"checkout", "--detach", "FETCH_HEAD"}, + } + } + + if reuseWorktree { + return [][]string{ + append([]string{"-C", worktree}, fetchCmd...), + {"-C", worktree, "checkout", "--detach", "FETCH_HEAD"}, + } + } + return [][]string{ + fetchCmd, + {"worktree", "add", "--detach", "--", worktree, "FETCH_HEAD"}, + } +} + +// syncBranchCmds syncs a branch to ref: a hard reset when force is set, +// otherwise a fast-forward-only merge. A non-empty path runs the commands there. +func syncBranchCmds(path, ref string, force bool) [][]string { + var prefix []string + if path != "" { + prefix = []string{"-C", path} + } + if force { + return [][]string{append(prefix, "reset", "--hard", ref)} + } + // TODO: check if non-fast-forward and suggest to use `--force` + return [][]string{append(prefix, "merge", "--ff-only", ref)} +} + +func worktreeCheckoutCmds(path, branch, ref string, force bool) [][]string { + cmds := [][]string{{"-C", path, "checkout", branch}} + cmds = append(cmds, syncBranchCmds(path, ref, force)...) + return cmds +} + +// ensureWorktreePathSafe validates a --worktree target before we write to it: +// it must be a non-existent path (git will create it) or an existing directory, +// and never a symlink at its final component. A symlinked ancestor (e.g. macOS +// /tmp -> /private/tmp) is fine; os.Lstat checks only the leaf so it is not +// followed. Rejecting a leaf symlink guards against writing PR content through a +// planted link. +func ensureWorktreePathSafe(path string) error { + fi, err := os.Lstat(path) + switch { + case os.IsNotExist(err): + return nil + case err != nil: + return err + case fi.Mode()&os.ModeSymlink != 0: + return fmt.Errorf("--worktree path must not be a symlink: %s", path) + case !fi.IsDir(): + return fmt.Errorf("--worktree path must be a directory: %s", path) + } + return nil +} + func executeCmds(client *git.Client, credentialPattern git.CredentialPattern, cmdQueue [][]string) error { for _, args := range cmdQueue { + // Determine the git sub-command, skipping any -C prefix. + subCmd := args[0] + if len(args) >= 3 && args[0] == "-C" { + subCmd = args[2] + } + var err error var cmd *git.Command - switch args[0] { + switch subCmd { case "submodule": - cmd, err = client.AuthenticatedCommand(context.Background(), credentialPattern, args...) + cmd, err = authenticatedCommand(client, credentialPattern, args) case "fetch": - cmd, err = client.AuthenticatedCommand(context.Background(), git.AllMatchingCredentialsPattern, args...) + cmd, err = authenticatedCommand(client, git.AllMatchingCredentialsPattern, args) default: cmd, err = client.Command(context.Background(), args...) } @@ -290,6 +480,22 @@ func executeCmds(client *git.Client, credentialPattern git.CredentialPattern, cm return nil } +// authenticatedCommand builds an authenticated git command, transparently +// handling a leading -C prefix. AuthenticatedCommand prepends +// credential-helper flags before all args, so a -C prefix would be displaced; +// instead we strip it and apply it as cmd.Dir. +func authenticatedCommand(client *git.Client, credentialPattern git.CredentialPattern, args []string) (*git.Command, error) { + if args[0] == "-C" { + cmd, err := client.AuthenticatedCommand(context.Background(), credentialPattern, args[2:]...) + if err != nil { + return nil, err + } + cmd.Dir = args[1] + return cmd, nil + } + return client.AuthenticatedCommand(context.Background(), credentialPattern, args...) +} + type PRResolver interface { Resolve() (*api.PullRequest, ghrepo.Interface, error) } diff --git a/pkg/cmd/pr/checkout/checkout_test.go b/pkg/cmd/pr/checkout/checkout_test.go index 496139423e9..73beaa6dcb3 100644 --- a/pkg/cmd/pr/checkout/checkout_test.go +++ b/pkg/cmd/pr/checkout/checkout_test.go @@ -5,6 +5,8 @@ import ( "errors" "io" "net/http" + "os" + "path/filepath" "strings" "testing" @@ -61,6 +63,18 @@ func TestNewCmdCheckout(t *testing.T) { BranchName: "test-branch", }, }, + { + name: "worktree", + args: "--worktree /path/to/wt 123", + wantsOpts: CheckoutOptions{ + Worktree: "/path/to/wt", + }, + }, + { + name: "when --worktree is given a blank path, returns an error", + args: `--worktree "" 123`, + wantErr: cmdutil.FlagErrorf("--worktree cannot be blank"), + }, { name: "when there is no selector and no TTY, returns an error", args: "", @@ -100,6 +114,7 @@ func TestNewCmdCheckout(t *testing.T) { require.Equal(t, tt.wantsOpts.Force, spiedOpts.Force) require.Equal(t, tt.wantsOpts.Detach, spiedOpts.Detach) require.Equal(t, tt.wantsOpts.BranchName, spiedOpts.BranchName) + require.Equal(t, tt.wantsOpts.Worktree, spiedOpts.Worktree) }) } } @@ -173,6 +188,7 @@ func Test_checkoutRun(t *testing.T) { promptStubs func(*prompter.MockPrompter) remotes map[string]string + stdoutTTY bool wantStdout string wantStderr string wantErr bool @@ -293,6 +309,384 @@ func Test_checkoutRun(t *testing.T) { cs.Register(`git config branch\.foobar\.merge refs/heads/feature`, 0, "") }, }, + { + name: "checkout new branch into a worktree", + opts: &CheckoutOptions{ + Worktree: "/path/to/wt", + PRResolver: func() PRResolver { + baseRepo, pr := stubPR("OWNER/REPO:master", "OWNER/REPO:feature") + return &stubPRResolver{ + pr: pr, + baseRepo: baseRepo, + } + }(), + Config: func() (gh.Config, error) { + return config.NewBlankConfig(), nil + }, + Branch: func() (string, error) { + return "main", nil + }, + }, + remotes: map[string]string{ + "origin": "OWNER/REPO", + }, + stdoutTTY: true, + runStubs: func(cs *run.CommandStubber) { + cs.Register(`git rev-parse --path-format=absolute --show-toplevel --git-common-dir`, 0, "/repo/main\n/repo/.git\n") + cs.Register(`git -C .+path.to.wt rev-parse --path-format=absolute --show-toplevel --show-prefix --git-common-dir`, 128, "") + cs.Register(`git show-ref --verify -- refs/heads/feature`, 1, "") + cs.Register(`git fetch origin \+refs/heads/feature:refs/remotes/origin/feature --no-tags`, 0, "") + cs.Register(`git worktree add --track -b feature -- /path/to/wt origin/feature`, 0, "") + }, + wantStderr: "✓ Checked out PR #123 in worktree /path/to/wt\n To start working: cd /path/to/wt\n", + }, + { + name: "checkout into a worktree with recurse submodules runs submodule commands inside the worktree", + opts: &CheckoutOptions{ + Worktree: "/path/to/wt", + RecurseSubmodules: true, + PRResolver: func() PRResolver { + baseRepo, pr := stubPR("OWNER/REPO:master", "OWNER/REPO:feature") + return &stubPRResolver{ + pr: pr, + baseRepo: baseRepo, + } + }(), + Config: func() (gh.Config, error) { + return config.NewBlankConfig(), nil + }, + Branch: func() (string, error) { + return "main", nil + }, + }, + remotes: map[string]string{ + "origin": "OWNER/REPO", + }, + stdoutTTY: true, + runStubs: func(cs *run.CommandStubber) { + cs.Register(`git rev-parse --path-format=absolute --show-toplevel --git-common-dir`, 0, "/repo/main\n/repo/.git\n") + cs.Register(`git -C .+path.to.wt rev-parse --path-format=absolute --show-toplevel --show-prefix --git-common-dir`, 128, "") + cs.Register(`git show-ref --verify -- refs/heads/feature`, 1, "") + cs.Register(`git fetch origin \+refs/heads/feature:refs/remotes/origin/feature --no-tags`, 0, "") + cs.Register(`git worktree add --track -b feature -- /path/to/wt origin/feature`, 0, "") + cs.Register(`git submodule sync --recursive`, 0, "") + cs.Register(`git submodule update --init --recursive`, 0, "") + }, + wantStderr: "✓ Checked out PR #123 in worktree /path/to/wt\n To start working: cd /path/to/wt\n", + }, + { + name: "checkout existing branch into a worktree and sync with merge", + opts: &CheckoutOptions{ + Worktree: "/path/to/wt", + PRResolver: func() PRResolver { + baseRepo, pr := stubPR("OWNER/REPO:master", "OWNER/REPO:feature") + return &stubPRResolver{ + pr: pr, + baseRepo: baseRepo, + } + }(), + Config: func() (gh.Config, error) { + return config.NewBlankConfig(), nil + }, + Branch: func() (string, error) { + return "main", nil + }, + }, + remotes: map[string]string{ + "origin": "OWNER/REPO", + }, + runStubs: func(cs *run.CommandStubber) { + cs.Register(`git rev-parse --path-format=absolute --show-toplevel --git-common-dir`, 0, "/repo/main\n/repo/.git\n") + cs.Register(`git -C .+path.to.wt rev-parse --path-format=absolute --show-toplevel --show-prefix --git-common-dir`, 128, "") + cs.Register(`git show-ref --verify -- refs/heads/feature`, 0, "") + cs.Register(`git fetch origin \+refs/heads/feature:refs/remotes/origin/feature --no-tags`, 0, "") + cs.Register(`git worktree add -- /path/to/wt feature`, 0, "") + cs.Register(`git -C /path/to/wt merge --ff-only refs/remotes/origin/feature`, 0, "") + }, + }, + { + name: "checkout existing branch into a worktree with force resets", + opts: &CheckoutOptions{ + Worktree: "/path/to/wt", + Force: true, + PRResolver: func() PRResolver { + baseRepo, pr := stubPR("OWNER/REPO:master", "OWNER/REPO:feature") + return &stubPRResolver{ + pr: pr, + baseRepo: baseRepo, + } + }(), + Config: func() (gh.Config, error) { + return config.NewBlankConfig(), nil + }, + Branch: func() (string, error) { + return "main", nil + }, + }, + remotes: map[string]string{ + "origin": "OWNER/REPO", + }, + runStubs: func(cs *run.CommandStubber) { + cs.Register(`git rev-parse --path-format=absolute --show-toplevel --git-common-dir`, 0, "/repo/main\n/repo/.git\n") + cs.Register(`git -C .+path.to.wt rev-parse --path-format=absolute --show-toplevel --show-prefix --git-common-dir`, 128, "") + cs.Register(`git show-ref --verify -- refs/heads/feature`, 0, "") + cs.Register(`git fetch origin \+refs/heads/feature:refs/remotes/origin/feature --no-tags`, 0, "") + cs.Register(`git worktree add -- /path/to/wt feature`, 0, "") + cs.Register(`git -C /path/to/wt reset --hard refs/remotes/origin/feature`, 0, "") + }, + }, + { + name: "checkout detached into a worktree", + opts: &CheckoutOptions{ + Worktree: "/path/to/wt", + Detach: true, + PRResolver: func() PRResolver { + baseRepo, pr := stubPR("OWNER/REPO:master", "OWNER/REPO:feature") + return &stubPRResolver{ + pr: pr, + baseRepo: baseRepo, + } + }(), + Config: func() (gh.Config, error) { + return config.NewBlankConfig(), nil + }, + Branch: func() (string, error) { + return "main", nil + }, + }, + remotes: map[string]string{ + "origin": "OWNER/REPO", + }, + runStubs: func(cs *run.CommandStubber) { + cs.Register(`git rev-parse --path-format=absolute --show-toplevel --git-common-dir`, 0, "/repo/main\n/repo/.git\n") + cs.Register(`git -C .+path.to.wt rev-parse --path-format=absolute --show-toplevel --show-prefix --git-common-dir`, 128, "") + cs.Register(`git fetch origin \+refs/heads/feature --no-tags`, 0, "") + cs.Register(`git worktree add --detach -- /path/to/wt FETCH_HEAD`, 0, "") + }, + }, + { + name: "checkout detached into the same worktree again fetches and checks out inside it", + opts: &CheckoutOptions{ + Worktree: "/path/to/wt", + Detach: true, + PRResolver: func() PRResolver { + baseRepo, pr := stubPR("OWNER/REPO:master", "OWNER/REPO:feature") + return &stubPRResolver{ + pr: pr, + baseRepo: baseRepo, + } + }(), + Config: func() (gh.Config, error) { + return config.NewBlankConfig(), nil + }, + Branch: func() (string, error) { + return "main", nil + }, + }, + remotes: map[string]string{ + "origin": "OWNER/REPO", + }, + runStubs: func(cs *run.CommandStubber) { + cs.Register(`git rev-parse --path-format=absolute --show-toplevel --git-common-dir`, 0, "/repo/main\n/repo/.git\n") + cs.Register(`git -C .+path.to.wt rev-parse --path-format=absolute --show-toplevel --show-prefix --git-common-dir`, 0, "/path/to/wt\n\n/repo/.git\n") + cs.Register(`git fetch origin \+refs/heads/feature --no-tags`, 0, "") + cs.Register(`git -C /path/to/wt checkout --detach FETCH_HEAD`, 0, "") + }, + }, + { + name: "checkout fork PR without a remote into a worktree", + opts: &CheckoutOptions{ + Worktree: "/path/to/wt", + PRResolver: func() PRResolver { + baseRepo, pr := stubPR("OWNER/REPO:master", "hubot/REPO:feature") + pr.MaintainerCanModify = true + return &stubPRResolver{ + pr: pr, + baseRepo: baseRepo, + } + }(), + Config: func() (gh.Config, error) { + return config.NewBlankConfig(), nil + }, + Branch: func() (string, error) { + return "main", nil + }, + }, + remotes: map[string]string{ + "origin": "OWNER/REPO", + }, + runStubs: func(cs *run.CommandStubber) { + cs.Register(`git rev-parse --path-format=absolute --show-toplevel --git-common-dir`, 0, "/repo/main\n/repo/.git\n") + cs.Register(`git -C .+path.to.wt rev-parse --path-format=absolute --show-toplevel --show-prefix --git-common-dir`, 128, "") + cs.Register(`git config branch\.feature\.merge`, 1, "") + cs.Register(`git fetch origin refs/pull/123/head:feature --no-tags`, 0, "") + cs.Register(`git worktree add -- /path/to/wt feature`, 0, "") + cs.Register(`git config branch\.feature\.remote https://github.com/hubot/REPO.git`, 0, "") + cs.Register(`git config branch\.feature\.pushRemote https://github.com/hubot/REPO.git`, 0, "") + cs.Register(`git config branch\.feature\.merge refs/heads/feature`, 0, "") + }, + }, + { + name: "checkout existing branch into the same worktree again switches and syncs it", + opts: &CheckoutOptions{ + Worktree: "/path/to/wt", + PRResolver: func() PRResolver { + baseRepo, pr := stubPR("OWNER/REPO:master", "OWNER/REPO:feature") + return &stubPRResolver{ + pr: pr, + baseRepo: baseRepo, + } + }(), + Config: func() (gh.Config, error) { + return config.NewBlankConfig(), nil + }, + Branch: func() (string, error) { + return "main", nil + }, + }, + remotes: map[string]string{ + "origin": "OWNER/REPO", + }, + runStubs: func(cs *run.CommandStubber) { + cs.Register(`git rev-parse --path-format=absolute --show-toplevel --git-common-dir`, 0, "/repo/main\n/repo/.git\n") + cs.Register(`git -C .+path.to.wt rev-parse --path-format=absolute --show-toplevel --show-prefix --git-common-dir`, 0, "/path/to/wt\n\n/repo/.git\n") + cs.Register(`git show-ref --verify -- refs/heads/feature`, 0, "") + cs.Register(`git fetch origin \+refs/heads/feature:refs/remotes/origin/feature --no-tags`, 0, "") + cs.Register(`git -C /path/to/wt checkout feature`, 0, "") + cs.Register(`git -C /path/to/wt merge --ff-only refs/remotes/origin/feature`, 0, "") + }, + }, + { + name: "checkout into an existing worktree with a new custom branch name creates the branch", + opts: &CheckoutOptions{ + Worktree: "/path/to/wt", + BranchName: "my-custom-name", + PRResolver: func() PRResolver { + baseRepo, pr := stubPR("OWNER/REPO:master", "OWNER/REPO:feature") + return &stubPRResolver{ + pr: pr, + baseRepo: baseRepo, + } + }(), + Config: func() (gh.Config, error) { + return config.NewBlankConfig(), nil + }, + Branch: func() (string, error) { + return "main", nil + }, + }, + remotes: map[string]string{ + "origin": "OWNER/REPO", + }, + stdoutTTY: true, + runStubs: func(cs *run.CommandStubber) { + cs.Register(`git rev-parse --path-format=absolute --show-toplevel --git-common-dir`, 0, "/repo/main\n/repo/.git\n") + cs.Register(`git -C .+path.to.wt rev-parse --path-format=absolute --show-toplevel --show-prefix --git-common-dir`, 0, "/path/to/wt\n\n/repo/.git\n") + cs.Register(`git show-ref --verify -- refs/heads/my-custom-name`, 1, "") + cs.Register(`git fetch origin \+refs/heads/feature:refs/remotes/origin/feature --no-tags`, 0, "") + cs.Register(`git -C /path/to/wt checkout -b my-custom-name --track origin/feature`, 0, "") + }, + wantStderr: "✓ Checked out PR #123 in worktree /path/to/wt\n To start working: cd /path/to/wt\n", + }, + { + name: "checkout fork PR without a remote into the same worktree again switches and syncs it", + opts: &CheckoutOptions{ + Worktree: "/path/to/wt", + PRResolver: func() PRResolver { + baseRepo, pr := stubPR("OWNER/REPO:master", "hubot/REPO:feature") + pr.MaintainerCanModify = true + return &stubPRResolver{ + pr: pr, + baseRepo: baseRepo, + } + }(), + Config: func() (gh.Config, error) { + return config.NewBlankConfig(), nil + }, + Branch: func() (string, error) { + return "main", nil + }, + }, + remotes: map[string]string{ + "origin": "OWNER/REPO", + }, + runStubs: func(cs *run.CommandStubber) { + cs.Register(`git rev-parse --path-format=absolute --show-toplevel --git-common-dir`, 0, "/repo/main\n/repo/.git\n") + cs.Register(`git -C .+path.to.wt rev-parse --path-format=absolute --show-toplevel --show-prefix --git-common-dir`, 0, "/path/to/wt\n\n/repo/.git\n") + cs.Register(`git show-ref --verify -- refs/heads/feature`, 0, "") + cs.Register(`git config branch\.feature\.merge`, 0, "refs/heads/feature") + cs.Register(`git fetch origin refs/pull/123/head --no-tags`, 0, "") + cs.Register(`git -C /path/to/wt checkout feature`, 0, "") + cs.Register(`git -C /path/to/wt merge --ff-only FETCH_HEAD`, 0, "") + }, + }, + { + name: "checkout fork PR without a remote into the same worktree again with force resets", + opts: &CheckoutOptions{ + Worktree: "/path/to/wt", + Force: true, + PRResolver: func() PRResolver { + baseRepo, pr := stubPR("OWNER/REPO:master", "hubot/REPO:feature") + pr.MaintainerCanModify = true + return &stubPRResolver{ + pr: pr, + baseRepo: baseRepo, + } + }(), + Config: func() (gh.Config, error) { + return config.NewBlankConfig(), nil + }, + Branch: func() (string, error) { + return "main", nil + }, + }, + remotes: map[string]string{ + "origin": "OWNER/REPO", + }, + runStubs: func(cs *run.CommandStubber) { + cs.Register(`git rev-parse --path-format=absolute --show-toplevel --git-common-dir`, 0, "/repo/main\n/repo/.git\n") + cs.Register(`git -C .+path.to.wt rev-parse --path-format=absolute --show-toplevel --show-prefix --git-common-dir`, 0, "/path/to/wt\n\n/repo/.git\n") + cs.Register(`git show-ref --verify -- refs/heads/feature`, 0, "") + cs.Register(`git config branch\.feature\.merge`, 0, "refs/heads/feature") + cs.Register(`git fetch origin refs/pull/123/head --no-tags`, 0, "") + cs.Register(`git -C /path/to/wt checkout feature`, 0, "") + cs.Register(`git -C /path/to/wt reset --hard FETCH_HEAD`, 0, "") + }, + }, + { + name: "checkout fork PR without a remote into an existing worktree whose branch does not exist yet creates it", + opts: &CheckoutOptions{ + Worktree: "/path/to/wt", + PRResolver: func() PRResolver { + baseRepo, pr := stubPR("OWNER/REPO:master", "hubot/REPO:feature") + pr.MaintainerCanModify = true + return &stubPRResolver{ + pr: pr, + baseRepo: baseRepo, + } + }(), + Config: func() (gh.Config, error) { + return config.NewBlankConfig(), nil + }, + Branch: func() (string, error) { + return "main", nil + }, + }, + remotes: map[string]string{ + "origin": "OWNER/REPO", + }, + runStubs: func(cs *run.CommandStubber) { + cs.Register(`git rev-parse --path-format=absolute --show-toplevel --git-common-dir`, 0, "/repo/main\n/repo/.git\n") + cs.Register(`git -C .+path.to.wt rev-parse --path-format=absolute --show-toplevel --show-prefix --git-common-dir`, 0, "/path/to/wt\n\n/repo/.git\n") + cs.Register(`git show-ref --verify -- refs/heads/feature`, 1, "") + cs.Register(`git config branch\.feature\.merge`, 1, "") + cs.Register(`git fetch origin refs/pull/123/head --no-tags`, 0, "") + cs.Register(`git -C /path/to/wt checkout -b feature FETCH_HEAD`, 0, "") + cs.Register(`git config branch\.feature\.remote https://github.com/hubot/REPO.git`, 0, "") + cs.Register(`git config branch\.feature\.pushRemote https://github.com/hubot/REPO.git`, 0, "") + cs.Register(`git config branch\.feature\.merge refs/heads/feature`, 0, "") + }, + }, { name: "when the PR resolver errors, then that error is bubbled up", opts: &CheckoutOptions{ @@ -309,6 +703,7 @@ func Test_checkoutRun(t *testing.T) { opts := tt.opts ios, _, stdout, stderr := iostreams.Test() + ios.SetStdoutTTY(tt.stdoutTTY) opts.IO = ios httpReg := &httpmock.Registry{} @@ -786,3 +1181,183 @@ func TestPRCheckout_detach(t *testing.T) { assert.Equal(t, "", output.String()) assert.Equal(t, "", output.Stderr()) } + +func Test_authenticatedCommand_stripsWorktreePrefix(t *testing.T) { + tests := []struct { + name string + args []string + wantDir string + wantArgs []string + }{ + { + name: "leading -C prefix is applied as cmd.Dir and stripped from args", + args: []string{"-C", "/path/to/wt", "submodule", "sync", "--recursive"}, + wantDir: "/path/to/wt", + wantArgs: []string{"submodule", "sync", "--recursive"}, + }, + { + name: "leading -C prefix is applied as cmd.Dir for a worktree-local fetch", + args: []string{"-C", "/path/to/wt", "fetch", "origin", "refs/pull/123/head", "--no-tags"}, + wantDir: "/path/to/wt", + wantArgs: []string{"fetch", "origin", "refs/pull/123/head", "--no-tags"}, + }, + { + name: "without a -C prefix cmd.Dir is left empty", + args: []string{"fetch", "origin", "refs/pull/123/head", "--no-tags"}, + wantDir: "", + wantArgs: []string{"fetch", "origin", "refs/pull/123/head", "--no-tags"}, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + client := &git.Client{ + GhPath: "/some/path/gh", + GitPath: "/some/path/git", + } + cmd, err := authenticatedCommand(client, git.AllMatchingCredentialsPattern, tt.args) + require.NoError(t, err) + + assert.Equal(t, tt.wantDir, cmd.Dir) + // The credential-helper flags are prepended, so assert the tail + // carries the real sub-command args and no -C prefix leaked in. + require.GreaterOrEqual(t, len(cmd.Args), len(tt.wantArgs)) + assert.Equal(t, tt.wantArgs, cmd.Args[len(cmd.Args)-len(tt.wantArgs):]) + assert.NotContains(t, cmd.Args, "-C") + }) + } +} + +func Test_resolveWorktreeTarget(t *testing.T) { + dir := t.TempDir() + + tests := []struct { + name string + stubs func(*run.CommandStubber) + wantReuse bool + wantErr string + }{ + { + name: "path is the current worktree", + stubs: func(cs *run.CommandStubber) { + cs.Register(`git rev-parse --path-format=absolute --show-toplevel --git-common-dir`, 0, "/repo/main\n/repo/.git\n") + cs.Register(`git -C .+rev-parse --path-format=absolute --show-toplevel --show-prefix --git-common-dir`, 0, "/repo/main\n\n/repo/.git\n") + }, + wantErr: "--worktree path points to the repository you're already in; omit --worktree to check out here", + }, + { + name: "path is a subdirectory of the current worktree", + stubs: func(cs *run.CommandStubber) { + cs.Register(`git rev-parse --path-format=absolute --show-toplevel --git-common-dir`, 0, "/repo/main\n/repo/.git\n") + cs.Register(`git -C .+ rev-parse --path-format=absolute --show-toplevel --show-prefix --git-common-dir`, 0, "/repo/main\nsub/\n/repo/.git\n") + }, + wantErr: "--worktree path points to the repository you're already in; omit --worktree to check out here", + }, + { + name: "path is a different worktree of this repo", + stubs: func(cs *run.CommandStubber) { + cs.Register(`git rev-parse --path-format=absolute --show-toplevel --git-common-dir`, 0, "/repo/main\n/repo/.git\n") + cs.Register(`git -C .+ rev-parse --path-format=absolute --show-toplevel --show-prefix --git-common-dir`, 0, "/path/to/wt\n\n/repo/.git\n") + }, + wantReuse: true, + }, + { + name: "path is a subdirectory of another worktree", + stubs: func(cs *run.CommandStubber) { + cs.Register(`git rev-parse --path-format=absolute --show-toplevel --git-common-dir`, 0, "/repo/main\n/repo/.git\n") + cs.Register(`git -C .+ rev-parse --path-format=absolute --show-toplevel --show-prefix --git-common-dir`, 0, "/path/to/wt\nsub/\n/repo/.git\n") + }, + wantErr: "--worktree path is inside an existing worktree", + }, + { + name: "path is inside a different repository", + stubs: func(cs *run.CommandStubber) { + cs.Register(`git rev-parse --path-format=absolute --show-toplevel --git-common-dir`, 0, "/repo/main\n/repo/.git\n") + cs.Register(`git -C .+ rev-parse --path-format=absolute --show-toplevel --show-prefix --git-common-dir`, 0, "/other/wt\n\n/other/.git\n") + }, + wantErr: "--worktree path is inside a different repository", + }, + { + name: "target is non-git or non-existent", + stubs: func(cs *run.CommandStubber) { + cs.Register(`git rev-parse --path-format=absolute --show-toplevel --git-common-dir`, 0, "/repo/main\n/repo/.git\n") + cs.Register(`git -C .+ rev-parse --path-format=absolute --show-toplevel --show-prefix --git-common-dir`, 128, "") + }, + wantReuse: false, + }, + { + name: "current worktree cannot be determined", + stubs: func(cs *run.CommandStubber) { + cs.Register(`git rev-parse --path-format=absolute --show-toplevel --git-common-dir`, 128, "") + }, + wantReuse: false, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + cs, teardown := run.Stub() + defer teardown(t) + tt.stubs(cs) + + client := &git.Client{ + GhPath: "/some/path/gh", + GitPath: "/some/path/git", + } + reuse, err := resolveWorktreeTarget(client, dir) + if tt.wantErr != "" { + require.EqualError(t, err, tt.wantErr) + return + } + require.NoError(t, err) + assert.Equal(t, tt.wantReuse, reuse) + }) + } +} + +func Test_ensureWorktreePathSafe(t *testing.T) { + base := t.TempDir() + + existingDir := filepath.Join(base, "dir") + require.NoError(t, os.Mkdir(existingDir, 0o755)) + + regularFile := filepath.Join(base, "file") + require.NoError(t, os.WriteFile(regularFile, []byte("x"), 0o644)) + + symlink := filepath.Join(base, "link") + require.NoError(t, os.Symlink(existingDir, symlink)) + + tests := []struct { + name string + path string + wantErr string + }{ + { + name: "non-existent path is allowed", + path: filepath.Join(base, "does-not-exist"), + }, + { + name: "existing directory is allowed", + path: existingDir, + }, + { + name: "leaf symlink is rejected", + path: symlink, + wantErr: "--worktree path must not be a symlink", + }, + { + name: "existing non-directory is rejected", + path: regularFile, + wantErr: "--worktree path must be a directory", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := ensureWorktreePathSafe(tt.path) + if tt.wantErr == "" { + require.NoError(t, err) + return + } + require.Error(t, err) + assert.Contains(t, err.Error(), tt.wantErr) + }) + } +} diff --git a/skills/gh/SKILL.md b/skills/gh/SKILL.md index 65b3479dce8..626a41080eb 100644 --- a/skills/gh/SKILL.md +++ b/skills/gh/SKILL.md @@ -160,6 +160,8 @@ Sometimes useful data isn't on the typed commands. Examples: - `gh pr checkout ` switches branches. Use `gh pr diff ` or `gh pr view ` if you only need to read. +- `gh pr checkout --worktree ` checks the PR out into a git worktree + at `` instead of switching the current branch. - `NO_COLOR`, `CLICOLOR_FORCE`, and `GH_FORCE_TTY` are honored. Set `GH_FORCE_TTY=1` if you want TTY-style output (colors, tables, the pager, interactivity) inside an agent harness; leave it unset unless needed.