fix dylib name on mac - Githubissues

matt-phylum commented 1 year ago

Linux uses the pattern "lib{}.so", but Mac OS X uses the pattern "lib{}.dylib" and Windows uses the pattern "{}.dll".

This bug fix will likely cause build failures. When you do the default brew install libmagic, you get the following files:

libmagic.1.dylib
libmagic.a
libmagic.dylib -> libmagic.1.dylib

If build.rs is updated to locate shared libraries correctly, it will start finding both libraries and prompting users to select whether they want static or dynamic linking.

robo9k commented 1 year ago

Hej @matt-phylum thanks for your contribution. Your merge request looks good to me 👍

A couple remarks:

I had to fix the macOS build in #32 so please rebase your branch onto my origin/main and just force push
"This bug fix will likely cause build failures." means that the user and CI build have to set the MAGIC_STATIC environment variable, like you explain later, right? Is there more to be aware of?
I'm not familiar with using macOS, so in the spirit of #3 do you think there could be better UX than having to set an env var for the default brew install libmagic && cargo build workflow? How would you distribute such a Rust exe, would you as a developer expect static linking as default so you don't have to put the libmagic.dylib into your app bundle or whatever?

robo9k commented 1 year ago

Okay, so the macOS CI run passed - which is not what I expected:


Run cargo build --verbose
  cargo build --verbose
  shell: /bin/bash -e {0}
  env:
    CARGO_TERM_COLOR: always
    Updating crates.io index
 Downloading crates ...
  Downloaded vcpkg v0.2.15
  Downloaded libc v0.2.138
   Compiling vcpkg v0.2.15
   Compiling libc v0.2.138
     Running `rustc --crate-name vcpkg /Users/runner/.cargo/registry/src/github.com-1ecc6299db9ec823/vcpkg-0.2.15/src/lib.rs --error-format=json --json=diagnostic-rendered-ansi,artifacts,future-incompat --crate-type lib --emit=dep-info,metadata,link -C embed-bitcode=no -C split-debuginfo=unpacked -C debuginfo=2 -C metadata=8841c0faefcfac99 -C extra-filename=-8841c0faefcfac99 --out-dir /Users/runner/work/rust-magic-sys/rust-magic-sys/target/debug/deps -L dependency=/Users/runner/work/rust-magic-sys/rust-magic-sys/target/debug/deps --cap-lints allow`
     Running `rustc --crate-name build_script_build /Users/runner/.cargo/registry/src/github.com-1ecc6299db9ec823/libc-0.2.138/build.rs --error-format=json --json=diagnostic-rendered-ansi,artifacts,future-incompat --crate-type bin --emit=dep-info,link -C embed-bitcode=no -C split-debuginfo=unpacked -C debuginfo=2 -C metadata=ea92d77b853efbc9 -C extra-filename=-ea92d77b853efbc9 --out-dir /Users/runner/work/rust-magic-sys/rust-magic-sys/target/debug/build/libc-ea92d77b853efbc9 -L dependency=/Users/runner/work/rust-magic-sys/rust-magic-sys/target/debug/deps --cap-lints allow`
     Running `/Users/runner/work/rust-magic-sys/rust-magic-sys/target/debug/build/libc-ea92d77b853efbc9/build-script-build`
     Running `rustc --crate-name libc /Users/runner/.cargo/registry/src/github.com-1ecc6299db9ec823/libc-0.2.138/src/lib.rs --error-format=json --json=diagnostic-rendered-ansi,artifacts,future-incompat --crate-type lib --emit=dep-info,metadata,link -C embed-bitcode=no -C split-debuginfo=unpacked -C debuginfo=2 -C metadata=41d7e5253b3d5b1c -C extra-filename=-41d7e5253b3d5b1c --out-dir /Users/runner/work/rust-magic-sys/rust-magic-sys/target/debug/deps -L dependency=/Users/runner/work/rust-magic-sys/rust-magic-sys/target/debug/deps --cap-lints allow --cfg freebsd11 --cfg libc_priv_mod_use --cfg libc_union --cfg libc_const_size_of --cfg libc_align --cfg libc_int128 --cfg libc_core_cvoid --cfg libc_packedN --cfg libc_cfg_target_vendor --cfg libc_non_exhaustive --cfg libc_ptr_addr_of --cfg libc_underscore_const_names --cfg libc_const_extern_fn`
   Compiling magic-sys v0.3.0 (/Users/runner/work/rust-magic-sys/rust-magic-sys)
     Running `rustc --crate-name build_script_build build.rs --error-format=json --json=diagnostic-rendered-ansi,artifacts,future-incompat --crate-type bin --emit=dep-info,link -C embed-bitcode=no -C split-debuginfo=unpacked -C debuginfo=2 --cfg 'feature="default"' --cfg 'feature="v5-04"' --cfg 'feature="v5-05"' --cfg 'feature="v5-10"' --cfg 'feature="v5-13"' --cfg 'feature="v5-20"' --cfg 'feature="v5-21"' --cfg 'feature="v5-22"' --cfg 'feature="v5-23"' --cfg 'feature="v5-25"' --cfg 'feature="v5-27"' --cfg 'feature="v5-32"' --cfg 'feature="v5-35"' --cfg 'feature="v5-38"' -C metadata=a77ba2a611c7efdc -C extra-filename=-a77ba2a611c7efdc --out-dir /Users/runner/work/rust-magic-sys/rust-magic-sys/target/debug/build/magic-sys-a77ba2a611c7efdc -C incremental=/Users/runner/work/rust-magic-sys/rust-magic-sys/target/debug/incremental -L dependency=/Users/runner/work/rust-magic-sys/rust-magic-sys/target/debug/deps --extern vcpkg=/Users/runner/work/rust-magic-sys/rust-magic-sys/target/debug/deps/libvcpkg-8841c0faefcfac99.rlib`
     Running `/Users/runner/work/rust-magic-sys/rust-magic-sys/target/debug/build/magic-sys-a77ba2a611c7efdc/build-script-build`
     Running `rustc --crate-name magic_sys src/lib.rs --error-format=json --json=diagnostic-rendered-ansi,artifacts,future-incompat --crate-type lib --emit=dep-info,metadata,link -C embed-bitcode=no -C split-debuginfo=unpacked -C debuginfo=2 --cfg 'feature="default"' --cfg 'feature="v5-04"' --cfg 'feature="v5-05"' --cfg 'feature="v5-10"' --cfg 'feature="v5-13"' --cfg 'feature="v5-20"' --cfg 'feature="v5-21"' --cfg 'feature="v5-22"' --cfg 'feature="v5-23"' --cfg 'feature="v5-25"' --cfg 'feature="v5-27"' --cfg 'feature="v5-32"' --cfg 'feature="v5-35"' --cfg 'feature="v5-38"' -C metadata=292323c8a6c889b2 -C extra-filename=-292323c8a6c889b2 --out-dir /Users/runner/work/rust-magic-sys/rust-magic-sys/target/debug/deps -C incremental=/Users/runner/work/rust-magic-sys/rust-magic-sys/target/debug/incremental -L dependency=/Users/runner/work/rust-magic-sys/rust-magic-sys/target/debug/deps --extern libc=/Users/runner/work/rust-magic-sys/rust-magic-sys/target/debug/deps/liblibc-41d7e5253b3d5b1c.rmeta -l dylib=magic`
    Finished dev [unoptimized + debuginfo] target(s) in 2m 03s

