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

Avoid need for Base64 gem at runtime #182

Merged
merged 1 commit into from
May 25, 2024

Conversation

Bo98
Copy link
Contributor

@Bo98 Bo98 commented May 21, 2024

This follows a common practice that's been adopted by the majority of the Ruby community: rubocop/rubocop@b2b29da, rack/rack@696ed9e, lostisland/faraday@9487833, octokit/octokit.rb@a787bf4.

The personal reason for my need for this is that bootsnap doesn't seem to work properly on Ruby < 3.4 when using base64, which is preventing me from using json_schemer 2.2.x:

<internal:/ruby/3.3.1/lib/ruby/3.3.0/rubygems/core_ext/kernel_require.rb>:37:in `require': cannot load such file -- /vendor/bundle/ruby/3.3.0/gems/base64-0.2.0/lib/base64.rb (LoadError)
	from <internal:/ruby/3.3.1/lib/ruby/3.3.0/rubygems/core_ext/kernel_require.rb>:37:in `require'
	from /vendor/bundle/ruby/3.3.0/gems/bootsnap-1.18.3/lib/bootsnap/load_path_cache/core_ext/kernel_require.rb:30:in `require'

This is technically a Bundler/Bootsnap problem, but I felt it was worthwhile reducing the dependency tree here anyway.

@Bo98
Copy link
Contributor Author

Bo98 commented May 21, 2024

macos-latest failure on Ruby 2.5 is expected since macos-latest now points to an arm64 runner: https://github.blog/changelog/2024-05-20-actions-upcoming-changes-to-github-hosted-macos-runners/.

Only Ruby >= 2.6 is supported there: ruby/setup-ruby@e8677f0

@davishmcclurg
Copy link
Owner

Hi @Bo98—thanks for opening this!

If I understand correctly, base64 will still be bundled with Ruby in 3.4—though not as a default gem. I'm a bit sad to see code getting more confusing (ie, what does unpack1("m0") mean?) in order to remove a gem dependency that ships with Ruby. Do you know of any discussion around bundled gem use in libraries? Are library authors meant to avoid using them?

This is a pretty small change, though, so I'm ok merging it if it makes it easier to use the library.

Thanks for looking into that Ruby 2.5 failure—I'll clean that up as part of #178.

@Bo98
Copy link
Contributor Author

Bo98 commented May 25, 2024

If I understand correctly, base64 will still be bundled with Ruby in 3.4—though not as a default gem. I'm a bit sad to see code getting more confusing (ie, what does unpack1("m0") mean?) in order to remove a gem dependency that ships with Ruby. Do you know of any discussion around bundled gem use in libraries? Are library authors meant to avoid using them?

Bundled gems are competely ignored with Bundler so the behaviour from Ruby 3.4 is it will be that base64 will be a dependency that is always downloaded from RubyGems. This already happens in some cases on 3.3 and earlier if the default gem doesn't exactly match the latest version of base64 hosted on RubyGems.

I'm OK to close this though if you prefer the extra dependency for readability.

@davishmcclurg
Copy link
Owner

Bundled gems are competely ignored with Bundler so the behaviour from Ruby 3.4 is it will be that base64 will be a dependency that is always downloaded from RubyGems. This already happens in some cases on 3.3 and earlier if the default gem doesn't exactly match the latest version of base64 hosted on RubyGems.

Oh I didn't realize that—thanks for the clarification.

I'm OK to close this though if you prefer the extra dependency for readability.

No, I think this is good if bundler treats it as a regular dependency. I'll merge. Thanks for the contribution!

@davishmcclurg davishmcclurg merged commit b476026 into davishmcclurg:main May 25, 2024
31 of 32 checks passed
@Bo98 Bo98 deleted the base64-runtime branch May 25, 2024 21:56
@davishmcclurg
Copy link
Owner

This has been released in 2.3.0.

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.

2 participants