Enhancement: Major overhaul of installation

lovell commented 11 months ago

TL;DR - see https://github.com/lovell/sharp/issues/3750#issuecomment-1751172728

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 https://github.com/npm/npm-install-checks/pull/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 https://github.com/npm/rfcs/issues/438#issuecomment-1753711862

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:

@img - see https://www.npmjs.com/org/img
Many thanks to the team at npm for transferring ownership.

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:

Consuming app/package can use dependencies to include a list of specific @img/sharp-platform-arch packages.
Use yarn v3.1+ supportedArchitectures - see https://yarnpkg.com/configuration/yarnrc#supportedArchitectures
npm has a (closed but not fully-implemented) RFC relating to this at https://github.com/npm/rfcs/issues/612 -

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 https://github.com/lovell/sharp/pull/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:
- ~~Can we use an npm postpack script to run advdef --recompress --shrink-insane over tarballs before uploading?~~ No we cannot, as postpack is broken in slightly different ways in different versions of npm e.g. https://github.com/npm/cli/issues/4533

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:
- It ignores os/cpu filters so people will complain about huge download/install e.g. https://github.com/evanw/esbuild/issues/2949
- It generates errors for unknown engines so will require --ignore-engines flag.
- If you're using yarn v1 there's never been a better time to upgrade.
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.

falkenhawk commented 9 months ago

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

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

lovell commented 9 months ago

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

lovell commented 9 months ago

:tada: 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 https://github.com/npm/cli/pull/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

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

:sob: 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 commented 9 months ago

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 commented 9 months ago

@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 commented 9 months ago

@Julusian There's a new sharp@0.33.0-alpha.6 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 https://github.com/lovell/sharp/commit/f7da2e5970d5c333768d8bbe979066d7d504e8c9 for more details).

SivaramPg commented 9 months ago

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

lovell commented 9 months ago

@SivaramPg If sharp v0.32.6 fails in a similar manner, please open a new sharp issue to discuss. If sharp v0.32.6 works but v0.33.0-alpha.6 does not work, please contact Vercel support to get them to help you debug this first.

SivaramPg commented 9 months ago

@SivaramPg If sharp v0.32.6 fails in a similar manner, please open a new sharp issue to discuss. If sharp v0.32.6 works but v0.33.0-alpha.6 does not work, please contact Vercel support to get them to help you debug this first.

@lovell Been using v0.32.6 without any issues. Just wanted to test this out :)

lovell commented 8 months ago

@SivaramPg It looks like Vercel introduced a change to Next.js 13.5.4 that is removing sharp from deployments - see https://github.com/vercel/next.js/pull/56048 and https://github.com/vercel/next.js/issues/56357 - this is unrelated to either sharp or these installation improvements.

nCrafts commented 8 months ago

When I try this on Mac I get this error with v0.33.0-alpha.6

error: Uncaught Error: Could not load the "sharp" module at runtime
Possible solutions:
- Add explicit dependencies for the runtime platform:
    npm install --force @sharpen/sharp-darwin-arm64
    npm install --force @sharpen/sharp-libvips-darwin-arm64
