Repository navigation
Research the feasibility to providing a VFS extension mechanism to fs, require(), child_process, etc #37
Description
Activity
- addedgood first issueGood for newcomersGood for newcomershelp wantedExtra attention is neededExtra attention is needed
on Sep 16, 2022 Btw, after nodejs/node#44537 landed, we are having some related conversation in nodejs/node#44713.
The user-facing surface of this is pretty large, and I wonder if providing an extension point at the lower levels is actually easier.
Sounds like nodejs/node#39711 (comment) is a potential approach we can try experimenting with:
// node.h class UvDelegate { public: explicit UvDelegate(node::Environment* env); virtual int uv_fs_open(...); virtual int uv_fs_close(...); }; // node.cc void InternalModuleReadJSON() { ... int fd = env->delegate()->uv_fs_open(); ... } // user.cc class EmbedderUvDelegate : node::UvDelegate { int uv_fs_open(...) override; int uv_fs_close(...) override; }; node::CreateEnvironment(..., delegate, ...);
or something simpler:
// node.h struct UvAPIs { UvFsOpenFunc uv_fs_open = uv_fs_open; UvFsCloseFunc uv_fs_close = uv_fs_close; ... }; // user.cc UvAPIs uv_apis; uv_apis.uv_fs_open = ...; uv_apis.uv_fs_close = ...; node::CreateEnvironment(..., uv_apis, ...);
but I'm not sure if this is actually "easier".
Reacted by Juan Cruz Viotti@RaisinTen Yeah, this is exactly what I'm talking about!
but I'm not sure if this is actually "easier".
I do think it's easier from an "amount of code" point of view and I'd be interested in exploring this further. Maybe do a demo patch if you are up for it @RaisinTen ?
The only limitation I can see here is that while it does make it potentially easier to implement a VFS on C++, it would be hard to use the same APIs from JavaScript, right? I wonder if we could also expose the same "delegate" concept or something like that to JS too?
The only limitation I can see here is that while it does make it potentially easier to implement a VFS on C++, it would be hard to use the same APIs from JavaScript, right?
My understanding was that the patching would take place completely at the C++ level and there should be no extra APIs exposed to JS?
It's still unclear to me why you want/need C++ hooks? Why isn't patching
fsenough?Patching
fsis problematic because it's hard to keep those patches in sync with the upstream fs API changes from node. This is a problem Electron's ASAR has faced and we have sent a PR to address at least one of the API changes in electron/electron#34764.Yes, I'm familiar with this problem; still:
-
if the goal of this project is to have one "blessed" VFS implementation specifically made for reading files from archives, then it doesn't have to have a hook API, it can be integrated with Node. By this point, having it in JS is fine, since tests will be part of the Node codebase itself.
-
if the goal is instead to allow for multiple VFS implementation, then native is out of the question since it's not portable (Node scripts and packages are portable by default, and I think it's something very important to preserve).
And for experimentations, patching
fsourselves isn't perfect for the reasons you said, but we've done that for some time and we can keep doing it until we find a format we all agree on. We probably don't need a C++ API just for this?-
if the goal is instead to allow for multiple VFS implementation
I think that was the general agreement.
We probably don't need a C++ API just for this?
I don't think doing C++ hooks is a strict requirement. We're trying to find the most optimal level where allowing the configuration of these APIs would reduce maintenance burden for both node and userland. It probably should be somewhere between monkey-patching of node's fs apis and allowing overloads for libuv c++ functions.
Today @jviotti and I were discussing about doing this in the
fdlevel by just monkey-patchingfs.openbut it seems that there are a lot of path based APIs that would remain unaffected, so we should think of a different approach.We also realized that supporting the plain monkey-patching approach would require patching:
- fs
- child_process
- cjs module loader - Module._stat and Module._readPackage (not sure if esm needs patching of some other components)
I wonder if we can come up with a single API that allows users to configure all of these at once.
if the goal is instead to allow for multiple VFS implementation
I think that was the general agreement.
This wasn't my understanding, and under this prompt I believe the work drastically changes. If, instead of finding a blessed format, and integrating it transparently inside Node itself (similar to phar, eggs, or jar archives), the intent is to simply make it easier to support multiple VFS implementations, then:
-
It's much less useful imo, performance and transparent behaviors were the two main user-facing advantages I saw to this project, over existing solutions.
-
Node.js already has the proper layers (patching for CJS, a resolver overriding
node:fsfor ESM). All that remains then is to build a proper way for userland authors to test that their VFS implementations have the expected behavior compared to the standard Node APIs (essentially, a testsuite-as-a-library). And potentially to add a couple of VFS in CITGM, although it's almost there already, only waiting for lookup: add yarn to lookup.json citgm#905 to land.
-
cc @nodejs/loaders
if the goal of this project is to have one "blessed" VFS implementation specifically made for reading files from archives, then it doesn't have to have a hook API, it can be integrated with Node. By this point, having it in JS is fine, since tests will be part of the Node codebase itself.
The observation here is that even if we provide a "blessed" VFS implementation in the context of SEAs, there are other projects that implement VFSs for reasons other than SEAs, and they still monkey-patch a lot of functions. If there is a way we can provide hooking capabilities in Node.js, then we would both solve the SEA problem AND solve the overall monkey-patching situation that everybody seems to agree sucks and should not happen (and it's incompatible with ESM).
I think that if we widen our scope a bit more, we might be able to come up with a solution that fixes the entire problem for once and for all.
That said, I don't think blocks the rest of the SEA work. For now, we can monkey patch as everybody else is doing (or patch Node.js directly for the sake of our experiments) while we try to find a proper solution to the problem, and hopefully use that later on.
This wasn't my understanding, and under this prompt I believe the work drastically changes. If, instead of finding a blessed format, and integrating it transparently inside Node itself (similar to phar, eggs, or jar archives), the intent is to simply make it easier to support multiple VFS implementations, then:
We want to find a "blessed" one, but based on all the previous discussions we had over the past weeks, it seems that identifying a VFS that checks all the boxes will be tricky. The decision that came out of the last meeting was:
- Accepting that we don't know of a VFS implementation that checks all the boxes
- Embracing that knowledge gap as the ground for further experimentation on the VFS area
- Run SEA experiments with multiple VFS and see what we learn from that
- Then properly revisit the problem of selecting a blessed one
For point 3, we will need to integrate with many VFS (for the time being) as we experiment with them, so the problem of hooking in into Node.js becomes more problematic for us. This is all internal experimentation and we would hope to ship a blessed VFS at some point, but we don't know which yet.
I wrote a VFS a long time ago https://github.com/matrixai/js-virtualfs and indeed Node's FS API has changed quite a bit since then. My company hasn't had resources to keep it up to date since we moved to js-encryptedfs.
If nodejs itself supported a FS interface "blessed", then enabled different underlying implementations, that might be quite useful.
6 remaining items
I think a lot can be obtained by registering new URL schemes in our filesystem API, e.g.
fs.readFile('yarn://mymodule.This has a few benefits:
- avoid most overhead for the current fs calls
- minimize breakage
- compatible with the ESM/loader ecosystem
- compatible with SEA
An alternative with similar properties is "mounting" a filesystem on a given path.
I would stay away from providing an "overlay" filesystem, as I think it would have a significant issues with performance.
I don't think the Node API should enforce any kind of format over the paths. If it goes through
fs, it should be available to the FS layer implementers.Mounting paths would be vastly better than a protocol1, but it still feels like an arbitrary restriction to me; whether the filtering is performed by Node or by the userland code, the performances will be the same.
Also keep in mind we have prior work here. Both Electron and Yarn have been operating production-grade virtual filesystems for many years now, and perhaps there's no need to reinvent the wheel.
Footnotes
-
A protocol would break a major chunk of the ecosystem relying on path manipulation. Anything using
path.resolveorpath.relativewould be unable to use virtual filesystems. ↩
-
Mounting paths would be vastly better than a protocol1, but it still feels like an arbitrary restriction to me; whether the filtering is performed by Node or by the userland code, the performances will be the same.
I agree with you on mounting a path. We do not agree on the performance of filtering on top on an existing path. The problem is that in that case, you'd need to check both the vfs and the actual file system for each API call, and mix the result. This is going to be very complex, likely bug prone and difficult for certain APIs.
Have you got links to the vfs implementations of electron and yarn handy?
I imagine we would just implement it as a if (UNLIKELY(realm->has_fs_delegate()) kind of check in C++ and if there is a delegate, we can call a user-defined delegate (we can call into JS too if that’s configured). And for certain commonly used fs operations (fs.open etc) we can just coalesce the calls in JS. That looks sufficiently performant to me if we are only checking a boolean.
Electron: bundles an entire
v8runtime and achrome, so even a hello world is at least 200MB in size. Also comes with loads of custom interfaces to cooperate the two. Not exactly a plug'n'play nodejs solution.
Yarn: calling its vfs implementation "production-grade" is quite a stretch, since I have yet to see a repo which uses yarn above version 1. My experience of migrating from version 1 to 3, which ended up migrating to npm due to various issues, such as requiring a custom extension for IDE (that still means typescript won't work with them) and a pretty cryptic docker config step. So not very production-ready even if the idea is pretty interesting.The core problem of tying
vfsto existingfsAPI is ambiguity of path resolutions.
Let's say we have a potential SEA with this structure (details of internal implementation don't matter):index.js /a/index.js /b/index.jsThen we put
sea.exefile into a folder which ends up in this structure:/sea /index.js /a/index.js /b/index.js /sea.exeWhere does the path
/sea/a/index.jspoint to? Having/sea.exe/a/index.jsis not an option, since it implies/sea.exeis a folder. Which is not true, because it's clearly a file, anddirent.isFile()will be true, which is quite a footgun for anyone accustomed to file system APIs but not thevfsspecifics. And we didn't even get to multiplatform issues, of which there will be plenty.
So the "smart" way, akafsmodules automagically understandingvfs, will have a bunch of side effects and unintuitive (and potentially inconsistent between platforms) assumptions about the folder wherevfsfile is called.
The other way is to create a module to interact withvfsfiles in NodeJS. This has the benefit of separating (potentially inlined)vfscalls from normalfscalls inside a givenvfswithout relying on arcane tricks which require monkey-patchingfsin the first place.Reacted by Maël NisonHere is the kind of API that I would like to see:
import { mount } from 'node:fs' await mount('/path/to/nonexistent/dir', { async open (path) { return new FakeFDHandle() }, async createReadStream (fakeFdHandle) { // ... } // not sure I would support anything else as most things can be derived by those two })
Have you got links to the vfs implementations of electron and yarn handy?
- Electron: ASAR (implementation)
- Yarn:
@yarnpkg/fslib(FakeFSinterface)
Generally the ASAR implementation is a much more limited version of Yarn's one; since Yarn is used for a very large variety of tools (Eslint, Parcel, Next, ...) we follow very closely the Node.js
fsAPI. By contrast Electron can get away with skipping some rarely used primitives likefstatetc.Here is the kind of API that I would like to see:
We do something similar, it's just that the check is left up to the implementer (because what constitutes a virtual file is rarely decided based on the folder; it's rather the file being accessed):
async function readFile(p, opts, nextReadFile) { if (p.includes('.zip/')) { return readFileFromZip(p, opts); } else { return nextReadFile(p, opts); } }
This mechanism is also closer from how other loader hooks work, as they delegate to the next resolver:
async function resolve(specifier, context, nextResolve) { if (specifier.match(/whatever/)) { return myCustomResolve(specifier, context); } else { return nextResolve(specifier, context); } }
Reacted by Tony GorezI think the API node.js should implement is the minimal one needed to allow 3rd party modules to hook it to specific file formats. The reason why I mention mounting something to a specific folder is that that can be checked extremely quickly via a prefix trie.
Here is the kind of API that I would like to see:
Okay, what is the expected behaviour of the resulting VFS (and by extension SEA) file?
mount()is a quite specific name and can mean somewhat different things in different platforms. Considering a lot offsfunctions are named after linux utils, it can be quite confusing to have a method with the same name but mechanics unrelated to at least linuxmount(). Hence why I think it should be a separate module, because it's only tangentially related tofs, as in you need to open the file of the VFS using the host file system interfaces and at that point it's almost custom logic, which might or might not involve host file system interfaces.In order for all things in the ecosystem to work correctly, support needs to be built in inside the
node:fsmodule, because dependencies uses that to load files all the time.The name of the function can be whatever we want:
exposeVirtualFileSystemmight be long and descriptive enough.I'd be a bit wary of jumping quickly to perf things like a prefix trie; for small numbers of mounts (I suspect this will be the normal case) there are likely other more efficient checks that could be done.
Reacted by Matteo CollinaAgreed. What I want to avoid is to have to iterate through a full list of regexp for each mount for every file system operation.
When we designed
import { register } from 'node:module'we envisioned the pattern would be repeated for other systems. Soimport { register } from 'node:fs', and it can either directly register an object whose properties are functions or it can register a module that's run in a separate thread like the module hooks. The advantage of the latter is that users can write async functions that can run as hooks within sync calls, for example an asyncopenhook that can run withinreadFileSync.Reacted by Jacob SmithReacted by Matteo CollinaClosing this a I think we did a lot of research already. Any action items can be extracted a new issues
Refer to #31 (comment) for a better explanation of the problem.
Monkey-patching all of these functions is non-trivial and requires a lot of code. The user-facing surface of this is pretty large, and I wonder if providing an extension point at the lower levels is actually easier.
Can we do some research to evaluate the feasibility of this?
@RaisinTen You have dug a bit on this on Postman's internal bootstrapper patch, but we do welcome any help or insights from the community.