-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
[prebuild] feat(prebuild): support of Alpine binaries #2370
Open
Delagen
wants to merge
7
commits into
Automattic:prebuilds
Choose a base branch
from
Delagen:rework-prebuilds
base: prebuilds
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
7 commits
Select commit
Hold shift + click to select a range
04276a5
maybe fix Linux build
zbjornson b554283
try to build v3.0.0-rc1b
zbjornson 02c2c7c
another attempt at linux fix
zbjornson 8439b94
another attempt at macos fix
zbjornson 8d37634
maybe fix linux
zbjornson 89811a9
maybe fix macos
zbjornson f22fbfe
feat(prebuild): support of Alpine binaries
Delagen File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
Large diffs are not rendered by default.
Oops, something went wrong.
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,3 @@ | ||
#!/usr/bin/env sh | ||
|
||
apk --no-cache add build-base cairo-dev jpeg-dev pango-dev giflib-dev librsvg-dev pixman-dev patchelf |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,3 @@ | ||
#!/usr/bin/env sh | ||
|
||
apk --purge del build-base cairo* jpeg* pango* giflib* librsvg* pixman* |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,9 @@ | ||
#!/usr/bin/env sh | ||
|
||
TARGET=./source/build/Release | ||
|
||
for lib in $(ldd "${TARGET}/canvas.node" | grep '=>' | cut -d " " -f 3); do | ||
echo "Copy ${lib}" | ||
cp -L "${lib}" "${TARGET}" | ||
patchelf --force-rpath --set-rpath '$ORIGIN' "${TARGET}/$(basename -- "${lib}")" | ||
done |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,4 @@ | ||
#!/usr/bin/env sh | ||
|
||
sudo apt-get update | ||
sudo apt-get install -y build-essential libcairo2-dev libjpeg-dev libpango1.0-dev libgif-dev librsvg2-dev libpixman-1-dev patchelf | ||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,3 @@ | ||
# FIXME: error if removing with build-essential libcairo2* libjpeg* libpango1.0* libgif* librsvg2* libpixman-1* | ||
sudo apt-get purge -y build-essential libcairo2-dev libjpeg-dev libpango1.0-dev libgif-dev librsvg2-dev libpixman-1-dev | ||
sudo apt-get autoremove --purge -y |
This file was deleted.
Oops, something went wrong.
This file was deleted.
Oops, something went wrong.
This file was deleted.
Oops, something went wrong.
This file was deleted.
Oops, something went wrong.
This file was deleted.
Oops, something went wrong.
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,69 @@ | ||
#!/usr/bin/env sh | ||
|
||
LIBS=() | ||
|
||
WINDIR=$(cygpath -u -W) | ||
|
||
TARGET=./source/build/Release | ||
|
||
# Can be totally replaced with this, but MSYS throw format error if DLL has not .dll extension | ||
#for lib in $(ldd "${TARGET}/canvas.node" | grep '=>' | cut -d " " -f 3); do | ||
# if [[ "${lib}" == "${WINDIR}"* ]]; then | ||
# : | ||
# else | ||
# echo "copy ${lib} to destination" | ||
# cp "${lib}" "${TARGET}" | ||
# fi | ||
#done | ||
|
||
shopt -s nocasematch | ||
function addToLibs() { | ||
local lib=${1} && shift | ||
if [[ "$(which "${lib}")" == "${WINDIR}"* ]]; then | ||
: | ||
else | ||
for libItem in ${LIBS[@]}; do | ||
if [[ "${libItem}" == "${lib}" ]]; then | ||
return | ||
fi | ||
done | ||
echo $lib | ||
fi | ||
} | ||
|
||
RECURSE_INDEX=0 | ||
|
||
function getLibsForBinary() { | ||
local binary=$1 && shift | ||
|
||
local -i count=0 | ||
for binLib in $(objdump -p "$(cygpath -u "${binary}")" | grep "DLL Name:" | sed -e 's/^\s*DLL\sName:\s*//'); do | ||
if [[ ! -z "$(addToLibs ${binLib})" ]]; then | ||
echo "added ${binLib}" | ||
count=$count+1 | ||
LIBS+=("${binLib}") | ||
fi | ||
done | ||
|
||
if [[ count -gt 0 ]]; then | ||
local currentIndex=${RECURSE_INDEX} | ||
RECURSE_INDEX=${#LIBS[@]} | ||
# recurse if any added from last checked | ||
echo "recurse check after ${count} added" | ||
for lib in ${LIBS[@]:${currentIndex}}; do | ||
getLibsForBinary "$(which "${lib}")" | ||
done | ||
fi | ||
} | ||
|
||
function copyLibs() { | ||
local DEST=$1 && shift | ||
local DEST_PATH=$(cygpath -u "${DEST}") | ||
for lib in ${LIBS[@]}; do | ||
echo "copy ${lib} to destination" | ||
cp "$(which "${lib}")" "${DEST_PATH}" | ||
done | ||
} | ||
|
||
getLibsForBinary "${TARGET}/canvas.node" | ||
copyLibs "${TARGET}" |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@chearon did you build all of the Linux deps from source for a reason, or can we use precompiled packages as this PR does?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, we need to build on an old OS for maximum compatibility. Programs built against a newer libc may have symbols that don't exist on older OSes (this happened to real users). But we want newer libraries, so have to build from source.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 forgot about this part.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You describe preference to run code on old OS, but you require at least NodeJS 18.
Node 18 via NVM ubuntu 14.04
Node 18 via NVM Ubuntu 18.04
Node-source distribution doesn't target any Ubuntu lower than 20.04
https://github.com/nodesource/distributions?tab=readme-ov-file#ubuntu-versions
Current build with not precompiled libraries works perfectly on Ubuntu 20.04 with Node 18 from NVM.
Npm test result
That pretty the same issues as Alpine build.
Any other suggestions to build from source?
I thinks that build from source was designed as a way to bundle libraries with compiled node binding, because base libraries without patching tries to resolve dependencies via precompiled path. Patching libraries with
patchelf
resolves this issue. So I don't think that manually compile libraries is overkill.