-
Notifications
You must be signed in to change notification settings - Fork 37
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: add -z noexecstack
in Makevars
#1212
Conversation
-z noexecstack
in Makevars
src/Makevars.in
Outdated
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.
Hello! The flag you found goes to the linker, not cargo
itself. The location should be somewhere that gets passed to R's build step of the package, in which the linker is invoked (oddly enough, the linker interface is gcc
too lol). I think you can start by putting this in PKG_LIBS
line and see if it helps.
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.
Thanks, let's see how it goes
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.
@CGMossa it seems that this flag only exists on Linux (just my guess since ubuntu passes but macos workflows fail with ld: unknown options: -z
). Any idea how to fix it?
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.
My bad, I think this option should go in the configure file, no makevars.in
This is so good to know.... I'm wondering if this needs to be upstreamed to extendr. |
I still don't know if this worked, I need to wait for 0.19.1 to be available on R-universe. Also, this is the first time I see this, I never had a glibc issue in CI before. But you know more than me what should and shouldn't be handled in extendr (might be part of a FAQ / "good to know" section in the docs). |
@@ -118,5 +118,5 @@ Collate: | |||
'zzz.R' | |||
Config/rextendr/version: 0.3.1 | |||
VignetteBuilder: knitr | |||
Config/polars/LibVersion: 0.42.0 | |||
Config/polars/LibVersion: 0.42.1 |
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.
Was this change necessary?
Isn't this flag unrelated to the Rust binary?
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.
Maybe it wasn't, I think I misinterpreted the failures in the first commit because I've been bitten before by compilation errors when I forgot to update the libversion
Error seen in ubuntu CI workflow of tidypolars