[build] Regenerate Ruby lockfile in rb:pin task - #17768
Conversation
rb:pin only synced gem checksums from the existing Gemfile.lock into MODULE.bazel; it never re-ran bundle to update the lockfile itself. Every other language's :pin task (java, py, dotnet, node, rust) does regenerate its lockfile, so rb:pin was the odd one out. This let rb/Gemfile.lock drift after a release: the version-bump commit updates rb/lib/selenium/webdriver/version.rb, but the release workflow's reset-dependencies step (./go rb:pin) left the self-referencing selenium-webdriver path-gem entry pinned to the old released version. Bundler's frozen mode then rejects the mismatch, breaking `bazel run //rb:rubocop` on trunk (e.g. run 29134193142). Run //rb:bundle-update before syncing checksums so rb:pin is self-sufficient, matching the other languages' :pin contract.
PR Summary by QodoRegenerate Ruby Gemfile.lock during rb:pin
AI Description
Diagram
High-Level Assessment
Files changed (1)
|
Code Review by Qodo
Context used✅ Compliance rules (platform):
11 rules 1. Pin upgrades gem versions
|
| desc 'Sync gem checksums from Gemfile.lock to MODULE.bazel (use force to re-download all)' | ||
| task :pin, [:force] do |_task, arguments| | ||
| Bazel.execute('run', [], '//rb:bundle-update') | ||
|
|
||
| gemfile_lock = 'rb/Gemfile.lock' |
There was a problem hiding this comment.
1. No tests for rb:pin 📘 Rule violation ▣ Testability
The PR changes rb:pin behavior by running //rb:bundle-update, but no regression test is added to ensure this contract stays enforced. Without coverage, this bug fix can silently regress and break rb:pin/frozen-bundler workflows again.
Agent Prompt
## Issue description
`rb:pin` now runs `//rb:bundle-update` before syncing checksums, but there is no automated test ensuring this behavior remains true.
## Issue Context
This is a bug fix that changes the behavior/contract of the `rb:pin` tooling task; it should be protected by at least one automated regression test that would fail if the `bundle-update` step were removed or reordered.
## Fix Focus Areas
- rake_tasks/ruby.rake[190-246]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| Bazel.execute('run', [], '//rb:bundle-update') | ||
|
|
There was a problem hiding this comment.
2. Pin upgrades gem versions 🐞 Bug ≡ Correctness
rb:pin now invokes //rb:bundle-update, but that target runs bundle update, which can change locked gem versions (not just regenerate the lockfile metadata) and unexpectedly modify rb/Gemfile.lock and MODULE.bazel when pinning. This is inconsistent with other languages’ pin tasks that regenerate lockfiles without upgrading dependencies and can introduce unintended dependency bumps in automation that runs rb:pin.
Agent Prompt
### Issue description
`rb:pin` now calls `//rb:bundle-update`, which executes `bundle update` and can upgrade dependency versions as a side effect of pinning.
### Issue Context
Other `:pin` tasks (node/dotnet/rust) regenerate lockfiles without upgrading. For Ruby, the desired behavior is to refresh the lockfile (e.g., to pick up local path-gem version changes) without broad dependency upgrades.
### Fix Focus Areas
- rake_tasks/ruby.rake[190-196]
- rb/support/bundle_update.rb[20-39]
- rb/BUILD.bazel[180-191]
### Suggested fix
- Add a separate Bazel runnable (e.g., `//rb:bundle-install` or `//rb:bundle-lock`) that runs `bundle install` (or `bundle lock`) to refresh `Gemfile.lock` without upgrading versions.
- Update `rb:pin` to call that new target instead of `//rb:bundle-update`.
- Keep `//rb:bundle-update` (running `bundle update`) only for the explicit `rb:update` workflow.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| Bazel.execute('run', [], '//rb:bundle-update') | ||
|
|
There was a problem hiding this comment.
3. Bundle-update executed twice 🐞 Bug ➹ Performance
rb:update runs //rb:bundle-update and then invokes rb:pin, which now runs //rb:bundle-update again, doubling the work (and network/CPU) in the update flow. This redundancy can significantly slow down rb:update with no functional benefit.
Agent Prompt
### Issue description
`rb:update` currently runs `//rb:bundle-update` and then calls `rb:pin`, but `rb:pin` now also runs `//rb:bundle-update`, causing duplicate work.
### Issue Context
This redundancy was introduced by adding `//rb:bundle-update` to `rb:pin`.
### Fix Focus Areas
- rake_tasks/ruby.rake[191-193]
- rake_tasks/ruby.rake[248-254]
### Suggested fix
Pick one:
- If `rb:pin` remains responsible for lockfile regeneration, remove the explicit `Bazel.execute('run', [], '//rb:bundle-update')` from `rb:update`.
- Or, if you split responsibilities (recommended): make `rb:pin` run a non-upgrading lockfile refresh (e.g., `bundle install`), and keep `rb:update` as the only place that runs `bundle update`.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
|
@titusfortner does this change make sense? After merging the release PR, I saw in |
💥 What does this PR do?
rb:pin only synced gem checksums from the existing Gemfile.lock into MODULE.bazel; it never re-ran bundle to update the lockfile itself. Every other language's :pin task (java, py, dotnet, node, rust) does regenerate its lockfile, so rb:pin was the odd one out.
This let rb/Gemfile.lock drift after a release: the version-bump commit updates rb/lib/selenium/webdriver/version.rb, but the release workflow's reset-dependencies step (./go rb:pin) left the self-referencing selenium-webdriver path-gem entry pinned to the old released version. Bundler's frozen mode then rejects the mismatch, breaking
bazel run //rb:rubocopon trunk (e.g. run 29134193142).Run //rb:bundle-update before syncing checksums so rb:pin is self-sufficient, matching the other languages' :pin contract.
🤖 AI assistance
💡 Additional Considerations
🔄 Types of changes