Skip to content

Conversation

titusfortner
Copy link
Member

@titusfortner titusfortner commented Oct 18, 2025

User description

💥 What does this PR do?

I got tired of my ruby versions being out of sync, and I kept forgetting where bazel was hiding the ruby binary.

Now, when you run ./go rb:local_dev, in addition to building the gems and the grid, if you have rbenv installed, it will create a symlink to the bazel installed ruby and call it selenium-ruby
If you also have direnv installed, it will execute off of the new .envrc to set $RBENV_VERSION to selenium-ruby to override what is in .ruby-version file. There is a conditional in .envrc that says not to do anything if the rbenv symlink isn't there, so it shouldn't get in anyone's way.

You still need to select this in Rubymine, but you can go from the rbenv directory symlink instead of bazel nested confusion.

So far this seems to work pretty well for me, but it may be overkill. What do you think?


PR Type

Enhancement


Description

  • Automates Ruby version synchronization between rbenv, RubyMine, and Bazel

  • Creates symlink to Bazel-built Ruby in rbenv versions directory

  • Adds .envrc configuration for direnv integration with RBENV_VERSION

  • Simplifies local development setup by eliminating manual version management


Diagram Walkthrough

flowchart LR
  A["rb:local_dev task"] --> B["Build Bazel Ruby"]
  B --> C["manage_selenium_ruby!"]
  C --> D["Create rbenv symlink"]
  D --> E["Configure direnv"]
  E --> F["Install bundles with correct version"]
  G[".envrc file"] --> H["Detect rbenv installation"]
  H --> I["Set RBENV_VERSION if symlink exists"]
Loading

File Walkthrough

Relevant files
Enhancement
Rakefile
Add Ruby version management automation                                     

Rakefile

  • Added manage_selenium_ruby! method to automate Ruby version setup
  • Creates symlink from Bazel-built Ruby to rbenv versions directory
  • Integrates direnv to set RBENV_VERSION environment variable
  • Builds rubocop dependency and handles errors gracefully
+26/-0   
Configuration changes
.envrc
Add direnv configuration for Ruby environment                       

rb/.envrc

  • New direnv configuration file for local Ruby environment
  • Detects rbenv installation and validates symlink existence
  • Sets RBENV_VERSION to selenium-ruby when symlink is available
  • Watches Ruby binary for changes and safely skips if rbenv unavailable
+18/-0   

@titusfortner titusfortner requested review from aguspe and p0deje October 18, 2025 02:29
@selenium-ci selenium-ci added C-rb Ruby Bindings B-build Includes scripting, bazel and CI integrations labels Oct 18, 2025
Copy link
Contributor

qodo-merge-pro bot commented Oct 18, 2025

PR Compliance Guide 🔍

Below is a summary of compliance checks for this PR:

Security Compliance
Command injection risk

Description: The code shells out to rbenv and runs direnv allow/direnv reload and bundle install via
system calls without validating paths or sanitizing environment, which could execute
unintended commands if PATH or rbenv is manipulated.
Rakefile [761-776]

Referred Code
rbenv_path = `command -v rbenv`.strip
rbenv_root = `#{rbenv_path} root`.strip
dest = File.join(rbenv_root, 'versions', label)
return if File.symlink?(dest)

rb_dir = File.join(Dir.pwd, 'rb')
bazel_bin  = File.expand_path('../bazel-selenium/external/rules_ruby++ruby+ruby/dist/bin', rb_dir)
bazel_dist = File.dirname(bazel_bin)
FileUtils.ln_s(bazel_dist, dest)
system(rbenv_path, 'rehash')

Dir.chdir(rb_dir) do
  system('direnv allow')
  system('direnv reload')
  system({ "RBENV_VERSION" => label }, 'bundle', 'install')
end
Untrusted env loading

Description: Automatically running direnv allow may trust and load environment from .envrc files,
potentially enabling malicious environment modifications if the repo or directory is
untrusted.
Rakefile [770-776]

Referred Code
system(rbenv_path, 'rehash')

Dir.chdir(rb_dir) do
  system('direnv allow')
  system('direnv reload')
  system({ "RBENV_VERSION" => label }, 'bundle', 'install')
end
Ticket Compliance
🎫 No ticket provided
- [ ] Create ticket/issue <!-- /create_ticket --create_ticket=true -->

</details></td></tr>
Codebase Duplication Compliance
Codebase context is not defined

Follow the guide to enable codebase context checks.

Custom Compliance
No custom compliance provided

Follow the guide to enable custom compliance check.

  • Update
Compliance status legend 🟢 - Fully Compliant
🟡 - Partial Compliant
🔴 - Not Compliant
⚪ - Requires Further Human Verification
🏷️ - Compliance label

Copy link
Contributor

qodo-merge-pro bot commented Oct 18, 2025

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
High-level
Dynamically query Bazel for Ruby path

The hardcoded path to the Bazel-built Ruby is fragile. It should be replaced
with a dynamic query to Bazel to make the automation more robust against future
changes.

Examples:

Rakefile [767-768]
    bazel_bin  = File.expand_path('../bazel-selenium/external/rules_ruby++ruby+ruby/dist/bin', rb_dir)
    bazel_dist = File.dirname(bazel_bin)

Solution Walkthrough:

Before:

