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

Improve build performance #2261

Merged
merged 1 commit into from
Dec 1, 2023
Merged

Improve build performance #2261

merged 1 commit into from
Dec 1, 2023

Conversation

kroese
Copy link
Contributor

@kroese kroese commented Dec 1, 2023

Description

This change improves the buildtime of the Docker container massively, by executing the build stage only once, instead of multiple times for each platform.

Currently when building the ARM64 image it is executing the build-stage under QEMU emulation, but after this change it re-uses the files build for AMD64 (since they are all javascript that does not matter).

@SleeplessOne1917 SleeplessOne1917 merged commit 07aba95 into LemmyNet:main Dec 1, 2023
@kroese kroese deleted the patch-1 branch December 1, 2023 03:11
@kroese
Copy link
Contributor Author

kroese commented Dec 1, 2023

@SleeplessOne1917

Sorry, but it does not work:

Node.js v20.10.0
/app/node_modules/sharp/lib/sharp.js:37
  throw new Error(help.join('\n'));
  ^
Error: 
Something went wrong installing the "sharp" module
Cannot find module '../build/Release/sharp-linuxmusl-arm64v8.node'
Require stack:
- /app/node_modules/sharp/lib/sharp.js
- /app/node_modules/sharp/lib/constructor.js
- /app/node_modules/sharp/lib/index.js
- /app/dist/js/server.js
Possible solutions:
- Install with verbose logging and look for errors: "npm install --ignore-scripts=false --foreground-scripts --verbose sharp"
- Install for the current linuxmusl-arm64v8 runtime: "npm install --platform=linuxmusl --arch=arm64v8 sharp"
- Consult the installation documentation: https://sharp.pixelplumbing.com/install
    at Object.<anonymous> (/app/node_modules/sharp/lib/sharp.js:37:9)
    at Module._compile (node:internal/modules/cjs/loader:1376:14)
    at Module._extensions..js (node:internal/modules/cjs/loader:1435:10)
    at Module.load (node:internal/modules/cjs/loader:1207:32)
    at Module._load (node:internal/modules/cjs/loader:1023:12)
    at Module.require (node:internal/modules/cjs/loader:1235:19)
    at require (node:internal/modules/helpers:176:18)
    at Object.<anonymous> (/app/node_modules/sharp/lib/constructor.js:11:1)
    at Module._compile (node:internal/modules/cjs/loader:1376:14)
    at Module._extensions..js (node:internal/modules/cjs/loader:1435:10)

So this commit has to be reverted unfortunately. I am still convinced it should be possible to cross-compile in the builder-stage, but it will require some more changes than just the one line in this commit.

@kroese
Copy link
Contributor Author

kroese commented Dec 1, 2023

According to https://sharp.pixelplumbing.com/install#cross-platform it can be fixed by installing both packages at the same time:

npm install --cpu=x64 --os=linux sharp
npm install --cpu=arm64 --os=linux sharp

There might be other packages that Lemmy depends on that contain binaries (I just assumed node packages only contained Javascript). So the final fix would look something like this:

npm install --cpu=$TARGET_PLATFORM

But until we figure this out this pullrequest needs to be reversed. I thought I tested it, but I got tricked because Cloudflare was serving a cached response so it looked like the image was functioning correctly. I should have checked the container log.

SleeplessOne1917 added a commit that referenced this pull request Dec 1, 2023
SleeplessOne1917 added a commit that referenced this pull request Dec 1, 2023
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.

2 participants