-
Notifications
You must be signed in to change notification settings - Fork 797
Build libyaml from source when Homebrew/FreeBSD packages unavailable #2595
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
base: master
Are you sure you want to change the base?
Conversation
When brew is not in PATH, use_homebrew_yaml fails silently and Ruby compiles without libyaml/psych support. This adds a fallback that downloads and builds libyaml from source, similar to how OpenSSL is handled. Changes: - Add build_package_yaml to install libyaml to PREFIX_PATH/libyaml - Add fetch_libyaml_latest_version to get latest version from GitHub API with fallback to 0.2.5 - Add build_yaml_from_source as fallback in the yaml detection chain - Fix use_freebsd_yaml to return failure when not on FreeBSD
|
This builds That would be a regression on Linux, e.g. OpenSSL is not built on Linux if already available on the system. BTW in that scenario psych links to the system libyaml: which e.g. could break if the versions differ. Probably the simplest & safest for now is to |
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 for the contribution.
On top of everything @eregon already pointed out, I would also like to note that within the ruby-build project, we prefer that the declaration of dependencies is explicit in the ruby definition itself, for example how we did with libyaml up to Ruby 2.1.0:
ruby-build/share/ruby-build/2.1.0
Line 1 in 079b5af
| install_package "yaml-0.1.6" "http://pyyaml.org/download/libyaml/yaml-0.1.6.tar.gz#7da6971b4bd08a986dd2a61353bc422362bd0edcc67d7ebaac68c95f74182749" --if needs_yaml |
Starting from Ruby 2.2, we stopped using the needs_yaml declaration in ruby formulae, although my memory is fuzzy and I'm not sure why. It might have something to do with the psych gem bundling its own libyaml at some point? /cc @hsbt d646595
In any case, if we added the needs_yaml dependency back to Ruby definitions, we would need to make sure that libyaml would not be downloaded and compiled on systems that already have libyaml already present and discoverable in the build path. This last part is tricky since discovering system libraries isn't exactly straightforward. For example, we try to only download and build OpenSSL when it's not present on the system or when the system version is incompatible, but our detection approach for openssl is somewhat complex and cannot be easily extended to libyaml.
|
Based on https://github.com/ruby/psych/blob/master/ext/psych/extconf.rb, I would be down with something like the following. First, adding this directive to modern (i.e. non EOL'd) Ruby definitions: install_package "yaml-0.2.5" "https://github.com/yaml/libyaml/archive/refs/tags/0.2.5.tar.gz#SHA" yaml --if needs_yamlThen, redefining
And finally, defining a build_package_yaml() {
local YAML_PREFIX_PATH="${PREFIX_PATH}/libyaml"
package_option ruby configure --with-libyaml-dir="$YAML_PREFIX_PATH"
package_option yaml configure --disable-dependency-tracking
capture_command ./bootstrap
build_package_standard "$@"
} |
Yes, psych used to vendor libyaml, but then more recently no longer does that and instead has the |
This should not happen, because of Lines 711 to 714 in 079b5af
Did this break in CRuby or does this work as expected and ruby-build fails in that case? |
|
For context, ruby-build in general does not install dependencies itself, instead they are documented here. The list is long so I'm unsure if just handling libyaml is a big help. ruby-build or CRuby should definitely fail early if libyaml is missing though. |
I tried on my machine by removing From that, I think keeping things as they are is best, it's already a clear error and AFAIK it's not hard to install libyaml (unlike OpenSSL which is messy due to Ruby needing very specific versions). |
Summary
When
brewis not inPATH,use_homebrew_yamlfails silently and Ruby compiles without libyaml/psych support.This PR adds a fallback that downloads and builds libyaml from source when brew isn't in the path, similar to how OpenSSL is already handled.
Changes
build_package_yamlto install libyaml to$PREFIX_PATH/libyamlfetch_libyaml_latest_versionto get latest version from GitHub API (falls back to 0.2.5)build_yaml_from_sourceas fallback in the yaml detection chainuse_freebsd_yamlto return failure when not on FreeBSD (was silently succeeding)After