-
Notifications
You must be signed in to change notification settings - Fork 83
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
Add sysroot support for GNU based toolchains #158
Conversation
…use the correct headers when cross compiling.
Side note; building for aarch64-unknown-linux-musl will currently fail do this MbedTLS 2.26.0 issue: Mbed-TLS/mbedtls#4237. As a temporary workaround until it's fixed, it's possible to build by setting |
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! Can you also add a target that uses this to CI?
mbedtls-sys/build/build.rs
Outdated
.strip_suffix("\r\n") | ||
.or(path.strip_suffix("\n")) | ||
.unwrap_or(&path); | ||
cflags.push(format!("--sysroot={}", trimmed_path)); |
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.
This sends the flag to both bindgen and CMake, I think that's not desirable?
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.
Edited in ce2864a.
mbedtls-sys/build/build.rs
Outdated
// Determine the sysroot for this compiler so that bindgen | ||
// uses the correct headers | ||
let compiler = cc::Build::new().get_compiler(); | ||
let output = compiler.to_command().args(&["--print-sysroot"]).output(); |
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.
Please run this only if compiler.is_like_gnu()
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.
Edited in ce2864a.
I'll see if I can add the |
The three Aarch64 tests are slower since it's running in Qemu. |
I pushed some changes and now some builds are failing and it's entirely unclear to me why. ¯\_(ツ)_/¯ |
Dang haha, it looks like on the latest nightly the clang-sys crate is failing to build. Issue was created here an hour ago. The aarch64 stable build is failing here But I'm not sure what's causing MBEDTLS_ZLIB_SUPPORT to be defined. Since it looks to be undefined in config.rs. |
Man, I was thinking of making that change just because I thought it would be nicer! I'll merge this after we get a new nightly |
Sounds good, thanks! |
Except, it didn't fix ARM |
Yep, looks like the the unquoted "-n" was always evaluating to true. |
bors r+ |
Build succeeded: |
Do you need this released soon? Otherwise this will release with the next MbedTLS version. |
Next release sounds good! For now I can just build from the master branch. Thanks for the help @jethrogb |
…x#158) Instead of cloning most of `Request`'s fields individually when creating a `Unit`, this PR switches to just cloning `Request` and stuffing it in `Unit`, and changes references to `unit.[field]` to `unit.req.[field]` where appropriate. Fixes fortanix#155
This PR adds sysroot detection for GNU based toolchains for targets like
aarch64-unknown-linux-musl
. This prevents bindgen from using incorrect headers when generating bindings.For instance when using cross, the aarch64 musl sysroot is installed in /usr/local/aarch64-linux-musl/ but bindgen/clang will attempt to look for headers in /usr/include unless
--sysroot
is provided.