From a94ce40e2caf32b56c2e5037b28db20d88ef0097 Mon Sep 17 00:00:00 2001 From: samzong Date: Fri, 3 Apr 2026 13:32:44 +0800 Subject: [PATCH 1/2] feat(worktree): add client initialization cache and batch remove Signed-off-by: samzong --- cmd/worktree.go | 9 +- internal/worktree/pr.go | 11 +- internal/worktree/prune.go | 14 +- internal/worktree/resource.go | 36 +-- internal/worktree/share_discover.go | 2 +- internal/worktree/sync.go | 13 +- internal/worktree/worktree.go | 325 +++++++++++++++++++--------- internal/worktree/worktree_test.go | 304 ++++++++++++++++++++++++++ 8 files changed, 567 insertions(+), 147 deletions(-) diff --git a/cmd/worktree.go b/cmd/worktree.go index f157304..8906f26 100644 --- a/cmd/worktree.go +++ b/cmd/worktree.go @@ -340,15 +340,18 @@ func runWorktreeRemove(wtClient *worktree.Client, names []string) error { DeleteBranch: wtDeleteBranch, DryRun: wtDryRun, } + + result := wtClient.RemoveBatch(names, opts) + printWorktreeReport(result.Report) + var failed []string for _, name := range names { - report, err := wtClient.Remove(name, opts) - printWorktreeReport(report) - if err != nil { + if err, ok := result.Failed[name]; ok { fmt.Fprintf(errWriter(), "Error removing '%s': %v\n", name, err) failed = append(failed, name) } } + if len(failed) > 0 { return fmt.Errorf("failed to remove worktrees: %s", strings.Join(failed, ", ")) } diff --git a/internal/worktree/pr.go b/internal/worktree/pr.go index 71c31e2..c210ebb 100644 --- a/internal/worktree/pr.go +++ b/internal/worktree/pr.go @@ -64,12 +64,10 @@ func (c *Client) PRExists(prNumber int, remote, repoDir string) (bool, string, e func (c *Client) AddPR(prNumber int, remote string) (Report, error) { var report Report - // Setup paths to get repoDir - root, err := c.GetWorktreeRoot() - if err != nil { + if err := c.ensureInit(); err != nil { return report, fmt.Errorf("failed to find worktree root: %w", err) } - repoDir := repoDirForGit(root) + repoDir := c.repoDir // Auto-detect remote if not specified if remote == "" { @@ -92,7 +90,7 @@ func (c *Client) AddPR(prNumber int, remote string) (Report, error) { // Prepare worktree paths branchName := fmt.Sprintf("pr-%d", prNumber) - targetPath := filepath.Join(root, branchName) + targetPath := filepath.Join(c.worktreeRoot, branchName) if _, err := os.Stat(targetPath); err == nil { return report, fmt.Errorf("directory already exists: %s", targetPath) @@ -115,7 +113,8 @@ func (c *Client) AddPR(prNumber int, remote string) (Report, error) { return report, gitutil.WrapGitError("failed to create worktree", result, err) } - // Sync shared resources + c.InvalidateList() + sharedReport, err := c.SyncSharedResources(branchName) report.Merge(sharedReport) if err != nil { diff --git a/internal/worktree/prune.go b/internal/worktree/prune.go index 64fb928..f181a7d 100644 --- a/internal/worktree/prune.go +++ b/internal/worktree/prune.go @@ -59,17 +59,19 @@ func ghRunDefault(repoDir string, args ...string) ([]byte, error) { func (c *Client) Prune(opts PruneOptions) (PruneResult, error) { var result PruneResult - root, err := c.GetWorktreeRoot() - if err != nil { + if err := c.ensureInit(); err != nil { return result, fmt.Errorf("failed to find worktree root: %w", err) } + if !opts.DryRun { + defer c.InvalidateList() + } - baseBranch, err := c.resolveBaseBranch(root, opts.BaseBranch) + baseBranch, err := c.resolveBaseBranch(c.worktreeRoot, opts.BaseBranch) if err != nil { return result, err } - candidates, repoDir, err := c.collectPruneCandidates(root, baseBranch, &result.Report) + candidates, repoDir, err := c.collectPruneCandidates(c.worktreeRoot, baseBranch, &result.Report) if err != nil { return result, err } @@ -77,13 +79,13 @@ func (c *Client) Prune(opts PruneOptions) (PruneResult, error) { if opts.PRAware { return c.prunePRAware(opts, candidates, repoDir, result) } - return c.pruneClassic(opts, candidates, root, baseBranch, repoDir, result) + return c.pruneClassic(opts, candidates, c.worktreeRoot, baseBranch, repoDir, result) } func (c *Client) collectPruneCandidates(root, baseBranch string, report *Report) ([]pruneCandidate, string, error) { baseBranchName := localBranchName(baseBranch) - worktrees, err := c.List() + worktrees, err := c.ListCached() if err != nil { return nil, "", err } diff --git a/internal/worktree/resource.go b/internal/worktree/resource.go index 2dc8e82..0babb87 100644 --- a/internal/worktree/resource.go +++ b/internal/worktree/resource.go @@ -63,13 +63,12 @@ func (c *Client) syncSharedResourcesToPath(targetRoot string, runHooks bool) (Re return report, nil } - repoRoot, err := c.GetRepoRoot() - if err != nil { + if err := c.ensureInit(); err != nil { return report, err } for _, res := range cfg.Resources { - resourceReport, err := c.syncOneResource(repoRoot, targetRoot, res) + resourceReport, err := c.syncOneResource(c.worktreeRoot, targetRoot, res) report.Merge(resourceReport) if err != nil { return report, err @@ -149,10 +148,12 @@ func (c *Client) syncOneResource(repoRoot, targetRoot string, res SharedResource } func (c *Client) LoadSharedConfig() (*SharedConfig, string, error) { + c.once.Do(c.init) + commonDir, err := c.GetGitCommonDir() if err != nil { - if root, bareErr := FindBareRoot(""); bareErr == nil { - commonDir = filepath.Join(root, ".bare") + if c.bareRoot != "" { + commonDir = filepath.Join(c.bareRoot, ".bare") } else { return nil, "", err } @@ -163,10 +164,10 @@ func (c *Client) LoadSharedConfig() (*SharedConfig, string, error) { filepath.Join(commonDir, legacySharedConfigYML), filepath.Join(commonDir, legacySharedConfigYAML), } - if repoRoot, rootErr := c.GetRepoRoot(); rootErr == nil && repoRoot != "" { + if c.worktreeRoot != "" { legacyCandidates = append(legacyCandidates, - filepath.Join(repoRoot, legacySharedConfigYML), - filepath.Join(repoRoot, legacySharedConfigYAML), + filepath.Join(c.worktreeRoot, legacySharedConfigYML), + filepath.Join(c.worktreeRoot, legacySharedConfigYAML), ) } @@ -338,21 +339,21 @@ func (c *Client) RemoveHook(index int) (Report, error) { func (c *Client) SyncAllSharedResources() (Report, error) { var report Report - worktrees, err := c.List() + worktrees, err := c.ListCached() if err != nil { return report, err } - root, _ := c.GetRepoRoot() - repoDir := repoDirForGit(root) - isBare := repoDir != root // bare layout: repoDir points to .bare + if err := c.ensureInit(); err != nil { + return report, err + } + isBare := c.repoDir != c.worktreeRoot var targets []Info for _, wt := range worktrees { if wt.IsBare || filepath.Base(wt.Path) == ".bare" { continue } - // In bare layout, skip worktrees outside the managed root directory - if isBare && isExternalPath(root, wt.Path) { + if isBare && isExternalPath(c.worktreeRoot, wt.Path) { continue } targets = append(targets, wt) @@ -380,8 +381,9 @@ func (c *Client) resolveWorktreePath(worktreeName string) (string, error) { return "", errors.New("worktree name cannot be empty") } - repoRoot, _ := c.GetRepoRoot() - worktrees, err := c.List() + c.once.Do(c.init) + repoRoot := c.worktreeRoot + worktrees, err := c.ListCached() if err != nil { if repoRoot != "" { candidate := filepath.Join(repoRoot, worktreeName) @@ -479,7 +481,7 @@ func (c *Client) resolveSharedPaths(repoRoot, targetRoot string, res SharedResou parts := strings.SplitN(res.Path, string(filepath.Separator), 2) if len(parts) == 2 { - worktrees, listErr := c.List() + worktrees, listErr := c.ListCached() if listErr == nil { var baseMatches []string for _, wt := range worktrees { diff --git a/internal/worktree/share_discover.go b/internal/worktree/share_discover.go index 5c37436..3a9469a 100644 --- a/internal/worktree/share_discover.go +++ b/internal/worktree/share_discover.go @@ -119,7 +119,7 @@ func (c *Client) AddDiscoveredResources(results []DiscoverResult) (Report, error } func (c *Client) findMainWorktreePath() (string, error) { - worktrees, err := c.List() + worktrees, err := c.ListCached() if err != nil { return "", fmt.Errorf("failed to list worktrees: %w", err) } diff --git a/internal/worktree/sync.go b/internal/worktree/sync.go index 3b15c30..669445b 100644 --- a/internal/worktree/sync.go +++ b/internal/worktree/sync.go @@ -31,23 +31,20 @@ type syncContext struct { // ResolveSyncBaseBranch resolves the base branch used for syncing. func (c *Client) ResolveSyncBaseBranch(override string) (string, error) { - root, err := c.GetWorktreeRoot() - if err != nil { + if err := c.ensureInit(); err != nil { return "", fmt.Errorf("failed to find worktree root: %w", err) } - return c.resolveSyncBaseBranch(repoDirForGit(root), override) + return c.resolveSyncBaseBranch(c.repoDir, override) } -// Sync updates the base branch refs and main worktree using fast-forward only. func (c *Client) Sync(opts SyncOptions) (Report, error) { var report Report - root, err := c.GetWorktreeRoot() - if err != nil { + if err := c.ensureInit(); err != nil { return report, fmt.Errorf("failed to find worktree root: %w", err) } - repoDir := repoDirForGit(root) + repoDir := c.repoDir remote, err := c.selectSyncRemote(repoDir) if err != nil { return report, err @@ -63,7 +60,7 @@ func (c *Client) Sync(opts SyncOptions) (Report, error) { remoteFull := "refs/remotes/" + remoteRef localFull := "refs/heads/" + baseName - worktrees, err := c.List() + worktrees, err := c.ListCached() if err != nil { return report, err } diff --git a/internal/worktree/worktree.go b/internal/worktree/worktree.go index 46f009f..465cd8b 100644 --- a/internal/worktree/worktree.go +++ b/internal/worktree/worktree.go @@ -7,6 +7,7 @@ import ( "path/filepath" "strconv" "strings" + "sync" "time" "github.com/samzong/gmc/internal/gitcmd" @@ -20,6 +21,17 @@ type Options struct { type Client struct { runner gitcmd.Runner verbose bool + + once sync.Once + bareRoot string + worktreeRoot string + searchRoot string + repoDir string + initErr error + + listMu sync.Mutex + listCache []Info + listValid bool } func NewClient(opts Options) *Client { @@ -29,6 +41,64 @@ func NewClient(opts Options) *Client { } } +func (c *Client) init() { + bareRoot, err := FindBareRoot("") + if err == nil { + c.bareRoot = bareRoot + c.worktreeRoot = bareRoot + } else { + commonDir, cdErr := c.GetGitCommonDir() + if cdErr != nil { + c.initErr = cdErr + return + } + c.worktreeRoot = filepath.Dir(commonDir) + } + + c.repoDir = repoDirForGit(c.worktreeRoot) + + if c.repoDir != c.worktreeRoot { + c.searchRoot = c.worktreeRoot + } else { + c.searchRoot = filepath.Dir(c.worktreeRoot) + } +} + +func (c *Client) ensureInit() error { + c.once.Do(c.init) + return c.initErr +} + +func (c *Client) ListCached() ([]Info, error) { + c.listMu.Lock() + defer c.listMu.Unlock() + + if c.listValid { + out := make([]Info, len(c.listCache)) + copy(out, c.listCache) + return out, nil + } + + list, err := c.List() + if err != nil { + return nil, err + } + + c.listCache = list + c.listValid = true + + out := make([]Info, len(list)) + copy(out, list) + return out, nil +} + +func (c *Client) InvalidateList() { + c.listMu.Lock() + c.listCache = nil + c.listValid = false + c.listMu.Unlock() +} + // RepoType represents the type of git repository type RepoType int @@ -198,21 +268,10 @@ func (c *Client) GetGitCommonDir() (string, error) { // GetRepoRoot returns the main worktree/repository root for the current repository family. func (c *Client) GetRepoRoot() (string, error) { - // First try bare-root discovery for gmc's preferred layout. - root, err := FindBareRoot("") - if err == nil { - return root, nil - } - - commonDir, err := c.GetGitCommonDir() - if err != nil { + if err := c.ensureInit(); err != nil { return "", err } - - if filepath.Base(commonDir) == ".bare" { - return filepath.Dir(commonDir), nil - } - return filepath.Dir(commonDir), nil + return c.worktreeRoot, nil } // GetWorktreeRoot returns the root directory for worktrees (parent of .bare or main repo root). @@ -222,30 +281,27 @@ func (c *Client) GetWorktreeRoot() (string, error) { // IsBareWorktree checks if the current repository uses the .bare worktree pattern func (c *Client) IsBareWorktree() bool { - _, err := FindBareRoot("") - return err == nil + c.once.Do(c.init) + return c.bareRoot != "" } // List returns all worktrees for the current repository func (c *Client) List() ([]Info, error) { - // Find the bare root to support running from any directory - root, err := FindBareRoot("") - if err != nil { - // Fallback to current directory if not in a bare repo structure - result, err := c.runner.RunLogged("worktree", "list", "--porcelain") + c.once.Do(c.init) + + if c.bareRoot != "" { + bareDir := filepath.Join(c.bareRoot, ".bare") + result, err := c.runner.RunLogged("-C", bareDir, "worktree", "list", "--porcelain") if err != nil { return nil, fmt.Errorf("failed to list worktrees: %w", err) } return parseWorktreeList(string(result.Stdout)) } - // Run git command from the .bare directory - bareDir := filepath.Join(root, ".bare") - result, err := c.runner.RunLogged("-C", bareDir, "worktree", "list", "--porcelain") + result, err := c.runner.RunLogged("worktree", "list", "--porcelain") if err != nil { return nil, fmt.Errorf("failed to list worktrees: %w", err) } - return parseWorktreeList(string(result.Stdout)) } @@ -315,8 +371,8 @@ func (c *Client) Add(name string, opts AddOptions) (Report, error) { return report, gitutil.WrapGitError("failed to create worktree", result, err) } - // Pass the directory name (base of targetPath), not the branch name, - // since SyncSharedResources resolves by worktree path. + c.InvalidateList() + sharedReport, err := c.SyncSharedResources(filepath.Base(ctx.targetPath)) report.Merge(sharedReport) if err != nil { @@ -373,9 +429,109 @@ func (c *Client) Remove(name string, opts RemoveOptions) (Report, error) { report.Warn(fmt.Sprintf("Deleted branch '%s'", ctx.wtInfo.Branch)) } + c.InvalidateList() + return report, nil } +type RemoveBatchResult struct { + Succeeded []string + Failed map[string]error + Report Report +} + +func (c *Client) RemoveBatch(names []string, opts RemoveOptions) RemoveBatchResult { + result := RemoveBatchResult{ + Failed: make(map[string]error), + } + + if err := c.ensureInit(); err != nil { + for _, n := range names { + result.Failed[n] = err + } + return result + } + + worktrees, err := c.ListCached() + if err != nil { + for _, n := range names { + result.Failed[n] = err + } + return result + } + + var targets []removeContext + for _, name := range names { + ctx, resolveErr := c.resolveRemoveTarget(name, worktrees) + if resolveErr != nil { + result.Failed[name] = resolveErr + continue + } + targets = append(targets, ctx) + } + + if len(result.Failed) > 0 { + return result + } + + type pendingBranch struct { + branch string + name string + } + var branchesToDelete []pendingBranch + for _, t := range targets { + if opts.DryRun { + status := c.GetWorktreeStatus(t.targetPath) + result.Report.Warn("Would remove worktree: " + t.targetPath) + result.Report.Warn(" Branch: " + t.wtInfo.Branch) + result.Report.Warn(" Status: " + status) + if opts.DeleteBranch && t.wtInfo.Branch != "" && t.wtInfo.Branch != "(detached)" { + result.Report.Warn("Would delete branch: " + t.wtInfo.Branch) + } + if status == "modified" && !opts.Force { + result.Report.Warn("Note: Worktree has uncommitted changes. Use -f to force removal.") + } + result.Succeeded = append(result.Succeeded, t.name) + continue + } + + args := []string{"-C", c.repoDir, "worktree", "remove"} + if opts.Force { + args = append(args, "--force") + } + args = append(args, t.targetPath) + + runResult, err := c.runner.RunLogged(args...) + if err != nil { + result.Failed[t.name] = gitutil.WrapGitError("failed to remove worktree", runResult, err) + continue + } + + result.Report.Warn(fmt.Sprintf("Removed worktree '%s'", t.name)) + result.Succeeded = append(result.Succeeded, t.name) + + if opts.DeleteBranch && t.wtInfo.Branch != "" && t.wtInfo.Branch != "(detached)" { + branchesToDelete = append(branchesToDelete, pendingBranch{branch: t.wtInfo.Branch, name: t.name}) + } + } + + for _, pb := range branchesToDelete { + args := []string{"-C", c.repoDir, "branch", "-D", pb.branch} + runResult, err := c.runner.RunLogged(args...) + if err != nil { + result.Failed[pb.name] = gitutil.WrapGitError("failed to delete branch", runResult, err) + continue + } + result.Report.Warn(fmt.Sprintf("Deleted branch '%s'", pb.branch)) + } + + if !opts.DryRun && len(result.Succeeded) > 0 { + c.InvalidateList() + } + + return result +} + func (c *Client) prepareAdd(name string, opts AddOptions) (addContext, error) { if name == "" { return addContext{}, errors.New("worktree name cannot be empty") @@ -384,25 +540,17 @@ func (c *Client) prepareAdd(name string, opts AddOptions) (addContext, error) { return addContext{}, err } - root, err := c.GetWorktreeRoot() - if err != nil { + if err := c.ensureInit(); err != nil { return addContext{}, fmt.Errorf("failed to find worktree root: %w", err) } - // Directory name: replace "/" with "--" so branch names like "feat/login" - // become "feat--login" instead of a nested directory. dirName := strings.ReplaceAll(name, "/", "--") - // Build target path: - // bare layout → / (sibling of .bare) - // non-bare → /-- (sibling of the repo) var targetPath string - repoDir := repoDirForGit(root) - if repoDir != root { // bare layout: .bare exists - targetPath = filepath.Join(root, dirName) + if c.repoDir != c.worktreeRoot { + targetPath = filepath.Join(c.worktreeRoot, dirName) } else { - // Non-bare: create as a sibling of the repo, prefixed with the repo name. - targetPath = filepath.Join(filepath.Dir(root), filepath.Base(root)+"--"+dirName) + targetPath = filepath.Join(filepath.Dir(c.worktreeRoot), filepath.Base(c.worktreeRoot)+"--"+dirName) } if _, err := os.Stat(targetPath); err == nil { @@ -416,7 +564,7 @@ func (c *Client) prepareAdd(name string, opts AddOptions) (addContext, error) { return addContext{ name: name, - repoDir: repoDir, + repoDir: c.repoDir, targetPath: targetPath, baseBranch: baseBranch, }, nil @@ -448,32 +596,16 @@ func (c *Client) appendAddSummary(report *Report, ctx addContext, branchExists b report.Info("Next step: cd " + ctx.targetPath) } -func (c *Client) prepareRemove(name string) (removeContext, error) { +func (c *Client) resolveRemoveTarget(name string, worktrees []Info) (removeContext, error) { if name == "" { return removeContext{}, errors.New("worktree name cannot be empty") } - root, err := c.GetWorktreeRoot() - if err != nil { - return removeContext{}, fmt.Errorf("failed to find worktree root: %w", err) - } - - // searchRoot is where linked worktrees live (parent of repo for non-bare). - searchRoot, err := c.worktreeSearchRoot() - if err != nil { - return removeContext{}, fmt.Errorf("failed to determine worktree search root: %w", err) - } - - targetPath := filepath.Join(searchRoot, name) - worktrees, err := c.List() - if err != nil { - return removeContext{}, err - } - + targetPath := filepath.Join(c.searchRoot, name) var found bool var wtInfo Info for _, wt := range worktrees { - relPath := strings.TrimPrefix(wt.Path, searchRoot+string(filepath.Separator)) + relPath := strings.TrimPrefix(wt.Path, c.searchRoot+string(filepath.Separator)) if wt.Path == targetPath || relPath == name { wtInfo = wt targetPath = wt.Path @@ -488,20 +620,32 @@ func (c *Client) prepareRemove(name string) (removeContext, error) { return removeContext{}, errors.New("cannot remove the main bare worktree") } - // Reject agent/external worktrees (outside searchRoot). - rel, err := filepath.Rel(searchRoot, wtInfo.Path) + rel, err := filepath.Rel(c.searchRoot, wtInfo.Path) if err != nil || strings.HasPrefix(rel, "..") { return removeContext{}, fmt.Errorf("worktree '%s' is external (not managed by gmc wt)", name) } return removeContext{ name: name, - repoDir: repoDirForGit(root), + repoDir: c.repoDir, targetPath: targetPath, wtInfo: wtInfo, }, nil } +func (c *Client) prepareRemove(name string) (removeContext, error) { + if err := c.ensureInit(); err != nil { + return removeContext{}, fmt.Errorf("failed to find worktree root: %w", err) + } + + worktrees, err := c.ListCached() + if err != nil { + return removeContext{}, err + } + + return c.resolveRemoveTarget(name, worktrees) +} + // GetWorktreeStatus returns the git status of a worktree with detailed file counts func (c *Client) GetWorktreeStatus(path string) string { result, err := c.runner.Run("-C", path, "status", "--porcelain") @@ -554,24 +698,14 @@ func (c *Client) GetWorktreeStatus(path string) string { return strings.Join(parts, ", ") } -// branchExists checks if a branch exists in the repository func (c *Client) branchExists(name string) (bool, error) { - // Try to find the bare repo root for proper -C path - root, _ := c.GetWorktreeRoot() + c.once.Do(c.init) var args []string - if root == "" { - // Fallback to current directory - args = []string{"rev-parse", "--verify", "refs/heads/" + name} + if c.repoDir != "" { + args = []string{"-C", c.repoDir, "rev-parse", "--verify", "refs/heads/" + name} } else { - // Check if .bare directory exists - bareDir := filepath.Join(root, ".bare") - if _, statErr := os.Stat(bareDir); statErr == nil { - args = []string{"-C", bareDir, "rev-parse", "--verify", "refs/heads/" + name} - } else { - // Standard repo - args = []string{"-C", root, "rev-parse", "--verify", "refs/heads/" + name} - } + args = []string{"rev-parse", "--verify", "refs/heads/" + name} } _, err := c.runner.Run(args...) @@ -591,7 +725,6 @@ type DupResult struct { Warnings []string // Non-fatal warnings generated during creation } -// Dup creates multiple worktrees with temporary branches for parallel development func (c *Client) Dup(opts DupOptions) (*DupResult, error) { if opts.BaseBranch == "" { opts.BaseBranch = "HEAD" @@ -600,11 +733,9 @@ func (c *Client) Dup(opts DupOptions) (*DupResult, error) { opts.Count = 2 } - root, err := c.GetWorktreeRoot() - if err != nil { + if err := c.ensureInit(); err != nil { return nil, fmt.Errorf("failed to find worktree root: %w", err) } - repoDir := repoDirForGit(root) timestamp := strconv.FormatInt(getCurrentTimestamp(), 10) dupResult := &DupResult{ @@ -615,21 +746,20 @@ func (c *Client) Dup(opts DupOptions) (*DupResult, error) { for i := 1; i <= opts.Count; i++ { dirName := fmt.Sprintf(".dup-%d", i) branchName := fmt.Sprintf("_dup/%s/%s-%d", opts.BaseBranch, timestamp, i) - targetPath := filepath.Join(root, dirName) + targetPath := filepath.Join(c.worktreeRoot, dirName) - // Check if directory already exists if _, err := os.Stat(targetPath); err == nil { return nil, fmt.Errorf("directory already exists: %s", targetPath) } - // Create worktree with new branch - args := []string{"-C", repoDir, "worktree", "add", "-b", branchName, targetPath, opts.BaseBranch} + args := []string{"-C", c.repoDir, "worktree", "add", "-b", branchName, targetPath, opts.BaseBranch} runResult, err := c.runner.RunLogged(args...) if err != nil { return nil, gitutil.WrapGitError("failed to create worktree "+dirName, runResult, err) } - // Sync shared resources + c.InvalidateList() + sharedReport, err := c.SyncSharedResources(dirName) if err != nil { dupResult.Warnings = append( @@ -665,12 +795,11 @@ func (c *Client) Promote(worktreeName, newBranchName string) (Report, error) { return report, err } - searchRoot, err := c.worktreeSearchRoot() - if err != nil { + if err := c.ensureInit(); err != nil { return report, fmt.Errorf("failed to determine worktree search root: %w", err) } - targetPath := filepath.Join(searchRoot, worktreeName) + targetPath := filepath.Join(c.searchRoot, worktreeName) // Verify worktree exists if _, err := os.Stat(targetPath); os.IsNotExist(err) { @@ -704,21 +833,6 @@ func getCurrentTimestamp() int64 { return time.Now().Unix() } -// worktreeSearchRoot returns the directory in which linked worktrees are expected to live. -// Bare layout: root (parent of .bare) — managed worktrees are direct children. -// Non-bare layout: parent of the repo dir — linked worktrees can be siblings of the repo. -func (c *Client) worktreeSearchRoot() (string, error) { - root, err := c.GetWorktreeRoot() - if err != nil { - return "", err - } - repoDir := repoDirForGit(root) - if repoDir != root { // bare layout: .bare exists - return root, nil - } - return filepath.Dir(root), nil -} - // isExternalPath reports whether wtPath is outside the managed root directory. func isExternalPath(root, wtPath string) bool { if root == "" { @@ -733,12 +847,11 @@ func isExternalPath(root, wtPath string) bool { // listGitRefs runs a git command in the repo dir and splits output by newline. func (c *Client) listGitRefs(errLabel string, gitArgs ...string) ([]string, error) { - root, _ := c.GetWorktreeRoot() - repoDir := repoDirForGit(root) + c.once.Do(c.init) var args []string - if repoDir != "" { - args = append([]string{"-C", repoDir}, gitArgs...) + if c.repoDir != "" { + args = append([]string{"-C", c.repoDir}, gitArgs...) } else { args = gitArgs } diff --git a/internal/worktree/worktree_test.go b/internal/worktree/worktree_test.go index d0ff11a..f18d40f 100644 --- a/internal/worktree/worktree_test.go +++ b/internal/worktree/worktree_test.go @@ -1,6 +1,7 @@ package worktree import ( + "fmt" "os" "os/exec" "path/filepath" @@ -635,3 +636,306 @@ func writeFile(t *testing.T, path string, content string) { t.Fatalf("writeFile(%s) failed: %v", path, err) } } + +func initBareLayoutRepo(t *testing.T) string { + t.Helper() + tmpDir := t.TempDir() + + bareDir := filepath.Join(tmpDir, ".bare") + runGit(t, tmpDir, "init", "--bare", bareDir) + runGit(t, bareDir, "config", "user.name", "Test User") + runGit(t, bareDir, "config", "user.email", "test@example.com") + + mainDir := filepath.Join(tmpDir, "main") + runGit(t, bareDir, "worktree", "add", mainDir, "-b", "main") + writeFile(t, filepath.Join(mainDir, "README.md"), "init") + runGit(t, mainDir, "add", ".") + runGit(t, mainDir, "commit", "-m", "init") + + return tmpDir +} + +func TestClientCacheInitialization(t *testing.T) { + repoDir := initBareLayoutRepo(t) + repoDir, _ = filepath.EvalSymlinks(repoDir) + mainDir := filepath.Join(repoDir, "main") + + cwd, err := os.Getwd() + if err != nil { + t.Fatal(err) + } + defer func() { _ = os.Chdir(cwd) }() + if err := os.Chdir(mainDir); err != nil { + t.Fatal(err) + } + + client := NewClient(Options{}) + + root, err := client.GetWorktreeRoot() + if err != nil { + t.Fatalf("GetWorktreeRoot() error = %v", err) + } + if root != repoDir { + t.Errorf("GetWorktreeRoot() = %q, want %q", root, repoDir) + } + + if !client.IsBareWorktree() { + t.Error("IsBareWorktree() should return true for bare layout") + } + + if client.bareRoot != repoDir { + t.Errorf("bareRoot = %q, want %q", client.bareRoot, repoDir) + } + if client.worktreeRoot != repoDir { + t.Errorf("worktreeRoot = %q, want %q", client.worktreeRoot, repoDir) + } + expectedRepoDir := filepath.Join(repoDir, ".bare") + if client.repoDir != expectedRepoDir { + t.Errorf("repoDir = %q, want %q", client.repoDir, expectedRepoDir) + } + if client.searchRoot != repoDir { + t.Errorf("searchRoot = %q, want %q", client.searchRoot, repoDir) + } + + root2, _ := client.GetWorktreeRoot() + if root2 != root { + t.Errorf("second GetWorktreeRoot() = %q, want %q (should be cached)", root2, root) + } +} + +func TestListCacheInvalidation(t *testing.T) { + repoDir := initBareLayoutRepo(t) + mainDir := filepath.Join(repoDir, "main") + + cwd, err := os.Getwd() + if err != nil { + t.Fatal(err) + } + defer func() { _ = os.Chdir(cwd) }() + if err := os.Chdir(mainDir); err != nil { + t.Fatal(err) + } + + client := NewClient(Options{}) + + list1, err := client.ListCached() + if err != nil { + t.Fatalf("ListCached() error = %v", err) + } + initialCount := len(list1) + + list2, err := client.ListCached() + if err != nil { + t.Fatalf("ListCached() second call error = %v", err) + } + if len(list2) != initialCount { + t.Errorf("ListCached() returned different count: %d vs %d", len(list2), initialCount) + } + + bareDir := filepath.Join(repoDir, ".bare") + featureDir := filepath.Join(repoDir, "feature-test") + runGit(t, bareDir, "worktree", "add", "-b", "feature-test", featureDir, "main") + + staleList, _ := client.ListCached() + if len(staleList) != initialCount { + t.Error("ListCached() should return stale data before invalidation") + } + + client.InvalidateList() + + freshList, err := client.ListCached() + if err != nil { + t.Fatalf("ListCached() after invalidation error = %v", err) + } + if len(freshList) != initialCount+1 { + t.Errorf("ListCached() after invalidation = %d worktrees, want %d", len(freshList), initialCount+1) + } +} + +func TestRemoveBatch(t *testing.T) { + repoDir := initBareLayoutRepo(t) + mainDir := filepath.Join(repoDir, "main") + + cwd, err := os.Getwd() + if err != nil { + t.Fatal(err) + } + defer func() { _ = os.Chdir(cwd) }() + if err := os.Chdir(mainDir); err != nil { + t.Fatal(err) + } + + client := NewClient(Options{}) + bareDir := filepath.Join(repoDir, ".bare") + + names := []string{"wt-a", "wt-b", "wt-c"} + for _, name := range names { + dir := filepath.Join(repoDir, name) + runGit(t, bareDir, "worktree", "add", "-b", name, dir, "main") + } + client.InvalidateList() + + result := client.RemoveBatch(names, RemoveOptions{Force: true, DeleteBranch: true}) + + if len(result.Failed) > 0 { + t.Fatalf("RemoveBatch() had failures: %v", result.Failed) + } + if len(result.Succeeded) != 3 { + t.Errorf("RemoveBatch() succeeded = %d, want 3", len(result.Succeeded)) + } + + for _, name := range names { + dir := filepath.Join(repoDir, name) + if _, err := os.Stat(dir); !os.IsNotExist(err) { + t.Errorf("worktree %s still exists", name) + } + } + + for _, name := range names { + if _, err := client.runner.Run("-C", bareDir, "rev-parse", "--verify", "refs/heads/"+name); err == nil { + t.Errorf("branch %s still exists", name) + } + } +} + +func TestRemoveBatchPartialFailure(t *testing.T) { + repoDir := initBareLayoutRepo(t) + mainDir := filepath.Join(repoDir, "main") + + cwd, err := os.Getwd() + if err != nil { + t.Fatal(err) + } + defer func() { _ = os.Chdir(cwd) }() + if err := os.Chdir(mainDir); err != nil { + t.Fatal(err) + } + + client := NewClient(Options{}) + bareDir := filepath.Join(repoDir, ".bare") + + dir := filepath.Join(repoDir, "wt-good") + runGit(t, bareDir, "worktree", "add", "-b", "wt-good", dir, "main") + client.InvalidateList() + + result := client.RemoveBatch([]string{"wt-good", "wt-nonexistent"}, RemoveOptions{Force: true}) + + if len(result.Failed) == 0 { + t.Fatal("RemoveBatch() should have failures for nonexistent worktree") + } + if _, ok := result.Failed["wt-nonexistent"]; !ok { + t.Error("expected wt-nonexistent in Failed map") + } + + if len(result.Succeeded) != 0 { + t.Error("RemoveBatch() should not have succeeded entries when validation fails") + } + + if _, err := os.Stat(dir); os.IsNotExist(err) { + t.Error("wt-good should NOT be removed when batch validation fails (fail-fast)") + } +} + +func TestRemoveBatchDryRun(t *testing.T) { + repoDir := initBareLayoutRepo(t) + mainDir := filepath.Join(repoDir, "main") + + cwd, err := os.Getwd() + if err != nil { + t.Fatal(err) + } + defer func() { _ = os.Chdir(cwd) }() + if err := os.Chdir(mainDir); err != nil { + t.Fatal(err) + } + + client := NewClient(Options{}) + bareDir := filepath.Join(repoDir, ".bare") + + dir := filepath.Join(repoDir, "wt-dry") + runGit(t, bareDir, "worktree", "add", "-b", "wt-dry", dir, "main") + client.InvalidateList() + + result := client.RemoveBatch([]string{"wt-dry"}, RemoveOptions{DryRun: true, DeleteBranch: true}) + + if len(result.Failed) > 0 { + t.Fatalf("RemoveBatch(DryRun) failures: %v", result.Failed) + } + if len(result.Succeeded) != 1 { + t.Errorf("RemoveBatch(DryRun) succeeded = %d, want 1", len(result.Succeeded)) + } + + if _, err := os.Stat(dir); os.IsNotExist(err) { + t.Error("DryRun should not actually remove the worktree") + } + + if len(result.Report.Events) == 0 { + t.Error("DryRun should produce report events") + } +} + +func BenchmarkRemoveBatch(b *testing.B) { + for range b.N { + b.StopTimer() + + tmpDir := b.TempDir() + bareDir := filepath.Join(tmpDir, ".bare") + + cmd := exec.Command("git", "init", "--bare", bareDir) + cmd.Dir = tmpDir + if out, err := cmd.CombinedOutput(); err != nil { + b.Fatalf("git init --bare: %v\n%s", err, out) + } + + mainDir := filepath.Join(tmpDir, "main") + cmd = exec.Command("git", "-C", bareDir, "worktree", "add", mainDir, "-b", "main") + if out, err := cmd.CombinedOutput(); err != nil { + b.Fatalf("git worktree add main: %v\n%s", err, out) + } + + cmd = exec.Command("git", "-C", mainDir, "config", "user.name", "Bench") + if _, err := cmd.CombinedOutput(); err != nil { + b.Fatal(err) + } + cmd = exec.Command("git", "-C", mainDir, "config", "user.email", "b@b.com") + if _, err := cmd.CombinedOutput(); err != nil { + b.Fatal(err) + } + + if err := os.WriteFile(filepath.Join(mainDir, "f"), []byte("x"), 0644); err != nil { + b.Fatal(err) + } + cmd = exec.Command("git", "-C", mainDir, "add", ".") + if _, err := cmd.CombinedOutput(); err != nil { + b.Fatal(err) + } + cmd = exec.Command("git", "-C", mainDir, "commit", "-m", "init") + if _, err := cmd.CombinedOutput(); err != nil { + b.Fatal(err) + } + + names := make([]string, 5) + for j := range 5 { + name := fmt.Sprintf("bench-%d", j) + names[j] = name + dir := filepath.Join(tmpDir, name) + cmd = exec.Command("git", "-C", bareDir, "worktree", "add", "-b", name, dir, "main") + if out, err := cmd.CombinedOutput(); err != nil { + b.Fatalf("add worktree %s: %v\n%s", name, err, out) + } + } + + origDir, _ := os.Getwd() + if err := os.Chdir(mainDir); err != nil { + b.Fatal(err) + } + + client := NewClient(Options{}) + + b.StartTimer() + client.RemoveBatch(names, RemoveOptions{Force: true, DeleteBranch: true}) + b.StopTimer() + + _ = os.Chdir(origDir) + } +} From d12c9dc7eff505c6bfdbd36aca8f1bc7521d64d7 Mon Sep 17 00:00:00 2001 From: samzong Date: Fri, 3 Apr 2026 13:38:30 +0800 Subject: [PATCH 2/2] perf(worktree): batch branch -D and use syncSharedResourcesToPath directly Signed-off-by: samzong --- internal/worktree/pr.go | 6 +++--- internal/worktree/worktree.go | 29 ++++++++++++++++++----------- 2 files changed, 21 insertions(+), 14 deletions(-) diff --git a/internal/worktree/pr.go b/internal/worktree/pr.go index c210ebb..3c0a200 100644 --- a/internal/worktree/pr.go +++ b/internal/worktree/pr.go @@ -113,14 +113,14 @@ func (c *Client) AddPR(prNumber int, remote string) (Report, error) { return report, gitutil.WrapGitError("failed to create worktree", result, err) } - c.InvalidateList() - - sharedReport, err := c.SyncSharedResources(branchName) + sharedReport, err := c.syncSharedResourcesToPath(targetPath, true) report.Merge(sharedReport) if err != nil { report.Warn(fmt.Sprintf("Warning: failed to sync shared resources: %v", err)) } + c.InvalidateList() + report.Info(fmt.Sprintf("Created PR worktree '%s' at %s", branchName, targetPath)) report.Info("Commit: " + commitHash[:7]) report.Info("Next step: cd " + targetPath) diff --git a/internal/worktree/worktree.go b/internal/worktree/worktree.go index 465cd8b..70bf543 100644 --- a/internal/worktree/worktree.go +++ b/internal/worktree/worktree.go @@ -371,14 +371,14 @@ func (c *Client) Add(name string, opts AddOptions) (Report, error) { return report, gitutil.WrapGitError("failed to create worktree", result, err) } - c.InvalidateList() - - sharedReport, err := c.SyncSharedResources(filepath.Base(ctx.targetPath)) + sharedReport, err := c.syncSharedResourcesToPath(ctx.targetPath, true) report.Merge(sharedReport) if err != nil { report.Warn(fmt.Sprintf("Warning: failed to sync shared resources: %v", err)) } + c.InvalidateList() + c.appendAddSummary(&report, ctx, branchExists) return report, nil } @@ -515,14 +515,21 @@ func (c *Client) RemoveBatch(names []string, opts RemoveOptions) RemoveBatchResu } } - for _, pb := range branchesToDelete { - args := []string{"-C", c.repoDir, "branch", "-D", pb.branch} + if len(branchesToDelete) > 0 { + args := []string{"-C", c.repoDir, "branch", "-D"} + for _, pb := range branchesToDelete { + args = append(args, pb.branch) + } runResult, err := c.runner.RunLogged(args...) if err != nil { - result.Failed[pb.name] = gitutil.WrapGitError("failed to delete branch", runResult, err) - continue + for _, pb := range branchesToDelete { + result.Failed[pb.name] = gitutil.WrapGitError("failed to delete branch", runResult, err) + } + } else { + for _, pb := range branchesToDelete { + result.Report.Warn(fmt.Sprintf("Deleted branch '%s'", pb.branch)) + } } - result.Report.Warn(fmt.Sprintf("Deleted branch '%s'", pb.branch)) } if !opts.DryRun && len(result.Succeeded) > 0 { @@ -758,9 +765,7 @@ func (c *Client) Dup(opts DupOptions) (*DupResult, error) { return nil, gitutil.WrapGitError("failed to create worktree "+dirName, runResult, err) } - c.InvalidateList() - - sharedReport, err := c.SyncSharedResources(dirName) + sharedReport, err := c.syncSharedResourcesToPath(targetPath, true) if err != nil { dupResult.Warnings = append( dupResult.Warnings, @@ -777,6 +782,8 @@ func (c *Client) Dup(opts DupOptions) (*DupResult, error) { dupResult.Branches = append(dupResult.Branches, branchName) } + c.InvalidateList() + return dupResult, nil }