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.

Sign up for GitHub

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

Jump to bottom

Link against shared zlib library when cross-compiling #201

Open
hodrob84 opened this issue Aug 8, 2024 · 8 comments · May be fixed by #206
Open

Link against shared zlib library when cross-compiling #201

hodrob84 opened this issue Aug 8, 2024 · 8 comments · May be fixed by #206
Labels
bug help wanted

Comments

@hodrob84
Copy link

hodrob84 commented Aug 8, 2024 •
edited
Loading

Currently libz-sys is always building a static zlib library when cross-compiling, except on Apple platforms. Can I ask the reason of this? I have a shared zlib library in my rootfs, and pkg-config is working fine. With this config, I have a build failure because of library collision, like this:

/tmp/rustc0ZqD0F/liblibz_sys-84983a050a121d20.rlib(inflate.o): relocation R_AARCH64_ADR_PREL_PG_HI21 against symbol '__stack_chk_guard' which may bind externally can not be used when making a shared object; recompile with -fPIC

If I remove forcing this static library build, it is building fine:

index cab160ae6e6f..87e3e8664fd6 100644
--- a/build.rs
+++ b/build.rs
@@ -81,11 +81,11 @@ fn main() {
     //
     // Apple platforms have libz.1.dylib, and it's usually available even when
     // cross compiling (via fat binary or in the target's Xcode SDK)
-    let cross_compiling = target != host;
+    // let cross_compiling = target != host;
     if target.contains("msvc")
         || target.contains("pc-windows-gnu")
         || want_static
-        || (cross_compiling && !target.contains("-apple-"))
+        // || (cross_compiling && !target.contains("-apple-"))
     {
         return build_zlib(&mut cfg, &target);
     }```
@Byron
Copy link
Member

Byron commented Aug 9, 2024

Thanks for reporting!

I'd hope that this information is available in the Git history somewhere.

As insinuated, the intention behind a lot of things might not be clear from looking at the code and if unlucky isn't even discovered when checking the Git history. Further, due to the nature of this crate, it's hard to test on all platforms so a test is unlikely to fail if any change to build.rs is made.

And in a way, this makes changes like these hard to make as it alters semantics that many might rely upon, at least implicitly.

I don't really know how to solve this except for trying to build actual tests for all the involved platforms, essentially rebuilding the build-script in a test-driven fashion and finally, releasing a new major version as one should probably fix a lot of things on the way.

@hodrob84
Copy link
Author

I tried to figure this out from git log, but I failed. To workaround this, we could add another ENV var, similar to LIBZ_SYS_STATIC, e.g. LIBZ_SYS_FORCE_PKG_CONFIG=1, or similar to override this behavior if someone wants it. This would solve my issue (on a probably not that nice way), but I would still think the default behavior would be to link with the installed shared library.

@Byron
Copy link
Member

Byron commented Aug 10, 2024

It's a good idea to have a variable of sorts - instead of an environment variable, would a Cargo feature work?

Regarding the default behaviour, I am afraid this can't change without a new breaking release, and it's something I'd consider doing if the build script would be re-built with full documentation and full testing.

@hodrob84
Copy link
Author

It's a good idea to have a variable of sorts - instead of an environment variable, would a Cargo feature work?

Yes, I think, that should work.

Regarding the default behaviour, I am afraid this can't change without a new breaking release, and it's something I'd consider doing if the build script would be re-built with full documentation and full testing.

I agree.

hodrob84 pushed a commit to hodrob84/libz-sys that referenced this issue Aug 15, 2024
@rhodaszi
Add LIB_SYS_TRY_SHARED ENV var to try to link with shared library
2422982 
On cross-compilation libz-sys is always building static zlib library,
except for Apple platforms. If target has an installed zlib shared
library, this is problem, because:

  1. the binary is increased, because we are including a static zlib as
     well
  2. it may cause build issues if the pkg-config sets the linker
     arguments, like this:

     /tmp/rustc0ZqD0F/liblibz_sys-84983a050a121d20.rlib(inflate.o): relocation R_AARCH64_ADR_PREL_PG_HI21 against symbol '__stack_chk_guard' which may bind externally can not be used when making a shared object; recompile with -fPIC

To workaround the issue, add a LIB_SYS_TRY_SHARED=1 ENV var (similarly
to LIBZ_SYS_STATIC=1) to force the use of the shared library, recognized
by the pkg-config.

fixes rust-lang#201
@hodrob84 hodrob84 linked a pull request Aug 15, 2024 that will close this issue
Open
@hodrob84
Copy link
Author

I couldn't make it working with a Cargo feature for me. Unfortunately I'm using a workspace Cargo.toml, and libz-sys is pulled in by a dependency. So I couldn't really find a good way to set a feature for libz-sys. ENV var is a better fit.

@Byron
Copy link
Member

Byron commented Aug 16, 2024

I understand. The way this could work, very explicitly, with cargo features is by adding the libz-siyz dependency to your own project, possibly renaming it to something indicating that it's 'just for configuration'.

libz-sys-for-configuration = { version = "X", package = "libz-sys", features = ["whateveryouneed"] }

@hodrob84
Copy link
Author

So you mean, adding it to "[workspace.dependencies]". I already tried that, but it wasn't working. Note, that I'm using "[patch.create-io]" to be able to modify libz-sys, but I don't think that should change anything.

Another use-case for ENV var: what if we are thinking in build systems, like buildroot? If there are multiple packages which are pulling in libz-sys as a dependency (multiple builds), I should have to patch each recipes to add this feature to their Cargo.toml. But using an ENV variable, I can just add it to the generic Cargo ENV vars, and if something is pulling in libz-sys, it will work automagically.

@Byron
Copy link
Member

Byron commented Aug 17, 2024

So you mean, adding it to "[workspace.dependencies]". I already tried that, but it wasn't working. Note, that I'm using "[patch.create-io]" to be able to modify libz-sys, but I don't think that should change anything.

No, it would have to be a [dependency], as only then it's truly used.

Another use-case for ENV var: what if we are thinking in build systems, like buildroot? If there are multiple packages which are pulling in libz-sys as a dependency (multiple builds), I should have to patch each recipes to add this feature to their Cargo.toml. But using an ENV variable, I can just add it to the generic Cargo ENV vars, and if something is pulling in libz-sys, it will work automagically.

That's a valid point, and to make that possible, it should certainly be an environment variable. I think the issue to solve really is to make it less magic. At least I'd expect to be able to learn about this magic somewhere, and the docs.rs is usually the place which at least lists cargo features automatically. Probably there would be no harm in documenting it there by hand though.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug help wanted
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Add LIB_SYS_TRY_SHARED ENV var to try to link with shared library hodrob84/libz-sys
2 participants
@Byron @hodrob84