diff --git a/bin/README-tree-me.md b/bin/README-tree-me.md index fa45064..fe0b071 100644 --- a/bin/README-tree-me.md +++ b/bin/README-tree-me.md @@ -110,6 +110,8 @@ Removes the worktree for the specified branch. A full or unique abbreviated comm | Uncommitted changes or untracked files | `-f` | | Locked worktree (`git worktree lock`, or a tool like Supacode that locks the ones it manages) | `-f -f` (or `-ff`) | +Before git removes a worktree, `tree-me` gives you write permission on any read-only directory in it, such as a Go module cache. Without that, git stops partway through the directory and leaves behind a checkout that `git worktree list` no longer shows. A worktree that git will refuse keeps its permissions. + For a pattern, `-f` also skips the "Remove N worktree(s)?" confirmation. Every match is attempted even if git refuses one of them; refused worktrees are listed as skipped and the command exits non-zero. If several detached worktrees share the same commit, `tree-me` asks before removing all of them. Pass `-f` to skip that confirmation. diff --git a/bin/git-bclean-local b/bin/git-bclean-local index 1694915..02f3f37 100755 --- a/bin/git-bclean-local +++ b/bin/git-bclean-local @@ -7,6 +7,10 @@ set -e +SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +# shellcheck source=bin/lib/git-worktree.sh +source "$SCRIPT_DIR/lib/git-worktree.sh" + # Build a map of branch -> worktree path from porcelain output declare -A worktree_for current_wt="" @@ -34,13 +38,7 @@ for branch in "${gone_branches[@]}"; do wt_path="${worktree_for[$branch]:-}" if [ -n "$wt_path" ]; then - # Skip worktrees with uncommitted changes - if [ -n "$(git -C "$wt_path" status --porcelain 2>/dev/null)" ]; then - echo "Skipping $branch (worktree has uncommitted changes: $wt_path)" - continue - fi - - if err=$(git worktree remove "$wt_path" 2>&1); then + if err=$(worktree_remove "$wt_path" 2>&1); then echo "Removed worktree: $wt_path" else # A locked worktree (e.g. an active supacode build) can't be removed @@ -50,6 +48,8 @@ for branch in "${gone_branches[@]}"; do reason="locked" [[ "$err" == *'"owner":"supacode"'* ]] && reason="locked by supacode" echo "Skipping $branch (worktree $reason): $wt_path" + elif [[ "$err" == *"contains modified or untracked files"* ]]; then + echo "Skipping $branch (worktree has uncommitted changes: $wt_path)" else echo "Skipping $branch (could not remove worktree): $wt_path" echo " Reason: $err" diff --git a/bin/lib/git-worktree.sh b/bin/lib/git-worktree.sh index df4014c..f42d33a 100644 --- a/bin/lib/git-worktree.sh +++ b/bin/lib/git-worktree.sh @@ -58,3 +58,51 @@ worktree_create() { echo "$path" } + +# Gives the owner write permission on every directory under . +make_writable() { + find "$1" -type d ! -perm -u+w -exec chmod u+w {} + +} + +# Succeeds when `git worktree remove` with -f flags will refuse the +# worktree at because it is the main worktree, is locked, or has changes. +# git also refuses a worktree that has populated submodules. This check misses +# that case, and the owner then gains write permission on directories that +# they already own. +worktree_remove_refused() { + local path="$1" force="$2" git_dir + git_dir=$(git -C "$path" rev-parse --absolute-git-dir 2>/dev/null) || return 1 + if [ "$git_dir" = "$(git -C "$path" rev-parse --path-format=absolute --git-common-dir)" ]; then + return 0 + fi + if [ "$force" -lt 2 ] && [ -e "$git_dir/locked" ]; then + return 0 + fi + [ "$force" -lt 1 ] && [ -n "$(git -C "$path" status --porcelain --ignore-submodules=none 2>/dev/null)" ] +} + +# Runs `git worktree remove` with the given arguments, which are -f/--force +# flags and one worktree path. +# +# Go makes its module cache read-only, so `git worktree remove` fails partway +# through it. git then deletes the worktree's admin dir anyway, which leaves a +# checkout that no worktree command can see. This makes every directory under +# the path writable first. It leaves permissions alone when the directory is +# missing or git will refuse the removal. It does not run git when a directory +# stays read-only or unreadable. +worktree_remove() { + local arg path force=0 + for arg in "$@"; do + case "$arg" in + -f|--force) force=$((force + 1)) ;; + *) path="$arg" ;; + esac + done + + if [ -d "$path" ] && ! worktree_remove_refused "$path" "$force" && ! make_writable "$path"; then + echo "error: could not make every directory under $path writable" >&2 + return 1 + fi + + git worktree remove "$@" +} diff --git a/bin/lib/test-git-bclean-local.sh b/bin/lib/test-git-bclean-local.sh new file mode 100755 index 0000000..6224fe3 --- /dev/null +++ b/bin/lib/test-git-bclean-local.sh @@ -0,0 +1,106 @@ +#!/bin/bash +# Tests for git-bclean-local's worktree removal. +# +# Usage: test-git-bclean-local.sh +# +# Builds a throwaway repo whose branches track a remote branch that is gone, +# gives each branch a worktree, runs bin/git-bclean-local against it, and +# cleans up on exit. + +set -euo pipefail + +SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +# shellcheck source=bin/lib/test-helpers.sh +source "$SCRIPT_DIR/test-helpers.sh" +BIN="$(cd "$SCRIPT_DIR/.." && pwd)/git-bclean-local" +find_bash4 + +# ── Fixture: a repo whose worktrees live under $WT_BASE ──────────────────── + +# Resolve through symlinks (macOS /var -> /private/var) so the literal paths we +# build match the canonical paths `git worktree list` reports. +REPO_DIR=$(cd "$(mktemp -d)" && pwd -P) +WT_BASE=$(cd "$(mktemp -d)" && pwd -P) +cleanup() { + # A plain `rm -rf` of the repo's admin files leaves locked worktrees behind. + # The loop unlocks every worktree first. + git -C "$REPO_DIR" worktree list --porcelain 2>/dev/null | + sed -n 's/^worktree //p' | + while read -r wt; do git -C "$REPO_DIR" worktree unlock "$wt" 2>/dev/null || true; done + # `rm -rf` cannot empty a read-only directory. + chmod -R u+w "$REPO_DIR" "$WT_BASE" 2>/dev/null || true + rm -rf "$REPO_DIR" "$WT_BASE" +} +trap cleanup EXIT + +git -C "$REPO_DIR" init -q +git -C "$REPO_DIR" config user.email test@example.com +git -C "$REPO_DIR" config user.name "Test" +git -C "$REPO_DIR" checkout -q -b main +echo one > "$REPO_DIR/file" +# git ignores cache/ the way posthog ignores .flox/cache. A read-only directory +# under it then leaves the worktree clean. +echo cache/ > "$REPO_DIR/.gitignore" +git -C "$REPO_DIR" add file .gitignore +git -C "$REPO_DIR" commit -qm "one" +# The remote is never fetched. A branch whose upstream ref is missing reports +# [gone], which is what gh poi leaves behind after a merge. +git -C "$REPO_DIR" remote add origin "$REPO_DIR/no-such-remote" + +# Creates a worktree for at $WT_BASE/, makes the branch track +# a gone remote branch, gives the worktree a read-only directory, and echoes +# that directory's path. +mkgoneworktree() { + local branch="$1" path="$WT_BASE/$1" + git -C "$REPO_DIR" worktree add -q "$path" -b "$branch" main + git -C "$REPO_DIR" config "branch.$branch.remote" origin + git -C "$REPO_DIR" config "branch.$branch.merge" "refs/heads/$branch" + mkreadonlydir "$path" +} + +branch_exists() { + git -C "$REPO_DIR" show-ref --verify --quiet "refs/heads/$1" +} + +mkgoneworktree gone-clean >/dev/null +mkgoneworktree gone-missing >/dev/null +chmod -R u+w "$WT_BASE/gone-missing" +rm -rf "$WT_BASE/gone-missing" +dir_dirty=$(mkgoneworktree gone-dirty) +echo changed > "$WT_BASE/gone-dirty/file" +dir_locked=$(mkgoneworktree gone-locked) +git -C "$REPO_DIR" worktree lock "$WT_BASE/gone-locked" --reason '{"owner":"supacode"}' + +# git refuses to remove the main worktree, so its permissions must not change. +git -C "$REPO_DIR" config branch.main.remote origin +git -C "$REPO_DIR" config branch.main.merge refs/heads/main +dir_main=$(mkreadonlydir "$REPO_DIR") + +rc=0 +out=$(cd "$REPO_DIR" && "$BASH4" "$BIN" 2>&1) || rc=$? + +# ── Test: a read-only directory doesn't stop the removal partway ────────── + +assert "the run succeeds" test "$rc" -eq 0 +assert "removes a worktree holding a read-only directory" test ! -e "$WT_BASE/gone-clean" +assert_not "deletes the branch of the removed worktree" branch_exists gone-clean + +assert_not "deletes the branch of a worktree whose directory is gone" branch_exists gone-missing + +# ── Test: skipped worktrees keep their permissions ───────────────────────── + +assert "skips a dirty worktree" test -e "$dir_dirty" +assert "keeps the dirty worktree's branch" branch_exists gone-dirty +assert_not "a skipped dirty worktree's directory stays read-only" test -w "$dir_dirty" + +assert "skips a locked worktree" test -e "$dir_locked" +assert "keeps the locked worktree's branch" branch_exists gone-locked +assert "names the lock holder" test "${out#*locked by supacode}" != "$out" +assert_not "a skipped locked worktree's directory stays read-only" test -w "$dir_locked" + +assert "keeps the main worktree's branch" branch_exists main +assert_not "the main worktree's directory stays read-only" test -w "$dir_main" + +# ── Results ──────────────────────────────────────────────────────────────── + +print_results diff --git a/bin/lib/test-helpers.sh b/bin/lib/test-helpers.sh index 7297781..d3c8c89 100644 --- a/bin/lib/test-helpers.sh +++ b/bin/lib/test-helpers.sh @@ -4,8 +4,9 @@ # Usage: source "$SCRIPT_DIR/test-helpers.sh" # # Provides assert/assert_not functions, socket/agent fixtures for SSH-agent -# tests, a bounded runner for the script under test, and a print_results -# finalizer. Each test file should call print_results at the end. +# tests, a read-only directory fixture for worktree tests, a bounded runner for +# the script under test, and a print_results finalizer. +# Each test file should call print_results at the end. _helpers_dir="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" # shellcheck source=bin/lib/bounded.sh @@ -101,6 +102,17 @@ start_agent() { printf '%s\n' "$out" | sed -n 's/^SSH_AGENT_PID=\([0-9]*\);.*/\1/p' } +# Gives worktree $1 a read-only directory holding a file, the way Go leaves its +# module cache, and echoes the directory's path. The directory is under cache/, +# which the caller's fixture should ignore so the worktree stays clean. +mkreadonlydir() { + local dir="$1/cache/go/mod/pkg" + mkdir -p "$dir" + echo x > "$dir/f" + chmod 555 "$dir" "$1/cache/go/mod" + printf '%s\n' "$dir" +} + assert() { local description="$1" shift diff --git a/bin/lib/test-tree-me-remove.sh b/bin/lib/test-tree-me-remove.sh index 6294719..73a2440 100755 --- a/bin/lib/test-tree-me-remove.sh +++ b/bin/lib/test-tree-me-remove.sh @@ -3,8 +3,9 @@ # # Usage: test-tree-me-remove.sh # -# Builds a throwaway repo with a few worktrees (clean, dirty, locked) in a temp -# dir, drives bin/tree-me against it, and cleans up on exit. +# Builds a throwaway repo with a few worktrees (clean, dirty, locked, holding a +# read-only directory) in a temp dir, drives bin/tree-me against it, and cleans +# up on exit. set -euo pipefail @@ -25,6 +26,8 @@ cleanup() { git -C "$REPO_DIR" worktree list --porcelain 2>/dev/null | sed -n 's/^worktree //p' | while read -r wt; do git -C "$REPO_DIR" worktree unlock "$wt" 2>/dev/null || true; done + # `rm -rf` cannot empty a read-only directory. + chmod -R u+w "$REPO_DIR" "$WT_BASE" 2>/dev/null || true rm -rf "$REPO_DIR" "$WT_BASE" } trap cleanup EXIT @@ -35,7 +38,10 @@ git -C "$REPO_DIR" config user.name "Test" # A stable default branch name regardless of the host's init.defaultBranch. git -C "$REPO_DIR" checkout -q -b main echo one > "$REPO_DIR/file" -git -C "$REPO_DIR" add file +# git ignores cache/ the way posthog ignores .flox/cache. A read-only directory +# under it then leaves the worktree clean. +echo cache/ > "$REPO_DIR/.gitignore" +git -C "$REPO_DIR" add file .gitignore git -C "$REPO_DIR" commit -qm "one" FIRST_SHA=$(git -C "$REPO_DIR" rev-parse HEAD) echo two > "$REPO_DIR/file" @@ -175,6 +181,44 @@ tree_me rm -f dirty-one assert "-f removes a dirty worktree" test "$rc" -eq 0 assert "dirty worktree directory is gone" test ! -d "$path" +# ── Test: a read-only directory doesn't stop the removal partway ────────── + +path=$(mkworktree readonly-one) +mkreadonlydir "$path" >/dev/null +tree_me rm readonly-one +assert "removes a worktree holding a read-only directory" test "$rc" -eq 0 +assert "the read-only worktree directory is gone" test ! -e "$path" + +path=$(mkworktree readonly-forced) +mkreadonlydir "$path" >/dev/null +echo changed > "$path/file" +tree_me rm -f readonly-forced +assert "-f removes a dirty worktree holding a read-only directory" test "$rc" -eq 0 +assert "the forced read-only worktree directory is gone" test ! -e "$path" + +path=$(mkworktree missing-dir) +rm -rf "$path" +tree_me rm missing-dir +assert "removes a worktree whose directory is already gone" test "$rc" -eq 0 +assert "the missing worktree is no longer registered" \ + test -z "$(git -C "$REPO_DIR" worktree list --porcelain | grep -F "$path")" + +# A removal git refuses leaves the worktree's permissions as they were. +path=$(mkworktree readonly-dirty) +dir=$(mkreadonlydir "$path") +echo changed > "$path/file" +tree_me rm readonly-dirty +assert "refuses a dirty read-only worktree without -f" test "$rc" -ne 0 +assert_not "a refused dirty removal leaves the directory read-only" test -w "$dir" + +path=$(mkworktree readonly-locked) +dir=$(mkreadonlydir "$path") +git -C "$REPO_DIR" worktree lock "$path" +tree_me rm -f readonly-locked +assert "refuses a locked read-only worktree with a single -f" test "$rc" -ne 0 +assert_not "a refused locked removal leaves the directory read-only" test -w "$dir" +git -C "$REPO_DIR" worktree unlock "$path" + # ── Test: the flag is positional-agnostic ────────────────────────────────── path=$(mkworktree dirty-two) diff --git a/bin/tree-me b/bin/tree-me index b433749..68be0e3 100755 --- a/bin/tree-me +++ b/bin/tree-me @@ -189,7 +189,7 @@ checkout_pr() { path="$candidate_path" break fi - git worktree remove --force "$candidate_path" 2>/dev/null || true + worktree_remove --force "$candidate_path" 2>/dev/null || true done if [ -z "$path" ]; then @@ -534,7 +534,7 @@ EOF for i in "${matches[@]}"; do name="${worktree_names[$i]}" path="${worktree_paths[$i]}" - if output=$(git worktree remove "${force_args[@]}" "$path" 2>&1); then + if output=$(worktree_remove "${force_args[@]}" "$path" 2>&1); then echo "✓ Removed worktree: $path" else echo "$output" >&2