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

LoadedFeaturesIndex#register: Ignore absolute paths entirely #385

Merged
merged 1 commit into from Jan 10, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 4 additions & 0 deletions CHANGELOG.md
@@ -1,5 +1,9 @@
# Unreleased

* Ignore absolute paths in the loaded feature index. (#385)
This fixes a compatibility issue with Zeitwerk when Zeitwerk is loaded before bootsnap. It also should
reduce the memory usage and improve load performance of Zeitwerk managed files.

* Automatically invalidate the load path cache whenever the Ruby version change. (#387)
This is to avoid issues in case the same installation path is re-used for subsequent ruby patch releases.

Expand Down
10 changes: 10 additions & 0 deletions lib/bootsnap.rb
Expand Up @@ -123,4 +123,14 @@ def self.default_setup
end
end
end

if RbConfig::CONFIG['host_os'] =~ /mswin|mingw|cygwin/
def self.absolute_path?(path)
path[1] == ':'
end
else
def self.absolute_path?(path)
path.start_with?('/')
end
end
end
12 changes: 1 addition & 11 deletions lib/bootsnap/load_path_cache/cache.rb
Expand Up @@ -48,7 +48,7 @@ def find(feature, try_extensions: true)
reinitialize if (@has_relative_paths && dir_changed?) || stale?
feature = feature.to_s.freeze

return feature if absolute_path?(feature)
return feature if Bootsnap.absolute_path?(feature)

if feature.start_with?('./', '../')
return try_extensions ? expand_path(feature) : File.expand_path(feature).freeze
Expand Down Expand Up @@ -98,16 +98,6 @@ def find(feature, try_extensions: true)
raise(LoadPathCache::FallbackScan, '', []) if @development_mode
end

if RbConfig::CONFIG['host_os'] =~ /mswin|mingw|cygwin/
def absolute_path?(path)
path[1] == ':'
end
else
def absolute_path?(path)
path.start_with?(SLASH)
end
end

def unshift_paths(sender, *paths)
return unless sender == @path_obj
@mutex.synchronize { unshift_paths_locked(*paths) }
Expand Down
5 changes: 5 additions & 0 deletions lib/bootsnap/load_path_cache/loaded_features_index.rb
Expand Up @@ -83,6 +83,11 @@ def key?(feature)
# 2. Inspect $LOADED_FEATURES upon return from yield to find the matching
# entry.
def register(short, long = nil)
# Absolute paths are not a concern.
if Bootsnap.absolute_path?(short.to_s)
return yield
end

if long.nil?
len = $LOADED_FEATURES.size
ret = yield
Expand Down
14 changes: 13 additions & 1 deletion test/load_path_cache/loaded_features_index_test.rb
Expand Up @@ -136,11 +136,23 @@ def test_derives_initial_state_from_loaded_features
end

def test_works_with_pathname
path = '/tmp/bundler.rb'
path = 'bundler.rb'
pathname = Pathname.new(path)
@index.register(pathname, path) { true }
assert(@index.key?(pathname))
end

def test_ignores_absolute_paths
path = "#{Dir.mktmpdir}/bundler.rb"
@index.register(path) { true }
refute(@index.key?(path))

path = "#{Dir.mktmpdir}/bundler.rb"
pathname = Pathname.new(path)
@index.register(pathname, path) { true }
refute(@index.key?(path))
refute(@index.key?(pathname))
end
end
end
end