Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion rake_tasks/ruby.rake
Original file line number Diff line number Diff line change
Expand Up @@ -187,8 +187,10 @@ task :lint do |_task, arguments|
)
end

desc 'Sync gem checksums from Gemfile.lock to MODULE.bazel (use force to re-download all)'
desc 'Reconcile Gemfile.lock and sync gem checksums to MODULE.bazel (use force to re-download all)'
task :pin, [:force] do |_task, arguments|
Bazel.execute('run', [], '//rb:bundle-lock')

Comment thread
titusfortner marked this conversation as resolved.
Comment on lines +190 to +193

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Remediation recommended

1. Bundler runs twice in update 🐞 Bug ➹ Performance

rb:update runs //rb:bundle-update and then invokes rb:pin, which now always runs
//rb:bundle-lock, causing an extra Bundler invocation on every update and potentially rewriting
rb/Gemfile.lock a second time. This increases runtime and can introduce non-essential lockfile
churn in the upgrade workflow.
Agent Prompt
### Issue description
`rb:update` already runs Bundler (`//rb:bundle-update`) and then calls `rb:pin`, which now runs Bundler again (`//rb:bundle-lock`). This adds unnecessary work and may produce extra lockfile churn.

### Issue Context
- `rb:pin` now executes `//rb:bundle-lock` unconditionally.
- `rb:update` executes `//rb:bundle-update` and then invokes `rb:pin`.

### Fix Focus Areas
- rake_tasks/ruby.rake[190-193]
- rake_tasks/ruby.rake[248-254]

### Suggested fix
Introduce a way for `rb:update` to reuse the already-updated lockfile without re-running `bundle lock` (e.g., split checksum syncing into its own task, or add an optional `skip_lock` argument to `rb:pin` and use it from `rb:update`).

β“˜ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

gemfile_lock = 'rb/Gemfile.lock'
module_bazel = 'MODULE.bazel'
force = arguments[:force] == 'force'
Expand Down
19 changes: 17 additions & 2 deletions rb/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -177,16 +177,31 @@ rb_binary(
],
)

rb_binary(
name = "bundle-lock",
args = [
"rb/support/bundle.rb",
"lock",
],
data = [
"Gemfile",
],
deps = [
"//rb/support:bundle",
],
)

rb_binary(
name = "bundle-update",
args = [
"rb/support/bundle_update.rb",
"rb/support/bundle.rb",
"update",
],
data = [
"Gemfile",
],
deps = [
"//rb/support:bundle_update",
"//rb/support:bundle",
],
)

Expand Down
4 changes: 2 additions & 2 deletions rb/support/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,8 @@ rb_library(
)

rb_library(
name = "bundle_update",
srcs = ["bundle_update.rb"],
name = "bundle",
srcs = ["bundle.rb"],
)

rb_library(
Expand Down
4 changes: 3 additions & 1 deletion rb/support/bundle_update.rb β†’ rb/support/bundle.rb
Original file line number Diff line number Diff line change
Expand Up @@ -35,5 +35,7 @@

FileUtils.mkdir_p(ENV["BUNDLE_PATH"])

# The bundle subcommand (e.g. "lock" or "update") and any flags are supplied by
# the caller so a single script backs both //rb:bundle-lock and //rb:bundle-update.
ruby = RbConfig.ruby
exec ruby, "-S", "bundle", "update", *ARGV
exec ruby, "-S", "bundle", *ARGV
Comment on lines +38 to +41

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Informational

2. No bundle subcommand check 🐞 Bug ☼ Reliability

rb/support/bundle.rb now forwards caller-supplied ARGV directly to bundle without validating a
subcommand is present, so a misconfigured Bazel target or direct invocation can trigger Bundler’s
default/help behavior with unclear diagnostics. Adding a small guard makes failures deterministic
and easier to debug.
Agent Prompt
### Issue description
`rb/support/bundle.rb` now executes `bundle` with `*ARGV` and does not enforce that `ARGV[0]` is a supported subcommand (e.g., `lock` or `update`). If invoked without args (or with a typo), behavior/exit codes are delegated to Bundler and can be confusing.

### Issue Context
This wrapper is intended to be shared by multiple Bazel entrypoints (`//rb:bundle-lock`, `//rb:bundle-update`). Those currently supply arguments correctly, but adding validation prevents future misconfiguration.

### Fix Focus Areas
- rb/support/bundle.rb[38-41]

### Suggested fix
Add a guard like:
- if `ARGV.empty?`, print usage and exit non-zero
- optionally validate `ARGV.first` is in an allowlist (`lock`, `update`) before calling `exec`

β“˜ Copy this prompt and use it to remediate the issue with your preferred AI generation tools