-
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(npm) support compiling on linux/aarch64 #16207 #16208
Conversation
8e46906
to
7186df0
Compare
@@ -375,6 +378,10 @@ fn main() { | |||
} | |||
} | |||
|
|||
// Linux + aarch64 does not support a glibc version that supports `--export-dynamic-symbol`. | |||
#[cfg(all(target_os = "linux", target_arch = "aarch64"))] | |||
println!("cargo:rustc-link-arg-bin=deno=-rdynamic"); |
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.
GNU aarch64 ld did not release 2.38? That's weird 🤔
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.
https://man.archlinux.org/man/aarch64-linux-gnu-ld.1.en#export~2 It did. Can you upgrade your local binutils to >=2.38 and revert this back to --export-dynamic-symbol
? -rdynamic
will drastically increase the binary size (>160MB)
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.
The issue is that the compiled binary is used in OSes with old versions of glibc.
I’m not very knowledgeable about this, but I had a hard time upgrading binutils without also depending on a newer glibc.
Hypothetically, if I upgraded just binutils to >= 2.38, could I leave glibc at 2.27 (the version that ships with Ubuntu 18.04). Seems like a headache.
Suggestion: can I update build.rs to check on the glibc version is >= 2.28, and de-optimise the build in that case, maybe with a warning?)
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.
can I update build.rs to check on the glibc version is >= 2.28, and de-optimise the build in that case, maybe with a warning?
Yup that sounds great. It can be done in a follow up. I'll merge this PR as-is
35dab1f
to
dfff0ac
Compare
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, thanks
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]>
Changes introduced in #13633 have broken the ability to compile for linux/aarch64 - specifically the use of a `i8` as a char type, which is an `u8` on linux/aarch64. This PR: - Replaces instances of `i8` with the architecture-aware wrapper type `c_char` - Skips the use of `--export-dynamic-symbol` on linux-aarch64, because the target environments often rely on older libc/binutils versions
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]>
Changes introduced in #13633 have broken the ability to compile for linux/aarch64 - specifically the use of a
i8
as a char type, which is anu8
on linux/aarch64.This PR:
i8
with the architecture-aware wrapper typec_char
--export-dynamic-symbol
on linux-aarch64, because the target environments often rely on older libc/binutils versions