Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Ref: #402
Ref: https://bugs.ruby-lang.org/issues/10222
Ref: ruby/ruby@5754f15
Ref: ruby/ruby@b6d3927
Up to Ruby 2.3,
require
would resolve symlinks, butrequire_relative
wouldn't:This would easily cause double loading issue when
require
andrequire_relative
were mixed, but was fixed in 2.4 (https://bugs.ruby-lang.org/issues/10222).The problem is that
Bootsnap
kinda negated this fix, becauserealpath()
wouldn't be applied to absolute paths:And for performance reasons, Bootsnap tried really hard not to call
realpath
, as it's a syscall, instead it usedexpand_path
, which is entirely in use space and doesn't reach to the file system. So if you had asymlink
in$LOAD_PATH
,bootsnap
would perpetuate this bug, which led to the addition of #136.This was ultimately fixed in Ruby 3.1 (https://bugs.ruby-lang.org/issues/17885), now
realpath
is applied even on absolute paths.Solution
While
realpath
is indeed expensive, I think the performance impact is ok if we only call it for$LOAD_PATH
members, rather than for all requirable files. So if you have X gems, it's going to be more or less Xrealpath
calls.It would stay a problem if a gem actually contained symlinks and used
require_relative
, but it's quite the stretch, and with 3.1 now handling it, it's not worth keeping such workaround.#402