# Rakefile
def manage_selenium_ruby!
  # ...
  rb_dir = File.join(Dir.pwd, 'rb')
  # Path is hardcoded and fragile
  bazel_bin  = File.expand_path('../bazel-selenium/external/rules_ruby++ruby+ruby/dist/bin', rb_dir)
  bazel_dist = File.dirname(bazel_bin)
  FileUtils.ln_s(bazel_dist, dest)
  # ...
end

After:

# Rakefile
def manage_selenium_ruby!
  # ...
  # Dynamically query Bazel for the path to the Ruby distribution.
  # The exact target and command may vary.
  ruby_dist_target = '@rules_ruby_dist//:dist' # Example target
  bazel_dist = `bazel cquery "kind(filegroup, #{ruby_dist_target})" --output=location`.strip
  
  # Or query for the binary directly
  # bazel_bin = `bazel cquery "somepath(@rules_ruby_toolchains//:ruby, ...)" --output=location`.strip
  # bazel_dist = File.dirname(File.dirname(bazel_bin))

  FileUtils.ln_s(bazel_dist, dest)
  # ...
end
Suggestion importance[1-10]: 9

__

Why: The suggestion correctly identifies a critical fragility in the hardcoded path to the Bazel-built Ruby, which is a core part of the PR's logic and is likely to break, thus making the proposed automation unreliable.

High
Possible issue
Handle missing rbenv installation gracefully

Add a check to verify rbenv is installed at the beginning of the
manage_selenium_ruby! method, providing a clear message and exiting gracefully
if it is not found.

Rakefile [758-779]

 def manage_selenium_ruby!
   label = 'selenium-ruby'
 
   rbenv_path = `command -v rbenv`.strip
+  if rbenv_path.empty?
+    puts 'rbenv not found, skipping Ruby version management.'
+    return
+  end
+
   rbenv_root = `#{rbenv_path} root`.strip
   dest = File.join(rbenv_root, 'versions', label)
   return if File.symlink?(dest)
 
   rb_dir = File.join(Dir.pwd, 'rb')
   bazel_bin  = File.expand_path('../bazel-selenium/external/rules_ruby++ruby+ruby/dist/bin', rb_dir)
   bazel_dist = File.dirname(bazel_bin)
   FileUtils.ln_s(bazel_dist, dest)
   system(rbenv_path, 'rehash')
 
   Dir.chdir(rb_dir) do
     system('direnv allow')
     system('direnv reload')
     system({ "RBENV_VERSION" => label }, 'bundle', 'install')
   end
 rescue StandardError => e
   puts "Unable to set Bazel Selenium as Default: #{e.message}"
 end
  • Apply / Chat
Suggestion importance[1-10]: 7

__

Why: The suggestion correctly identifies that the script fails with a cryptic error if rbenv is not installed and proposes adding a check to handle this case gracefully, which significantly improves user experience and script robustness.

Medium
General
Respect custom rbenv root directory

Modify the .envrc script to respect the RBENV_ROOT environment variable when
determining the rbenv root directory, aligning its behavior with rbenv's
standard configuration.

rb/.envrc [4-18]

 if command -v rbenv >/dev/null 2>&1; then
   LABEL=selenium-ruby
 
-  RBENV_ROOT_DEFAULT="$(rbenv root 2>/dev/null || echo "$HOME/.rbenv")"
+  RBENV_ROOT_DEFAULT="${RBENV_ROOT:-$(rbenv root 2>/dev/null || echo "$HOME/.rbenv")}"
   RUBY_SYMLINK="${RBENV_ROOT_DEFAULT}/versions/${LABEL}"
   RUBY_BIN="${RUBY_SYMLINK}/bin/ruby"
 
   # Always track the binary, even if missing now
   watch_file "${RUBY_BIN}"
 
   # Only set environment variable if active
   if [ -x "${RUBY_BIN}" ]; then
     export RBENV_VERSION="${LABEL}"
   fi
 fi
  • Apply / Chat
Suggestion importance[1-10]: 6

__

Why: The suggestion correctly points out that the script does not respect the RBENV_ROOT environment variable, which can cause issues for users with custom rbenv setups. The proposed fix aligns the script's behavior with rbenv's own logic, making it more robust.

Low
Learned
best practice
Check subprocess exit statuses

Check return values of system calls and abort or warn on failure to avoid
continuing after a failed step.

Rakefile [772-776]

 Dir.chdir(rb_dir) do
-  system('direnv allow')
-  system('direnv reload')
-  system({ "RBENV_VERSION" => label }, 'bundle', 'install')
+  unless system('direnv', 'allow')
+    warn 'direnv allow failed; environment may not be loaded'
+  end
+  unless system('direnv', 'reload')
+    warn 'direnv reload failed; environment may not be loaded'
+  end
+  unless system({ "RBENV_VERSION" => label }, 'bundle', 'install')
+    raise 'bundle install failed under RBENV_VERSION=selenium-ruby'
+  end
 end
  • Apply / Chat
Suggestion importance[1-10]: 5

__

Why:
Relevant best practice - Validate inputs and states early and check subprocess results; handle non-zero exits to prevent hidden errors.

Low
  • Update

@titusfortner
Copy link
Member Author

I was really hoping this would make it easy to switch versions, but, well... jruby

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

B-build Includes scripting, bazel and CI integrations C-rb Ruby Bindings Review effort 2/5

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants