[WIP] Bundler::Runtime#clean: remove extensions dirs #5816
Conversation
|
Thanks for opening a pull request and helping make Bundler better! Someone from the Bundler team will take a look at your pull request shortly and leave any feedback. Please make sure that your pull request has tests for any changes or added functionality. We use Travis CI to test and make sure your change works functionally and uses acceptable conventions, you can review the current progress of Travis CI in the PR status window below. If you have any questions or concerns that you wish to ask, feel free to leave a comment in this PR or join our #bundler channel on Slack. For more information about contributing to the Bundler project feel free to review our CONTRIBUTING guide |
|
HI, can you please fill out the questionnaire. edit: sorry didn't realise this was still a WIP |
igorbozato
changed the title
spec/commands/clean_spec.rb
Outdated
|
|
||
| bundle "install --path vendor/bundle" | ||
|
|
||
| expect(vendored_gems("extensions/x86_64-linux/2.3.0-static/c_extension-1.0")).to exist |
There was a problem hiding this comment.
please don't hardcode the platform, use something like bundled_app(*["vendor/bundle", Gem.ruby_engine, Gem::ConfigMap[:ruby_version], c_extention-1.0"].compact) instead
| require "mkmf" | ||
| name = "c_extension_bundle" | ||
| dir_config(name) | ||
| raise "OMG" unless with_config("c_extension") == "hello" |
There was a problem hiding this comment.
this seems to be raising on CI
| @@ -193,6 +196,9 @@ 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_dirs | |||
|
|
|||
| stale_extension_dirs.collect {|dir| FileUtils.rm_rf(dir) } unless dry_run | |||
There was a problem hiding this comment.
this should assign to a variable that gets added to output a couple of lines below and use the remove_dir helper, same as 2 lines below
There was a problem hiding this comment.
Using remove_dir for this would duplicate the output of the removed gem, what should be the output text for this?
|
|
||
| bundle "install --path vendor/bundle" | ||
|
|
||
| expect(vendored_gems.join("extensions", Gem::Platform.local.to_s, Gem::ConfigMap[:ruby_version] + "-static", "c_extension-1.0")).to exist |
There was a problem hiding this comment.
Gem::ConfigMap[:ruby_version] + "-static" still hardcoded, not sure where "2.3.0-static" comes from.
Thanks so much for the contribution!
To make reviewing this PR a bit easier, please fill out answers to the following questions.
What was the end-user problem that led to this PR?
The problem was...
See #5596
Was was your diagnosis of the problem?
My diagnosis was...
What is your fix for the problem, implemented in this PR?
My fix...
Why did you choose this fix out of the possible options?
I chose this fix because...