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

Avoid requiring json for chruby activation #2430

Merged
merged 1 commit into from
Aug 13, 2024

Conversation

vinistock
Copy link
Member

Motivation

There's an annoying bug in our chruby activation because of the dependency on the json default gem.

These are the steps to reproduce it:

  1. Ensure you are using Ruby 3.3.4 (or any Ruby with a patch higher than zero)
  2. Chruby overrides the GEM_HOME to use ~/.gem/ruby/3.3.4 instead of the default ~/.gem/ruby/3.3.0. We perform this correction inside our activation script (in Ruby)
  3. However, the -rjson to require json will also require strscan. This happens before we had a chance to activate the chruby environment, which means it will try to require strscan with a GEM_HOME still pointing to ~/.gem/ruby/3.3.0, even though the Ruby version is 3.3.4 and chruby would've pointed us to ~/.gem/ruby/3.3.4
  4. This leads to the linked to incompatible Ruby version error and breaks the activation

Implementation

I can't think of another way for fixing this that isn't removing the json dependency. The approach that I took is quite simple: I started printing a separator string, so that we can easily break the pieces that we need from the activation script's result.

Automated Tests

It's not very trivial to write tests for this specific scenario because it would involve installing more than one Ruby version.

@vinistock vinistock added bugfix This PR will fix an existing bug vscode This pull request should be included in the VS Code extension's release notes labels Aug 12, 2024
@vinistock vinistock self-assigned this Aug 12, 2024
@vinistock vinistock requested a review from a team as a code owner August 12, 2024 20:42
@vinistock vinistock requested review from andyw8 and st0012 August 12, 2024 20:42
vscode/src/ruby/chruby.ts Outdated Show resolved Hide resolved
@vinistock vinistock force-pushed the vs-avoid-requiring-json-for-chruby branch from 6f7bd11 to 53dfb04 Compare August 13, 2024 13:31
@vinistock vinistock enabled auto-merge (squash) August 13, 2024 13:32
@vinistock vinistock merged commit c7d0edb into main Aug 13, 2024
34 checks passed
@vinistock vinistock deleted the vs-avoid-requiring-json-for-chruby branch August 13, 2024 13:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bugfix This PR will fix an existing bug vscode This pull request should be included in the VS Code extension's release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants