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

Enhancement: Major overhaul of installation #3750

Closed
lovell opened this issue Aug 4, 2023 · 41 comments
Closed

Enhancement: Major overhaul of installation #3750

lovell opened this issue Aug 4, 2023 · 41 comments

Comments

@lovell
Copy link
Owner

lovell commented Aug 4, 2023

TL;DR - see #3750 (comment)


In 2015 sharp first started providing prebuilt libvips binaries and in 2018 started providing prebuilt binaries of itself.

The primary aim of this meta-improvement is to attempt to use only package manager mechanics at install time, without custom scripts, and without downloading binaries from hosts other than those controlled by a package manager.

(What appears below is a bit of a brain-dump and will evolve over time.)

What will this change allow/support?

Non-exhaustive list:

  • Reproducible builds
  • Offline installation
  • Secure/sandbox environments where npm scripts cannot be run at install time
  • Environments (countries, organisations) where access to GitHub is limited
  • Vulnerability reporting via package manager for upstream dependencies

The approach taken by esbuild has now proven that this is possible with its use of optionalDependencies and platform-based cpu/os/libc filters.

A small but important piece of the puzzle fell into place with libc filtering via npm/npm-install-checks#54, which is part of npm v9.6.5 and therefore included with the latest Node.js 18 and 20. but sadly it's not quite available in the npm CLI yet - see npm/rfcs#438 (comment)

This should remove an entire class of install and runtime error, whilst no doubt introducing a new class of error, but a change is as good as a rest.

What needs to happen?

Register a suitable npm org for the namespace:

Publish per-platform prebuilt binaries of libvips and its dependencies specifically for use by sharp:

  • These will tale the form @img/sharp-libvips-platform-arch e.g. @img/sharp-libvips-linuxmusl-x64

Publish per-platform prebuilt binaries of sharp:

  • These will take the form @sharpen/sharp-platform-arch e.g. @img/sharp-linuxmusl-x64
  • Set rpath values for Linux and macOS that match the various package manager filesystem layouts.
    Example: if sharp.node is located at node_modules/@img/sharp-platform-arch/lib/sharp.node then its rpath needs to contain:
    • $$ORIGIN/../../sharp-libvips-platform-arch/lib
    • $$ORIGIN/../node_modules/@img/sharp-libvips-platform-arch/lib
    • Others?
  • Provide 32-bit ARM binaries via QEMU that support lowest common denominator of ARMv6 arch as package managers do not differentiate on arm_version.

Support multi-platform within same installation tree:

Support building from source:

  • This will be opt-in, a breaking change. The assumption is that most people will not do this, so make it the exception rather than the rule.
  • Compile sharp against prebuilt libvips:
    • Publish header files as common/generic @img/sharp-libvips-dev package and add to sharp's devDependencies
  • Compile sharp against global libvips:
    • A consumer will need to add node-addon-api (and for yarn v3 node-gyp) to their package.json file.

Windows will be a bit different, natch:

  • Need to ship sharp and libvips together as a single package to keep DLLs together = 8.9MB tarball
  • Still need libvips-only packages and DLL copying logic etc. to allow (re)build of sharp from source e.g. @img/sharp-libvips-win32-x64.
  • Need to do something different again for win32-arm64 (fewer than 30 downloads/month, so could drop then re-add later?)
  • Currently no support for a globally-installed libvips DLLs, which will continue to be the case.

Support Wasm fallback:

  • Will rely on the great work from WebAssembly build #3522
  • Can we declare an optional package that does not support a list of otherwise-natively-supported os/cpu/libc combos, e.g. not linux-x64 and not darwin-x64 and not ...?
  • What values do "sandbox" or "web container" platforms provide for process.platform and process.arch?
  • Can we detect when Node.js is run with --no-addons at runtime? Yes, catch ERR_DLOPEN_DISABLED error code.
  • Must display helpful error message for unsupported environments e.g. browsers and Wasm runtimes without threads/workers.

How is this going to be tested?

Need to be able to run sharp's unit tests against:

  • sharp built from source against prebuilt libvips binaries.
  • Prebuilt binaries of both sharp and libvips.
  • Using non-Node.js runtimes too e.g. Deno, Bun.

What are the possible problems?

Impossible to please everyone all the time:

  • Approach will fail when a package manager is configured to skip optional dependencies e.g. via npm's --no-optional flag.
  • Filesize increase as Brotli-ball option for libvips binaries no longer an option = ~10% increased download size:

Other breaking changes:

  • Drop support for Node.js 14 and 16, minimum will be (semver) ^18.17.0 || ^20.3.0 || >=21.0.0
  • Upgrade to latest Node-API version 9.
  • Upgrade to libvips 8.15.0.
  • Building from source becomes opt-in.
  • Specify a minimum version of the various package managers based on support for libc limitation.
  • yarn v3 PNP (Plug'n'Play) is unsupported, all the other nodeLinker settings should work. PNP support works.
  • Drop support for EOL yarn v1:
  • Remove sharp.vendor runtime API as this logic will now be "owned" by the package manager.
  • Someone, somewhere, is going to realise for the first time that libvips is LGPL (sharp uses it under LGPLv3); insert "always has been" meme.
lovell added a commit that referenced this issue Sep 26, 2023
- Remove all custom download logic for prebuilt binaries
- Add scripts to populate package contents
- Specify minimum versions of common package managers
- Remove sharp.vendor runtime API as no-longer relevant
- Update installation docs and issue templates
@lovell lovell added this to the v0.33.0 milestone Sep 26, 2023
@lovell lovell pinned this issue Sep 27, 2023
@falkenhawk
Copy link

  • Drop support for yarn v1 and yarn v2 - what problems might this cause?

Please don't drop support for yarn v1 just yet 🙏

@lovell
Copy link
Owner Author

lovell commented Sep 28, 2023

@falkenhawk It might work with older versions of yarn, but evanw/esbuild#2949 provides a good example of the sort of problems you're going to run into.

@lovell
Copy link
Owner Author

lovell commented Oct 6, 2023

🎉 There is now a v0.33.0-rc.2 release candidate of sharp available that has no custom install scripts and relies entirely on the npm registry.

I've tested it locally on Linux and via a new packaging test CI job - see https://github.com/lovell/sharp/actions/runs/6957160711

  • Node.js v20.9.0 + npm, pnpm and yarn (older versions require --ignore-engines flag)
  • Bun v1.0.13
  • Deno v1.38.2

...both with and without the --ignore-scripts flag where available. It also still detects a globally-installed libvips.

If you would like to test this locally then you can install this via:

npm install sharp@next

