Skip to content

[AIX] Use zlibNX if available.#268

Open
amy-kwan wants to merge 2 commits intorust-lang:mainfrom
amy-kwan:amy-kwan/AIX-Use-zlibNX
Open

[AIX] Use zlibNX if available.#268
amy-kwan wants to merge 2 commits intorust-lang:mainfrom
amy-kwan:amy-kwan/AIX-Use-zlibNX

Conversation

@amy-kwan
Copy link
Copy Markdown

This change prevents libz-sys from building its own zlib by using zlibNX if available, which is an version of the zlib library that supports hardware accelerated data compression and decompression.

This change prevents libz-sys from building its own zlib by using zlibNX
if available, which is an version of the zlib library that supports hardware
accelerated data compression and decompression.
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Updates libz-sys’s build script to prefer the AIX zlibNX installation when present, avoiding building a bundled zlib in that case.

Changes:

  • Detect AIX targets and probe /usr/opt/zlibNX for libz.a and zlib.h.
  • When found, configure Cargo to link against -lz from zlibNX and expose the include path, then exit early.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread build.rs Outdated
Comment on lines +35 to +41
let zlibNX_lib = "/usr/opt/zlibNX/lib";
let zlibNX_include = "/usr/opt/zlibNX/include";
if std::path::Path::new(zlibNX_lib).join("libz.a").exists() &&
std::path::Path::new(zlibNX_include).join("zlib.h").exists() {
println!("cargo:rustc-link-search=native={}", zlibNX_lib);
println!("cargo:rustc-link-lib=z");
println!("cargo:include={}", zlibNX_include);
Copy link

Copilot AI Apr 22, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Local variable names zlibNX_lib/zlibNX_include don’t follow Rust’s usual snake_case naming used throughout this file (e.g., want_static, host_and_target_contain). Consider renaming to snake_case (e.g., zlibnx_lib, zlibnx_include or zlibnx_lib_dir, zlibnx_include_dir) for consistency.

Suggested change
let zlibNX_lib = "/usr/opt/zlibNX/lib";
let zlibNX_include = "/usr/opt/zlibNX/include";
if std::path::Path::new(zlibNX_lib).join("libz.a").exists() &&
std::path::Path::new(zlibNX_include).join("zlib.h").exists() {
println!("cargo:rustc-link-search=native={}", zlibNX_lib);
println!("cargo:rustc-link-lib=z");
println!("cargo:include={}", zlibNX_include);
let zlibnx_lib = "/usr/opt/zlibNX/lib";
let zlibnx_include = "/usr/opt/zlibNX/include";
if std::path::Path::new(zlibnx_lib).join("libz.a").exists() &&
std::path::Path::new(zlibnx_include).join("zlib.h").exists() {
println!("cargo:rustc-link-search=native={}", zlibnx_lib);
println!("cargo:rustc-link-lib=z");
println!("cargo:include={}", zlibnx_include);

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Member

@Byron Byron left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks a lot. Here is a review by Codex, based on my hunch, which ended up P1. Please do also take a look at P2, which may or may not make sense.

Thanks


  • P1: Honor explicit static mode before preferring zlibNX
    In build.rs, the new AIX fast-path returns as soon as /usr/opt/zlibNX exists, before should_link_static() is evaluated. That changes existing behavior: callers using the static feature or LIBZ_SYS_STATIC=1 can no longer force the bundled static build when zlibNX is present. This regresses a supported build mode.

  • P2: Validate zlibNX is actually linkable before skipping fallback
    In build.rs, the AIX special case short-circuits based only on file existence. The current non-Windows system path only avoids the vendored build after zlib_installed() successfully compiles and links src/smoke.c. If /usr/opt/zlibNX exists but is unusable for the active compiler configuration, this change will fail later at link time instead of falling back to the bundled zlib.

This change addresses two issues with respect to AIX zlibNX support:
1. The zlibNX check is moved after should_link_static() to allow users to force
   static builds even when zlibNX is present.
2. zlibNX linkability is validated prior to using it. The zlibnx_installed()
   function is added and compiles/links a test program (similar to zlib_installed()
   validation).
@amy-kwan
Copy link
Copy Markdown
Author

Thanks a lot. Here is a review by Codex, based on my hunch, which ended up P1. Please do also take a look at P2, which may or may not make sense.

Thanks

  • P1: Honor explicit static mode before preferring zlibNX
    In build.rs, the new AIX fast-path returns as soon as /usr/opt/zlibNX exists, before should_link_static() is evaluated. That changes existing behavior: callers using the static feature or LIBZ_SYS_STATIC=1 can no longer force the bundled static build when zlibNX is present. This regresses a supported build mode.
  • P2: Validate zlibNX is actually linkable before skipping fallback
    In build.rs, the AIX special case short-circuits based only on file existence. The current non-Windows system path only avoids the vendored build after zlib_installed() successfully compiles and links src/smoke.c. If /usr/opt/zlibNX exists but is unusable for the active compiler configuration, this change will fail later at link time instead of falling back to the bundled zlib.

Ah, those are good points. Thank you for the review, @Byron! Appreciate it!
I've attempted to address these comments in the newest commit.

@amy-kwan amy-kwan requested a review from Byron April 25, 2026 23:48
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.

3 participants