vfs: drop --vfs-mount, keep --vfs-load alone - #66162
Draft
pipobscure wants to merge 2 commits into
Draft
pipobscure wants to merge 2 commits into
pipobscure wants to merge 2 commits into
Conversation
Collaborator
|
Review requested:
|
nodejs-github-bot
added
lib / src
Contributor
Author
|
This is the alternate to #66119 so we can either have names for mounts, in which case --vfs-mount makes sense, or we go with this PR, which drops the idea of --vfs-mount entirely and just allows for --vfs-load. @mcollina @jasnell @bakkot since you have been the most actively engaged on the topic and much more in tune with the philosophy of what should go into node-core, I'm asking for your guidance. I'm fine with either. |
The reserved root `${os.devNull}/vfs`, which holds the mount points of
all virtual file systems, could not be read: fs calls on it fell through
to the real file system, so nothing could list what was mounted.
While any file system is mounted, serve the root as a read-only
directory. It lists every mount point by its layer id, a recursive
listing descends into each mounted file system, and paths under it that
no mount serves report ENOENT. Creating, removing or changing entries in
it fails with EROFS. When nothing is mounted it does not exist, as
before.
A mount point cannot be removed or renamed, nor replaced by a rename:
rmdir() and rename() fail with EBUSY, and a recursive rm() empties the
file system and then fails the same way. Before, rmdir() of an empty
mount point reported success without doing anything.
Reserve layer 0 for the file system --vfs-load mounts, and number the
others from 1. That source is then at the same reserved mount point in
every thread, whatever else a thread mounts and wherever --vfs-load is
written among the other mounts, so a path into it stays valid in a
worker - including a worker created with its own execArgv, which
inherits none of the parent's options and has to be given --vfs-load
again. A worker still does not run that entry point, but it now has to
recognize which source it belongs to in order to mount it at that layer.
The callback and promise forms of readdir() with `withFileTypes` now
report each Dirent's parentPath as a host path, as readdirSync() did,
instead of the provider-relative one, and split recursive names such as
`dir/file.txt` into their directory and base name. A recursive listing
joins subdirectories with the host separator rather than `/`, which
mixed separators on Windows. realpath() of a mount point no longer
returns it with a trailing separator.
Add vfs.vfsBase(), which returns that directory, so a program can list
what is mounted without spelling out `path.join(os.devNull, 'vfs')`. The
note under vfs.mount() said the path scheme must not be relied on, which
read as a contradiction of the root being listable; it now says where a
mount point comes from, and that only the name within the root is
assigned at runtime.
Signed-off-by: Philipp Dunkel <pip@pipobscure.com>
--vfs-mount mounted a source without running it, and shared one ordered list of sources with --vfs-load, so neither option could say which entry it had contributed. Both the entry point and the layer reserved for it were recovered from the position of --vfs-load among the mounts, in a list NODE_OPTIONS could prepend to. Nothing needs more than one mount from the command line: a program that wants more can mount them itself through node:vfs, where it also gets the instance. Remove --vfs-mount. --vfs-load keeps its own single source, so nothing has to recover which flag contributed which mount: the one source is mounted at the layer reserved for it, and the entry point comes from there. Workers are unchanged: the source is inherited but the entry point is not, so a worker mounts it and runs its own entry, which may live inside the mount. ERR_VFS_INVALID_TARGET now names --vfs-load as the source's origin, and the startup test moves to test-vfs-load.js, with the cases that covered mounting without loading removed. Signed-off-by: Philipp Dunkel <pip@pipobscure.com>
pipobscure
force-pushed
the
vfs-load-only
branch
from
September 20, 2026 15:31
1e49322 to
0abd2c8
Compare
jasnell
approved these changes
Sep 20, 2026
bakkot
reviewed
Sep 20, 2026
bakkot
left a comment
bakkot
left a comment
Contributor
There was a problem hiding this comment.
Great, thanks, I think this ends up being a lot simpler/clearer.
| The source is mounted at the same reserved mount point in every thread, | ||
| whatever else that thread mounts, so a path into it stays valid in a worker - | ||
| including one created with its own `execArgv`, which does not inherit the | ||
| parent's options and has to be given `--vfs-load` again. |
Contributor
There was a problem hiding this comment.
I would say something stronger here (assuming I've understood correctly): if you are going to provide execArgv to a Worker loaded from --vfs-load'd vfs, you must include the same --vfs-load whatever in execArgv or else the worker will not be able to laod.
Contributor
Author
There was a problem hiding this comment.
Is that true though? You could load a worker from an entirely different source if you wanted. Heck you could even fetch the worker-code from a remote source if you wanted and point at it.
but you are rigtht in the sense that if you want to load from theb same archive you must pass the flag.
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
4 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.
--vfs-mount mounted a source without running it, and shared one ordered
list of sources with --vfs-load, so neither option could say which entry
it had contributed. Both the entry point and the layer reserved for it
were recovered from the position of --vfs-load among the mounts, in a
list NODE_OPTIONS could prepend to. Nothing needs more than one mount
from the command line: a program that wants more can mount them itself
through node:vfs, where it also gets the instance.
Remove --vfs-mount. --vfs-load keeps its own single source, so nothing
has to recover which flag contributed which mount: the one source is
mounted at the layer reserved for it, and the entry point comes from
there.
Workers are unchanged: the source is inherited but the entry point is
not, so a worker mounts it and runs its own entry, which may live inside
the mount.
ERR_VFS_INVALID_TARGET now names --vfs-load as the source's origin, and
the startup test moves to test-vfs-load.js, with the cases that covered
mounting without loading removed.
Based on #66140, which is the first commit here; only
vfs: drop --vfs-mount, keep --vfs-load aloneis new. Draft, because the direction is still open for discussion.