- Consult the installation documentation: https://sharp.pixelplumbing.com/install
    at Object.<anonymous> (file:///Users/nishant/Library/Caches/deno/npm/registry.npmjs.org/sharp/0.33.0-alpha.6/lib/sharp.js:64:11)
    at Object.<anonymous> (file:///Users/nishant/Library/Caches/deno/npm/registry.npmjs.org/sharp/0.33.0-alpha.6/lib/sharp.js:68:4)
    at Module._compile (node:module:733:34)
    at Object.Module._extensions..js (node:module:747:10)
    at Module.load (node:module:658:32)
    at Function.Module._load (node:module:539:12)
    at Module.require (node:module:677:19)
    at require (node:module:791:16)
    at Object.<anonymous> (file:///Users/nishant/Library/Caches/deno/npm/registry.npmjs.org/sharp/0.33.0-alpha.6/lib/constructor.js:10:1)
    at Object.<anonymous> (file:///Users/nishant/Library/Caches/deno/npm/registry.npmjs.org/sharp/0.33.0-alpha.6/lib/constructor.js:440:4)

I then went into the cache folder and ran an npm install but the same error remains. Oddly when I try to use v0.32.6 it works (after using npm install). Thanks for working on this.

More info:

deno 1.37.1 (release, aarch64-apple-darwin)
v8 11.8.172.6
typescript 5.2.2

oscarotero commented 8 months ago

@nCrafts @lovell I got the same error trying to install the new alpha.7 version on Deno.

I had a similar issue with a different package which uses the same strategy to distribute binary files. It's already fixed but not released yet, I guess it will be available in the next version of Deno.

@dsherret Do you think your changes can also fix this problem?

I'm using this code to test it: https://github.com/lumeland/experimental-plugins/tree/main/sharp

lovell commented 8 months ago

Thanks for testing. It looks the current Deno 1.37.1 only supports optionalDependencies that are tagged latest so you won't be able to test prereleases with Deno until https://github.com/denoland/deno/pull/20772 is included in a release.

Julusian commented 8 months ago

@lovell I have verified that sharp@0.33.0-alpha.8 is working with electron-builder without my hack now.

I have just noticed something odd about sharp@0.33.0-alpha.4. yarn installed @sharpen/sharp-libvips-*-* 0.0.1-alpha.2 and 0.0.1-alpha.1.
It looks like @sharpen/sharp-darwin-arm64 0.0.1-alpha.4 depends on @sharpen/sharp-libvips-darwin-arm64 0.0.1-alpha.1 and sharp 0.33.0-alpha.4 depends on @sharpen/sharp-libvips-darwin-arm64 0.0.1-alpha.2. I am seeing the same pattern for each of the platform packages.
Perhaps some version number update didn't work properly? Something to keep an eye on for future releases.

On a related note, should sharp be depending on these libvips packages directly, or should that solely be done via the platform packages?

lovell commented 8 months ago

@Julusian Thanks again for testing, I probably messed up the dependency tree in alpha.4 as it's all a bit manual at the moment (this is something I'd like to automate once the process is smoother).

The filesystem layout for co-dependent shared libraries has to be known at compile-time to set the correct rpath values, so to appease the strategies of both pnpm and yarn, the @sharpen/sharp-libvips-darwin-... packages have to be optionalDependencies of both the @sharpen/sharp-darwin-... and sharp itself.

The sharp-libvips packages are quite big and I'd like to publish these less often, keeping everyone's bandwidth usage as low as possible. Windows is (always) the exception as the DLLs have to live in the same directory, so @sharpen/sharp-win32-... subsumes @sharpen/sharp-libvips-win32-....

oscarotero commented 8 months ago

@lovell Just want to let you know that I tested the alpha.9 version in Deno on darwin-x64 and I got the following error:

Uncaught Error: Could not load the "sharp" module at runtime
undefined: dlopen(/Users/oscarotero/Library/Caches/deno/npm/registry.npmjs.org/@img/sharp-darwin-x64/0.33.0-alpha.9/lib/sharp-darwin-x64.node, 0x0001): Library not loaded: '@rpath/libvips-cpp.42.dylib'
  Referenced from: '/Users/oscarotero/Library/Caches/deno/npm/registry.npmjs.org/@img/sharp-darwin-x64/0.33.0-alpha.9/lib/sharp-darwin-x64.node'
  Reason: tried:
    '/Users/oscarotero/Library/Caches/deno/npm/registry.npmjs.org/@img/sharp-darwin-x64/0.33.0-alpha.9/lib/../../sharp-libvips-darwin-x64/lib/libvips-cpp.42.dylib' (no such file),
    '/Users/oscarotero/Library/Caches/deno/npm/registry.npmjs.org/@img/sharp-darwin-x64/0.33.0-alpha.9/lib/../../node_modules/@img/sharp-libvips-darwin-x64/lib/libvips-cpp.42.dylib' (no such file),
    '/Users/oscarotero/Library/Caches/deno/npm/registry.npmjs.org/@img/sharp-darwin-x64/0.33.0-alpha.9/lib/../../../node_modules/@img/sharp-libvips-darwin-x64/lib/libvips-cpp.42.dylib' (no such file),
    '/Users/oscarotero/Library/Caches/deno/npm/registry.npmjs.org/@img/sharp-darwin-x64/0.33.0-alpha.9/lib/../../sharp-libvips-darwin-x64/lib/libvips-cpp.42.dylib' (no such file),
    '/Users/oscarotero/Library/Caches/deno/npm/registry.npmjs.org/@img/sharp-darwin-x64/0.33.0-alpha.9/lib/../../node_modules/@img/sharp-libvips-darwin-x64/lib/libvips-cpp.42.dylib' (no such file),
    '/Users/oscarotero/Library/Caches/deno/npm/registry.npmjs.org/@img/sharp-darwin-x64/0.33.0-alpha.9/lib/../../../node_modules/@img/sharp-libvips-darwin-x64/lib/libvips-cpp.42.dylib' (no such file),
    '/usr/local/lib/libvips-cpp.42.dylib' (no such file), '/usr/lib/libvips-cpp.42.dylib' (no such file)
Possible solutions:
- Add an explicit dependency for the runtime platform:
    npm install --force @img/sharp-darwin-x64
- Consult the installation documentation: https://sharp.pixelplumbing.com/install
    at Object.<anonymous> (file:///Users/oscarotero/Library/Caches/deno/npm/registry.npmjs.org/sharp/0.33.0-alpha.9/lib/sharp.js:67:11)
    at Object.<anonymous> (file:///Users/oscarotero/Library/Caches/deno/npm/registry.npmjs.org/sharp/0.33.0-alpha.9/lib/sharp.js:71:4)
    at Module._compile (node:module:733:34)
    at Object.Module._extensions..js (node:module:747:10)
    at Module.load (node:module:658:32)
    at Function.Module._load (node:module:539:12)
    at Module.require (node:module:677:19)
    at require (node:module:791:16)
    at Object.<anonymous> (file:///Users/oscarotero/Library/Caches/deno/npm/registry.npmjs.org/sharp/0.33.0-alpha.9/lib/constructor.js:10:1)
    at Object.<anonymous> (file:///Users/oscarotero/Library/Caches/deno/npm/registry.npmjs.org/sharp/0.33.0-alpha.9/lib/constructor.js:440:4)

Deno stores the NPM packages differently than node_modules folder: Due the packages are stored globaly, every package has one or more subfolders with the versions downloaded. Maybe this affects to how sharp tries to locate the dependencies?

lovell commented 8 months ago

@oscarotero Thanks for testing, are you able to check with the deno run --node-modules-dir flag?

oscarotero commented 8 months ago

@lovell Adding --node-modules-dir flag it works perfectly.

lovell commented 8 months ago

@oscarotero Great, thanks for confirming. I've added support for the Deno package cache filesystem layout via commit https://github.com/lovell/sharp/commit/8d1747aa6a0c1d13204b1d4e9c4948c464686149 so hopefully the --node-modules-dir won't be necessary from the next (pre-)release.

d3lm commented 8 months ago

Hey @lovell 👋 Awesome to see progress on this front.

I am the lead for WebContainers and wanted to chime in on the following:

What values do "sandbox" or "web container" platforms provide for process.platform and process.arch?

WebContainers specifically aims to comply with Linux behavior which means that process.platform === 'linux' and process.arcg === 'x64'. We have infra that allows us to configure "polyfills" which will be installed in place of other dependencies if you run npm i, for example npm i sharp. In this case, if for the requested version a polyfill is configured, it will instead install the polyfill. We have published a sharp polyfill (from #3522 and curtesy of @RReverser) to @webassembly/sharp.

Maybe what we could do for platforms that don't support native addons, is to detect that (dlopen will fail), and dynamically download the Wasm module. This is similar to what Next.js does for SWC.

lovell commented 8 months ago

@d3lm Thanks for the extra info. There will be a runtime check for the ERR_DLOPEN_DISABLED error code to which I will add a help message that says something like "try adding the Wasm version to your dependencies".

Due to licensing differences (LGPLv3 vs Apache 2.0) the Wasm version will be opt-it only. All of the install-time behaviour will be controlled by a user's choice of package manager and its relevant configuration; there will be no more downloading anything within sharp itself.

Hopefully this will be suitable for WebContainers/StackBlitz, but let me know if not and we can try to work out something.

RReverser commented 8 months ago

Due to licensing differences (LGPLv3 vs Apache 2.0) the Wasm version will be opt-it only.

Btw, I was thinking about that one a bit more after our conversation in another issue. Doesn't LGPLv3 linking exception apply here? I think it was added specifically to allow scenarios similar to one we have.

As in, I believe (IANAL) you can statically link LGPLv3 into Apache 2.0 project as long as you provide source code (which you do) so users can substitute the LGPLv3 dependency with another implementation and build their own binaries.

RReverser commented 8 months ago

Combined Works. You may convey a Combined Work under terms of your choice that, taken together, effectively do not restrict modification of the portions of the Library contained in the Combined Work and reverse engineering for debugging such modifications, if you also do each of the following:

a) Give prominent notice with each copy of the Combined Work that the Library is used in it and that the Library and its use are covered by this License. b) Accompany the Combined Work with a copy of the GNU GPL and this license document. c) For a Combined Work that displays copyright notices during execution, include the copyright notice for the Library among these notices, as well as a reference directing the user to the copies of the GNU GPL and this license document. d) Do one of the following: 0) Convey the Minimal Corresponding Source under the terms of this License, and the Corresponding Application Code in a form suitable for, and under terms that permit, the user to recombine or relink the Application with a modified version of the Linked Version to produce a modified Combined Work, in the manner specified by section 6 of the GNU GPL for conveying Corresponding Source. 1) Use a suitable shared library mechanism for linking with the Library. A suitable mechanism is one that (a) uses at run time a copy of the Library already present on the user's computer system, and (b) will operate properly with a modified version of the Library that is interface-compatible with the Linked Version. e) Provide Installation Information, but only if you would otherwise be required to provide such information under section 6 of the GNU GPL, and only to the extent that such information is necessary to install and execute a modified version of the Combined Work produced by recombining or relinking the Application with a modified version of the Linked Version. (If you use option 4d0, the Installation Information must accompany the Minimal Corresponding Source and Corresponding Application Code. If you use option 4d1, you must provide the Installation Information in the manner specified by section 6 of the GNU GPL for conveying Corresponding Source.)

Basically we don't want to do d) 1) (shared linking) but we can do d) 0) alternative - static linking but provide the source and building instructions - to allow e) recombining the application with a modified version of the linked version. Which then allows the overarching "You may convey a Combined Work under terms of your choice" aka your own license.

d3lm commented 8 months ago

Hopefully this will be suitable for WebContainers/StackBlitz, but let me know if not and we can try to work out something.

For WebContainer it's not the most ideal experience because people can't just use their projects on both local and with WebContainer unless they install another dependency. However, if the Wasm version is just a drop-in replacement which would be very important for us, then we can just configure it as a "polyfill" and it would be installed in place of sharp. But it'd be really nice if we didn't have to rely on the polyfill infra for this if possible. If that's not possible then, as I mentioned, it'd be very important that it's a drop in replacement so it has the same set of dependencies, package.json, etc. That's the case for the polyfill that we have currently published to npm.

zoellner commented 8 months ago

Is there an option in addition to "use globally installed libvips" that is more like "use exactly this libvips, I'm pointing you to the folder"? Trying to work through some build issues and this would help

lovell commented 8 months ago

@zoellner You can provide a custom value for the PKG_CONFIG_PATH environment variable to tell pkg-config where to find everything at install time (this already works with the current sharp and will continue to work after the changes under discussion here).

lovell commented 8 months ago

@d3lm I've opened https://github.com/stackblitz/webcontainer-core/issues/1236 with some thoughts - feel free to shoot them down :)

lovell commented 8 months ago

New sharp v0.33.0-alpha.10 pre-release now available with libvips v8.15.0-rc2. It should "just work" using Node.js, Deno and Bun on pretty much anything from a $5 Pi Zero to a $500,000 IBM mainframe. Please see https://github.com/lovell/sharp/issues/3750#issuecomment-1751172728 for more details.

d3lm commented 8 months ago

Ok, thanks @lovell 🙏 I think changing the arch to wasm32 may have much less implications than changing the platform. We'll investigate this and see if we can change that on our end without risking breakage.

oscarotero commented 8 months ago

Hi @lovell! I'm afraid the latest version v0.33.0-alpha.10 still requires node_modules folder enabled in Deno to work. Without this option I continue having errors:

Error: Could not load the "sharp" module at runtime
undefined: dlopen(/Users/oscarotero/Library/Caches/deno/npm/registry.npmjs.org/@img/sharp-darwin-x64/0.33.0-alpha.10/lib/sharp-darwin-x64.node, 0x0001): Library not loaded: '@rpath/libvips-cpp.42.dylib'
  Referenced from: '/Users/oscarotero/Library/Caches/deno/npm/registry.npmjs.org/@img/sharp-darwin-x64/0.33.0-alpha.10/lib/sharp-darwin-x64.node'
  Reason: tried: '/Users/oscarotero/Library/Caches/deno/npm/registry.npmjs.org/@img/sharp-darwin-x64/0.33.0-alpha.10/lib/../../sharp-libvips-darwin-x64/lib/libvips-cpp.42.dylib' (no such file), '/Users/oscarotero/Library/Caches/deno/npm/registry.npmjs.org/@img/sharp-darwin-x64/0.33.0-alpha.10/lib/../../../0.0.3/sharp-libvips-darwin-x64/lib/libvips-cpp.42.dylib' (no such file), '/Users/oscarotero/Library/Caches/deno/npm/registry.npmjs.org/@img/sharp-darwin-x64/0.33.0-alpha.10/lib/../../node_modules/@img/sharp-libvips-darwin-x64/lib/libvips-cpp.42.dylib' (no such file), '/Users/oscarotero/Library/Caches/deno/npm/registry.npmjs.org/@img/sharp-darwin-x64/0.33.0-alpha.10/lib/../../../node_modules/@img/sharp-libvips-darwin-x64/lib/libvips-cpp.42.dylib' (no such file), '/Users/oscarotero/Library/Caches/deno/npm/registry.npmjs.org/@img/sharp-darwin-x64/0.33.0-alpha.10/lib/../../sharp-libvips-darwin-x64/lib/libvips-cpp.42.dylib' (no such file), '/Users/oscarotero/Library/Caches/deno/npm/registry.npmjs.org/@img/sharp-darwin-x64/0.33.0-alpha.10/lib/../../../0.0.3/sharp-libvips-darwin-x64/lib/libvips-cpp.42.dylib' (no such file), '/Users/oscarotero/Library/Caches/deno/npm/registry.npmjs.org/@img/sharp-darwin-x64/0.33.0-alpha.10/lib/../../node_modules/@img/sharp-libvips-darwin-x64/lib/libvips-cpp.42.dylib' (no such file), '/Users/oscarotero/Library/Caches/deno/npm/registry.npmjs.org/@img/sharp-darwin-x64/0.33.0-alpha.10/lib/../../../node_modules/@img/sharp-libvips-darwin-x64/lib/libvips-cpp.42.dylib' (no such file), '/usr/local/lib/libvips-cpp.42.dylib' (no such file), '/usr/lib/libvips-cpp.42.dylib' (no such file)

lovell commented 8 months ago

@oscarotero Thank you for testing, I thought I'd checked this but you are quite right. Here's a subset of what my local Deno cache contains, which also doesn't match any of the rpath values:

.../deno/npm/registry.npmjs.org/@img/sharp-linux-x64/0.33.0-alpha.10/lib/sharp-linux-x64.node
.../deno/npm/registry.npmjs.org/@img/sharp-libvips-linux-x64/0.0.3/lib/libvips-cpp.so.42

So it looks like I got the directory ordering wrong (package name vs package version), now fixed via commit https://github.com/lovell/sharp/commit/8c5a493f68cf9c7c8bee0c464a7d88ce3a37c47f

oscarotero commented 8 months ago

No problem, thanks for the quick fix! I'll check the new alpha version when released.

lovell commented 7 months ago

A new sharp v0.33.0-alpha.11 pre-release is now available, which has been tested as working out of the box with Deno and has (drum roll please) a WebAssembly version - please see https://github.com/lovell/sharp/issues/3750#issuecomment-1751172728

oscarotero commented 7 months ago

@lovell It works like a charm with Deno. Thanks!

laktek commented 7 months ago

@lovell Is there a way to load the WASM version in Deno?

lovell commented 7 months ago

@laktek Deno provides Node-ABI compatibility and supports native code so you may not need it. However, I suspect your question might relate to the Deno permissions model e.g. when --deny-ffi is used. If so, try explicitly adding @img/sharp-wasm32 to your dependencies. Please remember that support for the Web Workers API is required in the Wasm runtime.

lovell commented 7 months ago

A release candidate is now available - please see https://github.com/lovell/sharp/issues/3750#issuecomment-1751172728 for details.

alex-kinokon commented 7 months ago

The rc version hard-coded ENVIRONMENT_IS_NODE to be true. Is that expected?

lovell commented 7 months ago

@alex-kinokon Does your question refer to the following configuration?

https://github.com/lovell/sharp/blob/61057f25bc56437d65cd4a1cafa934a9e689016b/src/binding.gyp#L191

If so, please see https://github.com/lovell/sharp/pull/3522 for additional context around the environments in which the experimental WebAssembly version is expected to work.

lovell commented 7 months ago

v0.33.0 is now available, thank you very much everyone for all your help with testing.

d3lm commented 7 months ago

Wow, thanks a ton @lovell 🎉 This is a huge milestone.

lovell / sharp