Skip to content

Fix LoadError on Windows when loading RuboCop from symlinks#12062

Merged
bbatsov merged 1 commit intorubocop:masterfrom
p0deje:patch-1
Jul 25, 2023
Merged

Fix LoadError on Windows when loading RuboCop from symlinks#12062
bbatsov merged 1 commit intorubocop:masterfrom
p0deje:patch-1

Conversation

@p0deje
Copy link
Copy Markdown
Contributor

@p0deje p0deje commented Jul 22, 2023

There is a particular edge case when RuboCop is loaded from a fully symlinked environment. This is the default for Bazel which creates a tree of symlinks for every dependency, mimicking the real file tree structure.

In such a setup, RuboCop would fail to load with an error. Resolving the path to an absolute one fixes it.

No such file or directory - //?/D:/_bazel/external/bundle/ruby/3.0.0/gems/rubocop-1.36.0/lib/rubocop/../../exe
...
//?/D:/_bazel/external/bundle/ruby/3.0.0/gems/rubocop-1.36.0/lib/rubocop/result_cache.rb:208:in `each'

I am not sure what is the best way to write a test for this, I could use some guidance.


Before submitting the PR make sure the following are checked:

  • The PR relates to only one subject with a clear title and description in grammatically correct, complete sentences.
  • Wrote good commit messages.
  • Commit message starts with [Fix #issue-number] (if the related issue exists).
  • Feature branch is up-to-date with master (if not - rebase it).
  • Squashed related commits together.
  • Added tests.
  • Ran bundle exec rake default. It executes all tests and runs RuboCop on its own code.
  • Added an entry (file) to the changelog folder named {change_type}_{change_description}.md if the new code introduces user-observable changes. See changelog entry format for details.

There is a particular edge case when RuboCop is loaded from a fully symlinked environment. This is the default for Bazel which creates a tree of symlinks for every dependency, mimicking the real file tree structure.

In such a setup, RuboCop would fail to load with an error. Resolving the path to an absolute one fixes it.

```
No such file or directory - //?/D:/_bazel/external/bundle/ruby/3.0.0/gems/rubocop-1.36.0/lib/rubocop/../../exe
...
//?/D:/_bazel/external/bundle/ruby/3.0.0/gems/rubocop-1.36.0/lib/rubocop/result_cache.rb:208:in `each'
```
@bbatsov
Copy link
Copy Markdown
Collaborator

bbatsov commented Jul 24, 2023

This is the default for Bazel which creates a tree of symlinks for every dependency, mimicking the real file tree structure.

Does it work differently on Unix-like OSes? I'm just trying to understand if this is truly Windows-specific.

@p0deje
Copy link
Copy Markdown
Contributor Author

p0deje commented Jul 24, 2023

Does it work differently on Unix-like OSes? I'm just trying to understand if this is truly Windows-specific.

It works the same way on Unix-like OSes, but the issue with Windows is that its symlink support is way different - as far as I know there is no real symlinks at all. You can see the //? prefix in the path which denotes this is a UNC path - I suppose that's what is being used internally.

I wish I could find a way configure Windows to avoid these problems rather than patching gems. If you are not willing to merge the PR, I can totally understand that.

@bbatsov bbatsov merged commit d379d8e into rubocop:master Jul 25, 2023
@bbatsov
Copy link
Copy Markdown
Collaborator

bbatsov commented Jul 25, 2023

The patch seems harmless enough and it does solve a real problem. I'll merge it as is, given that adding tests for this would be quite complicated.

@bbatsov
Copy link
Copy Markdown
Collaborator

bbatsov commented Jul 25, 2023

Ops, just noticed you forgot to add a changelog entry for this.

@p0deje p0deje deleted the patch-1 branch July 25, 2023 14:35
p0deje added a commit to p0deje/rubocop that referenced this pull request Jul 25, 2023
@p0deje p0deje mentioned this pull request Jul 25, 2023
8 tasks
koic added a commit that referenced this pull request Jul 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants