rfcs icon indicating copy to clipboard operation
rfcs copied to clipboard

[RRFC] Add libc fields to select optionalDependencies should be installed or skipped

Open Brooooooklyn opened this issue 4 years ago • 24 comments

Motivation ("The Why")

Example

For my packages published as prebuilt native addons, I config the native packages cross-platform as optionalDependencies to let npm choose the suitable one to download. And I can avoid written postinstall scripts to download them in this way. Like canvas package:

The @napi-rs/canvas package is published for users depending on it directly:

{
  "name": "@napi-rs/canvas",
  "version": "0.1.6",
  "optionalDependencies": {
    "@napi-rs/canvas-linux-x64-gnu": "0.1.6",
    "@napi-rs/canvas-linux-x64-musl": "0.1.6",
    "@napi-rs/canvas-darwin-x64": "0.1.6",
    "@napi-rs/canvas-win32-x64": "0.1.6",
    "@napi-rs/canvas-linux-aarch64-gnu": "0.1.6",
    "@napi-rs/canvas-linux-aarch64-musl": "0.1.6"
  }
}

And the packages include prebuilt native addons:

{
  "name": "@napi-rs/canvas-linux-x64-musl",
  "version": "0.1.6",
  "os": ["linux"],
  "cpu": ["x64"],
  "main": "skia.linux-x64-musl.node",
  "files": ["skia.linux-x64-musl.node"]
}
{
  "name": "@napi-rs/canvas-linux-x64-gnu",
  "version": "0.1.6",
  "os": ["linux"],
  "cpu": ["x64"],
  "main": "skia.linux-x64-gnu.node",
  "files": ["skia.linux-x64-gnu.node"]
}

In the macOS/Windows/FreeBSD this mechanism works fine, but on the Linux systems, npm will download two native binding packages, both gnu and musl. It will significantly increase the install size. The install size of @napi-rs/canvas on linux-x64 systems is 37mb which could be the half of it.

References

  • Related: https://github.com/nodejs/node/issues/39877

Brooooooklyn avatar Aug 25 '21 11:08 Brooooooklyn

Also as process.versions.modules, npm should also add modules field to support downloading the appropriate package of the current nodejs version, such as:

