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

feat: introduce rb_bundle_fetch() repository rule #48

Merged
merged 2 commits into from
Jan 15, 2024

Conversation

p0deje
Copy link
Member

@p0deje p0deje commented Dec 9, 2023

This repository rule deprecates existing rb_bundle() and provides a
number of benefits over existing implementation:

  1. Hermeticity of bundle install - it's run as a regular rule.
  2. Ruby toolchain no longer needs to be installed during WORKSPACE
    loading.
  3. Downloading of gems is taken care of by Bazel. This also means that
    gems are stored in a repository cache.
  4. RBE is potentially supported (at least with JRuby).

The new rule however is not complete yet and lacks few important
features that will be added later.

  1. Support for gems installed from Git repositories.
  2. Support for checksums introduced in Bundler 2.50.

For most cases, the migration from rb_bundle() to rb_bundle_fetch()
is straightforward - use the same parameters with an extra gemfile_lock.
The main breaking change is that your targets that used to depend on
Bundler binstubs a new bin package. For example:

rb_bundle() rb_bundle_fetch
@bundle//:bin/rspec @bundle//bin:rspec

Fixes #16 and #45.

UPD: I just made Selenium CI pass using rb_bundle_fetch() so IMO this is good enough to merge and improve over time - SeleniumHQ/selenium#13422.

@p0deje p0deje force-pushed the bundle-fetch-attempt-304 branch 2 times, most recently from ffce7f1 to 79d5bc6 Compare January 9, 2024 21:34
@p0deje p0deje changed the title Bundle fetch attempt 304 feat: introduce rb_bundle_fetch() repository rule Jan 9, 2024
@p0deje p0deje marked this pull request as ready for review January 9, 2024 21:52
@p0deje p0deje requested a review from alexeagle January 9, 2024 23:47
Copy link
Collaborator

@alexeagle alexeagle left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Generally looks pretty great!

@p0deje p0deje force-pushed the bundle-fetch-attempt-304 branch 2 times, most recently from 966800e to 99320aa Compare January 13, 2024 16:35
@p0deje p0deje force-pushed the bundle-fetch-attempt-304 branch 4 times, most recently from 3288c92 to fb005f4 Compare January 13, 2024 17:40
"""
Parses a Gemfile.lock purely in Starlark.

Largely based on https://github.com/sushain97/rules_ruby/blob/master/tools/ruby/gemfile_parser.bzl (private).
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks @sushain97 !

@@ -0,0 +1,191 @@
"""
Parses a Gemfile.lock purely in Starlark.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

did we also inherit some unit tests for this?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, there were no unit tests.

This repository rule deprecates existing `rb_bundle()` and provides a
number of benefits over existing implementation:

1. Hermeticity of `bundle install` - it's run as a regular rule.
2. Ruby toolchain no longer needs to be installed during WORKSPACE
   loading.
2. Downloading of gems is taken care of by Bazel. This also means that
   gems are stored in a repository cache.
3. RBE is potentially supported (at least with JRuby).

The new rule however is not complete yet and lacks few important
features that will be added later.

1. Support for gems installed from Git repositories.
2. Support for checksums introduced in Bundler 2.50.

For most cases, the migration from `rb_bundle()` to `rb_bundle_fetch()`
is straightforward - use the same parameters with an extra `gemfile_lock`.
The main breaking change is that your targets that used to depend on
Bundler binstubs a new `bin` package. For example:

| rb_bundle()         | rb_bundle_fetch    |
|---------------------|--------------------|
| @bundle//:bin/rspec | @bundle//bin:rspec |
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.

bundler downloads gem dependencies rather than Bazel downloader
2 participants