When installed on Linux, Package Phobia suggests this has a disk space requirement of 16.4MB with yarn v3 and 32.4MB with npm 10 (the latter is still ignoring the libc filter - see npm/cli#6914)

There is also an experimental WebAssembly version. You'll need to use an up-to-date version of npm 10 with support for the --cpu flag for this to install properly.

npm install --cpu=wasm32 sharp@next

👍 If it works, brilliant, add a thumbs up please (no need to comment).

😭 I expect there will be a few problems, so if it doesn't work and no-one else has already mentioned it, please add a minimal comment that includes the output of npx envinfo --binaries --system.

@Julusian
Copy link

Julusian commented Oct 6, 2023

I often have painful fights with electron when it comes to native libraries, so I am giving this a try in a project now.

First problem is that the latest electron release is based on nodejs 18.16.1 which is older than 0.33.0-alpha.4 supports. That doesnt appear to have been a problem in my testing.

The second problem is the binding.gyp confuses electron-builder. It ends up failing to build with:

  ⨯ cannot execute  cause=exit status 1
                    out=sharp: Detected --build-from-source flag
    sharp: Attempting to build from source via node-gyp
    sharp: See https://sharp.pixelplumbing.com/install#building-from-source
    Usage Error: Couldn't find a script name "node-gyp" in the top-level (used by sharp@npm:0.33.0-alpha.4). This typically happens because some package depends on "node-gyp" to build itself, but didn't list it in their dependencies. To fix that, please run "yarn add node-gyp" into your top-level workspace. You also can open an issue on the repository of the specified package to suggest them to use an optional peer dependency.
    
    $ yarn run [--inspect] [--inspect-brk] [-T,--top-level] [-B,--binaries-only] <scriptName> ...

Yarn3 does recommend explicitly declaring node-gyp as a dependency now, which could be the cause of this failure.
Should binding.gyp and associated logic should still be in sharp, or should that move to @sharpen/sharp-libvips-dev, as that is where the source files needed are?
For now, I have a workaround of deleting the binding.gyp file, so that electron-builder doesnt try to rebuild it. Every platform I build for has prebuilds.

I did have to make a change to my .yarnrc.yml so that every architecture I want to cross-build for is listed there explicitly. Which is tedious, but not that bad https://github.com/bitfocus/companion-satellite/blob/sharp0.33/.yarnrc.yml#L7-L12

I also had to change some paths in my electron-builder config to point to the new location of libvips, but that is expected and simple. Simpler than before actually, as the path no longer includes a version number which changes for each release.

With that, electron-builder is able to produce builds and it will bundle every installed version of the sharp native components. It would be nice for it to only include the relevant one, but thats an electron-builder issue.
And electron builder appears to be able to make cross-architecture builds too. building darwin-arm64 on a darwin-x64 machine ran on an m1 mac.
My application is crashing shortly after startup, but that happens after sharp has been used a few times, so I expect is not relevant here (and could well not be the fault of sharp).


I was concerned going into this, but I am pleasantly surprised with how it went.
This gives a much simpler build process than what I was having to do before, where I had a script which would force sharp to reinstall libvips during the build script.

I did also have to change the project to be yarn3 based, but I am fine with that. It was the kick needed to make it happen. I expect you will get pushback on this, but yarn1 has been EOL for over 2 years now, so I think it reasonable to 'force' users to change.

@lovell
Copy link
Owner Author

lovell commented Oct 7, 2023

@Julusian Thank you very much, this is really useful feedback. I've been considering moving the sharp C++ source code, gyp config etc. into a separate opt-in package and you've provided a good reason for doing so, namely that lots of the current tooling makes assumptions about the presence of a binding.gyp file. I've updated the issue details with this proposed change.

The use of supportedArchitectures with yarn looks like the best approach for your use case, and it appears Electron 27 will ship with at least version 18.18.0 of Node.js.

@lovell
Copy link
Owner Author

lovell commented Oct 9, 2023

@Julusian There's a new [email protected] prerelease if you'd like to test again. This version moves the binding.gyp file to a subdirectory so most tooling should (by default) no longer act as if sharp is a native module. To build from source add node-addon-api and node-gyp to your dependencies alongside use of the existing --build-from-source flag (see commit f7da2e5 for more details).

@SivaramPg
Copy link

Trying to use the alpha.6 with Next.js serverless function on Vercel leading to the following error on invocation. Build is successful though.

Command:- bun install v1.0.3 (25e69c71)

Env Info:-
"nextVersion": "13.5.4",
"nodeVersion": "v18.17.1",

Error: Cannot find module 'sharp'
Require stack:
- /var/task/.next/server/app/i/[assetType]/[id]/[width]/route.js
- /var/task/node_modules/next/dist/server/require.js
- /var/task/node_modules/next/dist/server/next-server.js
- /var/task/___next_launcher.cjs
    at Module._resolveFilename (node:internal/modules/cjs/loader:1077:15)
    at /var/task/node_modules/next/dist/server/require-hook.js:54:36
    at Module._load (node:internal/modules/cjs/loader:922:27)
    at e.<computed>._module.Module._load (/var/task/___vc/__launcher/__launcher.js:14:2079)
    at Module.require (node:internal/modules/cjs/loader:1143:19)
    at mod.require (/var/task/node_modules/next/dist/server/require-hook.js:64:28)
    at require (node:internal/modules/cjs/helpers:121:18)
    at 5168 (/var/task/.next/server/app/i/[assetType]/[id]/[width]/route.js:1:478)
    at __webpack_require__ (/var/task/.next/server/webpack-runtime.js:1:146)
    at __webpack_exec__ (/var/task/.next/server/app/i/[assetType]/[id]/[width]/route.js:1:2316) {
  code: 'MODULE_NOT_FOUND',
  requireStack: [
    '/var/task/.next/server/app/i/[assetType]/[id]/[width]/route.js',
    '/var/task/node_modules/next/dist/server/require.js',
    '/var/task/node_modules/next/dist/server/next-server.js',
    '/var/task/___next_launcher.cjs'
  ],
  page: '/i/avatars/17/200'
}
Error: Cannot find module 'sharp'
Require stack:
- /var/task/.next/server/app/i/[assetType]/[id]/[width]/route.js
- /var/task/node_modules/next/dist/server/require.js
- /var/task/node_modules/next/dist/server/next-server.js
- /var/task/___next_launcher.cjs
    at Module._resolveFilename (node:internal/modules/cjs/loader:1077:15)
    at /var/task/node_modules/next/dist/server/require-hook.js:54:36
    at Module._load (node:internal/modules/cjs/loader:922:27)
    at e.<computed>._module.Module._load (/var/task/___vc/__launcher/__launcher.js:14:2079)
    at Module.require (node:internal/modules/cjs/loader:1143:19)
    at mod.require (/var/task/node_modules/next/dist/server/require-hook.js:64:28)
    at require (node:internal/modules/cjs/helpers:121:18)
    at 5168 (/var/task/.next/server/app/i/[assetType]/[id]/[width]/route.js:1:478)
    at __webpack_require__ (/var/task/.next/server/webpack-runtime.js:1:146)
    at __webpack_exec__ (/var/task/.next/server/app/i/[assetType]/[id]/[width]/route.js:1:2316) {
  code: 'MODULE_NOT_FOUND',
  requireStack: [
    '/var/task/.next/server/app/i/[assetType]/[id]/[width]/route.js',
    '/var/task/node_modules/next/dist/server/require.js',
    '/var/task/node_modules/next/dist/server/next-server.js',
    '/var/task/___next_launcher.cjs'
  ],
  page: '/i/avatars/17/200'
}
Error: Runtime exited with error: exit status 1
Runtime.ExitError

kodiakhq bot pushed a commit to vercel/vercel that referenced this issue Feb 8, 2024
…d from `yarn` to `npm` (gated behind feature flag) (#11131)

This PR changes the default package manager from `yarn` to `npm` in the case that no lockfile is present.

Many years ago when `yarn` was first released, it was much faster than `npm` so we used it by default. That is no longer the case today and `yarn@1` is no longer receiving new features.

For example, `sharp` and `esbuild` no longer works with `yarn@1`:


### Related

- lovell/sharp#3750
- evanw/esbuild#2949

Note that this change will not impact most projects because most used a lockfile.
styfle added a commit to vercel/next.js that referenced this issue Apr 25, 2024
…ependency (#63321)

## History

Previously, we added support for `squoosh` because it was a wasm
implementation that "just worked" on all platforms when running `next
dev` for the first time. However, it was slow so we always recommended
manually installing `sharp` for production use cases running `next
build` and `next start`.

Now that [`sharp` supports
webassembly](https://sharp.pixelplumbing.com/install#webassembly), we no
longer need to maintain `squoosh`, so it can be removed. We also don't
need to make the user install sharp manually because it can be installed
under `optionalDependencies`. I left it optional in case there was some
platform that still needed to manually install the wasm variant with
`npm install --cpu=wasm32 sharp` such as codesandbox/stackblitz (I don't
believe sharp has any fallback built in yet).

Since we can guarantee `sharp`, we can also remove `get-orientation` dep
and upgrade `image-size` dep.

I also moved an [existing `sharp`
test](#56674) into its own fixture
since it was unrelated to image optimization.

## Related Issues
- Fixes #41417
- Related #54670
- Related #54708
- Related #44804
- Related #48820
- Related #61810
- Related #61696
- Related #44685
- Closes #64362

## Breaking Change

This is a breaking change because newer versions of `sharp` no longer
support `yarn@1`.

- lovell/sharp#3750

The workaround is to install with `yarn --ignore-engines` flag.

Also note that Vercel no longer defaults to yarn when no lockfile is
found

- vercel/vercel#11131
- vercel/vercel#11242

Closes NEXT-2823
ForsakenHarmony pushed a commit to vercel/next.js that referenced this issue Aug 16, 2024
…ependency (#63321)

## History

Previously, we added support for `squoosh` because it was a wasm
implementation that "just worked" on all platforms when running `next
dev` for the first time. However, it was slow so we always recommended
manually installing `sharp` for production use cases running `next
build` and `next start`.

Now that [`sharp` supports
webassembly](https://sharp.pixelplumbing.com/install#webassembly), we no
longer need to maintain `squoosh`, so it can be removed. We also don't
need to make the user install sharp manually because it can be installed
under `optionalDependencies`. I left it optional in case there was some
platform that still needed to manually install the wasm variant with
`npm install --cpu=wasm32 sharp` such as codesandbox/stackblitz (I don't
believe sharp has any fallback built in yet).

Since we can guarantee `sharp`, we can also remove `get-orientation` dep
and upgrade `image-size` dep.

I also moved an [existing `sharp`
test](#56674) into its own fixture
since it was unrelated to image optimization.

## Related Issues
- Fixes #41417
- Related #54670
- Related #54708
- Related #44804
- Related #48820
- Related #61810
- Related #61696
- Related #44685
- Closes #64362

## Breaking Change

This is a breaking change because newer versions of `sharp` no longer
support `yarn@1`.

- lovell/sharp#3750

The workaround is to install with `yarn --ignore-engines` flag.

Also note that Vercel no longer defaults to yarn when no lockfile is
found

- vercel/vercel#11131
- vercel/vercel#11242

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

No branches or pull requests