-
Notifications
You must be signed in to change notification settings - Fork 85
Correctly patch host ruby 3.2.3 and 3.2.4 when cross compiling #847
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
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
c42b749 to
433e811
Compare
Previously, we were installing the ffi gem twice, but with different arguments,
once in the base-rubygem component and again in the rubygem-ffi component. For
example:
/opt/puppetlabs/puppet/bin/gem install --no-document --local ffi-1.15.5.gem && \
/opt/puppetlabs/puppet/bin/gem install --no-document --local ffi-1.15.5.gem -- --enable-system-ffi
Building native extensions. This could take a while...
Successfully installed ffi-1.15.5
1 gem installed
Building native extensions with: '--enable-system-ffi'
This could take a while...
Successfully installed ffi-1.15.5
1 gem installed
Now we only execute the gem install command once. On Windows, we install ffi
gems with precompiled binaries (so we don't need to specify enable/disable
flags). On non-Windows, we continue to pass enable/disable based on the ruby
version.
The ffi gem depends on the libffi native library. When installing the ffi gem, we need to specify the name of the library in the enable/disable-system-libffi option.
Previously, our rubygems patch wasn't being applied when cross compiling mac 12
ARM, because homebrew updated to ruby 3.2.4 and builder.rb was modified:
- cmd = Gem.ruby.shellsplit
+ cmd = Shellwords.split(Gem.ruby)
The code for patching is very fragile and has broken multiple times without any
indication that the patch failed to apply, causing the resulting native
extensions to be compiled for the wrong architecture.
The code was also conflating the `settings[:ruby_version]` in our ruby component
with the host's ruby version (the one we're actually trying to patch). Also the
version that matters is the rubygems version (Gem::VERSION), not RUBY_VERSION.
For example, if `gem update --system` was executed during provisioning, then
our patch would fail to apply.
This commit creates a script that runs on the remote system and so can
introspect the host's ruby and rubygems versions. This way if brew updates to a
newer ruby/rubygems, we can fail the build.
…2 ARM Now that we're patching homebrew's ruby 3.2.4 correctly, we can remove our workaround and use platform=ruby as suggested in https://nokogiri.org/tutorials/installing_nokogiri.html#installing-using-standard-system-libraries Note --platform is an option to `gem install` so it comes before double dashes.
Settings are shared across all components, so `settings[:gem_install_options]`
in the ffi gem caused its options to be passed to gems that follow like gssapi
in the bolt-runtime:
/opt/puppetlabs/bolt/bin/gem install --no-document --local --bindir=/opt/puppetlabs/bolt/bin gssapi-1.3.1.gem -- --disable-system-libffi
This commit namespaces the options so they only apply when installing that
specific gem. The namespace is based on the full gem name like `rubygems-ffi`,
not the short name `ffi`.
433e811 to
9c19360
Compare
Collaborator
Author
agent-runtime-mainmacOS 12 ARMSolaris 11 SPARCagent-runtime-7.xmacOS 12 ARMSolaris 11 SPARCSolaris 10 SPARC |
cthorn42
approved these changes
May 14, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
https://builds.delivery.puppetlabs.net/puppet-runtime/9c19360894d2b776aea2bea9c524dbf848be706b/artifacts
Tested builds using https://gist.github.com/joshcooper/1b183a2cf11c461c3506b5d2b803555e The output for cross-compiled builds is in a comment below.
mac 11 ARM *(not supported)