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

[RubygemsIntegration] Add support for new activate_bin_path binstubs #4341

Merged
merged 1 commit into from
Mar 7, 2016
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
3 changes: 2 additions & 1 deletion lib/bundler/cli/exec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ def run
validate_cmd!
SharedHelpers.set_bundle_environment
if bin_path = Bundler.which(cmd)
kernel_load(bin_path, *args) && return if ruby_shebang?(bin_path)
kernel_load(bin_path, *args) if ruby_shebang?(bin_path)
# First, try to exec directly to something in PATH
kernel_exec([bin_path, cmd], *args)
else
Expand Down Expand Up @@ -61,6 +61,7 @@ def kernel_load(file, *args)
Bundler.ui = nil
require "bundler/setup"
Kernel.load(file)
exit
rescue SystemExit
raise
rescue Exception => e # rubocop:disable Lint/RescueException
Expand Down
36 changes: 21 additions & 15 deletions lib/bundler/rubygems_integration.rb
Original file line number Diff line number Diff line change
Expand Up @@ -361,25 +361,31 @@ def stub_source_index(specs)
# +specs+
def replace_bin_path(specs)
gem_class = (class << Gem; self; end)
redefine_method(gem_class, :bin_path) do |name, *args|
exec_name = args.first

return ENV["BUNDLE_BIN_PATH"] if exec_name == "bundle"

spec = nil
redefine_method(gem_class, :find_spec_for_exe) do |name, *args|
exec_name = args.first

if exec_name
spec = specs.find {|s| s.executables.include?(exec_name) }
raise(Gem::Exception, "can't find executable #{exec_name}") unless spec
unless spec.name == name
warn "Bundler is using a binstub that was created for a different gem.\n" \
"This is deprecated, in future versions you may need to `bundle binstub #{name}` " \
"to work around a system/bundle conflict."
end
spec = if exec_name
specs.find {|s| s.executables.include?(exec_name) }
else
spec = specs.find {|s| s.name == name }
raise Gem::Exception, "no default executable for #{spec.full_name}" unless exec_name = spec.default_executable
specs.find {|s| s.name == name }
end
raise(Gem::Exception, "can't find executable #{exec_name}") unless spec
raise Gem::Exception, "no default executable for #{spec.full_name}" unless exec_name ||= spec.default_executable
unless spec.name == name
warn "Bundler is using a binstub that was created for a different gem.\n" \
"This is deprecated, in future versions you may need to `bundle binstub #{name}` " \
"to work around a system/bundle conflict."
end
spec
end

redefine_method(gem_class, :bin_path) do |name, *args|
exec_name = args.first
return ENV["BUNDLE_BIN_PATH"] if exec_name == "bundle"
Copy link

Choose a reason for hiding this comment

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

Looks like the new Gem.activate_bin_path will be missing out the chance to hit this early return and breaks this feature on bundle-exec's man page:

make sure that it's still possible to shell out to bundle from inside a command invoked by bundle exec (using $BUNDLE_BIN_PATH)

Not sure what would be the best way to fix it :-/
/cc @segiddins @indirect

Copy link
Contributor

Choose a reason for hiding this comment

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

Looks like you were right :) We're about to make this change on this PR #4992. I ran across this while researching a write up of a fresh commit message. It's been a long and winding road to re-discover what you'd found in March.

Copy link

Choose a reason for hiding this comment

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

@chrismo I'm feeling a bit guilty now for not being able to help fixing the issue a few months back.. But it was the first time I was reporting an issue with Rubygems and Bundler. There was a tendency to think I was missing something in the code and not fully understand the details. But thanks so much for the work on #4992. Looking forward to the release!

Copy link
Contributor

Choose a reason for hiding this comment

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

No worries, there are a lot of moving parts (as I continue to learn) and it can be a tough decision to know what matters and what will just be fine. At least you recorded this comment which helps confirm what we're seeing. :)


spec = find_spec_for_exe(name, *args)
exec_name ||= spec.default_executable

gem_bin = File.join(spec.full_gem_path, spec.bindir, exec_name)
gem_from_path_bin = File.join(File.dirname(spec.loaded_from), spec.bindir, exec_name)
Expand Down
4 changes: 2 additions & 2 deletions spec/commands/exec_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -128,13 +128,13 @@
G
end

bundle "exec rackup", :expect_err => true
bundle! "exec rackup", :expect_err => true

expect(out).to eq("0.9.1")
expect(err).to match("deprecated")

Dir.chdir bundled_app2 do
bundle "exec rackup"
bundle! "exec rackup"
expect(out).to eq("1.0.0")
end
end
Expand Down