Open
Conversation
Move FUSE integration tests from sharness shell scripts (t0030, t0031, t0032) and test/cli/fuse_test.go into a dedicated test/cli/fuse/ Go sub-package, ensuring all FUSE test cases run in CI. - git mv test/cli/fuse_test.go to test/cli/fuse/ (package fuse) - convert all sharness FUSE tests to Go subtests under TestFUSE: mount failure, IPNS symlink, IPNS NS map resolution, MFS file/dir creation, xattr (Linux), files write, add --to-files, file removal, nested dirs, publish-while-mounted block, sharded directory reads - add xattr helpers with build tags (linux/other) using unix.Getxattr - split make test_fuse into test_fuse_unit (./fuse/...) and test_fuse_cli (./test/cli/fuse/...) sub-targets - set TEST_FUSE=0 in test_cli so FUSE tests skip in cli-tests CI job - increase fuse-tests CI timeout from 5m to 10m for CLI tests - delete sharness t0030, t0031, t0032 (were always skipped in CI)
Add cross-reference comments between the unit tests in fuse/readonly/, fuse/ipns/, fuse/mfs/ and the end-to-end CLI tests in test/cli/fuse/. Also fix AGENTS.md to use a temp dir for fusermount symlink instead of sudo.
On shared self-hosted runners, leftover mount points from previous runs can exhaust the kernel FUSE mount limit. - add job-level concurrency group so only one fuse-tests runs at a time - lazy-unmount stale /tmp/fusetest* mounts before running tests
The Flush handler wrapped fi.fi.Flush() in a goroutine so it could return early when the FUSE context was canceled. But the goroutine kept running in the background, and when Release arrived it called Close on the same file descriptor concurrently. The two paths both entered DagModifier.Sync, racing on its internal write buffer and causing a nil pointer panic. The fix is to call Flush directly without a goroutine. The MFS flush cannot be safely canceled mid-operation anyway, so the goroutine only added the illusion of cancellation while leaking work and masking the real error. Also bumps boxo to pick up the matching defense-in-depth fix that serializes FileDescriptor.Flush and Close with a mutex.
lidel
force-pushed
the
feat/consolidate-fuse-tests
branch
from
d60bb04 to
a588c23
Compare
bazil/fuse dispatches each FUSE request in its own goroutine. The IPNS File handle had no synchronization, so concurrent Read/Write/Flush/Release calls could overlap on the underlying DagModifier which is not safe for concurrent use. Add sync.Mutex to File, matching the pattern already used by the MFS FileHandler.
bazil/fuse only dispatches Forget to nodes via the NodeForgetter interface. File is a handle, not a node, so this method was never called. The /mfs mount has no equivalent.
The /mfs mount flushes the directory after Unlink and Rename so changes propagate to the MFS root immediately. The /ipns mount did not, leaving mutations pending until an unrelated flush. Also add an empty-directory check before removing directories, matching the /mfs mount's safety check.
New files created via the /ipns FUSE mount now inherit the CID builder from their parent directory, preventing CIDv0 nodes from appearing inside a CIDv1 tree. The directory is also flushed after AddChild so the new entry propagates to the MFS root immediately, matching the /mfs mount.
Cover the file removal path and the empty-directory safety check added in the previous commit. TestRemoveFile verifies a created file can be removed and is gone afterwards. TestRemoveNonEmptyDirectory verifies that rmdir on a directory with children fails, and succeeds once the children are removed first.
All three FUSE mounts now read mode and mtime from UnixFS metadata when present, falling back to POSIX defaults when absent. Most IPFS data does not include this optional metadata. Writing mode and mtime is opt-in via two new config flags: - Mounts.StoreMtime: persist mtime on file create and open-for-write - Mounts.StoreMode: persist mode on chmod Other changes in this commit: - align default file/dir modes across /ipns and /mfs to 0644/0755 - share mode constants via fuse/mount/mode.go - convert Mounts.FuseAllowOther from bool to Flag for consistency - add Setattr to /ipns FileNode and /mfs File for chmod and touch - move dead File.Setattr from IPNS handle to FileNode (node) - bump boxo for Directory.Mode() and Directory.ModTime() getters
All three FUSE mounts now expose the node's CID via the ipfs.cid extended attribute on both files and directories. The /mfs mount also accepts the old ipfs_cid name for backward compatibility. The /ipfs mount previously had a stub that returned nil for all xattrs; it now returns the correct CID. The xattr name follows the convention used by CephFS (ceph.*), Btrfs (btrfs.*), and GlusterFS (glusterfs.*).
lidel
force-pushed
the
feat/consolidate-fuse-tests
branch
from
03dac69 to
6ba3437
Compare
Member
Author
Status updateStarted as test consolidation, grew into a broader FUSE fix-up after CI exposed a race condition panic. What changedFixed the panic: The IPNS Flush handler leaked a background goroutine that raced with Release on the same file descriptor, causing a nil-pointer crash. Removed the goroutine (the flush can't be canceled anyway) and added a mutex in boxo as safety net. Brought /ipns up to /mfs quality: added mutex on file handle ops, flushing after directory mutations, CID builder inheritance on file creation, rmdir safety check. Removed dead code that bazil/fuse never called. UnixFS mode and mtime: all three mounts now show POSIX mode and mtime from UnixFS when present. New opt-in config flags (
Tests and CI: dedicated fuse-tests job, coverage for remove, chmod, mtime, default modes, xattr. What we can't fix with the current FUSE librarybazil.org/fuse (unmaintained since 2020) dispatches
Why hanwen/go-fuse would fix thishanwen/go-fuse v2 passes the file handle to It's actively maintained (v2.9.0, Oct 2025), used by gocryptfs and rclone. The stability issues rclone hit in 2021 were fixed in v2.5.1 and don't apply to our use case. I'll see if we can refactor without investing too much time. |
lidel
changed the title
Replace the unmaintained bazil.org/fuse (last commit 2020) with hanwen/go-fuse v2.9.0, fixing two architectural issues that could not be solved with the old library. ftruncate now works: hanwen/go-fuse passes the open file handle to NodeSetattrer, so Setattr can truncate through the existing write descriptor instead of trying to open a second one (which deadlocks on MFS's single-writer lock). fsync now works: FileFsyncer runs on the handle directly, flushing the write buffer through the open descriptor. Previously a no-op because bazil dispatched Fsync to the inode only. mount package: - NewMount takes (InodeEmbedder, mountpoint, *fs.Options) instead of (fs.FS, mountpoint, allowOther) - mount/unmount collapses to a single fs.Mount call - fusermount3 tried before fusermount in ForceUnmount all three mounts: - structs embed fs.Inode (hanwen's InodeEmbedder pattern) - Remove split into Unlink + Rmdir (separate FUSE interfaces) - ReadDirAll replaced with Readdir returning DirStream - fillAttr helper shared between Getattr and Lookup responses - kernel cache invalidation via NotifyContent after Flush - 1s entry/attr timeout for writable mounts (matches go-fuse default, gocryptfs, rclone) - O_APPEND tracked on file handle, writes seek to end - build tags standardized to (linux || darwin || freebsd) && !nofuse tests: - replaced bazil fstestutil.MountedT with shared fusetest.TestMount - fixed TestConcurrentRW: channel drain mismatch and missing sync between write Close and read start - added TestFsync, TestFtruncate, TestReadlink, TestSeekRead, TestLargeFile, TestRmdir, TestCrossDirRename, TestUnknownXattr - added StoreMtime disabled/enabled subtests
MFS enforces a single-writer lock, so a leaked write descriptor blocks all subsequent opens of that file until GC.
Without this, IsActive stays true after `fusermount -u` and Unmount returns nil instead of ErrNotMounted.
After confirming the child exists, an Unlink failure could be an IO error. Returning ENOENT would hide the real cause.
Readonly Open now returns a file handle holding a DagReader instead of recreating one per Read call. Sequential reads no longer re-traverse the DAG from the root on each kernel request. All three mounts now use CtxReadFull with the kernel's per-request context so killing a process mid-read cancels in-flight block fetches instead of letting them complete uselessly.
- remove dead `_ = mntDir` in TestXattrCID - comment why immutableAttrCacheTime and mutableCacheTime are var - add TODO for using IPNS record TTL as cache timeout
The old check tried to verify OSXFUSE >= 2.7.2 to avoid a kernel panic from 2015. It used sysctl, tried to `go install` a third-party tool at runtime, and referenced paths that no longer exist. Replace with a simple check for the macFUSE mount helper, matching the same paths go-fuse looks for. If neither macFUSE nor OSXFUSE is found, point the user to the install page. Also standardize build tags to (linux || darwin || freebsd) && !nofuse and use strings.ReplaceAll.
Closed
go-fuse's fusermount errors don't include the path, so tools that check error messages for the mountpoint name couldn't tell which mount failed.
go-fuse finds fusermount3 natively, no symlink needed. The stale mount cleanup was for bazil's fstestutil which we no longer use.
IPNS Rename now unlinks the target before AddChild, matching MFS. Without this, renaming onto an existing name returned "directory already has entry". Bump boxo to pick up the flushUp unlinked-entry fix (ipfs/boxo@8ae46d5): when a file descriptor outlives its directory entry (FUSE RELEASE racing with RENAME), flushUp no longer re-adds the stale name. Unskip TestTempFileRename and TestRsyncPattern on both mounts.
boxo@552d8e7 fixes File.setNodeData dropping content links when updating metadata (mode, mtime). chmod or touch after write no longer makes the file appear empty. Unskip TestVimSavePattern on both mounts. Remove debug logging and temporary test functions added during investigation.
go-fuse does not compile on windows/openbsd/netbsd/plan9. Move WritableMountCapabilities (which imports go-fuse) from mode.go (no build tag) to caps.go (platform-gated). Align build tags on fusetest and core/commands/mount stubs so unsupported platforms don't pull in go-fuse transitively.
The doUnmount helper hardcoded fusermount, but systems with only fuse3 installed have fusermount3. Try fusermount3 first, matching what go-fuse and our ForceUnmount already do.
Add NodeSymlinker to MFS and IPNS directories. Symlinks are stored as UnixFS TSymlink nodes in the DAG, the same format used by `ipfs add` for directories containing symlinks. The readonly /ipfs mount already rendered existing symlinks; now /mfs and /ipns can create them too. The target string is cached at Lookup time to avoid re-parsing the DAG node on every Readlink call. Symlink permissions are always 0777 per POSIX convention (access control uses the target's mode).
The direct type assertion on newParent could panic if the kernel passed a non-directory inode. Use a checked assertion with EINVAL fallback, matching the type-switch pattern in the IPNS mount.
Missing continue after error sends let execution fall through to nil type assertions (read.(files.File)) that would panic on error. Also cancel the context before continuing to avoid leaking it.
Abort the directory listing instead of silently omitting the unretrievable entry. Callers get EIO, which is more honest than a partial listing that hides missing blocks.
On shared self-hosted runners, leftover mounts from crashed runs can exhaust the kernel mount_max limit. Lazy-unmount kubo-test and harness temp mounts before and after tests.
guillaumemichel
approved these changes
Apr 8, 2026
Contributor
guillaumemichel
left a comment
guillaumemichel
left a comment
There was a problem hiding this comment.
Great work fixing all these FUSE issues! This has been a long-standing blind spot in test coverage, so new tests running in the CI are welcome!
Quickly eyeballed the code changes, didn't see any major flaw. Switching to an actively maintained library and actually running FUSE tests in CI can only be an improvement.
| @@ -1,4 +1,4 @@ | |||
| //go:build !windows && nofuse | |||
| //go:build !windows && (nofuse || !(linux || darwin || freebsd)) | |||
Contributor
There was a problem hiding this comment.
What are we trying to achieve here? A comment would be welcome
Picks up flushUp unlinked-entry guard and setNodeData content link preservation.
Add a one-line comment above every //go:build directive explaining why the constraint exists. Normalize tag style: positive platform constraints first, then feature flags/negations. Simplify redundant expressions.
Tools like tar and rsync call utimensat on directories after extraction. Without Setattr on Dir, this returned ENOTSUP. Add Setattr to Dir (MFS) and Directory (IPNS) that handles mode and mtime the same way as the file-level Setattr. When StoreMtime or StoreMode is disabled the call succeeds silently, matching the file-level behavior.
- mention that touch and chmod work on both files and directories - note tar and rsync as practical use cases - link to UnixFS spec for optional metadata storage
Use files.UnixPermsToModePerms and files.ModePermsToUnixPerms for converting between FUSE kernel mode (unix 12-bit layout) and Go's os.FileMode (different bit positions for setuid/setgid/sticky). The UnixFS spec supports all 12 permission bits, but boxo's MFS layer (File.Mode, Directory.Mode) exposes only the lower 9. FUSE mounts are always nosuid so the upper 3 bits would have no effect. Add TestSetuidBitsStripped to both mounts confirming the behavior.
Wire the backing mfs.File into the FUSE Symlink struct so Setattr can call SetModTime when StoreMtime is enabled. boxo's File methods (SetModTime, ModTime) already work on TSymlink nodes since they operate on the FSNode protobuf without checking the type. Without Setattr, rsync -a fails with "failed to set times" on symlinks. Every major FUSE filesystem (gocryptfs, rclone, sshfs, s3fs) implements Setattr on symlinks for this reason. Mode is always 0777 per POSIX convention, so chmod requests are silently accepted but not stored.
lidel
force-pushed
the
feat/consolidate-fuse-tests
branch
from
13f9e11 to
05bdd59
Compare
Replace panic with log.Errorf + syscall.EIO in IPNS Directory.Lookup for unexpected MFS node types. Also remove duplicate comment block on File.Flush.
- fuse.md: replace stale OSXFUSE section with macFUSE, remove obsolete go-fuse-version tool, fix broken FreeBSD sudo echo, update xattr example to ipfs.cid with CIDv1, add mode/mtime section, add unixfs-v1-2025 tip, add debug logging section, add TOC, link to hanwen/go-fuse - changelog: refine bullet wording, link to fuse.md - config.md: fix double space, update fuse.md link text - experimental-features.md: fix double space, soften wording - README.md: add FUSE to features list and docs table
Extract duplicated code from fuse/mfs and fuse/ipns into a shared fuse/writable package, and consolidate duplicated tests into a reusable suite in fuse/fusetest. - fuse/writable: Dir, FileInode, FileHandle, Symlink types with all FUSE interface methods, shared by both mounts - fuse/fusetest: RunWritableSuite with helpers, exercised by both mfs and ipns via mount-specific factories - fix cache invalidation race: NotifyContent in Flush (synchronous) in addition to Release (async), so stat after close sees new size - drop deprecated ipfs_cid xattr, log error guiding users to ipfs.cid - mfs_unix.go: 632 -> 19 lines (thin wrapper over writable.Dir) - ipns_unix.go: 795 -> 170 lines (Root + key resolution only) - mfs_test.go: 1183 -> 95 lines (factory + persistence test) - ipns_test.go: 1309 -> 162 lines (factory + IPNS-specific tests) - tests that were only in one mount now run on both
Set volname, noapplexattr, and noappledouble on macOS via PlatformMountOpts, applied in NewMount so all three mounts benefit automatically. - volname: shows mount name in Finder instead of "macfuse Volume 0" - noapplexattr: suppresses Finder's com.apple.* xattr probes - noappledouble: prevents ._ resource fork sidecar files
Readdir on writable mounts now checks the underlying DAG node type for TFile entries, reporting S_IFLNK for symlinks instead of regular file. This makes ls -l and find -type l work correctly. - writable: Readdir checks SymlinkTarget for TFile entries - writablesuite: add SymlinkReaddir regression test - readonly: add TestReaddirSymlink regression test - test/cli/fuse: fix stale bazil.org/fuse reference in doc comment
Getxattr for the old "ipfs_cid" name now returns the CID instead of ENOATTR, keeping existing tooling working during the deprecation period. A log error is emitted on each access to nudge migration.
Member
Author
|
Changes since b8a0823 (reviewed by @guillaumemichel):
I think this is pretty solid as-is, amount of fixed bugs is just crazy. If we have any remaining bugs, we can fix them in future PRs, but this is good enough for 0.41 RC1. |
The go-fuse server dispatches each FUSE request in its own goroutine. On files larger than 128 KB the kernel issues concurrent readahead Read requests on the same file handle, racing on the shared DagReader's Seek+CtxReadFull sequence and corrupting its internal state. Add sync.Mutex to roFileHandle (matching the existing pattern in writable.FileHandle) and lock in Read and Release. - fuse/readonly/readonly_unix.go: add mu sync.Mutex to roFileHandle - fuse/readonly/ipfs_test.go: add TestConcurrentLargeFileRead - fuse/fusetest/writablesuite.go: add LargeFileConcurrentRead to shared writable suite (exercised by both /mfs and /ipns tests)
MFS uses an RWMutex (desclock) that holds RLock for the lifetime of a read descriptor and requires exclusive Lock for writes. Tools like rsync --inplace open the same file for reading and writing from separate processes, deadlocking on this mutex. For O_RDONLY opens, create a DagReader directly from the current DAG node instead of going through MFS. The reader gets a point-in-time snapshot and never touches desclock, so writers proceed independently. - fuse/writable/writable.go: add roFileHandle with DagReader for read-only opens, add DAG field to Config - fuse/mfs/mfs_unix.go: pass ipfs.DAG to writable Config - fuse/ipns/ipns_unix.go: pass ipfs.Dag() to writable Config - fuse/fusetest/writablesuite.go: add ConcurrentReadWrite test exercising simultaneous read and write on the same file
Open a temporary write descriptor in Setattr when the kernel sends a size change without a file handle (the truncate(2) syscall, as opposed to ftruncate(fd) which passes the handle). Previously this returned ENOTSUP. - fuse/writable: open, truncate, flush, close in Setattr else branch - fuse/fusetest: add TruncatePath to the shared writable suite - test/cli/fuse: add end-to-end truncation test covering ftruncate(fd), syscall.Truncate(path), and open(O_TRUNC) through a real daemon
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
2 participants
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.
This PR migrates to hanwen/go-fuse for reasons noted in #11249 (review) and #11272 (comment)
The library is much better, and we can leverage modern FUSE/kernel APIs to map IPFS/MFS/UnixFS abstractions more closely to a real filesystem.
It also fixes over a decade of FUSE bugs, making it much more useful.
Test consolidation
Consolidate all FUSE tests so they actually run in CI.
test/cli/fuse_test.gointo atest/cli/fuse/sub-packagemake test_fuseintotest_fuse_unit(./fuse/...) andtest_fuse_cli(./test/cli/fuse/...)TEST_FUSE=0intest_cliso FUSE tests skip in thecli-testsCI jobfuse-testsCI jobFUSE library migration
Replaced unmaintained
bazil.org/fusewith actively maintainedhanwen/go-fusev2. This fixes two architectural deadlocks that bazil could never solve:ftruncateandfsyncnow work because go-fuse passesthe open file handle to Setattr/Fsync, letting us use the existing write descriptor.
Fixes
rsync --inplaceand tools that shrink/grow files via ftruncate no longer get ENOTSUPln -sworks on/mfsand/ipns, stored as UnixFS TSymlink nodesls -landfind -type lsee the right file typecatworks: interrupting a read cancels in-flight block fetchesfusermount -ufrom outside the daemon marks the mount inactiveipfs_cidxattr normalized: returns the CID (same asipfs.cid) with an error log, instead of ENOATTRNew features
Mounts.StoreMtimeandMounts.StoreMode: opt-in config flags to persist mtime and POSIX mode in UnixFS metadata on writable mountsipfs.cidxattr on all mounts: extended attribute exposing the node's CIDCAP_ATOMIC_O_TRUNC: kernel sends O_TRUNC in Open instead of a separate SETATTR, avoiding a second write descriptor deadlockvolname,noapplexattr,noappledoubleset automaticallyArchitecture
Dir,FileInode,FileHandle,Symlink) intofuse/writable/package, used by both/mfsand/ipnsfusetest.RunWritableSuite) with ~30 scenarios run by both mountstest/cli/fuse/sub-package with new test cases (sharded dirs, IPNS resolution, publish-while-mounted)(linux || darwin || freebsd) && !nofuseto match actual go-fuse platform supportfusermount3handled natively by go-fuse, CI symlink workaround removedmake test_fusesplit intotest_fuse_unitandtest_fuse_clitest_cliviaTEST_FUSE=0, run in dedicatedfuse-testsCI jobReferences
O_TRUNC) does not work on/mfsand/ipnsmounts #11268echo/cptruncation fixed (same root cause as FUSE: file truncation (O_TRUNC) does not work on/mfsand/ipnsmounts #11268)fuse.hheader check removedfuse-testsjob (unit + CLI)Likely fixed (no way to reproduce reporter's exact environment)
server.Wait()server.Unmount(), CI uses lazy unmount for cleanup