diff --git a/AGENTS.md b/AGENTS.md index 4e6e039..520a907 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -81,6 +81,21 @@ created from `origin/main` silently gets `origin/main` as its upstream and will push there. The new-branch path must pass `--no-track`, then let `cmd_track` set the correct upstream. Live in `cmd_add`; any change there needs a fresh test. +## Git pitfall: a start-point can override `-b` + +`git worktree add --no-track -b ` does **not** guarantee a +worktree on ``. When `` is a bare name matching a branch that exists +only on the remote, git's DWIM reads it as "create a local branch tracking +`origin/`" and overrides `-b ` entirely: the worktree comes up on +``, `` is never created, a stray local `` ref is left to go +stale, and the exit status is 0. `--no-track` does not help — it governs the +upstream, not the branch name. + +`cmd_add` resolves the base through `_base_sha` first (local commit-ish, else +`origin/`) and passes the sha, which leaves nothing for the DWIM to latch +onto, and then asserts the new worktree's `HEAD` really is `
`. Keep both: +the resolution is the fix, the assertion is what makes a future regression loud. + ## Git pitfall: worktree paths are physical `git worktree list` reports the *physical* path. Resolve any user-supplied @@ -129,6 +144,10 @@ What the suite covers: exactly `origin/feature-x` for an existing remote branch and exactly `origin/brandnew` for a new one; directory collision; `--print-path` emitting only a path; argument errors; nonzero exit when `track`/push fails +- **add with a remote-only base** — the worktree lands on the requested branch + (not the base), starts at `origin/`, leaves no stray local ref, and + tracks its own remote; an explicit `origin/` behaves identically; an + unresolvable base fails and creates no worktree - **add with a slash in the branch** — the directory is slugged (`feature/x` → `feature-x/`, `deep/new/branch` → `deep-new-branch/`) while the ref keeps its slash and tracks `origin/feature/x`; a second branch slugging to a taken diff --git a/README.md b/README.md index e5588cf..07bc436 100644 --- a/README.md +++ b/README.md @@ -212,6 +212,11 @@ Creates a worktree, handling three cases: | Branch exists on `origin` | Fetch, create with `--track` | | Branch is new | Create from `base` (default `origin/`) with `--no-track` | +`base` is resolved to a commit before the worktree is created. A bare name that +exists only on `origin` resolves to `origin/`, so `add newwork v1.2.0` +starts the branch where you meant and leaves no local `v1.2.0` behind; a base +that resolves to nothing is an error rather than a worktree on something else. + > **`add` writes to the remote by default.** When pushing is enabled, upstream is > set afterward via `track`. If the branch does not exist on `origin`, that runs > `git push -u origin HEAD` — **which creates the branch on the remote.** This diff --git a/git-trees b/git-trees index 786496a..83e5a92 100755 --- a/git-trees +++ b/git-trees @@ -139,6 +139,26 @@ _default_branch() { echo main } +# _base_sha -> commit sha of , or nonzero +# +# `git worktree add -b ` DWIMs a base that names a branch +# existing only on the remote: it creates a local branch named tracking +# it and overrides the explicit `-b ` entirely, so the worktree lands on +# the wrong branch, is never created, and the command exits 0. `--no-track` +# does not prevent it — that flag governs the upstream, not the branch name. +# Resolving the base to a sha first leaves nothing for the DWIM to latch onto, +# and creates no stray local ref that would later go stale as origin advances. +# +# A bare remote-only name is not resolvable as a commit-ish, so it is retried +# against `origin/` — the branch the user meant. +_base_sha() { + local sha + sha=$(git rev-parse --verify --quiet "$1^{commit}") && { printf '%s\n' "$sha"; return 0; } + sha=$(git rev-parse --verify --quiet "refs/remotes/origin/$1^{commit}") \ + && { printf '%s\n' "$sha"; return 0; } + return 1 +} + _ref_info() { # _ref_info -> upstream|track|date git for-each-ref --format='%(upstream:short)|%(upstream:track)|%(committerdate:short)' \ "refs/heads/$1" @@ -377,7 +397,7 @@ cmd_root() { # --- add --------------------------------------------------------------------- cmd_add() { - local br="" base="" print_path=0 no_push=0 root path dir owner + local br="" base="" print_path=0 no_push=0 root path dir owner base_sha created while [ $# -gt 0 ]; do case "$1" in @@ -449,9 +469,27 @@ cmd_add() { git worktree add --track -b "$br" "$dir" "origin/$br" >&2 || return 1 else : "${base:=origin/$(_default_branch)}" + # A sha, never the base name: see _base_sha for the wrong-branch DWIM this + # avoids. Resolving here also turns a bogus base into a clear error instead + # of a worktree on something the user did not ask for. + base_sha=$(_base_sha "$base") || { + echo "git trees add: base '$base' is not a valid commit" >&2 + return 1 + } # --no-track: a new branch must not inherit the base ref's upstream, # or it would silently push to the base branch. - git worktree add --no-track -b "$br" "$dir" "$base" >&2 || return 1 + git worktree add --no-track -b "$br" "$dir" "$base_sha" >&2 || return 1 + fi + + # Belt and braces against a future DWIM: `add` must never leave a worktree + # checked out on a branch other than the one asked for, silently or otherwise. + # Asked of the worktree itself, not `git worktree list`: that reports physical + # paths, and $dir is built from _root, so a symlinked container root would make + # the lookup miss and this check cry wolf. + created=$(git -C "$dir" symbolic-ref --quiet --short HEAD 2>/dev/null) + if [ "$created" != "$br" ]; then + echo "git trees add: worktree at $dir is on '${created:-detached HEAD}', not '$br'" >&2 + return 1 fi if [ "$no_push" -eq 1 ]; then diff --git a/tests/smoke.sh b/tests/smoke.sh index 63d4f59..4b6374d 100755 --- a/tests/smoke.sh +++ b/tests/smoke.sh @@ -361,6 +361,55 @@ assert_not_contains "list does not show it as having no worktree" \ cd "$C" || exit 1 +# --- add with a remote-only base --------------------------------------------- + +# A base naming a branch that exists only on the remote used to be handed to +# `git worktree add -b ` verbatim, where git's DWIM turned it +# into "create a local branch tracking origin/" and overrode `-b ` +# entirely: the worktree came up on , was never created, a stray +# local ref was left behind to go stale, and the command exited 0. +section "add — remote-only base" +RB=$(new_container remote-base-c) + +# A branch on origin that the container has never had locally: created after the +# clone, so only the remote-tracking ref exists. Its own commit, so "based on it" +# is distinguishable from "based on main". +RB_SHA=$(git -C "$ORIGIN" commit-tree "$(git -C "$ORIGIN" rev-parse 'main^{tree}')" \ + -p main -m relbase) +git -C "$ORIGIN" branch relbase "$RB_SHA" >/dev/null 2>&1 +cd "$RB" || exit 1 +git fetch -q origin +assert_fail "the base branch is remote-only" \ + git show-ref --verify --quiet refs/heads/relbase + +assert_ok "add on a remote-only base" bash "$T" add newfromremote relbase +assert_eq "the worktree is on the requested branch, not the base" \ + "$(git -C newfromremote symbolic-ref --short HEAD 2>/dev/null)" "newfromremote" +assert_eq "the new branch starts at origin/" \ + "$(git -C newfromremote rev-parse HEAD 2>/dev/null)" "$RB_SHA" +assert_fail "no stray local branch named after the base" \ + git show-ref --verify --quiet refs/heads/relbase +assert_eq "the new branch tracks its own remote, not the base" \ + "$(git -C newfromremote rev-parse --abbrev-ref '@{upstream}' 2>/dev/null)" \ + "origin/newfromremote" + +# The same base spelled out explicitly must behave identically. +assert_ok "add on an explicit origin/" bash "$T" add explicitbase origin/relbase +assert_eq "the explicit form checks out the requested branch" \ + "$(git -C explicitbase symbolic-ref --short HEAD 2>/dev/null)" "explicitbase" +assert_eq "the explicit form starts at that commit" \ + "$(git -C explicitbase rev-parse HEAD 2>/dev/null)" "$RB_SHA" + +# A base that resolves to nothing is an error, not a worktree on something else. +out=$(bash "$T" add frombogus no/such/base 2>&1) +assert_fail "add on a nonexistent base fails" bash "$T" add frombogus no/such/base +assert_contains "the error names the bad base" "$out" "no/such/base" +assert_fail "no worktree was left behind for a bad base" test -e "$RB/frombogus" + +git -C "$ORIGIN" branch -D relbase >/dev/null 2>&1 + +cd "$C" || exit 1 + # A failed upstream setup must fail `add` (the worktree may still exist). BROKE=$(new_container add-nopush-remote) cd "$BROKE" || exit 1