-
Notifications
You must be signed in to change notification settings - Fork 5.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
fix(build) Refactor build.rs to better handle old glibc #16238
Conversation
c27e63f
to
2662e32
Compare
@LukeChannings What version of glibc does it support with this? It would be great if this allowed us to run on glibc 2.23 (Ubuntu 16) |
Seems to work fine against the ubuntu:16.04 docker image, tested against aarch64 only, see output below.
|
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.
@LukeChannings Thank you for looking into this. The symbol generation script was there to avoid doing this at build-time since its very rarely updated. Can we generalize generate_link_win.js
for Linux and macOS?
The glibc check LGTM
@littledivy I can look into it. Are we ok with having it output three separate files? I'd probably end up with |
21a3cd9
to
248b50e
Compare
@littledivy Can I get another review? |
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.
LGTM!
… image, lowering the glibc requirement to version 2.23
This reverts commit fa22956.
Follow-up to #16208. - Refactors build.rs behaviour to use `-exported_symbols_list` / `--export-dynamic-symbol-list` - Since all build systems now rely on a symbols list file, I have added `generate_exported_symbols_list`, which derives the symbol list file depending on the platform, which makes `tools/napi/generate_link_win.js` redundant. - Fixes a missed instance of `i8` being used instead of `c_char` Co-authored-by: Divy Srivastava <[email protected]>
Follow-up to #16208.
-exported_symbols_list
/--export-dynamic-symbol-list
generate_exported_symbols_list
, which derives the symbol list file depending on the platform, which makestools/napi/generate_link_win.js
redundant.i8
being used instead ofc_char