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

Remove manual libatomic reference #2058

Closed
wants to merge 1 commit into from

Conversation

link2xt
Copy link

@link2xt link2xt commented Oct 15, 2023

Reverts revert of 32a303a from ddd0478

Fixes #2043

@sfackler
Copy link
Owner

The build is red.

@link2xt
Copy link
Author

link2xt commented Oct 15, 2023

It fails for arm-unknown-linux-gnueabihf-openssl-vendored-true and arm-unknown-linux-gnueabihf-openssl-vendored-false.

@sfackler
Copy link
Owner

Those are the builds that are failing, yes.

@BlackDex
Copy link
Contributor

@sfackler I don't know about Android, but the Linux builds should work just fine with changing dylib to static i think.
Also, for that exact same system i need to use RUSTFLAGS: "-Clink-args=-latomic" else building on that platform fails for anything that uses OpenSSL (v1.1.1 or v3.x.x).
See:
https://github.com/BlackDex/rust-musl/blob/fe53e9361b6f9e0d9efd578f606dcad0dffd9730/.github/workflows/rust-musl.yml#L265C36-L268

And:
https://github.com/dani-garcia/vaultwarden/blob/f863ffb89a0f2a0a682c67110af32c731b5b9fcb/docker/Dockerfile.alpine#L70-L71

What i find a bit strange is that the check is done to see if it is static, and then the atomic lib is added dynamic.
That will always break builds which try to build statically compiled binaries, since now that is a dynamically linked library.

One compromise would be to either say, if kind is static use static, and else use dylib.
Because of this bug/regression i'm currently forced to pin the crates to a version before this, and people are having issues now trying to build Vaultwarden on FreeBSD for example.

What would be the best way to go to have this solved?

@sfackler
Copy link
Owner

Yeah, I think it makes sense to statically link to libatomic in these contexts. I'm not totally sure why we're dynamically linking currently.

@BlackDex
Copy link
Contributor

Ill test what happens if i convert it to static and use a newer version and report back.

@BlackDex
Copy link
Contributor

@sfackler, if i change it to:

println!("cargo:rustc-link-lib=atomic");

Then it works just fine for me on both gnueabi and musleabi.

@link2xt
Copy link
Author

link2xt commented Nov 15, 2023

Let's close it favor of #2094

@link2xt link2xt closed this Nov 15, 2023
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.

[REGRESSION] lib atomic is loaded dynamic again on armv6 with openssl-sys v0.9.93
3 participants