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

RFC: Package Distributions #519

Closed
wants to merge 7 commits into from
Closed

RFC: Package Distributions #519

wants to merge 7 commits into from

Conversation

darcyclarke
Copy link
Contributor

@darcyclarke darcyclarke commented Feb 2, 2022

  • adds RFC for new distributions capabilities

See rendered RFC

Copy link
Contributor

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

Is "engines" the only condition?

What about when someone only wants the "browser" version of the package?

What about the use case of "i want the default package to omit tests, but i want a way to opt in to installing with tests"?

"node": "10"
},
"platform": "win32",
"package": "foo-native-win32-10@1.x"
Copy link
Contributor

Choose a reason for hiding this comment

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

does this mean that "distributions" only points to additional packages? How will this impact dependency graph analysis tools?

accepted/0000-package-distributions.md Outdated Show resolved Hide resolved
Comment on lines +129 to +131
{
"package": "fs-readdir-native@1"
}
Copy link
Contributor

Choose a reason for hiding this comment

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

what condition applies here?


## Summary

Today, maintainers utilize various strategies to distribute platform-specific versions of their software under a singular package namespace. Often, these strategies rely on `install` scripts or `optionalDependencies` with some type of bootloader implementation (ex. [`esbuild`](https://npmjs.com/package/esbuild?activeTab=explore)); borth are not ideal. The `npm` CLI should support a first-class/standard way for maintainers to define conditions in which a package distribution is reified in place of the origin target.
Copy link

@lydell lydell Feb 3, 2022

Choose a reason for hiding this comment

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

Emphasis mine:

Often, these strategies rely on install scripts or optionalDependencies with some type of bootloader implementation (ex. esbuild)

In the case of esbuild it should be and: It uses both optionalDependencies and a postinstall script:

evanw/esbuild#1621

  • There is still a small post-install script but it's now optional in that the esbuild package should still function correctly if post-install scripts are disabled (such as with npm --ignore-scripts). This post-install script optimizes the installed package by replacing the esbuild JavaScript command shim with the actual binary executable at install time. This avoids the overhead of launching another node process when using the esbuild command. So keep in mind that installing with --ignore-scripts will result in a slower esbuild command.

https://github.com/evanw/esbuild/blob/049e765d95532055f87b1655196c3dc4505b72cf/lib/npm/node-install.ts#L150-L195

For a tool like esbuild – which can compile a small project in a couple of milliseconds – it’s a bit funny that the JavaScript wrapper around the executable can take more time than the compilation itself. If you only run esbuild once in a while it doesn’t matter super much of course.

But that extra startup time does add up in the Elm ecosystem. Editor plugins run both Elm and elm-format on save, and there you want an as snappy an experience as possible, so shaving 100 ms or so definitely counts.

Many people install elm-format with npm locally in each project (so that every contributor is on the exact same version, which is very important for formatters). So a JavaScript wrapper around the project hurts!

So – is this something that this RFC seeks to improve on too? (Just checking!) In my opinion it would be awesome to somehow achieve maximum performance and --ignore-scripts with no “shenanigans” to pull from package authors!

Copy link

@lydell lydell Feb 3, 2022

Choose a reason for hiding this comment

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

Hmm … like this?

{
  "name": "foo",
  "version": "1.2.3",
  "distributions": [
    {
      "platform": "win32",
      "package": "foo-native-win32-10@1.x",
      "bin": "./bin.js"
    },
    {
      "platform": "linux",
      "arch": "x64",
      "package": "foo-native-linux-x64@2.x",
      "bin": "./foo-native-linux-x64"
    },
    "..."
  ]
}

In other words – one "bin" per distributions object, pointing directly to the executables on Linux and macOS, and to a wrapper on Windows (Node.js based, or CMD based, or whatever you like, or maybe you can point directly to an .exe file even?)

If the above is correct, I’d love to see text about this added to the RFC!

Edit: The "bin" fields in foo points to files in foo-native-linux-x64 and other “sub” packages? 🤔

@Jolg42
Copy link

Jolg42 commented Feb 8, 2022

Hi there! Really happy to see this RFC and can provide an example from how we currently achieve the distribution of binaries at Prisma

We support something we called binaryTargets, see https://www.prisma.io/docs/reference/api-reference/prisma-schema-reference#binarytargets-options
The list looks like this

[
  'darwin',
  'darwin-arm64',
  'debian-openssl-1.0.x',
  'debian-openssl-1.1.x',
  'rhel-openssl-1.0.x',
  'rhel-openssl-1.1.x',
  'linux-arm64-openssl-1.1.x',
  'linux-arm64-openssl-1.0.x',
  'linux-arm-openssl-1.1.x',
  'linux-arm-openssl-1.0.x',
  'linux-musl',
  'linux-nixos',
  'windows',
  'freebsd11',
  'freebsd12',
  'openbsd',
  'netbsd',
  'arm',
]
  • We build our binaries for each of these "targets"
  • You can also build your own engine, for example
    • if you "forked" or want to build it yourself
    • if we don't build for your "target"

The download of the binaries is achieved on postinstall of the @prisma/engines package, which fetches the binaries from CloudFront/S3.

Here you can see there are quite a few requirements to replace this with Package Distributions

  • platform (covered by RFC)
  • arch (covered by RFC)
  • openssl version (for example, rhel-openssl-1.0.x is needed for AWS lambda)
  • Escape hatch to provide your own custom-built binaries (not sure how this is covered at the moment)

Happy to be part of a next meeting if you find this interesting 😃

@Jolg42
Copy link

Jolg42 commented Feb 8, 2022

I forgot to mention that we also need in some cases to install more than one "binaryTarget" locally.

Example:
For AWS Lambda a zip archive needs to prepared with what you want to deploy.
If you are on macOS and are deploying a Lambda like this you then require 2 "binaryTarget",

  • darwin if you want to execute the code locally on macOS
  • rhel-openssl-1.0.x so that it can be added in the zip archive.

@styfle
Copy link

styfle commented Feb 9, 2022

There are two features we need for Next.js that optionalDependencies doesn't currently provide:

  1. Support for libc detection: glibc vs musl. Today we need a custom install script to run detect-libc and install the appropriate binary. This is necessary for every Linux install.
  2. Support for WASM fallback. Today we need a custom install script to run and detect if the current platform fails to match known distributions with prebuilt binaries. This is necessary for CodeSandbox/StackBlitz/etc.

Hopefully distributions can solve these problems.

@Brooooooklyn
Copy link

Brooooooklyn commented Feb 10, 2022

Besides detecting engines and other conditions automatically, also we need a mechanism to install dependencies for the other platform for cross-compiling:

Refs:

yarn 1.x can use --ignore-platform flag to download ignore all conditions.
yarn 2+ can use supportedArchitectures to download specified platforms dependencies.
pnpm can use --force flag to download ignore all conditions.

yisibl added a commit to yisibl/resvg-js that referenced this pull request Mar 20, 2022
On Linux, it is not possible to tell exactly what kind of C library a native modules depends on just by os/cpu, so yarn 3.2 and cnpm added libc fields to further distinguish this case. This avoids downloading both `gnu` and `musl` packages at the same time.

Currently only [yarn 3.2+](yarnpkg/berry#3981) and [cnpm](cnpm/npminstall#387) are supported, the npm implementation is [still under discussion](npm/rfcs#519).
yisibl added a commit to yisibl/resvg-js that referenced this pull request Mar 20, 2022
On Linux, it is not possible to tell exactly what kind of C library a native modules depends on just by os/cpu, so yarn 3.2 and cnpm added libc fields to further distinguish this case. This avoids downloading both `gnu` and `musl` packages at the same time.

Currently only [yarn 3.2+](yarnpkg/berry#3981) and [cnpm](cnpm/npminstall#387) are supported, the npm implementation is [still under discussion](npm/rfcs#519).
},
{
"platform": "linux",
"arch": "x64",

Choose a reason for hiding this comment

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

Don't forget libc here

@saquibkhan
Copy link

saquibkhan commented Feb 8, 2023

  • can check with vercel and esbuild
  • can we achieve the same results without this feature
  • will this help to remove post install script
  • rethink script lifecycle

@Brooooooklyn
Copy link

Brooooooklyn commented Feb 22, 2023

Feedback from users of next.js

Feedback from users of NAPI-RS

Noway to install prebuild packages for another platform.

This is really important for Electron developers and Docker users.

For example, users need to build their Windows ia32 distribution Electron app on Windows x64 and they need to install prebuild packages for windows-ia32 platform on it.
Some users want to installation packages on Linux x64 gnu host so they can reuse the cache on it, then add the installed node_modules into node:lts-alpine (Linux x64 musl) containers to build their production image.
In cross platform testing CI, developers want to install packages on host platform because it’s fast and can be benefited from the cache on the host. Install the packages is extremely slow in the qemu enumerate. https://github.com/Brooooooklyn/snappy/blob/main/.github/workflows/CI.yaml#L428-L444

Yarn 3 is the only solution for this case at present: https://yarnpkg.com/configuration/yarnrc#supportedArchitectures.

There are some issues mentioned about it:

Install wasm fallback without postinstall script

Always including the wasm fallback package as dependency will increase the installation size. There is no way to install the wasm fallback package without postinstall script when all the platform specified optionalDependencies are not suitable for the host platform.

What we are doing for this case is to use postinstall script to detect if the native binaries are installed successfully, if not, we will install the wasm fallback package in the postinstall script.

Install the fallback package contains sources and provide a way to rebuild it

It could be solved by 0049-link-packages-to-source-and-build

napi-rs/napi-rs#1474

Like wasm package, we also can’t contain the sources for prebuild binary in the package by default due to the installation size concern.
But some developers who are working for Large international companies complain about dependencies on prebuild native binaries; because the compliance policy at their company wanted to audit the source code (and how can we prove that the downloaded binary was built from the code?).
So we need a way to publish the sources & infra codes for the prebuild binaries, but not install for users by default.

@ljharb
Copy link
Contributor

ljharb commented Feb 22, 2023

@saquibkhan the use case I find valuable here is if I could make the "default" distribution for my package omit tests, source files, docs, etc, but provide some kind of "full" distribution that includes all those things.

The beneficial install bandwith impact of applying this to my 300-400 packages alone would likely save npm millions of dollars a year, conservatively.

@saquibkhan saquibkhan requested a review from a team as a code owner February 28, 2023 00:23
saquibkhan and others added 3 commits February 27, 2023 19:23
Co-authored-by: Jordan Harband <ljharb@gmail.com>
Co-authored-by: Steven <steven@ceriously.com>
Co-authored-by: Steven <steven@ceriously.com>
@nlf
Copy link
Contributor

nlf commented Mar 16, 2023

i've made some updates to this document including several unanswered questions as well as detailing a known risk and adding in some other alternatives

Co-authored-by: Jordan Harband <ljharb@gmail.com>
@JoshuaKGoldberg
Copy link
Contributor

👋 @darcyclarke we can see that you closed this on Dec 1 but it's not clear why. npm/statusboard#647 says to refer to this PR for information. Is there somewhere that says why this RFC was closed? Thanks!

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.

None yet

9 participants