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

os/mac/pkgconfig/15: add bzip2.pc for rust formulae #18545

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
Draft

Conversation

cho-m
Copy link
Member

@cho-m cho-m commented Oct 10, 2024

  • Have you followed the guidelines in our Contributing document?
  • Have you checked to ensure there aren't other open Pull Requests for the same change?
  • Have you added an explanation of what your changes do and why you'd like us to include them?
  • Have you written new tests for your changes? Here's an example.
  • Have you successfully run brew style with your changes locally?
  • Have you successfully run brew typecheck with your changes locally?
  • Have you successfully run brew tests with your changes locally?

Mainly since the only way to avoid bundled bzip2 in rust formulae is via pkg-config - https://github.com/alexcrichton/bzip2-rs/blob/master/bzip2-sys/build.rs#L16-L23

Similar to pkgconfig file created inside bzip2 formula based on https://gitlab.com/bzip2/bzip2/-/blob/master/bzip2.pc.in (note that we currently only ship shared libraries & pkg-config file on Linux, so using brew bzip2 is particularly bad option on macOS).

Removed bindir= and -I${includedir} in Cflags.


Only added macOS 15 for initial discussion. If acceptable, can add older macOS versions.

@Bo98
Copy link
Member

Bo98 commented Oct 10, 2024

Didn't even realise there was headers in the SDK for this. Nice!

Removed bindir=

Any reason for this? Seems safe enough to have ${exec_prefix}/bin.

If acceptable, can add older macOS versions.

Seems like it's good for all OS versions (though note that pre-10.14 files are slightly different).

macOS 12 and later use v1.0.8.
macOS 11 and earlier use v1.0.6.

Also add a test to test/os/mac/pkgconfig_spec.rb.

@carlocab
Copy link
Member

note that we currently only ship shared libraries & pkg-config file on Linux, so using brew bzip2 is particularly bad option on macOS

We should probably fix that.

@cho-m
Copy link
Member Author

cho-m commented Oct 10, 2024

Removed bindir=

Any reason for this? Seems safe enough to have ${exec_prefix}/bin.

No major reason so can add.


note that we currently only ship shared libraries & pkg-config file on Linux, so using brew bzip2 is particularly bad option on macOS

We should probably fix that.

Part of issue is latest bzip2 stable release only cleanly supports Linux .so and would require manual effort to get .dylib. Pkg-config is also not officially supported yet, but worth having as projects expect it due to major Linux distros.

Future 1.1.0 fixes this, but slightly complicates things due to switch to meson/cmake so need to be careful about dependency loops. Should be fine as long as they continue cmake support as we use bundled bzip2 on Linux.

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