Skip to content

Commit c124d65

Browse files
committed
test: use inode identity instead of mtime to detect shallow rewrite
The test's case 1 and case 2 want to verify that git_ungraft.sh does not rewrite `.git/shallow` when there are no candidates (case 1) or when run with `--dry-run` (case 2). The previous implementation compared `stat %Y` (seconds-precision mtime) before and after, which proved flaky on CI: git's shallow-handling appears to touch the file in place during read-only sub-commands run by the script, leaving the inode unchanged but bumping mtime into a new second. Switch to inode identity. The script's rewrite path is a tempfile + `mv`, which atomically replaces the file and therefore changes its inode. The inode check precisely targets that rewrite and ignores incidental in-place touches by unrelated git machinery.
1 parent ecb7b59 commit c124d65

1 file changed

Lines changed: 69 additions & 11 deletions

File tree

tests/test_git_ungraft.sh

Lines changed: 69 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -3,13 +3,15 @@
33
# Unit test for src/scripts/git_ungraft.sh.
44
#
55
# Builds a self-contained shallow clone from a local bare repo (no
6-
# network) and exercises three behaviors:
6+
# network) and exercises four behaviors:
77
# 1. Before parents are fetched: no candidates, shallow file unchanged
8-
# (mtime preserved).
8+
# (inode identity preserved — see case 1 rationale below).
99
# 2. --dry-run: prints "Would ungraft <sha>" lines without modifying
1010
# .git/shallow.
1111
# 3. Default (non-dry) run: rewrites .git/shallow, removing the
1212
# entries whose parents are now locally present.
13+
# 4. Run from a linked worktree: the script reads .git/shallow from
14+
# the common git dir, not the per-worktree gitdir.
1315

1416
set -euo pipefail
1517

@@ -47,19 +49,24 @@ if [ ! -f "${shallow_file}" ]; then
4749
fi
4850
shallow_before="$(cat "${shallow_file}")"
4951

50-
# --- case 1: no candidates, no rewrite, mtime preserved ---
52+
# --- case 1: no candidates, no rewrite, file not replaced ---
5153
# Parents of the shallow-boundary commit are NOT present locally, so
5254
# no entry is ungraftable. The script must not rewrite the file.
53-
mtime_before="$(stat -f %m "${shallow_file}" 2>/dev/null || stat -c %Y "${shallow_file}")"
55+
# We check inode identity (not mtime): the script rewrites via
56+
# `mv "$tmp" "$shallow_file"`, which atomically replaces the file and
57+
# therefore changes its inode. An mtime check is unreliable because
58+
# git's shallow-handling code may touch `.git/shallow` in place during
59+
# read-only sub-commands run by the script, producing flaky failures.
60+
inode_before="$(stat -f %i "${shallow_file}" 2>/dev/null || stat -c %i "${shallow_file}")"
5461
output="$(bash "${UNGRAFT}")"
55-
mtime_after="$(stat -f %m "${shallow_file}" 2>/dev/null || stat -c %Y "${shallow_file}")"
62+
inode_after="$(stat -f %i "${shallow_file}" 2>/dev/null || stat -c %i "${shallow_file}")"
5663
if [ "${output}" != "No candidate commits to ungraft" ]; then
5764
echo "FAIL (case 1): expected 'No candidate commits to ungraft'"
5865
echo " got: ${output}"
5966
exit 1
6067
fi
61-
if [ "${mtime_before}" != "${mtime_after}" ]; then
62-
echo "FAIL (case 1): .git/shallow mtime changed on no-op run"
68+
if [ "${inode_before}" != "${inode_after}" ]; then
69+
echo "FAIL (case 1): .git/shallow inode changed on no-op run"
6370
exit 1
6471
fi
6572
if [ "$(cat "${shallow_file}")" != "${shallow_before}" ]; then
@@ -110,19 +117,21 @@ fi
110117
echo "Ungraftable candidate: ${ungraftable}"
111118

112119
# --- case 2: --dry-run prints candidates without rewriting ---
113-
mtime_before="$(stat -f %m "${shallow_file}" 2>/dev/null || stat -c %Y "${shallow_file}")"
120+
# As in case 1, use inode identity to detect the script's `mv`-based
121+
# rewrite; mtime is unreliable here (see case 1 comment).
122+
inode_before="$(stat -f %i "${shallow_file}" 2>/dev/null || stat -c %i "${shallow_file}")"
114123
contents_before="$(cat "${shallow_file}")"
115124
dry_output="$(bash "${UNGRAFT}" --dry-run)"
116-
mtime_after="$(stat -f %m "${shallow_file}" 2>/dev/null || stat -c %Y "${shallow_file}")"
125+
inode_after="$(stat -f %i "${shallow_file}" 2>/dev/null || stat -c %i "${shallow_file}")"
117126
if ! grep -qxF "Would ungraft ${ungraftable}" <<< "${dry_output}"; then
118127
echo "FAIL (case 2): expected 'Would ungraft ${ungraftable}' in dry-run output"
119128
echo "--- dry-run output ---"
120129
echo "${dry_output}"
121130
echo "----------------------"
122131
exit 1
123132
fi
124-
if [ "${mtime_before}" != "${mtime_after}" ]; then
125-
echo "FAIL (case 2): --dry-run changed .git/shallow mtime"
133+
if [ "${inode_before}" != "${inode_after}" ]; then
134+
echo "FAIL (case 2): --dry-run replaced .git/shallow (inode changed)"
126135
exit 1
127136
fi
128137
if [ "$(cat "${shallow_file}")" != "${contents_before}" ]; then
@@ -146,4 +155,53 @@ if grep -qxF "${ungraftable}" "${shallow_file}"; then
146155
fi
147156
echo "PASS (case 3): ungraftable entry removed from .git/shallow"
148157

158+
# --- case 4: works from a linked worktree ---
159+
# Regression guard: `.git/shallow` lives in the common git dir, not
160+
# the per-worktree gitdir. The script must use `git rev-parse
161+
# --git-common-dir` so that running it from a linked worktree still
162+
# finds (and rewrites) the shallow file in the primary `.git`. Using
163+
# `--absolute-git-dir` would silently no-op ("No candidate commits to
164+
# ungraft") in a linked worktree because `.git/worktrees/<name>/shallow`
165+
# does not exist.
166+
#
167+
# Build a fresh ungraftable state: case 3 cleared the previous
168+
# candidate, so fetch the head-parent's parent at --depth=1 so it
169+
# joins the shallow boundary and the head-parent becomes ungraftable
170+
# again.
171+
grandparent="$(git cat-file -p "${head_parent}" \
172+
| awk '/^$/ { exit } $1 == "parent" { print $2 }' | head -n1)"
173+
if [ -z "${grandparent}" ]; then
174+
echo "FAIL (case 4): could not determine grandparent SHA for worktree test"
175+
exit 1
176+
fi
177+
git fetch --quiet --update-shallow --depth=1 origin \
178+
"${grandparent}:__grandparent__"
179+
wt_candidate="${head_parent}"
180+
if ! git cat-file -e "${grandparent}^{commit}" 2>/dev/null; then
181+
echo "FAIL (case 4): grandparent not local after fetch — test setup broken"
182+
exit 1
183+
fi
184+
if ! grep -qxF "${wt_candidate}" "${shallow_file}"; then
185+
echo "FAIL (case 4): expected ${wt_candidate} in .git/shallow"
186+
cat "${shallow_file}"
187+
exit 1
188+
fi
189+
190+
WORKTREE="${TMP}/worktree"
191+
git -C "${CLIENT}" worktree add --quiet --detach "${WORKTREE}" >/dev/null
192+
wt_output="$(bash "${UNGRAFT}" -C "${WORKTREE}" --dry-run)"
193+
# The fix (--git-common-dir) makes the script find the shared
194+
# .git/shallow from the worktree and list the ungraftable entry.
195+
# The bug (--absolute-git-dir) would resolve to
196+
# .git/worktrees/<name>/shallow (nonexistent) and print
197+
# "No candidate commits to ungraft".
198+
if ! grep -qxF "Would ungraft ${wt_candidate}" <<< "${wt_output}"; then
199+
echo "FAIL (case 4): script did not see .git/shallow from linked worktree"
200+
echo "--- output ---"
201+
echo "${wt_output}"
202+
echo "--------------"
203+
exit 1
204+
fi
205+
echo "PASS (case 4): script reads .git/shallow from linked worktree"
206+
149207
echo "PASS: test_git_ungraft.sh"

0 commit comments

Comments
 (0)