Skip to content

Research the feasibility to providing a VFS extension mechanism to fs, require(), child_process, etc #37

Description

@jviotti

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.

Activity

  1. RaisinTen commented on Sep 19, 2022

    @RaisinTen
    Member

    Btw, after nodejs/node#44537 landed, we are having some related conversation in nodejs/node#44713.

  2. RaisinTen commented on Sep 19, 2022

    @RaisinTen
    Member

    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".

  3. jviotti commented on Sep 19, 2022

    @jviotti
    MemberAuthor

    @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?

  4. RaisinTen commented on Sep 20, 2022

    @RaisinTen
    Member

    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?

  5. arcanis commented on Sep 20, 2022

    @arcanis
    Contributor

    It's still unclear to me why you want/need C++ hooks? Why isn't patching fs enough?

  6. RaisinTen commented on Sep 20, 2022

    @RaisinTen
    Member

    Patching fs is 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.

  7. arcanis commented on Sep 20, 2022

    @arcanis
    Contributor

    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 fs ourselves 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?

  8. RaisinTen commented on Sep 21, 2022

    @RaisinTen
    Member

    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 fd level by just monkey-patching fs.open but 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.

  9. arcanis commented on Sep 21, 2022

    @arcanis
    Contributor

    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:fs for 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.

  10. GeoffreyBooth commented on Sep 21, 2022

    @GeoffreyBooth
    Member

    cc @nodejs/loaders

  11. jviotti commented on Sep 22, 2022

    @jviotti
    MemberAuthor

    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.

  12. CMCDragonkai commented on Jul 16, 2023

    @CMCDragonkai

    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.

  13. 6 remaining items

  14. mcollina commented on Nov 15, 2023

    @mcollina
    SponsorMember

    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:

    1. avoid most overhead for the current fs calls
    2. minimize breakage
    3. compatible with the ESM/loader ecosystem
    4. 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.

  15. arcanis commented on Nov 15, 2023

    @arcanis
    Contributor

    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

    1. A protocol would break a major chunk of the ecosystem relying on path manipulation. Anything using path.resolve or path.relative would be unable to use virtual filesystems. ↩

  16. mcollina commented on Nov 15, 2023

    @mcollina
    SponsorMember

    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?

  17. joyeecheung commented on Nov 15, 2023

    @joyeecheung
    Member

    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.

  18. GabenGar commented on Nov 15, 2023

    @GabenGar

    Electron: bundles an entire v8 runtime and a chrome, 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 vfs to existing fs API 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.js
    

    Then we put sea.exe file into a folder which ends up in this structure:

    /sea
      /index.js
      /a/index.js
      /b/index.js
    /sea.exe
    

    Where does the path /sea/a/index.js point to? Having /sea.exe/a/index.js is not an option, since it implies /sea.exe is a folder. Which is not true, because it's clearly a file, and dirent.isFile() will be true, which is quite a footgun for anyone accustomed to file system APIs but not the vfs specifics. And we didn't even get to multiplatform issues, of which there will be plenty.
    So the "smart" way, aka fs modules automagically understanding vfs, will have a bunch of side effects and unintuitive (and potentially inconsistent between platforms) assumptions about the folder where vfs file is called.
    The other way is to create a module to interact with vfs files in NodeJS. This has the benefit of separating (potentially inlined) vfs calls from normal fs calls inside a given vfs without relying on arcane tricks which require monkey-patching fs in the first place.

  19. mcollina commented on Nov 15, 2023

    @mcollina
    SponsorMember

    Here 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
    })
  20. arcanis commented on Nov 15, 2023

    @arcanis
    Contributor

    Have you got links to the vfs implementations of electron and yarn handy?

    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 fs API. By contrast Electron can get away with skipping some rarely used primitives like fstat etc.

    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);
      }
    }
  21. mcollina commented on Nov 15, 2023

    @mcollina
    SponsorMember

    I 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.

  22. GabenGar commented on Nov 15, 2023

    @GabenGar

    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 of fs functions are named after linux utils, it can be quite confusing to have a method with the same name but mechanics unrelated to at least linux mount(). Hence why I think it should be a separate module, because it's only tangentially related to fs, 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.

  23. mcollina commented on Nov 15, 2023

    @mcollina
    SponsorMember

    In order for all things in the ecosystem to work correctly, support needs to be built in inside the node:fs module, because dependencies uses that to load files all the time.

    The name of the function can be whatever we want: exposeVirtualFileSystem might be long and descriptive enough.

  24. bmeck commented on Nov 15, 2023

    @bmeck
    Member

    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.

  25. mcollina commented on Nov 15, 2023

    @mcollina
    SponsorMember

    Agreed. What I want to avoid is to have to iterate through a full list of regexp for each mount for every file system operation.

  26. GeoffreyBooth commented on Nov 16, 2023

    @GeoffreyBooth
    Member

    When we designed import { register } from 'node:module' we envisioned the pattern would be repeated for other systems. So import { 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 async open hook that can run within readFileSync.

  27. jviotti commented on May 17, 2024

    @jviotti
    MemberAuthor

    Closing this a I think we did a lot of research already. Any action items can be extracted a new issues

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions