Skip to content

Allow LIBZ_SYS_STATIC=0 to force system libz - #275

Open
Byron wants to merge 2 commits into
mainfrom
build-rs-docs-and-libz-override
Open

Allow LIBZ_SYS_STATIC=0 to force system libz#275
Byron wants to merge 2 commits into
mainfrom
build-rs-docs-and-libz-override

Conversation

@Byron

@Byron Byron commented Aug 4, 2026

Copy link
Copy Markdown
Member

This PR is an attempt to regain control over the build.rs script which is documented only in code, and has no tests beyond what normally executes on CI.

To achieve that, it adds module-level documentation to declare the status quo as intent. From there, it makes one change as requested in #201, which seems to make sense and I tend to consider it a fix, rather than a breaking change. But that's quite debatable.

Notes for the Reviewer

I hope we can figure out what to do about build.rs to make it maintainable again, and how to introduce validated changes.

Shortcomings of my review

  • I didn't double-check every claim made in the newly added docs.
  • I didn't see CI fail without the code-change to build.rs, but think the script would most definitely fail as it checks build.rs output.

Also, there are many other ways to address #201, including to not change LIBZ_SYS_STATIC behaviour at all, which this PR currently claims to fix. Otherwise, we'd probably be able to continue with something like #206.

An unanswered question is if LIBZ_SYS_STATIC=1 should also be overriding, which to me feels like it.

Tasks

This section is for Byron only. Models continuing this PR must not add, remove, check, uncheck, rename, or reorder checkboxes here.

  • refackiew review

Everything below this line was generated by Codex GPT-5.

Created by Codex on behalf of Byron. Byron will review before this is ready to merge.

Fixes #201.

Requested follow-up: #201 (comment)

Summary

  • document the complete build-script selection and bundled-build behavior in Markdown doc comments
  • make LIBZ_SYS_STATIC=0 override implementation and platform choices so an existing system libz is always used
  • preserve pkg-config and vcpkg link/include metadata while preventing compiler probing and bundled fallback
  • add CI coverage proving the override wins over the static feature

Validation

  • PKG_CONFIG=false CC=false LIBZ_SYS_STATIC=0 cargo test --features static
  • cargo test
  • cargo test --all-features
  • cargo run --manifest-path systest/Cargo.toml --all-features (264 tests passed)
  • LIBZ_SYS_STATIC=1 cargo test
  • rustfmt --check build.rs
  • git diff --check

Codex commit review was run once for each commit hash. The first review was clean; the behavior review identified missing include metadata, which was preserved in the refreshed commit, and the refreshed commit review was clean.

Byron and others added 2 commits August 4, 2026 10:29
<!-- agent -->
Describe the complete selection order for stock zlib and zlib-ng, including
feature and environment precedence, platform-specific discovery, fallback
behavior, bundled compilation details, generated files, and Cargo metadata.

Assisted-by: GPT 5.6
Co-authored-by: GPT 5.6 <codex@openai.com>
Issue #201 asks for a build-environment override that reliably links an existing libz instead of falling back to the bundled source. Previously LIBZ_SYS_STATIC=0 only disabled the static preference, so failed discovery still compiled vendored zlib.

Make zero override implementation and platform choices while retaining pkg-config and vcpkg metadata. Skip the compiler probe and bundled fallback, then link z directly. Add an Ubuntu CI script that enables the static feature with CC=false, builds tests without running them, requires the system z link directive, rejects a static z directive, and verifies that no bundled libz.a was produced.

Validation: ci/test-force-system.bash; cargo test; cargo test --all-features; cargo run --manifest-path systest/Cargo.toml --all-features (264 passed); LIBZ_SYS_STATIC=1 cargo test; rustfmt --check build.rs; git diff --check.
@Byron
Byron force-pushed the build-rs-docs-and-libz-override branch from cb92551 to e11f168 Compare August 4, 2026 08:45
@Byron
Byron marked this pull request as ready for review August 4, 2026 08:49
@Byron Byron assigned jongiddy and unassigned jongiddy Aug 4, 2026
@Byron
Byron requested a review from jongiddy August 4, 2026 08:56
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

3 participants