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

Update run.sh to exclude sys/random.h for this target #127

Merged
merged 2 commits into from
Apr 4, 2024

Conversation

scosol
Copy link
Contributor

@scosol scosol commented Mar 28, 2024

nodejs sources were updated with some glibc 2.25+ functionality around expecting/detecting sys/random.h and getrandom()
This target specifically is meant for lesser glibc versions, thus we specifically disable the new expectations

@scosol scosol marked this pull request as ready for review March 29, 2024 01:59
@scosol scosol marked this pull request as draft March 29, 2024 02:49
@scosol scosol marked this pull request as ready for review March 31, 2024 10:48
@rvagg
Copy link
Member

rvagg commented Apr 2, 2024

@scosol this seems OK to me, but have you managed to try building this locally to confirm it works?

Copy link
Member

@addaleax addaleax left a comment

Choose a reason for hiding this comment

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

Fwiw, this is what we do for our own usage of mongosh: https://github.com/mongodb-js/mongosh/blob/5b5f6073a1406f329ebe84518c5a9bb0f159eae7/scripts/nodejs-patches/003-no-sys-random-on-older-glibc-node-52223.patch

On older glibc versions that's effectively equivalent to this patch, so I'd say it works :)

recipes/x64-glibc-217/run.sh Show resolved Hide resolved
recipes/x64-glibc-217/run.sh Show resolved Hide resolved
@scosol
Copy link
Contributor Author

scosol commented Apr 3, 2024

@scosol this seems OK to me, but have you managed to try building this locally to confirm it works?

I've just built successfully for:
./local_build.sh -r x64-glibc-217 -v v20.12.1
./local_build.sh -r x64-glibc-217 -v v18.20.1

❯ ls -al ~/src/github/staging/release/v18.20.1
total 134592
drwxr-xr-x 4 scosol staff 128 Apr 3 10:59 .
drwxr-xr-x 4 scosol staff 128 Apr 3 11:31 ..
-rw-------@ 1 scosol staff 44713613 Apr 3 10:59 node-v18.20.1-linux-x64-glibc-217.tar.gz
-rw-------@ 1 scosol staff 24193232 Apr 3 10:59 node-v18.20.1-linux-x64-glibc-217.tar.xz
❯ ls -al ~/src/github/staging/release/v20.12.1
total 142088
drwxr-xr-x 4 scosol staff 128 Apr 3 12:49 .
drwxr-xr-x 4 scosol staff 128 Apr 3 11:31 ..
-rw-------@ 1 scosol staff 46947672 Apr 3 12:48 node-v20.12.1-linux-x64-glibc-217.tar.gz
-rw-------@ 1 scosol staff 25796820 Apr 3 12:49 node-v20.12.1-linux-x64-glibc-217.tar.xz
~/src/github/nodejs-unofficial-builds/bin  main 

@rvagg rvagg merged commit 6853f54 into nodejs:main Apr 4, 2024
@rvagg
Copy link
Member

rvagg commented Apr 4, 2024

don't suppose you'd like to tackle some of the others while your head is in here?

@scosol
Copy link
Contributor Author

scosol commented Apr 4, 2024

don't suppose you'd like to tackle some of the others while your head is in here?

Sure- but I don't see any formal issues directly calling any other failures out? Shall I assume that anything not showing up here: https://unofficial-builds.nodejs.org/download/release/v20.12.1/ is currently "broken"?

@scosol
Copy link
Contributor Author

scosol commented Apr 5, 2024

don't suppose you'd like to tackle some of the others while your head is in here?

#129

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.

3 participants