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

Prevent passing version to bin/bundle #2765

Merged
merged 1 commit into from
Oct 24, 2024

Conversation

vinistock
Copy link
Member

Motivation

Closes #2731

If the user has their local bin directory as part of the PATH and a bin/bundle binstub exists, we accidentally invoke the binstub rather than invoking bundle exec.

This is problematic because the binstub doesn't accept the same arguments as the bundle executable. Namely, we can't pass the version using _2.5.12_ to ensure that the locked Bundler version is used and no restarts happen.

I'd argue that overriding the bundle executable with something that doesn't support the same arguments is not great as it prevents any tools from invoking Bundler directly, which is what they may legitimately want to do (this is the Ruby LSP's case).

However, it appears that this affects a considerable number of users, so let's put in a fix to prevent this error from happening.

Implementation

If there's a bin/bundle and any parts of the path are pointing to the local bin directory, then we favour using the binstub with no version.

Otherwise, we continue doing the same as before. I'm hesitant to expand this to always use the binstub if present since people could have modified their binstubs and that may lead to other errors.

Automated Tests

I added two new tests.

@vinistock vinistock added bugfix This PR will fix an existing bug server This pull request should be included in the server gem's release notes labels Oct 23, 2024
@vinistock vinistock self-assigned this Oct 23, 2024
@vinistock vinistock requested a review from a team as a code owner October 23, 2024 18:18
@vinistock vinistock force-pushed the vs-prevent-accidental-usage-of-binstub branch 2 times, most recently from c00b6c8 to 778aa14 Compare October 23, 2024 18:34
test/integration_test.rb Outdated Show resolved Hide resolved
test/integration_test.rb Outdated Show resolved Hide resolved
@vinistock vinistock force-pushed the vs-prevent-accidental-usage-of-binstub branch from 778aa14 to 122ee64 Compare October 23, 2024 20:10
@vinistock vinistock requested a review from andyw8 October 23, 2024 20:11
test/integration_test.rb Outdated Show resolved Hide resolved
@vinistock vinistock force-pushed the vs-prevent-accidental-usage-of-binstub branch from 122ee64 to 35a4263 Compare October 24, 2024 13:37
@vinistock vinistock enabled auto-merge (squash) October 24, 2024 13:40
@vinistock vinistock merged commit 2f2f87d into main Oct 24, 2024
37 checks passed
@vinistock vinistock deleted the vs-prevent-accidental-usage-of-binstub branch October 24, 2024 14:00
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 server This pull request should be included in the server gem's release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

LSP fails to start: Could not find command "_2.5.22_".
3 participants