-
Notifications
You must be signed in to change notification settings - Fork 797
Add support for pkg-config #2547
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
Conversation
|
@headius @eregon It would be really nice to have ruby-build work better with MacPorts by using pkg-config as this PR enables. I stopped trusting Homebrew for dev build dependencies a long time ago. I think that there may be similar configuration changes required for TruffleRuby because it requires that I override the libyaml prefix for all runs. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi, thank you so much for the contribution, and sorry for the late review.
I generally agree that we should support discovering openssl from pkg-config if available instead of blindly downloading and compiling a new OpenSSL instance, but for the sake of a smaller diff, I would strongly prefer if the initial code change was scoped to just that.
Also, since Ruby's build system already supports discovering libraries from pkg-config, I would avoid trying to replicate that functionality too much in ruby-build. I know you suggested that explicitly passing --with-*-dir=... flags is good for visibility, but we also want to let Ruby's build system have a chance to do the right thing. We only added support for discovering Homebrew-installed libraries in ruby-build because Ruby doesn't have any capabilities to do that on its own.
So here's an idea for a minimal fix: instead of all the changes in this branch, how about scoping the fix to just needs_openssl so that it returns false if a compatible openssl version was found with pkg-config? Something like this:
local system_version
if has_broken_mac_openssl; then
system_version="$(pkg-config --modversion openssl 2>/dev/null || true)"
else
system_version="$(system_openssl_version)"
fiThe logic behind this would be: if on a Mac with a broken system OpenSSL version (the default for all macOS systems), treat the pkgconfig-discovered version as the "system" version and exit if that satisfies the requirement. Would this be enough to support MacPorts?
|
I'm not on the computer where I was looking at this, but if you look at the linked MacPorts PR, there's a few libraries which are available via MacPorts that aren't found by ruby-build without explicit configuration. I've seen a problem where ruby-build appears to favour Homebrew-discovered libraries over equivalent (better, IMO) installations from MacPorts. As I'm using mise mostly these days, this affects me since it is based on ruby-build, but I'm also a maintainer of the Port. |
I've taken a look at the linked PR, but I don't understand which are these libraries you are referring to. Are you saying that there can be MacPorts-installed libraries such as libyaml that are discoverable using
Ah, I think I understand this part. If Homebrew-discovered libraries were found by ruby-build, then indeed it will automatically configure Ruby by having We could potentially avoid doing the Homebrew discovery step if the same libraries were found using pkg-config. However, in that case I still wouldn't pass the paths to those libraries using Or, we could add an environment variable that would prevent |
|
Yes. I have both homebrew and MacPorts installed and want libraries used from MacPorts and never homebrew. Homebrew is an unreliable source for build libraries because they do rolling updates. MacPorts ensures that they are usable in the same way that Debian does. (I use Homebrew primarily for cask management and some tools which aren't yet available from MacPorts or mise; sometimes this drags in dependencies that then cause me to have broken Rubies.) My comment (macports/macports-ports#28748 (comment)) which triggered this PR from @baarde is specifically because:
IMO, homebrew should probably be the last choice for ruby-build when there are other options. |
I think this is subjective. In that case I think using either the library from Homebrew or MacPorts (or in general from one of the package managers where that package is installed) is correct. I'm pretty sure many people prefer Homebrew to MacPorts, and some might have both installed (though that's clearly a recipe for troubles).
That I understand would be better, to consult However, I have found that using
For these reasons I'm wary to use
TruffleRuby explicitly looks for Homebrew and MacPorts, documented here It should work, as it tries MacPorts if it doesn't find it in Homebrew. IMO these complications mostly stem from the fact macOS doesn't have a proper package manager where libraries are just found without extra config. On Linux it tends to just work because there is typically one system package manager and stuff just ends up in I like @mislav's suggestion in #2547 (review). |
|
I don't agree with your assessment that developers would prefer homebrew over MacPorts of both are installed—as I said earlier the problem is that Homebrew tends to move the files around when they decide that they're no longer where they think they should be. MacPorts (default install) is always in I'm not on my home computer to be able to assess the rest of what you said at this point, so this comment might be updated. |
When pkg-config is installed and finds OpenSSL, use that (i.e. do nothing) rather than relying on Homebrew or bundling OpenSSL.
Links to the Cellar break when packages are upgraded.
|
Hi, thank you for the contribution and for simplifying the diff like I asked! I merged this after making some tweaks of my own, namely:
|
| prefix="$(pkg-config --variable=prefix "$1" 2>/dev/null || true)" | ||
| [ -n "$prefix" ] || return 1 | ||
| brew_prefix="$(brew --prefix 2>/dev/null || true)" | ||
| if [[ -n $prefix_prefix && ( $prefix == "$brew_prefix"/Cellar/* || \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
$prefix_prefix -> $brew_prefix probably?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I do not understand?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| if [[ -n $prefix_prefix && ( $prefix == "$brew_prefix"/Cellar/* || \ | |
| if [[ -n $brew_prefix && ( $prefix == "$brew_prefix"/Cellar/* || \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, copy-paste mistake, I'll update my comment above for posterity.
But yes, what @halostatue showed (I tried to find the suggestion thing but wasn't available in the commit/diff view for some reason).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, thanks both! 🤦
When pkg-config is installed and finds the libraries, use those rather than relying on Homebrew or bundling OpenSSL.
Fixes #2544
A few comments about package managers
/opt/homebrew/Cellar/. However, those have their version number in the path, so I decided to ignore any library in*/Cellar/*to avoid breaking Ruby when runningbrew upgrade.Alternatives considered
Skip
--with-xyz-dir=pkg-config is used internally by Ruby build scripts. So my initial idea was to skip the
--with-xyz-dir=option when a library is discovered by pkg-config and let the build scripts do their job. However:--with-xyz-dir=options appear in the output is helpful to make sure that the correct libraries are used.Do nothing
My goal was to write a simple fix to have ruby-build work well with MacPorts (and other package managers). But doing so without breaking things turned out a little more complex than anticipated. Maybe it's just not worth it.