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

Really try dylib when compiling for Apple platforms #181

Merged
merged 2 commits into from
Feb 1, 2024
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 6 additions & 3 deletions build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -67,11 +67,10 @@ 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 apple_to_apple = host.contains("-apple-") && target.contains("-apple-");
if target.contains("msvc")
|| target.contains("pc-windows-gnu")
|| want_static
|| (cross_compiling && !apple_to_apple)
|| (cross_compiling && !target.contains("-apple-"))
{
return build_zlib(&mut cfg, &target);
}
Expand Down Expand Up @@ -185,7 +184,11 @@ fn try_vcpkg() -> bool {

fn zlib_installed(cfg: &mut cc::Build) -> bool {
let mut cmd = cfg.get_compiler().to_command();
cmd.arg("src/smoke.c").arg("-o").arg("/dev/null").arg("-lz");
cmd.arg("src/smoke.c")
.arg("-g0")
Copy link
Member

Choose a reason for hiding this comment

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

Are you sure this is working?

What I can gather from the CC documentation is the following:

       -o <file>
              Write output to file.

It seems like /dev/null wants to remain right behind it. Can you elaborate?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Duh, that's obviously because I misplaced the argument.

Copy link
Member

Choose a reason for hiding this comment

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

This would mean that this code probably never ran. Can you help me validate this, or share how you validated it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So, I did have the right patch originally, but I messed up something before I pushed, or something. It definitely worked. Anyways, here's how I checked: I added a panic at the beginning of build_zlib(). Then PATH=/path/to/cctools/bin:$PATH TARGET_CFLAGS="isysroot /path/to/MacOSX14.2.sdk" CC=clang cargo build --target aarch64-apple-darwin

Copy link
Member

@Byron Byron Jan 31, 2024

Choose a reason for hiding this comment

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

Thanks for your help.

That works for me, but it does so both on main and on this PR, after adding a panic right at the beginning of build_zlib().

This was my invocation (on MacOS): TARGET_CFLAGS="isysroot /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX14.2.sdk" CC=clang cargo build --target aarch64-apple-darwin, while noting that my SDK folder is empty, probably not downloaded. I also assume that isysroot is a magic value.

My expectation would be that there is an invocation that panics on main to indicate it's picking up the bundled zlib, but won't panic here as proof that it will pickup the system library more reliably. For me this seems to happen no matter what.

Does it panic for you on main but not on the branch? Is PATH important for specific cctools? Is isysroot magic or am I supposed to replace it - are these flags making all the difference?

Thanks for bearing with me.

Copy link
Member

Choose a reason for hiding this comment

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

Maybe on a Mac I wouldn't be able to reproduce it then anyway?
I do have a linux VM I could use, but would probably missing all the setup thats's needed to cross-compile.

Just to be clear, I can just merge this but after having found that one flaw I'd like to have proof that it's actually doing what it advertises. As the interested parties surely have a way to reproduce the issue they have with main the lack of issue with the code in this branch, maybe some logs could be shared to workaround difficulties in me reproducing it locally?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Actually, if you add a panic to build_zlib, it will fail to cargo build on mac on main.

Copy link
Member

Choose a reason for hiding this comment

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

That definitely doesn't happen for me :/. Here is my patch in case there is something I am missing.

diff --git a/build.rs b/build.rs
index 1368a12..2b11d3e 100644
--- a/build.rs
+++ b/build.rs
@@ -91,6 +91,7 @@ fn main() {
 }
 
 fn build_zlib(cfg: &mut cc::Build, target: &str) {
+    panic!("shouldn't be here");
     let dst = PathBuf::from(env::var_os("OUT_DIR").unwrap());
     let lib = dst.join("lib");

This is the slightly adjusted command-line I have been using: TARGET_CFLAGS="-isysroot /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX14.2.sdk" CC=clang cargo build --target aarch64-apple-darwin.

libz-sys ( main) +1 [$!]
❯ TARGET_CFLAGS="-isysroot /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX14.2.sdk" CC=clang cargo build --target aarch64-apple-darwin
   Compiling libc v0.2.152
   Compiling pkg-config v0.3.29
   Compiling cc v1.0.83
   Compiling libz-sys v1.1.15 (/Users/byron/dev/github.com/rust-lang/libz-sys)
warning: unreachable statement
  --> build.rs:95:5
   |
94 |     panic!("shouldn't be here");
   |     --------------------------- any code following this expression is unreachable
95 |     let dst = PathBuf::from(env::var_os("OUT_DIR").unwrap());
   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ unreachable statement
   |
   = note: `#[warn(unreachable_code)]` on by default

warning: unused variable: `cfg`
  --> build.rs:93:15
   |
93 | fn build_zlib(cfg: &mut cc::Build, target: &str) {
   |               ^^^ help: if this is intentional, prefix it with an underscore: `_cfg`
   |
   = note: `#[warn(unused_variables)]` on by default

warning: unused variable: `target`
  --> build.rs:93:36
   |
93 | fn build_zlib(cfg: &mut cc::Build, target: &str) {
   |                                    ^^^^^^ help: if this is intentional, prefix it with an underscore: `_target`

warning: `libz-sys` (build script) generated 3 warnings
    Finished dev [unoptimized + debuginfo] target(s) in 1.16s

libz-sys ( main) +1 [$!]

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is what I get on unpatched main:

% git clone https://github.com/rust-lang/libz-sys
Cloning into 'libz-sys'...
remote: Enumerating objects: 1377, done.
remote: Counting objects: 100% (419/419), done.
remote: Compressing objects: 100% (203/203), done.
remote: Total 1377 (delta 216), reused 380 (delta 200), pack-reused 958
Receiving objects: 100% (1377/1377), 902.13 KiB | 2.89 MiB/s, done.
Resolving deltas: 100% (656/656), done.
% cd libz-sys 
% patch -p1
diff --git a/build.rs b/build.rs
index 1368a12..2b11d3e 100644
--- a/build.rs
+++ b/build.rs
@@ -91,6 +91,7 @@ fn main() {
 }
 
 fn build_zlib(cfg: &mut cc::Build, target: &str) {
+    panic!("shouldn't be here");
     let dst = PathBuf::from(env::var_os("OUT_DIR").unwrap());
     let lib = dst.join("lib");
patching file build.rs
% cargo build
    Updating crates.io index
  Downloaded libc v0.2.153
  Downloaded 1 crate (740.6 KB) in 1.00s
   Compiling libc v0.2.153
   Compiling pkg-config v0.3.29
   Compiling vcpkg v0.2.15
   Compiling cc v1.0.83
   Compiling libz-sys v1.1.15 (/Users/glandium/libz-sys)
warning: unreachable statement
  --> build.rs:95:5
   |
94 |     panic!("shouldn't be here");
   |     --------------------------- any code following this expression is unreachable
95 |     let dst = PathBuf::from(env::var_os("OUT_DIR").unwrap());
   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ unreachable statement
   |
   = note: `#[warn(unreachable_code)]` on by default

warning: unused variable: `cfg`
  --> build.rs:93:15
   |
93 | fn build_zlib(cfg: &mut cc::Build, target: &str) {
   |               ^^^ help: if this is intentional, prefix it with an underscore: `_cfg`
   |
   = note: `#[warn(unused_variables)]` on by default

warning: unused variable: `target`
  --> build.rs:93:36
   |
93 | fn build_zlib(cfg: &mut cc::Build, target: &str) {
   |                                    ^^^^^^ help: if this is intentional, prefix it with an underscore: `_target`

warning: `libz-sys` (build script) generated 3 warnings
error: failed to run custom build command for `libz-sys v1.1.15 (/Users/glandium/libz-sys)`

Caused by:
  process didn't exit successfully: `/Users/glandium/libz-sys/target/debug/build/libz-sys-bdb7c6d6b951d63d/build-script-build` (exit status: 101)
  --- stdout
  cargo:rerun-if-env-changed=LIBZ_SYS_STATIC
  cargo:rerun-if-changed=build.rs
  cargo:rerun-if-env-changed=ZLIB_NO_PKG_CONFIG
  cargo:rerun-if-env-changed=PKG_CONFIG_aarch64-apple-darwin
  cargo:rerun-if-env-changed=PKG_CONFIG_aarch64_apple_darwin
  cargo:rerun-if-env-changed=HOST_PKG_CONFIG
  cargo:rerun-if-env-changed=PKG_CONFIG
  cargo:rerun-if-env-changed=ZLIB_STATIC
  cargo:rerun-if-env-changed=ZLIB_DYNAMIC
  cargo:rerun-if-env-changed=PKG_CONFIG_ALL_STATIC
  cargo:rerun-if-env-changed=PKG_CONFIG_ALL_DYNAMIC
  cargo:rerun-if-env-changed=PKG_CONFIG_PATH_aarch64-apple-darwin
  cargo:rerun-if-env-changed=PKG_CONFIG_PATH_aarch64_apple_darwin
  cargo:rerun-if-env-changed=HOST_PKG_CONFIG_PATH
  cargo:rerun-if-env-changed=PKG_CONFIG_PATH
  cargo:rerun-if-env-changed=PKG_CONFIG_LIBDIR_aarch64-apple-darwin
  cargo:rerun-if-env-changed=PKG_CONFIG_LIBDIR_aarch64_apple_darwin
  cargo:rerun-if-env-changed=HOST_PKG_CONFIG_LIBDIR
  cargo:rerun-if-env-changed=PKG_CONFIG_LIBDIR
  cargo:rerun-if-env-changed=PKG_CONFIG_SYSROOT_DIR_aarch64-apple-darwin
  cargo:rerun-if-env-changed=PKG_CONFIG_SYSROOT_DIR_aarch64_apple_darwin
  cargo:rerun-if-env-changed=HOST_PKG_CONFIG_SYSROOT_DIR
  cargo:rerun-if-env-changed=PKG_CONFIG_SYSROOT_DIR
  cargo:rerun-if-env-changed=ZLIB_STATIC
  cargo:rerun-if-env-changed=ZLIB_DYNAMIC
  cargo:rerun-if-env-changed=PKG_CONFIG_ALL_STATIC
  cargo:rerun-if-env-changed=PKG_CONFIG_ALL_DYNAMIC
  cargo:rerun-if-env-changed=PKG_CONFIG_PATH_aarch64-apple-darwin
  cargo:rerun-if-env-changed=PKG_CONFIG_PATH_aarch64_apple_darwin
  cargo:rerun-if-env-changed=HOST_PKG_CONFIG_PATH
  cargo:rerun-if-env-changed=PKG_CONFIG_PATH
  cargo:rerun-if-env-changed=PKG_CONFIG_LIBDIR_aarch64-apple-darwin
  cargo:rerun-if-env-changed=PKG_CONFIG_LIBDIR_aarch64_apple_darwin
  cargo:rerun-if-env-changed=HOST_PKG_CONFIG_LIBDIR
  cargo:rerun-if-env-changed=PKG_CONFIG_LIBDIR
  cargo:rerun-if-env-changed=PKG_CONFIG_SYSROOT_DIR_aarch64-apple-darwin
  cargo:rerun-if-env-changed=PKG_CONFIG_SYSROOT_DIR_aarch64_apple_darwin
  cargo:rerun-if-env-changed=HOST_PKG_CONFIG_SYSROOT_DIR
  cargo:rerun-if-env-changed=PKG_CONFIG_SYSROOT_DIR
  cargo-warning=Could not run `PKG_CONFIG_ALLOW_SYSTEM_CFLAGS=1 pkg-config --libs --cflags zlib`
  The pkg-config command could not be found.

  Most likely, you need to install a pkg-config package for your OS.
  Try `brew install pkg-config` if you have Homebrew.

  If you've already installed it, ensure the pkg-config command is one of the
  directories in the PATH environment variable.

  If you did not expect this build to link to a pre-installed system library,
  then check documentation of the libz-sys crate for an option to
  build the library from source, or disable features or dependencies
  that require pkg-config.
  OPT_LEVEL = Some("0")
  TARGET = Some("aarch64-apple-darwin")
  HOST = Some("aarch64-apple-darwin")
  cargo:rerun-if-env-changed=CC_aarch64-apple-darwin
  CC_aarch64-apple-darwin = None
  cargo:rerun-if-env-changed=CC_aarch64_apple_darwin
  CC_aarch64_apple_darwin = None
  cargo:rerun-if-env-changed=HOST_CC
  HOST_CC = None
  cargo:rerun-if-env-changed=CC
  CC = None
  cargo:rerun-if-env-changed=CRATE_CC_NO_DEFAULTS
  CRATE_CC_NO_DEFAULTS = None
  DEBUG = Some("true")
  CARGO_CFG_TARGET_FEATURE = Some("aes,crc,dit,dotprod,dpb,dpb2,fcma,fhm,flagm,fp16,frintts,jsconv,lor,lse,neon,paca,pacg,pan,pmuv3,ras,rcpc,rcpc2,rdm,sb,sha2,sha3,ssbs,v8.1a,v8.2a,v8.3a,v8.4a,vh")
  cargo:rerun-if-env-changed=CFLAGS_aarch64-apple-darwin
  CFLAGS_aarch64-apple-darwin = None
  cargo:rerun-if-env-changed=CFLAGS_aarch64_apple_darwin
  CFLAGS_aarch64_apple_darwin = None
  cargo:rerun-if-env-changed=HOST_CFLAGS
  HOST_CFLAGS = None
  cargo:rerun-if-env-changed=CFLAGS
  CFLAGS = None
  running "cc" "-O0" "-ffunction-sections" "-fdata-sections" "-fPIC" "-gdwarf-2" "-fno-omit-frame-pointer" "-arch" "arm64" "-Wall" "-Wextra" "src/smoke.c" "-o" "/dev/null" "-lz"

  --- stderr
  src/smoke.c:4:10: warning: cast to smaller integer type 'int' from 'uLong (*)(uLong, const Bytef *, uInt)' (aka 'unsigned long (*)(unsigned long, const unsigned char *, unsigned int)') [-Wpointer-to-int-cast]
    return (int) adler32;
           ^~~~~~~~~~~~~
  1 warning generated.
  error: cannot parse the debug map for '/dev/null': The file was not recognized as a valid object file
  clang: error: dsymutil command failed with exit code 1 (use -v to see invocation)
  thread 'main' panicked at build.rs:94:5:
  shouldn't be here
  note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It occurs to me that maybe you have pkg-config from brew?

.arg("-o")
.arg("/dev/null")
.arg("-lz");

println!("running {:?}", cmd);
if let Ok(status) = cmd.status() {
Expand Down
Loading