Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

doc: add minutes for meeting 5 Sep 2022 #31

Merged
merged 3 commits into from
Sep 9, 2022
Merged

Conversation

RaisinTen
Copy link
Contributor

@RaisinTen RaisinTen commented Sep 5, 2022

No description provided.

RaisinTen and others added 2 commits September 5, 2022 19:23
Signed-off-by: Darshan Sen <raisinten@gmail.com>
Signed-off-by: Juan Cruz Viotti <jv@jviotti.com>
@jviotti
Copy link
Member

jviotti commented Sep 5, 2022

@RaisinTen I added my own notes

Copy link
Member

@ljharb ljharb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My comments aren’t a review of the pr ofc, just discussion items that can be brought up during the next call.


To accomplish this decoupling, we hope to:

- Provide hooking capabilities to Node.js to allow developers to extend
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems like a really problematic thing to consider (require especially) if it will work outside a SEA.

itself)? Does an app that consists of the main executable along with sibling
".node" shared libraries still qualifies as a SEA? Clearly defining what an SEA
is (and is not) is key for identifying the core constrains for the ideal VFS
implementation later down the road.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the “s” is for “single”, so imo unless it results in just one file, it doesn’t qualify.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That was the discussion about. The project name doesn't say "single file applications", but "single executable applications". Therefore, the question is: does a Node.js CLI application compiled to a binary + a couple of .node shared libraries qualifies? Technically yes, as it still consists of a single "executable."

Some files must live outside of the main binary to be used in all operating systems (like native add-ons), and as @zcbenz and @arcanis pointed out in other discussions, extracting them on the fly every single time can be both problematic and slow. If an app consisting of an executable + shared objects still qualifies as a SEA, then that can solve a lot of problems.

Or to ask the reverse question: what do we gain from having a single file containing the entire thing vs a single executable + shared objects?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's much easier to distribute and manage.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's true that a single file its easier to distribute, however people rarely distribute standalone binary executables in the internet. Instead, even single executables are usually distributed on a ZIP, tarball, etc. One of the core reasons is that on i.e. macOS, the browser would strip the executable bit from the binary and users would need to be taught how to chmod +x.

So my thinking is: if you would zip/tar the single binary anyway, then having a few files rather than a single one doesn't really make distribution much harder.

Furthermore, having a single file representing an app with native add-ons on Windows is a no-go (#30 (reply in thread)), and it might be a hard constraint outside of our control.

The current status is: if a Node.js app has native add-ons and targets Windows, shipping a single file is not possible. So we either accommodate our definition or say that apps with native add-ons cannot be archived as an SEA.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ljharb @jviotti I've created a separate discussion for this in #34 because this is a very important topic and it might easily get lost here.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

Comment on lines +41 to +44
- Provide hooking capabilities to Node.js to allow developers to extend
`require()`, `fs` and related key components of Node.js to support arbitrary
virtual file systems without monkey-patching these modules like i.e.
Electron, Yarn and PKG do at the moment.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it should be clearer that "Provide hooking capabilities to Node.js" doesn't actually involve changing Node.js in any way at this time - even if non-official, we already have the "hooking capabilities" we need (monkey-patching) to implement a userland SEA.

Of course it won't the best in terms of quality / perfs, but since I think we agreed this userland idea was only to get a couple prototypes that could be used as basis for the discussion (and that it wasn't intended to be the final "deliverable"), it's good enough:

Once we have a better grasp on what the ideal VFS should look like, we will attempt to ship the SEA project with a blessed VFS implementation.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

True. However, we would still need to hook into these things indefinitely (see the Bootstrapper component discussed in https://github.com/nodejs/single-executable/blob/main/blog/2022-08-05-an-overview-of-the-current-state.md#bootstrapper), even with a blessed implementation. And if that's the case, should we just solve the entire thing for once and for all?

As we have seen with Yarn, PKG, Electron, etc a very common reason for monkey patching fs and require is packaged Node.js applications, so in a way, we could argue this falls within our scope.

As far as I understand, even require internally uses fs (or its native bindings?) and fs relies on libuv for most things, so I wonder if hooking into the lower-level layers of fs is actually much simpler than monkey patching all the upper level functions.

I guess @addaleax and @RaisinTen might be able to confirm or debunk :P Feels at least worth a look IMHO

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

True. However, we would still need to hook into these things indefinitely, even with a blessed implementation

The way I imagine it, once a format is selected, the support for reading it from a VFS would be integrated inside Node (similar to how Python supports importing modules from eggs without users having to hook anywhere). We'd then get the bootstrap "for free", since by running node ./path/to/app.sea Node (which uses fs, except for a couple of places which are being worked on) would transparently treat app.sea as a folder, and execute its index.js.

As we have seen with Yarn, PKG, Electron, etc a very common reason for monkey patching fs and require is packaged Node.js applications, so in a way, we could argue this falls within our scope.

I'd say it falls within the scope of https://github.com/nodejs/loaders, a separate WG working to add customization support to import/require.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd say it falls within the scope of https://github.com/nodejs/loaders, a separate WG working to add customization support to import/require.

That seems like a big stretch? By itself, fs hooking has nothing to do with module loading.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The loaders team has talked about renaming to the "customization" team and expanding their purview. Might make sense to double-check with them.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@arcanis

The way I imagine it, once a format is selected, the support for reading it from a VFS would be integrated inside Node (similar to how Python supports importing modules from eggs without users having to hook anywhere)

Yeah, I think we can eventually aim for that, once we have a VFS that we are confident to support long term and that works for all the scenarios we care about. Still, the "native" implementation on Node.js does require some hooking (even if it's internal hooking).

Signed-off-by: Darshan Sen <raisinten@gmail.com>
@RaisinTen RaisinTen marked this pull request as ready for review September 8, 2022 10:48
@RaisinTen RaisinTen changed the title [WIP] doc: add minutes for meeting 5 Sep 2022 doc: add minutes for meeting 5 Sep 2022 Sep 8, 2022
Copy link
Member

@mhdawson mhdawson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM great to see the active discusion on some key points. The minutes captures what was discussed in the meeting so can land without conversations being resolved.

@jviotti
Copy link
Member

jviotti commented Sep 9, 2022

Makes sense! Let's go ahead and merge this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants