feat: add rattler_vfs crate#2566
Conversation
improved permission checking, improved error message, improved slice usage in link.rs, Still have to look at the v2 backwards incompatibility comment
Finalise the offset-based prefix replacement: a structured Offsets enum (text vs binary), memchr-based NUL scanning, and the `copy_and_replace_placeholders_with_offsets` family, so install (and other consumers) can replace the install prefix at known byte offsets instead of rescanning each file. The `offsets` field on PathsEntry/PrefixPlaceholder is optional and skipped when absent, so paths.json stays backward-compatible (paths_version unchanged).
|
This is pretty amazing work! Thanks @chrisburr ! Coincidentally @Dagmar-Dinjens is also working on this in #2543. Ill be going over the code! |
|
I guess you meant @Dagmar-Dinjens? (Hi 👋! I'm glad to see the internship worked out!) I'd missed #2543 and I fear that PR is proabably orthogonal to this one... If you'd like to chat about this PR on a call I'm happy to do so as it might be easier for getting up to speed as there is a lot going on here. |
baszalmstra
left a comment
baszalmstra
left a comment
There was a problem hiding this comment.
I did a thorough review and tested the implementation on my windows machine. I think the code is already in a good state and apart from some tweaks here and there I would be fine if we merge this soon and then iterate on it in followup PRs.
Unfortunately I think this far surpasses the work that @Dagmar-Dinjens has pushed so far. @chrisburr I propose that we get this merged and let @Dagmar-Dinjens continue with followups? Would be happy to jump on a call to discuss this in more detail if that helps
I left way more comments than what I think is actually required to merge this. I think a lot of them we can address in follow-up PRs.
My main blockers are:
- I think we should name the crate
rattler_vfsinstead ofrattler_fs. But I can be persuaded. - We should not yet publish this crate automatically. This requires annotating the Cargo.toml with
package.publish = false.
Btw why is windows nfs not supported?
| pub(crate) struct UpperInodeMap { | ||
| path_to_ino: HashMap<PathBuf, u64>, | ||
| ino_to_path: HashMap<u64, PathBuf>, | ||
| next_ino: AtomicU64, |
There was a problem hiding this comment.
The rest of this struct requires locking for concurrency, so what does an atomic u64 buy us here.
| # NFS transport (optional) | ||
| nfs3_server = { version = "0.10", optional = true } | ||
| nfs3_types = { version = "0.5", optional = true } | ||
|
|
||
| [target.'cfg(target_os = "windows")'.dependencies] | ||
| windows = { version = "0.62", features = ["Win32_Storage_ProjectedFileSystem", "Win32_Foundation", "Win32_System_LibraryLoader"] } |
There was a problem hiding this comment.
Lets use workspace dependencies.
There was a problem hiding this comment.
@Dagmar-Dinjens If you can look into this perhaps.
| dbus-secret-service-keyring-store = { version = "1.0.0", default-features = false } | ||
| dunce = "1.0.5" | ||
| enum_dispatch = "0.3.13" | ||
| env_logger = "0.11.9" |
There was a problem hiding this comment.
I think this is left over from an earlier version of this before I figured out how to interface pixi more cleanly.
| @@ -0,0 +1,691 @@ | |||
| //! NFS transport adapter. | |||
There was a problem hiding this comment.
Out of curiosity, why do we use nfs3 instead of nfs4?
There was a problem hiding this comment.
I think the best supported crate I found only had v3 and I wasn't aware of anything about v4 really mattering.
There was a problem hiding this comment.
Another thing to add here is https://github.com/facebook/sapling/blob/61b3cde3bffdd7553094a9b4738af369abdf935b/eden/fs/docs/macOS.md
In general I found EdenFS's documentation to be a useful reference. For example the Windows docs were what made me finally give up on trying to support the read-only flag for ProjFS.
|
Also some notes from fable worth looking at: Review: #2566 —
|
Yes, I am very happy to have @Dagmar-Dinjens pick this up again, especially as I'd like to spend some time on the package cache itself to help with distribution via CVMFS which is orhtogonal but useful for this. If you want to arrange a chat, the conda-forge Zulip is probably the most reliable way for you to get my attention.
How do you want to proceed on that side? I'll work my way though replying and implement some of them? Or do you want to use the comments as a chance to onboard @Dagmar-Dinjens?
I'm not sure how many people know VFS as a concept. That said, the name has been in my brain for a long time so the problem might just be that it's different from what I've become used to. I don't have a strong opinion either way.
👍
Initially I tried to use only NFS and just ProjFS for later but I ran into a bunch of limitations. My memory is a bit fuzzy as this was a couple of months ago I think the main ones were:
|
I have created issues for all the non-blocking items in this PR (prefixed with vfs:). I also updated the fable review. So all the items that are still open should be tackled in this PR. I tagged @Dagmar-Dinjens in some of them, I hope she can take care of those? @chrisburr Would you be able to take care of the remaining ones? Once merged we can work on the individual issues. We can simply assign people to make sure we dont work on the same things. Does that help/make sense?
So I asked ChatGPT for the deciding vote:
@chrisburr Does that resonate with you? |
Correct the doc comments on `Offsets`, `PrefixPlaceholder::offsets` and `PrefixPlaceholder::shebang_length` to match the draft CEP "Prefix placeholder offsets in paths.json" (conda#2565): - `offsets` exclude occurrences inside the shebang region, and for binary files the group's last value is the NUL terminator offset or the file size when the final C string is unterminated at end-of-file. - `shebang_length` is present if and only if `offsets` is present, `file_mode` is text, and the file starts with `#!` — independent of whether the first line contains the placeholder. - The `Offsets` shape is determined normatively by `file_mode`, not inferred from the JSON structure.
Bring install-time prefix replacement into line with the draft CEP "Prefix placeholder offsets in paths.json" (conda#2565). `offsets` are absolute byte positions that exclude the shebang region and are spliced uniformly on every platform; the shebang region (first `shebang_length` bytes) is transformed separately. - Plumb `shebang_length` from `PrefixPlaceholder` into the offset functions and use it as the region boundary instead of re-deriving it from content, validating it equals first-0x0A-index + 1. - Add the missing non-rewriting-target rule: on Windows (e.g. a noarch package) the shebang region gets plain placeholder replacement rather than being left untouched. On Unix it is rewritten by the shebang rules (region minus its trailing newline, newline copied through verbatim). - Report producer non-conformance (in-region offset, shape/mode mismatch, out-of-range or unordered offsets, missing placeholder bytes, empty binary outer list, `#!` file without `shebang_length`) as a distinguishable `OffsetReplaceError::InconsistentMetadata`. The offset functions validate before writing anything, so `link_file` falls back to the search-based path (with a warning) using the still-empty destination instead of failing the install. - Fix a pre-existing panic in the search-based `copy_and_replace_textual_ placeholder`: a `#!` file with no newline fed an empty line into `replace_shebang`, tripping its `starts_with("#!")` assertion. - Fix the binary test that recorded the NUL terminator one byte past the actual `\x00`, and align the suite with the CEP's informative test-vector list (short/long Unix prefixes, non-rewriting target, no trailing newline, empty offsets, multiple in-shebang occurrences, over-long shebang with no occurrence, unterminated final C string, and the fallback for non-conformant input).
Add read-only `offsets` and `shebang_length` getters to the py-rattler `PrefixPlaceholder` so consumers can inspect the CEP fields (conda#2565). `offsets` returns `None`, a `list[int]` for text-mode files, or a `list[list[int]]` for binary-mode files (grouped by C string).
- "Check intra-doc links" (`-D rustdoc::private-intra-doc-links`): the public `copy_and_replace_textual_placeholder_offsets` doc linked to the private `replace_shebang` fn; demote it to a plain code span. - Windows-x86_64: `test_replace_long_prefix_in_text_file_offsets` hardcoded `shebang_length: Some(44)`, which is wrong when the `shebang_test.txt` fixture is checked out with CRLF (the carriage return shifts the first newline to 45). Derive `shebang_length` from the file contents instead.
|
That's a compelling arugment for
This is in contradiction with having For now I've gone with removing the feature and publishing the crate. The biggest open question from my perspective is conda/ceps#179 and especially the handling of non-UTF-8 strings. |
Dagmar's foundational implementation of the on-demand virtual filesystem that serves conda environments from the package cache. Later renamed to rattler_vfs and hardened (mount transports, overlay, codesign, prefix-replacement parity). (Reworded from the original "initial commit with multiple major problems still needing fixing"; authorship preserved.)
Rename Dagmar's rattler_fs foundation to rattler_vfs and harden it into the installable crate: FUSE/NFS/ProjFS transports, overlay, codesign, and the ranged prefix-replacement paths (byte-identical to rattler's install path, consuming its CEP-conformant offset APIs).
Add `rattler mount` to mount a locked environment through rattler_vfs. Gated behind the default-on `mount` feature (opt out with --no-default-features); the NFS transport is enabled for the bundled crate.
Add the rattler-vfs end-to-end harness: a nushell mount/verify script, a GitHub Actions workflow, the pixi `rattler-vfs` environment, and a test-data fixture environment for the mount tests.
Pull the shebang-region transformation out of copy_and_replace_textual_placeholder_offsets into a reusable, public `replace_shebang_region` helper: on Unix targets the region (minus its trailing newline) is rewritten by the shebang rules and may collapse to `#!/usr/bin/env <program>`; on non-rewriting targets it receives plain placeholder replacement. This makes the helper the single source of truth for how the shebang region is patched, shared by install-time replacement here and rattler_vfs mount-time ranged reads so the two stay byte-identical. Behavior is unchanged (verified by the existing link tests).
`text_ranged_read` spliced offsets uniformly, so a mounted shebang script kept its original (unreplaced) first line and reported the wrong size — diverging from an install once a CEP-conformant producer excludes the shebang region from `offsets`. Introduce `plan_text_replacement`, which mirrors the installer: it transforms the shebang region via the shared `replace_shebang_region` helper (collapsing an over-long line to `#!/usr/bin/env <program>` on Unix, or plain-replacing it on non-rewriting targets) and records the remaining occurrences as body offsets. `text_ranged_read` now emits the transformed region followed by the body-spliced tail, and `getattr` sizes are computed the same way. The install-vs-mount parity tests gain shebang coverage: line kept, collapsed for an over-long prefix, no trailing newline, multiple occurrences in the line, only-in-shebang, and a non-rewriting (Windows) target — plus ranged reads that cross the region/body boundary.
Two failures surfaced on the offsets work in CI: - "Check intra-doc links" (`-D rustdoc::private-intra-doc-links`): the public docs for `replace_shebang_region` and `copy_and_replace_textual_placeholder_offsets` linked to the private `replace_shebang` fn. Demote those to plain code spans. - Windows-x86_64: `test_replace_long_prefix_in_text_file_offsets` hardcoded `shebang_length: Some(44)`, which is wrong when the `shebang_test.txt` fixture is checked out with CRLF (the extra carriage return shifts the first newline to 45). Derive `shebang_length` from the file contents instead, so the test is robust to the checkout's line endings.
The Windows-only code paths never compile on the Linux/macOS CI, so three edition-2024 / `-D warnings` errors only surfaced on the Windows job: - projfs_adapter.rs: the `unsafe extern "system"` ProjFS callbacks perform raw pointer / FFI operations directly in the fn body, which Rust 2024's `unsafe_op_in_unsafe_fn` rejects. The whole module is inherently unsafe FFI, so allow the lint module-wide rather than wrapping ~30 sites. - lib.rs `mount_nfs`: on targets without NFS support the platform block `bail!`s, making the success tail (`tracing::info!` + `Ok(..)`) unreachable. Gate that tail behind `cfg(any(macos, linux))` so it is only compiled where it is reachable. - nfs_adapter.rs: `NfsMountHandle::mount_point` is only read by the macOS/Linux `umount` paths, so it is dead code elsewhere — allow it on non-NFS targets.
Add TextPlan::from_recorded, which builds a text replacement plan straight from the offsets recorded in paths.json instead of scanning the file, and use it during VirtualFS construction: one stat for the size arithmetic plus at most shebang_length bytes read for the shebang region. The recorded offsets are the producer's contract per the CEP and are trusted as-is; scanning remains as the fallback for pre-CEP packages and for metadata that fails the cheap shebang sanity check (logged so a non-conformant producer is self-diagnosing). Also log a warning before the macOS codesign materialization path falls back to serving raw bytes, instead of degrading silently.
3 participants
Description
This builds on top of #2565 and the work on @Dagmar-Dinjens to add a
rattler_fscrate. See #1182 for the original concept. Basically the idea is to serve data directly from the rattler cache directory using a virtual filesystem, therefore avoiding the overhead of actually making environments. The usual install-time tricks (prefix-subsitution, python entrypoints, shebang-fixing, apple's code signing, ...) are applied on-the-fly as the data is read.I see several motivations for this but the main ones are:
This PR includes 3 VFS backends for RattlerFS which are supported on the following platforms:
We default to FUSE on Linux, NFS on macOS and ProjFS on Windows which allows all versions to run in user space assuming the VFS drivers are available. Windows is notably less extensively tested than the others. I also looked into FSKit on macOS but it seems that it's not yet suitable for something like RattlerFS.
This PR also contains the concept of a writable overlay for FUSE and NFS which then allows for edits to be made on top of the read-only underlying environment. This also makes it possible to pip install packages on top, though in principle RattlerFS could be expanded to support wheels natively rather than needing the overlay for that case. I've laid the ground work for this but didn't finish it.
The main component here is the
rattler_fscrate which is used by the draft Pixi PR (prefix-dev/pixi#6548). There is arattler mountcommand added torattler_binwith an optional feature. I could see this as being useful in it's own right by systems which want to mount an environment from a lock file but don't want all of the the workspace functionality of pixi.How Has This Been Tested?
There are End-To-End tests included which have been useful during development to make sure all three platforms continue to work. Also see prefix-dev/pixi#6548.
AI Disclosure
There have been far too many sessions involved in the creation of this to track exactly what tools have been used and which parts are hand written vs AI generated. It's more AI-generated than I would like but equally it contains far more functionality and edge case handling than I would have managed to find time for alone. This project represents most of my rust experience so I'm not in a position to evaluate the quality of the code but I think it's in a reasonable state all things considered.
Checklist: