Skip to content

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 |
@p0deje p0deje force-pushed the bundle-fetch-attempt-304 branch from 46eb1ee to 47a3a51 Compare January 15, 2024 17:03
@p0deje p0deje merged commit f68794b into main Jan 15, 2024
@p0deje p0deje deleted the bundle-fetch-attempt-304 branch January 15, 2024 17:27
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

3 participants