-
-
Notifications
You must be signed in to change notification settings - Fork 2k
Ensure git is executed inside the gemspec dir #6191
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 |
@@ -28,7 +28,7 @@ Gem::Specification.new do |spec| | |||
"public gem pushes." | |||
end | |||
|
|||
spec.files = `git ls-files -z`.split("\x0").reject do |f| | |||
spec.files = Dir.chdir(__dir__){`git ls-files -z`}.split("\x0").reject do |f| |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
let's use File.expand_path("..", __FILE__)
for compatibility, and maybe add some spacing to conform with the style in the rest of the codebase?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done ✅, also moved the whole line inside the chdir block to make it more readable
When a Gemfile was pointing to a local gem using git to list its files the git command was executed from within the wrong dir. This fixes the issue without: 1. introducing new dependencies (e.g. `shellwords`) 2. any risks with non-trivial paths (e.g. paths containing spaces)
@bundlerbot r+ |
📌 Commit 107772e has been approved by |
Ensure git is executed inside the gemspec dir ### What was the end-user problem that led to this PR? Executables from bundled gems weren't available. ``` Gem::Exception: can't find executable <EXEC-FILENAME> for gem <GEM-NAME> /Users/elia/.rvm/rubies/ruby-2.3.4/lib/ruby/site_ruby/2.3.0/bundler/rubygems_integration.rb:458:in `block in replace_bin_path' /Users/elia/.rvm/rubies/ruby-2.3.4/lib/ruby/site_ruby/2.3.0/bundler/rubygems_integration.rb:478:in `block in replace_bin_path' … ``` ### What was your diagnosis of the problem? When a Gemfile was pointing to a local gem using git to list its files the git command was executed from within the wrong dir. ### What is your fix for the problem, implemented in this PR? Initially I added `-C #{__dir__}` to the `git ls-files -z` command. ### Why did you choose this fix out of the possible options? Realized `-C` wasn't safe and opted for stuff already available in the corelib, i.e. `Dir.chrid` instead of, say, escaping with `shellwords`. --- I know the line is long-ish, happy to fix it in the way you want
☀️ Test successful - status-travis |
What was the end-user problem that led to this PR?
Executables from bundled gems weren't available.
What was your diagnosis of the problem?
When a Gemfile was pointing to a local gem using git to list its files the git command
was executed from within the wrong dir.
What is your fix for the problem, implemented in this PR?
Initially I added
-C #{__dir__}
to thegit ls-files -z
command.Why did you choose this fix out of the possible options?
Realized
-C
wasn't safe and opted for stuff already available in the corelib, i.e.Dir.chrid
instead of, say, escaping withshellwords
.I know the line is long-ish, happy to fix it in the way you want