Skip to content
This repository was archived by the owner on May 30, 2024. It is now read-only.

Avoid mutating the environment with RUBYOPT#429

Merged
vinistock merged 2 commits intomainfrom
vs/avoid_mutating_environment_with_yjit
Mar 8, 2023
Merged

Avoid mutating the environment with RUBYOPT#429
vinistock merged 2 commits intomainfrom
vs/avoid_mutating_environment_with_yjit

Conversation

@vinistock
Copy link
Copy Markdown
Member

If we simply assign env = process.env, then when we change keys in env we're mutating process.env as well - which leaks across different extensions.

This means that we append RUBYOPT=--yjit in the process environment and it ends up applying to other extensions that spawn Ruby processes, which may lead to issues.

The fix is to simply create a clone of the environment, which we can do by destructuring it inside a new object.

@vinistock vinistock added the bugfix This PR will fix an existing bug label Mar 7, 2023
@vinistock vinistock requested a review from a team as a code owner March 7, 2023 21:13
@vinistock vinistock self-assigned this Mar 7, 2023
Comment thread package.json
"displayName": "Ruby LSP",
"description": "VS Code plugin for connecting with the Ruby LSP",
"version": "0.1.3",
"version": "0.1.4",
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.

I'm assuming the version bump is intentional. Maybe could be in its own commit.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Fair, I split the commits. Yeah, it's intentional to get a new version with the fix out there.

@vinistock vinistock force-pushed the vs/avoid_mutating_environment_with_yjit branch from 7552df5 to 586818d Compare March 7, 2023 21:27
@vinistock vinistock merged commit f714401 into main Mar 8, 2023
@vinistock vinistock deleted the vs/avoid_mutating_environment_with_yjit branch March 8, 2023 15:32
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

bugfix This PR will fix an existing bug

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants