Add an sshfs backend to eval-under - #13
Open
yarikoptic wants to merge 4 commits into
Open
yarikoptic wants to merge 4 commits into
yarikoptic wants to merge 4 commits into
Conversation
sshfs is the filesystem people actually reach for when they mount a
remote over ssh, and it breaks git-annex in a way no local filesystem
does. This adds it as a backend so a report against it can be reproduced
directly.
Two modes:
- Loopback (default): a throwaway sshd on the first free port at or
above 2222, with its own host key, authorized_keys and pid file inside
the run's scratch directory, and a fresh backing directory
sshfs-mounted back over it. The system sshd is not used and
~/.ssh/authorized_keys is never written to.
- `--host` mounts a real remote using the caller's ssh config, so a
reporter's own server and mount options can be used verbatim.
Knobs for the options reports actually turn on: `--no-cache`,
`--workaround`, `--opt`, `--port`, `--user`, `--remote-dir`, plus the
common `--mount-point` / `--set-home` / `--keep`.
Notes on the two non-obvious pieces:
- Dropping privileges deliberately calls `sudo -u` literally rather than
going through the `${SUDO[@]}` array the other backends use: that array
is empty when we are already root, which is exactly the case that needs
the drop.
- sshd runs with `UsePAM yes`. With it off, sshd refuses any account whose
shadow entry is locked (`!`), which is the normal state for service and
CI accounts, and the mount fails for a reason that looks nothing like
its cause. Its log is dumped on mount failure for the same reason.
`bin/ci/run-under.sh` refuses sshfs together with a root-requiring target
(pjdfstest): a FUSE mount belongs to whoever mounted it, there is no
--no-root-squash equivalent, and the run would measure privilege rather
than the filesystem. No matrix cell and no badge -- the backend exists for
on-demand reproduction, and GOTCHAS.md records what it does to hardlink
identity, mtime granularity and fifos before anyone reads a red result as
a filesystem bug.
Split out of the filesystem-probing branch, which had grown too large to
review as one change.
Verified: shellcheck clean, all 28 bats tests pass (the four "every
installed backend" cases now cover this backend), the mount works as root
and via the privilege-drop path, and the refusal above exits 2.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01E1fDiGVWKywpbhVps5hzQK
The backend was on-demand only, so nothing watched it. Add it as a matrix row: `sshfs (loopback) / git-annex test` and `sshfs (loopback) / git testsuite`, taking the matrix from 20 cells to 22. Not four cells. stress-ng and pjdfstest are needs-root, and a FUSE mount belongs to whoever mounted it -- there is no --no-root-squash equivalent the way there is for NFS -- so those two could only ever report on privilege rather than on the filesystem. run-under.sh already refuses that pair with exit 2; the matrix now expresses the same rule as data. Rather than a hand-kept exclusion list, the backend row carries `no-root: true` and `cell_enabled()` in matrix.sh derives the gap from it plus the target's existing `needs-root`. So the reason lives in one place, a future unprivileged backend gets the behaviour for free, and the grid cannot silently grow a cell that measures privilege. Every consumer asks `cell_enabled` instead of assuming backends x targets: - matrix-json.sh omits the pair, so the workflow never schedules it; - gen-readme-matrix.sh prints `n/a` instead of a badge (README regenerated); - update-status.py keeps it out of status.json, so it publishes no permanently-unknown badge; - render-report.py renders an explicit gap rather than an "unknown" badge, which would read as "not measured yet". `sshfs-git-annex` is expected red and is annotated as such on the report page and in GOTCHAS.md: invisible hardlinks (link() succeeds, st_ino differs, nlink=1) break `git annex add`'s post-link verification. `sshfs-git` is the control -- same mount, a suite that never hardlinks into an object store. Its result is not predicted here; the first run measures it, and GOTCHAS.md gets the answer either way. Verified: shellcheck clean and 28/28 bats; matrix-json.sh emits 22 cells with exactly the two sshfs ones; update-status.py's cell set agrees and omits sshfs-stress-ng / sshfs-pjdfstest; and render-report.py, run over a fabricated 22-cell status.json, writes 22 cell badges plus overall, shows two n/a gaps in the sshfs row, and carries the expected-red note on sshfs-git-annex. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01E1fDiGVWKywpbhVps5hzQK
The `sshfs / git testsuite` cell came back red, and the previous commit
had called it "the control: a suite that never hardlinks into an object
store". That was wrong, and measuring it turned up a bug in our own
reporting.
Root cause of the cell: `git clone <local path>` hardlinks each object
and then sanity-checks the result, comparing st_mode/st_ino/st_dev/
st_size/st_uid/st_gid against the source (builtin/clone.c). sshfs
synthesises st_ino per path, so the check fails and the clone dies with
"hardlink different from source". Plain `git clone` of a local path does
not work on sshfs at all -- a broader statement than the git-annex one,
and the same mechanism. It accounts for t1507-rev-parse-upstream (20),
t1013-read-tree-submodule (58) and t0035-safe-bare-repository (2), each
of which clones or adds a submodule in setup. A second, independent cause
is the absence of unix sockets: t0301-credential-cache (37, "unable to
bind ... Operation not permitted") and t0052-simple-ipc (9/9).
`-o disable_hardlink` fixes both this and the git-annex failure, which is
the counter-intuitive part worth telling a reporter: sshfs then fails
link() with EPERM instead of pretending, and every caller here has a copy
fallback that only an honest failure reaches. Measured, with an ext4
control:
default disable_hardlink ext4
git annex add (locked) ok ok ok
git annex add, annex.addunlocked=true FAIL ok ok
git clone <local path> FAIL ok ok
The reporting bug: `not ok N ... # TODO known breakage` is git's
test_expect_failure -- a TAP TODO directive that prove counts as an
expected result, not a failure. dump-failure-logs.sh counted those, so
this cell reported 351 failed assertions where prove saw 166, and the two
worst-looking scripts (t1517-outside-repo at 104, t0450-txt-doc-vs-help
at 54) are ones prove reports as *ok*. That sends triage after failures
that do not exist; it inflated every git cell, vfat included, not just
this one. Both the count and the detail listing now exclude the
directive, matching prove.
GOTCHAS.md gets the mechanisms, the workaround table, a single-script
reproduction recipe, and -- deliberately -- the list of small failures
that are *not* explained (t0003-attributes 6, t1091 8, t0610 3, t0021 3,
ten scripts with one each), so nobody assumes they share a cause.
Method: built the pinned git v2.55.0 locally and ran the whole t0*/t1*
range under the sshfs backend (174 scripts, 10366 assertions) plus an
ext4 control, which passes the five scripts under suspicion. Reproduced
the clone failure minimally, read git's check in clone.c, and confirmed
link() returns EPERM under disable_hardlink. One hypothesis was tested
and rejected rather than left implied: synthesised inode numbers do not
collide at 4000 entries, so that is not behind the whole-suite failures.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01E1fDiGVWKywpbhVps5hzQK
CI reported 179 failed assertions for the sshfs git cell on this very commit, where the local full-suite run measured 166. Both are post-fix numbers (the 351 was the TODO miscount), so the spread is run-to-run variance on a network filesystem, not a discrepancy to chase. Say so in the entry rather than leaving a precise-looking 166 for someone to read a future 179 against as a regression. Also record that both failures in the workaround table are now confirmed red in CI on the sshfs cells, not just locally: `sshfs / git-annex test` had never reported before this run (my previous push cancelled it mid-flight), so its expected-red was a local measurement until now. Docs only -- test.yaml's paths filter covers bin/**, provision/**, .github/matrix.yaml and itself, so this does not re-run the 22 cells. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01E1fDiGVWKywpbhVps5hzQK
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Split out of #5, which had grown too large to review as one change. This is the sshfs backend on its own, on top of current master; #5 keeps the probing / capabilities / survey work and has dropped its sshfs wiring.
Why sshfs
It is what people actually reach for to mount a remote over ssh, and it breaks git in a way no local filesystem does. SFTP's
ATTRScarries no inode number and no link count, so sshfs synthesisesst_inoper path and reportsnlink=1. Callers that hardlink and then verify the link cannot see that it happened, and they fail.Two such callers, both measured here against an ext4 control:
-o disable_hardlinkgit annex add(locked)git annex add,annex.addunlocked=truegit clone <local path>Plain
git cloneof a local path does not work on sshfs at all —builtin/clone.chardlinks each object and comparesst_mode/st_ino/st_dev/st_size/st_uid/st_gidagainst the source. That is broader than the git-annex failure and the same mechanism.-o disable_hardlinkfixes both, which is the counter-intuitive part worth telling a reporter: sshfs then failslink()withEPERMinstead of pretending, and both git and git-annex have copy fallbacks that only an honest failure reaches. A filesystem advertising hardlinks it cannot express is worse than one admitting it has none.What's here
bin/eval-under-sshfs, in two modes:authorized_keysand pid file inside the run's scratch directory — with a fresh backing directory sshfs-mounted back over it. The system sshd is not used and~/.ssh/authorized_keysis never written to.--host: mounts a real remote using the caller's ssh config, so a reporter's own server and mount options can be used verbatim.Flags for the options reports actually turn on:
--no-cache,--workaround,--opt,--port,--user,--remote-dir, plus the common--mount-point/--set-home/--keep.Wiring:
install-backend.shgrows aninstall_sshfs, andrun-under.shgrows the backend case plus a refusal — sshfs together with a root-requiring target exits 2 rather than producing a meaningless red result.In the matrix: two cells, not four
The matrix goes from 20 cells to 22 —
sshfs (loopback) / git-annex testandsshfs (loopback) / git testsuite. Both are red, both for the reasons above, and both are annotated as expected on the report page and inGOTCHAS.md.Not four.
stress-ngandpjdfstestareneeds-root, and a FUSE mount belongs to whoever mounted it, so those cells could only report on privilege rather than on the filesystem.Rather than a hand-kept exclusion list, the backend row carries
no-root: trueandcell_enabled()inmatrix.shderives the gap from that plus the target's existingneeds-root. The reason lives in one place, a future unprivileged backend gets the behaviour for free, and the grid cannot silently grow a cell that measures privilege. Every consumer askscell_enabledinstead of assuming backends × targets:matrix-json.shgen-readme-matrix.shn/ainstead of a badge (README regenerated)update-status.pystatus.json— no permanently-unknownbadgerender-report.pyA reporting bug this turned up
not ok N ... # TODO known breakageis git'stest_expect_failure— a TAP TODO directive that prove counts as an expected result, not a failure.dump-failure-logs.shcounted those, so the sshfs git cell reported 351 failed assertions where prove saw ~170, and the two worst-looking scripts (t1517-outside-repoat 104,t0450-txt-doc-vs-helpat 54) are ones prove reports as ok — triage aimed at failures that do not exist. This inflated every git cell, vfat included. Both the count and the detail listing now exclude the directive; the same cell reports 179 after the fix.Two non-obvious pieces in the backend
sudo -uliterally rather than going through the${SUDO[@]}array the other backends use. That array is empty when we are already root, which is exactly the case that needs the drop — routing through it made the documentedsudo bin/eval-under sshfsinvocation fail with-u: command not found.UsePAM yes. With it off, sshd refuses any account whose shadow entry is locked (!) — the normal state for service and CI accounts — and the mount fails for a reason that looks nothing like its cause. Its log is dumped on mount failure for the same reason.Verification
bin/ci/run-checks.sh: shellcheck clean, 28/28 bats — including the four "every installed backend" cases, which now cover this backend.nlink=1with distinct synthesised inodes for two names of one file.t0*/t1*range under the backend (174 scripts, 10366 assertions) plus an ext4 control that passes; the clone failure was then reproduced minimally. One hypothesis was tested and rejected rather than left implied: synthesised inode numbers do not collide at 4000 entries.sshfs-stress-ng/sshfs-pjdfstestare absent as theno-rootrule intends.GOTCHAS.mdnames the small failures that are not explained (t0003-attributes6,t10918,t06103,t00213, ten scripts with one each), so nobody assumes they share a cause.Note for whoever merges second
#5 also touches
update-status.pyandrender-report.py(to filter its on-demandcapabilitiestarget), in the same enumeration this PR changes. Expect a small conflict in those two files for whichever of #5 / #13 lands second; both changes are additive filters on the same loop and compose cleanly.🤖 Generated with Claude Code
https://claude.ai/code/session_01E1fDiGVWKywpbhVps5hzQK