Skip to content
This repository has been archived by the owner on Apr 14, 2021. It is now read-only.

Make bundle clean clean extension directories #6168

Merged
merged 1 commit into from
Jan 10, 2018
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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 8 additions & 2 deletions lib/bundler/runtime.rb
Original file line number Diff line number Diff line change
Expand Up @@ -163,13 +163,15 @@ def clean(dry_run = false)
gem_dirs = Dir["#{Gem.dir}/gems/*"]
gem_files = Dir["#{Gem.dir}/cache/*.gem"]
gemspec_files = Dir["#{Gem.dir}/specifications/*.gemspec"]
extension_dirs = Dir["#{Gem.dir}/extensions/*/*/*"]
Copy link
Member

Choose a reason for hiding this comment

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

Could the glob pattern be "select directories recursively"?

Such as: Dir["#{Gem.dir}/extensions/*/*/*"] => Dir["#{Gem.dir}/extensions/**/*"]

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We need exactly three levels of nesting. **/* will match the first level as well, i.e

Dir["#{Gem.dir}/extensions/**/*"]

Will match

extensions/x86_64-darwin-17
extensions/x86_64-darwin-17/2.4.0-static
extensions/x86_64-darwin-17/2.4.0-static/very_simple_binary-1.0
extensions/x86_64-darwin-17/2.4.0-static/very_simple_binary-1.0/gem_make.out
extensions/x86_64-darwin-17/2.4.0-static/very_simple_binary-1.0/gem.build_complete
extensions/x86_64-darwin-17/2.4.0-static/very_simple_binary-1.0/very_simple_binary_c.bundle
extensions/x86_64-darwin-17/2.4.0-static/simple_binary-1.0
extensions/x86_64-darwin-17/2.4.0-static/simple_binary-1.0/simple_binary_c.bundle
extensions/x86_64-darwin-17/2.4.0-static/simple_binary-1.0/gem_make.out
extensions/x86_64-darwin-17/2.4.0-static/simple_binary-1.0/gem.build_complete

Whereas

Dir["#{Gem.dir}/extensions/*/*/*"]

Will match

extensions/x86_64-darwin-17/2.4.0-static/very_simple_binary-1.0
extensions/x86_64-darwin-17/2.4.0-static/simple_binary-1.0

We don't want to remove extensions/x86_64-darwin-17, because this directory contains all extensions of all gems.

Please refer to following line for further details:
https://github.com/akhramov/bundler/blob/b016599152bddaf303618ca148ec59f623051893/lib/bundler/runtime.rb#L198

Copy link
Member

Choose a reason for hiding this comment

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

Thanks for the levels-explanation!

spec_gem_paths = []
# need to keep git sources around
spec_git_paths = @definition.spec_git_paths
spec_git_cache_dirs = []
spec_gem_executables = []
spec_cache_paths = []
spec_gemspec_paths = []
spec_extension_paths = []
specs.each do |spec|
spec_gem_paths << spec.full_gem_path
# need to check here in case gems are nested like for the rails git repo
Expand All @@ -181,6 +183,7 @@ def clean(dry_run = false)
end
spec_cache_paths << spec.cache_file
spec_gemspec_paths << spec.spec_file
spec_extension_paths << spec.extension_dir if spec.respond_to?(:extension_dir)
spec_git_cache_dirs << spec.source.cache_path.to_s if spec.source.is_a?(Bundler::Source::Git)
end
spec_gem_paths.uniq!
Expand All @@ -192,6 +195,7 @@ def clean(dry_run = false)
stale_gem_dirs = gem_dirs - spec_gem_paths
stale_gem_files = gem_files - spec_cache_paths
stale_gemspec_files = gemspec_files - spec_gemspec_paths
stale_extension_dirs = extension_dirs - spec_extension_paths

removed_stale_gem_dirs = stale_gem_dirs.collect {|dir| remove_dir(dir, dry_run) }
removed_stale_git_dirs = stale_git_dirs.collect {|dir| remove_dir(dir, dry_run) }
Expand All @@ -204,8 +208,10 @@ def clean(dry_run = false)
FileUtils.rm(file) if File.exist?(file)
end
end
stale_git_cache_dirs.each do |cache_dir|
SharedHelpers.filesystem_access(cache_dir) do |dir|

stale_dirs = stale_git_cache_dirs + stale_extension_dirs
stale_dirs.each do |stale_dir|
SharedHelpers.filesystem_access(stale_dir) do |dir|
FileUtils.rm_rf(dir) if File.exist?(dir)
end
end
Expand Down
35 changes: 35 additions & 0 deletions spec/commands/clean_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -733,4 +733,39 @@ def should_not_have_gems(*gems)
expect(vendored_gems("bundler/gems/extensions")).to exist
expect(vendored_gems("bundler/gems/very_simple_git_binary-1.0-#{revision[0..11]}")).to exist
end

it "removes extension directories", :rubygems => "2.2" do
gemfile <<-G
source "file://#{gem_repo1}"

gem "thin"
gem "very_simple_binary"
gem "simple_binary"
G

bundle! "install", forgotten_command_line_options(:path => "vendor/bundle")

very_simple_binary_extensions_dir =
Pathname.glob("#{vendored_gems}/extensions/*/*/very_simple_binary-1.0").first

simple_binary_extensions_dir =
Pathname.glob("#{vendored_gems}/extensions/*/*/simple_binary-1.0").first

expect(very_simple_binary_extensions_dir).to exist
expect(simple_binary_extensions_dir).to exist

gemfile <<-G
source "file://#{gem_repo1}"

gem "thin"
gem "simple_binary"
G

bundle! "install"
bundle! :clean
expect(out).to eq("Removing very_simple_binary (1.0)")

expect(very_simple_binary_extensions_dir).not_to exist
expect(simple_binary_extensions_dir).to exist
end
end
1 change: 1 addition & 0 deletions spec/support/builders.rb
Original file line number Diff line number Diff line change
Expand Up @@ -182,6 +182,7 @@ def build_repo1
end

build_gem "very_simple_binary", &:add_c_extension
build_gem "simple_binary", &:add_c_extension

build_gem "bundler", "0.9" do |s|
s.executables = "bundle"
Expand Down