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

Add LIB_SYS_TRY_SHARED ENV var to try to link with shared library #206

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

hodrob84
Copy link

@hodrob84 hodrob84 commented Aug 15, 2024

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 #201

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 force-pushed the hodrob/try_link_with_shared_on_cc branch from 612984b to 2422982 Compare August 15, 2024 15:24
@Byron
Copy link
Member

Byron commented Aug 16, 2024

Thanks a lot for tackling this.

@jongiddy I feel that libz-sys gets more complex by adding patch upon patch, and hoped you could involve yourself here more as well. Maybe you could help with making a case for using environment variables to alter build results, or for using Cargo Features. This is my related comment, to allow for backward compatible fixes for those who know about it.

Thanks everyone for your help.

@Byron Byron added the question There is a question to be answered label Aug 16, 2024
@jongiddy
Copy link

Neither option is ideal.

Using an environment variable means there is a hidden dependency that affects the build. When anyone needs support, it is an extra relevant piece of information they need to supply. And it is easy to not be aware whether it is set or not.

Using a feature is painful when a transitive dependency is deeply nested, and libz-sys is typically deeply nested. You either need to pass the feature setting down through the intermediate dependencies or include the dependency in the top-level crate, even though it is not used directly.

Pragmatically, since there is already an environment variable for controlling static vs dynamic, I'd accept using an environment variable here. However, I suggest that the existing variable is reused, so that setting LIBZ_SYS_STATIC=0 always forces dynamic linking, even for the supposedly non-supported targets (e.g. target.contains("msvc")).

@Byron Byron removed the question There is a question to be answered label Aug 17, 2024
@wezm
Copy link
Member

wezm commented Sep 3, 2024

FWIW there is precedent in other -sys crates for disabling building of vendored C code/forcing use of system library through an env var:

  • libgit2-sys: "LIBGIT2_NO_VENDOR": "1",
  • libssh2-sys: "LIBSSH2_SYS_USE_PKG_CONFIG": "1",
  • openssl-sys: "OPENSSL_NO_VENDOR": "1",
  • pcre2-sys: "PCRE2_SYS_STATIC": "0",
  • rustonig-sys: "RUSTONIG_SYSTEM_LIBONIG": "1",
  • zstd-sys: "ZSTD_SYS_USE_PKG_CONFIG": "1",
  • libsqlite3-sys: "LIBSQLITE3_SYS_USE_PKG_CONFIG": "1",

@Byron
Copy link
Member

Byron commented Sep 3, 2024

Thanks a lot for the summary, it's good to know the standards.

This PR is at a stage where the question remains if LIBZ_SYS_STATIC=0 can be used instead of introducing a new environment variable. On top of that, I think there should be a section in the crate-level docs to document this behaviour.

@jongiddy
Copy link

jongiddy commented Sep 3, 2024

  • pcre2-sys: "PCRE2_SYS_STATIC": "0",

This one is interesting. Matches the proposed one variable model with 0 meaning force dynamic and 1 meaning force static.

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.

Link against shared zlib library when cross-compiling
5 participants