feat(nfs): Windows support for hf-mount-nfs#140
Draft
XciD wants to merge 16 commits into
Draft
Conversation
Contributor
POSIX Compliance (pjdfstest) |
Contributor
Benchmark Results |
Make `hf-mount-nfs` build and run end-to-end on Windows, addressing #88. Verified on Windows Server 2022 (m5.xlarge EC2) with Rust 1.95 + MSVC 2022 and the "Client for NFS" feature: a HuggingFace dataset mounts cleanly to a drive letter and files read back correctly through xet-core/CAS. Cross-platform plumbing: - `daemon` is gated `cfg(unix)`; a Windows stub (`src/daemon_windows.rs`) preserves the public API so callers compile unchanged. Daemon control commands (`stop`/`status`/`daemonize`) return a clear "not supported" error. - `virtual_fs` replaces direct `libc::pread`/`libc::pwrite` on `Arc<File>` with cross-platform helpers built on `std::os::unix::fs::FileExt::read_at` / `std::os::windows::fs::FileExt::seek_read` (and the write equivalents). - `setup::raise_fd_limit` is a no-op on Windows; uid/gid default to 0. Mount-point creation now uses `ErrorKind::AlreadyExists` instead of `libc::EEXIST` for portability. - `nfs.rs` cfg-gates `tokio::signal::unix::SignalKind::terminate` (Unix only) and adds a Windows mount block that drives `mount.exe` with NFS-client options against `\\127.0.0.1\!`. Windows mount discovery (the actual blocker): - Windows `mount.exe` has no `mountport=` option to bypass portmapper, so it always queries portmapper at 127.0.0.1:111 to discover MOUNT and NFS service ports. nfsserve doesn't expose one. Linux/macOS skip this via `mountport=N`. - Bundle a minimal RFC 1833 portmapper (program 100000 v2) that answers GETPORT(MOUNT v3) and GETPORT(NFS v3) with nfsserve's actual TCP port, serving both UDP and TCP on 127.0.0.1:111. Implemented as a private module inside `nfs.rs` and only compiled `cfg(windows)`. Binding port 111 requires Administrator on Windows. Build/binary surface: - `fuser` is moved to `[target.'cfg(unix)'.dependencies]` so the `fuse` feature has no effect on Windows; `pub mod fuse` is gated `cfg(all(unix, feature = "fuse"))`. - `hf-mount`, `hf-mount-fuse`, `hf-mount-fuse-sidecar` print a clear "Unix-only" message and exit 1 on Windows. Limitations: - Windows requires the "Client for NFS" feature (`Install-WindowsFeature -Name NFS-Client`) and Administrator privileges (port 111 is privileged). - The neutral `hf-mount` daemon controller and FUSE backend remain Unix-only.
- Drop the `mod imp { pub fn run() }` wrapper in `hf-mount.rs` and gate at
item level instead, removing the 500-line re-indent.
- Rename `imp::run` -> `imp::main` in `hf-mount-fuse-sidecar.rs`.
- Restructure `unmount_nfs` to flat `linux`/`macos`/`windows` sibling cfgs
(matches `is_mounted`'s style, drops the nested cfg blocks).
- Replace the 200ms sleep before `mount.exe` with a deterministic readiness
signal: `portmapper::start()` binds UDP+TCP synchronously, then spawns the
accept loops, returning a JoinHandle. Caller awaits bind, then proceeds.
- Drop the misleading `std::mem::forget(portmapper_handle)` (forgetting a
JoinHandle is a no-op for the task lifecycle in tokio); abort cleanly in
the shutdown path instead.
- Replace `format!("nolock,...")` with `String::from(...)` + `push_str`
(clippy `useless_format`).
- Hoist the inline `async { #[cfg(unix)] {...} #[cfg(not(unix))] {...} }`
into a named `sigterm_fut` future at the top of the select scope.
- Inline the `read_u32`/`write_u32` helpers in the portmapper into direct
`u32::from_be_bytes` / `to_be_bytes` calls (already used elsewhere in the
module).
Drop the inline RFC 1833 portmapper module and depend on the matching upstream feature in nfsserve (huggingface/nfsserve#44, branch feat/portmap-listener via [patch.crates-io]). Net: -147 lines locally; the portmapper now lives where it logically belongs (next to nfsserve's existing portmap_handlers and rpc/xdr code, where future maintainers will look first). Drop the patch.crates-io entry once nfsserve cuts a release containing the new module.
`tokio::pin!(server_handle)` plus dropping it at function end does not cancel a tokio task — the JoinHandle's Drop is a no-op for the underlying task. The NFS accept loop in `nfsserve::handle_forever()` would keep running after `mount_nfs` returned, leaking the bound TCP port until process exit. Caught by codex review on the Windows port PR. Now: explicit `.abort(); let _ = .await;` in the shutdown sequence.
XciD
force-pushed
the
feat/windows-nfs-support
branch
from
eafd0f5 to
3386adb
Compare
- Add build-windows job to release.yml (matches Linux/macOS jobs) - Add windows-build.yml that builds hf-mount-nfs.exe and hf-mount.exe on push to feat/windows-** and on PRs touching src/Cargo, uploads the binaries as a workflow artifact for direct download.
XciD
force-pushed
the
feat/windows-nfs-support
branch
from
3386adb to
44fcef8
Compare
Setting the env var keeps the NFS server + portmapper running and prints the exact mount.exe command to run manually. Lets us debug which option mount.exe rejects without the binary tearing the server down on the first failure.
…T set Without this, the 2s poll sees the un-mounted drive letter and tears down the server, defeating the whole point of the skip flag.
The daemon controller (hf-mount) is Unix-only — its Windows main() just prints 'Unix-only' and exits 1. No reason to ship the binary.
The cfg(unix) wrap was unnecessary churn — hf-mount-fuse-sidecar declares 'required-features = ["fuse"]', and the fuse feature depends on fuser which is a [target.'cfg(unix)'.dependencies], so on Windows the binary is naturally skipped (the feature can't be enabled). Reverts the 859-line indentation diff.
Only DaemonGuard::from_env / notify_ready are needed by hf-mount-nfs on Windows; the full controller is Unix-only and not built. 49 lines of stub trimmed to 8 inline lines.
…red-openssl - Use tokio::process::Command instead of std::process::Command for the mount.exe call so it doesn't block a tokio worker thread. - Build the mount.exe command string once instead of duplicating it in the info log and the error message. - Drop --features vendored-openssl from Windows CI builds: native-tls uses Schannel on Windows, openssl-sys is not in the dep tree, so the feature was just compiling OpenSSL from source for nothing.
Adds Windows under 'System dependencies' with Client for NFS install commands (Server vs 10/11), Administrator requirement, and the EnableLinkedConnections registry tweak for non-admin Explorer visibility. Notes that only hf-mount-nfs.exe is built on Windows.
Required by the new non-blocking mount.exe spawn.
Adds three small helpers to tests/common/mod.rs:
- nfs_mount_point(slug): returns /tmp/hf-mount-nfs-{slug}-{pid} on Unix,
Z: on Windows (relies on --test-threads=1 for drive-letter reuse).
- nfs_binary_path(): appends .exe on Windows.
- nfs_is_mounted(): /proc/mounts on Linux, mount cmd on macOS,
std::fs::metadata on Windows.
unmount_nfs is cfg-gated to use 'sudo umount' on Unix and 'umount.exe -f'
on Windows (no sudo). create_dir_all on the mount point is skipped on
Windows since drive letters aren't directories.
The 4 mount_point bindings in tests/nfs_ops.rs swap their ad-hoc
format!() calls for nfs_mount_point(). bench.rs / fio_bench.rs left as
Unix-only paths since benches don't run on Windows CI.
Same NFS integration tests as the Linux smoke job, runs on windows-2022 with the Client for NFS feature enabled. Builds a fresh hf-mount-nfs.exe (cached via Swatinem/rust-cache) then exercises the cross-platform helpers added in the previous commit.
These tests use libc::dup + as_raw_fd which are Unix-only, breaking the Windows compile of tests/common/fs_tests.rs. The fcntl-style FD duplication semantics they exercise don't exist on Windows anyway.
Closed
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
1 participant
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.
Summary
Make
hf-mount-nfsbuild and run end-to-end on Windows. Closes #88 (NFS path; FUSE remains Unix-only).Verified on Windows Server 2022 (Rust 1.95 + MSVC 2022) with the "Client for NFS" feature:
datasets/XciD/hf-mount-poll-testmounts cleanly to driveZ:and files read back correctly through xet-core/CAS.What changed
Cross-platform plumbing:
daemongatedcfg(unix)with a Windows stub (src/daemon_windows.rs) preserving the public API. Daemon control commands return a clear "not supported" error on Windows.virtual_fsreplaces directlibc::pread/libc::pwriteonArc<File>with cross-platform helpers built onstd::os::unix::fs::FileExt::read_at(andstd::os::windows::fs::FileExt::seek_read).setup::raise_fd_limitis a no-op on Windows; uid/gid default to 0. Mount-point creation now usesErrorKind::AlreadyExistsinstead oflibc::EEXIST.nfs.rscfg-gatestokio::signal::unix::SignalKind::terminateand adds a Windows mount block drivingmount.exeagainst\\127.0.0.1\!.Windows mount discovery (the real blocker):
mount.exehas nomountport=option to bypass portmapper; it always queries portmapper at127.0.0.1:111to discover MOUNT and NFS service ports.nfsservedoesn't expose one. Linux/macOS skip this viamountport=N.GETPORT(MOUNT v3)andGETPORT(NFS v3)with nfsserve's actual TCP port, on UDP and TCP. Private module innfs.rs,cfg(windows)only.Build / binary surface:
fusermoved to[target.'cfg(unix)'.dependencies].pub mod fusegatedcfg(all(unix, feature = "fuse")).hf-mount,hf-mount-fuse,hf-mount-fuse-sidecarprint a clear "Unix-only" message and exit 1 on Windows.Windows runtime requirements
Install-WindowsFeature -Name NFS-ClientEnable-WindowsOptionalFeature -Online -FeatureName ServicesForNFS-ClientOnly,ClientForNFS-Infrastructure -AllEnableLinkedConnectionsand reboot, otherwise drives mounted by an elevated process are only visible to other elevated processes (MS doc):hf-mount-nfs.exe ... Z:in an admin shell exposes Z: to non-admin Explorer / apps.Debug knob
HF_MOUNT_SKIP_AUTO_MOUNT=1makes the binary spawn the NFS server + portmapper but skip themount.exeinvocation and the mount-disappearance poll, leaving the server up somount.execan be invoked manually for diagnosis.Limitations
hf-mount) and FUSE backend remain Unix-only.Follow-ups (out of scope)
windows-2022running at minimumcargo build --features nfs --bin hf-mount-nfs.