{
  "name": "example",
  "version": "0.0.1",
  "optionalDependencies": {
    // for node-v8.x
    "example-linux-x64-v57-gnu": "0.0.1",
    // for node-v10.x
    "example-linux-x64-v64-gnu": "0.0.1",
    // for node-v12.x
    "example-linux-x64-v72-gnu": "0.0.1",  
    // for node-v14.x
    "example-linux-x64-v83-gnu": "0.0.1",  
    // for node-v16.x
    "example-linux-x64-v93-gnu": "0.0.1",  
}

hyj1991 avatar Nov 08 '21 02:11 hyj1991

If node.js put more of that info in process.versions (as suggested in nodejs/node#39877) then it would be a relatively straightforward thing to provide a way to select based on it.

The other workaround is to use a preinstall script in each optional dep that checks the environment and exits with a non-zero status code if it's not suitable. This might be a good idea anyway, just to prevent someone using it in a place where it'll never work. If an optional dependency fails its install scripts, npm will clean it up. (Ie, remove it along with any dependencies that are no longer needed as a result of its removal.)

For selecting based on node version, you can use "engines": {"node":"<semver range>"} and npm will avoid even attempting to install optional deps with mismatched engine requirements.

(For non-optional deps, a failing install script will fail and roll back the entire install, and in the case of a mismatched engines.node range, it'll warn but continue, unless the user has specified --engines-strict.)

isaacs avatar Nov 10 '21 00:11 isaacs

The other workaround is to use a preinstall script in each optional dep that checks the environment and exits with a non-zero status code if it's not suitable. This might be a good idea anyway, just to prevent someone using it in a place where it'll never work.

It won't be a suitable solution if https://github.com/npm/rfcs/pull/488 landed

Brooooooklyn avatar Nov 10 '21 01:11 Brooooooklyn

@styfle here is glibcVersionCompiler and glibcVersionRuntime in process.report.getReport().header. Could we approach this feature via the glibcVersionRuntime filed?

Brooooooklyn avatar Dec 28 '21 04:12 Brooooooklyn

@Brooooooklyn Yeah that sounds like it could work 👍

I could imagine a new package.json field with "glibcVersionRuntime": ">=2 <3" kind of like engines and that would only install the glibc version of the package.

However, I'm not sure how this would work for the musl version. How would you negate it? "glibcVersionRuntime": "<=0"?

I'm almost thinking we need to expose family instead like detect-libc does. That way each optional dependency can match on the correct family.

styfle avatar Dec 28 '21 20:12 styfle

In my opinion, libc runtime is part of the system, not Node.js engine. So I prefer a filed libc: ['glibc', 'musl', 'msvc'] like os and cpu.

And I don't like the idea that introduce version restrict to the libc runtime. The version of libc is just like the version of OS. They do not follow the semver, and lack documentation to list the compatible matrix.

Brooooooklyn avatar Dec 29 '21 03:12 Brooooooklyn

I like the libc: ['glibc', 'musl', 'msvc'] idea 👍

Now the question is, does it make sense to implement this in Node.js core or should this be implemented in npm?

The advantage of Node.js core is that other package managers could easily implement the feature (yarn, pnpm, etc).

The disadvantage of Node.js core is that the user must update both node and npm in order to use this feature.

styfle avatar Jan 01 '22 20:01 styfle

Hello, I help maintain the detect-libc module mentioned here, which is now a dependency of quite a few install-time tools. It has not been updated in over 4 years and I'd like to simplify/modernise it, especially if doing so would help get this useful feature added to npm itself.

There's a list of possible improvements in https://github.com/lovell/detect-libc/issues/14 that might be of interest.

lovell avatar Jan 02 '22 20:01 lovell

While a little off-topic, I'm open to add support for a libc field in Yarn.

Main issue I'd have is the difficulty around detecting it (detect-libc is a good start, but it's currently sync; overall I'd prefer if this information came from Node, since it should be possible to get it at build-time), and decide whether the field should also expose individual majors (ie libc: ['glibc2']).

arcanis avatar Jan 10 '22 23:01 arcanis

/cc @zkochan

Brooooooklyn avatar Jan 11 '22 08:01 Brooooooklyn

overall I'd prefer if this information came from Node, since it should be possible to get it at build-time), and decide whether the field should also expose individual majors (ie libc: ['glibc2']).

Node.js already has the process.report.getReport() API to detect glibc versions: https://nodejs.org/api/report.html#diagnostic-report

{
  "header": {
    "reportVersion": 1,
    "nodejsVersion": "v12.0.0-pre",
    "glibcVersionRuntime": "2.17",
    "glibcVersionCompiler": "2.17",
    "wordSize": "64 bit",
    "arch": "x64",
    "platform": "linux",
    "componentVersions": {
      "node": "12.0.0-pre",
      "v8": "7.1.302.28-node.5",
      "uv": "1.24.1",
      "zlib": "1.2.11",
      "ares": "1.15.0",
      "modules": "68",
      "nghttp2": "1.34.0",
      "napi": "3",
      "llhttp": "1.0.1",
      "openssl": "1.1.0j"
    }
  }
}

But only for glibc systems. While I think it's enough for the native addons:

  • https://github.com/Brooooooklyn/canvas/pull/401

The postinstall parts in this pull request should be implemented in package managers.

Brooooooklyn avatar Jan 11 '22 09:01 Brooooooklyn

Right, the implementation of the skip mechanism will be up to each package manager, I'm not too worried about that.

My point is that knowing whether the system uses the glibc or musl should ideally come from either process.versions or process.report.getReport(), not from an external package (or at least, not long term). Spawning external process just for that seems a waste, considering that Node is supposed to have been built against the libc and should thus have the information available already.

arcanis avatar Jan 11 '22 09:01 arcanis

cc @Jarred-Sumner, I know he did some optimizations around optional dependencies in Bun

zkochan avatar Jan 12 '22 21:01 zkochan

cc @Jarred-Sumner, I know he did some optimizations around optional dependencies in Bun

Bun skips postinstall while retaining compatibility with native dependencies that have binary executables by allowlisting some packages to have the first dependency matching os/cpu constraints to become the source where "bin" is linked from. If you install esbuild on macOS x64, then the "bin" field in esbuild reads from esbuild-darwin-64's folder instead of esbuild. This is currently enabled by default for turbo & esbuild, but can be enabled for other packages with the --link-native-bins packageName flag. This also disables skipping optionalDependencies for the package (though it skips downloading tarballs if the platform doesn't match). esbuild & turbo's postinstall work by overwriting the package's executable with the platform-specific binary

Giving some packages special treatment is not great, but there currently is no way to specify this behavior in package.json or in the registry API response. This approach also only works with native executables, not native bindings

Instead of that, here is what I would propose

"nativeDependencies" field in package.json where the keys are something like target triples.

{
  "name": "@napi-rs/canvas",
  "version": "0.1.6",
  "nativeDependencies": {
    "linux-x64-gnu": {
        "@napi-rs/canvas-linux-x64-gnu": "0.1.6"
    },
    "linux-x64-musl": {
        "@napi-rs/canvas-linux-x64-musl": "0.1.6"
    },
    "darwin-x64" : {
        "@napi-rs/canvas-darwin-x64": "0.1.6"
    },
    "win32-x64" : {
        "@napi-rs/canvas-win32-x64": "0.1.6"
    },
    "linux-aarch64-gnu": {
        "@napi-rs/canvas-linux-aarch64-gnu": "0.1.6"
    },
    "linux-aarch64-musl": {
        "@napi-rs/canvas-linux-aarch64-musl": "0.1.6"
    }
  }
}

This would tell package managers they need to install the most specific matching package.

On Ubuntu 18.04 AMD64, a package manager would try:

  • linux-x64-gnu
  • linux-x64
  • linux

Ideally, these dependencies would not be allowed to have their own dependencies. That would enable package managers to improve installation times by skipping extra registry API metadata requests when the exact version of the native dependency is specified. This means adding additional native targets won't impact package installation time.

Jarred-Sumner avatar Jan 13 '22 02:01 Jarred-Sumner

"nativeDependencies" field in package.json where the keys are something like target triples.

I like this idea.

Ideally, these dependencies would not be allowed to have their own dependencies.

And this.

I think we can design a different installation logic for native packages than for normal npm packages, considering that native packages are platform-related.

The best solution I can imagine is:

  • For native packages authors, put all binaries into single npm packages. This will reduce a lot of works in CI for moving binaries cross build matrix.
  • For users, package managers will partially download the content of the published native npm package, by detecting the target triple on the installation host or config (for cross-compiling).

I think this RRFC should be turned into add nativeDependencies for prebuilt native packages rather than add libc fields

What do you think?

Brooooooklyn avatar Jan 14 '22 03:01 Brooooooklyn

There's already an open RFC discussing this feature, here:

  • https://github.com/yarnpkg/berry/issues/2751

It's still waiting for npm to join the discussion there, and I'm not interested to contribute to another RFC around this topic until they do.

The libc field is small enough that it requires little work and formal consensus, so that's what we're going to implement in the meantime.

arcanis avatar Jan 14 '22 07:01 arcanis

"nativeDependencies" field in package.json where the keys are something like target triples.

What if none of these match the current platform? Can it fallback to a wasm variant and how would you define that?

The libc field is small enough that it requires little work and formal consensus, so that's what we're going to implement in the meantime.

I'm happy with that for now! Its the least amount of changes to solve the original issue 👍 Is there a Yarn PR we can follow along?

styfle avatar Jan 14 '22 16:01 styfle

Is there a Yarn PR we can follow along?

Yes, it would be

  • https://github.com/yarnpkg/berry/pull/3981

arcanis avatar Jan 14 '22 17:01 arcanis

What if none of these match the current platform? Can it fallback to a wasm variant and how would you define that?

We can define platform triple like wasm32

Brooooooklyn avatar Jan 14 '22 18:01 Brooooooklyn

Relates to: https://github.com/npm/rfcs/pull/519

ruyadorno avatar Feb 02 '22 17:02 ruyadorno

Action item from our OpenRFC call: Let's follow up with the idea laid out in this RRFC and work on a sep RFC that can encompass more things that can possibly affect whether an optional dependency gets installed or not. This new RFC can complement the work from #519 and provide a better package distributions experience.

ruyadorno avatar Feb 09 '22 20:02 ruyadorno

Looks like both yarn and pnpm added support for the libc field.

  • https://github.com/yarnpkg/berry/pull/3981
  • https://github.com/pnpm/pnpm/pull/4605

Let's get this added to npm too? No need to wait for distributions.

styfle avatar May 23 '22 16:05 styfle

@styfle lets do it. We can chat about this tomorrow (ie. in the next RFC call) but I think this is something we can agree on given the previous implementations.

darcyclarke avatar Jul 12 '22 22:07 darcyclarke

Sounds good! I watched the recording and seems like there was no opposition 👍

https://youtu.be/UDftNDbx0nM?t=935

styfle avatar Jul 13 '22 19:07 styfle

@darcyclarke Is anyone implementing this feature? Whats the roadmap look like?

styfle avatar Oct 28 '22 14:10 styfle

~~I just confirmed this was fixed in https://github.com/npm/npm-install-checks/pull/54 and https://github.com/npm/npm-install-checks/pull/57 which landed in [email protected], so I think this can be closed~~

Update, seems to be not working.

styfle avatar Jul 24 '23 15:07 styfle

Has anyone confirmed that this feature is working as intended for people using the latest npm CLI, or are there further code or dependency updates required?

A popular package that has taken advantage of the new libc field is @swc/core, which defines optionalDependencies on @swc/core-linux-x64-musl and @swc/core-linux-x64-gnu (amongst others).

However the latest version of npm appears to be installing both the glibc and musl variants on Linux regardless of libc.

musl
$ docker run -it --rm node:20-alpine /bin/sh

# npm install -g npm@latest
# npm -v
10.2.0

# npm install @swc/core
added 5 packages in 11s

# npm ls --depth 1
`-- @swc/[email protected]
  +-- UNMET OPTIONAL DEPENDENCY @swc/[email protected]
  +-- UNMET OPTIONAL DEPENDENCY @swc/[email protected]
  +-- UNMET OPTIONAL DEPENDENCY @swc/[email protected]
  +-- UNMET OPTIONAL DEPENDENCY @swc/[email protected]
  +-- UNMET OPTIONAL DEPENDENCY @swc/[email protected]
  +-- @swc/[email protected]
  +-- @swc/[email protected]
  +-- UNMET OPTIONAL DEPENDENCY @swc/[email protected]
  +-- UNMET OPTIONAL DEPENDENCY @swc/[email protected]
  +-- UNMET OPTIONAL DEPENDENCY @swc/[email protected]
  +-- @swc/[email protected]
  +-- UNMET OPTIONAL DEPENDENCY @swc/helpers@^0.5.0
  `-- @swc/[email protected]

# ls -al node_modules/@swc/
total 28
drwxr-xr-x    7 root     root          4096 Oct  9 19:53 .
drwxr-xr-x    3 root     root          4096 Oct  9 19:53 ..
drwxr-xr-x    3 root     root          4096 Oct  9 19:52 core
drwxr-xr-x    2 root     root          4096 Oct  9 19:52 core-linux-x64-gnu
drwxr-xr-x    2 root     root          4096 Oct  9 19:52 core-linux-x64-musl
drwxr-xr-x    2 root     root          4096 Oct  9 19:52 counter
drwxr-xr-x    2 root     root          4096 Oct  9 19:52 types
glibc
$ docker run -it --rm node:20-bullseye /bin/sh

# npm install -g npm@latest
# npm -v
10.2.0

# npm install @swc/core
added 5 packages in 9s

# npm ls --depth 1
`-- @swc/[email protected]
  +-- UNMET OPTIONAL DEPENDENCY @swc/[email protected]
  +-- UNMET OPTIONAL DEPENDENCY @swc/[email protected]
  +-- UNMET OPTIONAL DEPENDENCY @swc/[email protected]
  +-- UNMET OPTIONAL DEPENDENCY @swc/[email protected]
  +-- UNMET OPTIONAL DEPENDENCY @swc/[email protected]
  +-- @swc/[email protected]
  +-- @swc/[email protected]
  +-- UNMET OPTIONAL DEPENDENCY @swc/[email protected]
  +-- UNMET OPTIONAL DEPENDENCY @swc/[email protected]
  +-- UNMET OPTIONAL DEPENDENCY @swc/[email protected]
  +-- @swc/[email protected]
  +-- UNMET OPTIONAL DEPENDENCY @swc/helpers@^0.5.0
  `-- @swc/[email protected]

# ls -al node_modules/@swc/
total 28
drwxr-xr-x 7 root root 4096 Oct  9 20:09 .
drwxr-xr-x 3 root root 4096 Oct  9 20:09 ..
drwxr-xr-x 3 root root 4096 Oct  9 20:09 core
drwxr-xr-x 2 root root 4096 Oct  9 20:09 core-linux-x64-gnu
drwxr-xr-x 2 root root 4096 Oct  9 20:09 core-linux-x64-musl
drwxr-xr-x 2 root root 4096 Oct  9 20:09 counter
drwxr-xr-x 2 root root 4096 Oct  9 20:09 types

I really want to take advantage of this feature so hopefully this is a case of me missing something obvious.

lovell avatar Oct 09 '23 20:10 lovell

libc is not overridable yet, only cpu and os.

This PR is where the libc override is happening. The underlying dependency supports it and landed in latest today, the PR now needs to rebase and drop the deps update and it should land. https://github.com/npm/cli/pull/6817

wraithgar avatar Oct 09 '23 20:10 wraithgar

@wraithgar I think we should reopen this issue.

The original post says that the libc field should be used to avoid postinstall by only installing the correct optionalDendencies.

I just tried the latest npm and latest sharp and it is still installing both musl and gnu binaries but alpine should only install musl binaries.

Dockerfile

FROM node:20-alpine3.18

RUN mkdir /app
WORKDIR /app
RUN npm i -g [email protected]
RUN npm add [email protected]
RUN ls -lA node_modules/@img

build command

docker build -t example --progress=plain --no-cache .

output

drwxr-xr-x    3 root     root          4096 Jan  4 18:27 sharp-libvips-linux-arm64
drwxr-xr-x    3 root     root          4096 Jan  4 18:27 sharp-libvips-linuxmusl-arm64
drwxr-xr-x    3 root     root          4096 Jan  4 18:27 sharp-linux-arm64
drwxr-xr-x    3 root     root          4096 Jan  4 18:27 sharp-linuxmusl-arm64

styfle avatar Jan 04 '24 18:01 styfle

  • This will fix it: https://github.com/npm/cli/pull/7126

styfle avatar Jan 10 '24 23:01 styfle