Looks like brew install libmagic in the previous step put the files in:

==> Downloading https://ghcr.io/v2/homebrew/core/libmagic/manifests/5.43
==> Downloading https://ghcr.io/v2/homebrew/core/libmagic/blobs/sha256:1de7baf672db48278eab882713c696e07ef0fbd5d410c6fa20975ee80b52497f
==> Downloading from https://pkg-containers.githubusercontent.com/ghcr1/blobs/sha256:1de7baf672db48278eab882713c696e07ef0fbd5d410c6fa20975ee80b52497f?se=2022-12-05T14%3A50%3A00Z&sig=E3xUk%2F5tnqnFktf4iBka0e9IJ2gt3BgrX2uzuqd3jfY%3D&sp=r&spr=https&sr=b&sv=2019-12-12
==> Pouring libmagic--5.43.monterey.bottle.tar.gz
🍺  /usr/local/Cellar/libmagic/5.43: 352 files, 9.5MB

I would have expected build.rs to complain about finding both static and shared lib. Might have to print the contents of that brew folder in CI 🤔

matt-phylum commented 1 year ago

"This bug fix will likely cause build failures." means that the user and CI build have to set the MAGIC_STATIC environment variable, like you explain later, right? Is there more to be aware of?

That's the issue I'm expecting.

I'm not familiar with using macOS, so in the spirit of #3 do you think there could be better UX than having to set an env var for the default brew install libmagic && cargo build workflow? How would you distribute such a Rust exe, would you as a developer expect static linking as default so you don't have to put the libmagic.dylib into your app bundle or whatever?

It depends on how you're distributing your code. If you're distributing the code via Homebrew, the library rpath is embedded such that it should work without needing to set any environment variables. If you're distributing the code via Cargo, it will build from source on the target machine so as long as the dylib is located during compilation it will work. If you're distributing the code via some other means, you probably need to copy the dylib into your distribution and update the executable to use relative rpaths. Changing the rpaths can be done during the build by setting linker flags or after the build by using install_name_tool.

Using a static library should make distribution easier. However, in my case I was trying to build a local test copy of something that normally runs in a different environment, and for some reason the static library was giving me an error about missing symbols related to bz2. I thought I was only seeing it in the smaller project because the larger project I have used bz2 for something else, but I can't replicate it now in either project. 🤔

matt-phylum commented 1 year ago

I would have expected build.rs to complain about finding both static and shared lib. Might have to print the contents of that brew folder in CI 🤔

Do you have MAGIC_DIR set? build.rs behaves differently depending on whether it's set. Does the GitHub runner have /opt/homebrew/lib on the default library search path? It looks like even in the build on main without this PR it's doing dynamic linking.

matt-phylum commented 1 year ago

The CI being able to find the library without MAGIC_DIR might have something to do with it being an amd64 machine. amd64 Homebrew puts packages in /usr/local but arm64 Homebrew puts packages in /opt/homebrew.

robo9k / rust-magic-sys

fix dylib name on mac #30