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

Commit

Permalink
Merge #7274
Browse files Browse the repository at this point in the history
7274: Fix more leaks to default copy of bundler r=hsbt a=deivid-rodriguez

### What was the end-user problem that led to this PR?

The problem was that in some places, it was still possible to end up requiring files in a different copy of bundler (the default copy). I noticed this when I removed a rubygems monkeypatch from the test suite that was preventing the default copy of bundler from being activated when requiring files.

This thing:

https://github.com/bundler/bundler/blob/e1c518363641208429f397170354054b3d28effd/spec/support/hax.rb#L15-L20

### What was your diagnosis of the problem?

My diagnosis was that I should use relative requires wherever they were missing.

### What is your fix for the problem, implemented in this PR?

My fix is to remove the rubygems hack, migrate the rest of the internal requires to be relative, and also introduce some hacks on our specs to make sure we never load the incorrect copy of bundler.

I think this PR should fix the issues in rubygems/rubygems#2863.


Co-authored-by: David Rodríguez <deivid.rodriguez@riseup.net>
  • Loading branch information
bundlerbot and deivid-rodriguez committed Aug 18, 2019
2 parents 64271ff + 8ef571e commit 6125cb3
Show file tree
Hide file tree
Showing 21 changed files with 86 additions and 104 deletions.
4 changes: 3 additions & 1 deletion Rakefile
Expand Up @@ -312,7 +312,9 @@ else
# This will be automated once https://github.com/segiddins/automatiek/pull/3
# is included in `automatiek` and we start using the new API for vendoring
# subdependencies.

# Besides that, we currently cherry-pick changes to use `require_relative`
# internally instead of regular `require`. They are pending review at
# https://github.com/drbrain/net-http-persistent/pull/106
desc "Vendor a specific version of net-http-persistent"
Automatiek::RakeTask.new("net-http-persistent") do |lib|
lib.download = { :github => "https://github.com/drbrain/net-http-persistent" }
Expand Down
22 changes: 19 additions & 3 deletions exe/bundle
Expand Up @@ -7,7 +7,14 @@ Signal.trap("INT") do
exit 1
end

require "bundler"
base_path = File.expand_path("../lib", __dir__)

if File.exist?(base_path)
require_relative "../lib/bundler"
else
require "bundler"
end

# Check if an older version of bundler is installed
$LOAD_PATH.each do |path|
next unless path =~ %r{/bundler-0\.(\d+)} && $1.to_i < 9
Expand All @@ -18,9 +25,18 @@ $LOAD_PATH.each do |path|
abort(err)
end

require "bundler/friendly_errors"
if File.exist?(base_path)
require_relative "../lib/bundler/friendly_errors"
else
require "bundler/friendly_errors"
end

Bundler.with_friendly_errors do
require "bundler/cli"
if File.exist?(base_path)
require_relative "../lib/bundler/cli"
else
require "bundler/cli"
end

# Allow any command to use --help flag to show help for that command
help_flags = %w[--help -h]
Expand Down
10 changes: 5 additions & 5 deletions lib/bundler/plugin.rb
Expand Up @@ -4,11 +4,11 @@

module Bundler
module Plugin
autoload :DSL, "bundler/plugin/dsl"
autoload :Events, "bundler/plugin/events"
autoload :Index, "bundler/plugin/index"
autoload :Installer, "bundler/plugin/installer"
autoload :SourceList, "bundler/plugin/source_list"
autoload :DSL, File.expand_path("plugin/dsl", __dir__)
autoload :Events, File.expand_path("plugin/events", __dir__)
autoload :Index, File.expand_path("plugin/index", __dir__)
autoload :Installer, File.expand_path("plugin/installer", __dir__)
autoload :SourceList, File.expand_path("plugin/source_list", __dir__)

class MalformattedPlugin < PluginError; end
class UndefinedCommandError < PluginError; end
Expand Down
@@ -1,7 +1,7 @@
require 'net/http'
require 'uri'
require 'cgi' # for escaping
require 'bundler/vendor/connection_pool/lib/connection_pool'
require_relative '../../../../connection_pool/lib/connection_pool'

begin
require 'net/http/pipeline'
Expand Down Expand Up @@ -1197,6 +1197,6 @@ def verify_callback= callback

end

require 'bundler/vendor/net-http-persistent/lib/net/http/persistent/connection'
require 'bundler/vendor/net-http-persistent/lib/net/http/persistent/pool'
require_relative 'persistent/connection'
require_relative 'persistent/pool'

Expand Up @@ -49,5 +49,5 @@ def shutdown
end
end

require 'bundler/vendor/net-http-persistent/lib/net/http/persistent/timed_stack_multi'
require_relative 'timed_stack_multi'

7 changes: 1 addition & 6 deletions lib/bundler/vendored_fileutils.rb
@@ -1,9 +1,4 @@
# frozen_string_literal: true

module Bundler; end
if RUBY_VERSION >= "2.4"
require_relative "vendor/fileutils/lib/fileutils"
else
# the version we vendor is 2.4+
require "fileutils"
end
require_relative "vendor/fileutils/lib/fileutils"
2 changes: 1 addition & 1 deletion spec/bundler/bundler_spec.rb
Expand Up @@ -176,7 +176,7 @@
let(:bundler_ui) { Bundler.ui }
it "should raise a friendly error" do
allow(File).to receive(:exist?).and_return(true)
allow(bundler_fileutils).to receive(:remove_entry_secure).and_raise(ArgumentError)
allow(::Bundler::FileUtils).to receive(:remove_entry_secure).and_raise(ArgumentError)
allow(File).to receive(:world_writable?).and_return(true)
message = <<EOF
It is a security vulnerability to allow your home directory to be world-writable, and bundler can not continue.
Expand Down
4 changes: 2 additions & 2 deletions spec/bundler/settings_spec.rb
Expand Up @@ -120,7 +120,7 @@

context "when it's not possible to write to the file" do
it "raises an PermissionError with explanation" do
expect(bundler_fileutils).to receive(:mkdir_p).with(settings.send(:local_config_file).dirname).
expect(::Bundler::FileUtils).to receive(:mkdir_p).with(settings.send(:local_config_file).dirname).
and_raise(Errno::EACCES)
expect { settings.set_local :frozen, "1" }.
to raise_error(Bundler::PermissionError, /config/)
Expand Down Expand Up @@ -161,7 +161,7 @@
describe "#set_global" do
context "when it's not possible to write to the file" do
it "raises an PermissionError with explanation" do
expect(bundler_fileutils).to receive(:mkdir_p).with(settings.send(:global_config_file).dirname).
expect(::Bundler::FileUtils).to receive(:mkdir_p).with(settings.send(:global_config_file).dirname).
and_raise(Errno::EACCES)
expect { settings.set_global(:frozen, "1") }.
to raise_error(Bundler::PermissionError, %r{\.bundle/config})
Expand Down
8 changes: 3 additions & 5 deletions spec/commands/exec_spec.rb
Expand Up @@ -837,14 +837,12 @@ def bin_path(a,b,c)
context "nested bundle exec" do
context "when bundle in a local path" do
before do
system_gems :bundler

gemfile <<-G
source "#{file_uri_for(gem_repo1)}"
gem "rack"
G
bundle "config path vendor/bundler"
bundle! :install, :system_bundler => true
bundle "config set path vendor/bundler"
bundle! :install
end

it "correctly shells out", :ruby_repo do
Expand All @@ -854,7 +852,7 @@ def bin_path(a,b,c)
puts `bundle exec echo foo`
RB
file.chmod(0o777)
bundle! "exec #{file}", :system_bundler => true
bundle! "exec #{file}"
expect(out).to eq("foo")
end
end
Expand Down
3 changes: 1 addition & 2 deletions spec/commands/pristine_spec.rb
Expand Up @@ -42,8 +42,7 @@
expect(changes_txt).to_not be_file
end

it "does not delete the bundler gem", :rubygems => ">= 2.6.2" do
ENV["BUNDLER_SPEC_KEEP_DEFAULT_BUNDLER_GEM"] = "true"
it "does not delete the bundler gem" do
system_gems :bundler
bundle! "install"
bundle! "pristine", :system_bundler => true
Expand Down
2 changes: 1 addition & 1 deletion spec/install/gemfile/groups_spec.rb
Expand Up @@ -333,7 +333,7 @@
G

ruby <<-R
require "bundler"
require "#{lib}/bundler"
Bundler.setup :default
Bundler.require :default
puts RACK
Expand Down
1 change: 0 additions & 1 deletion spec/plugins/source/example_spec.rb
Expand Up @@ -6,7 +6,6 @@
build_repo2 do
build_plugin "bundler-source-mpath" do |s|
s.write "plugins.rb", <<-RUBY
require "bundler/vendored_fileutils"
require "bundler-source-mpath"
class MPath < Bundler::Plugin::API
Expand Down
4 changes: 2 additions & 2 deletions spec/realworld/double_check_spec.rb
Expand Up @@ -25,9 +25,9 @@
RUBY

cmd = <<-RUBY
require "bundler"
require "#{lib}/bundler"
require #{File.expand_path("../../support/artifice/vcr.rb", __FILE__).dump}
require "bundler/inline"
require "#{lib}/bundler/inline"
gemfile(true) do
source "https://rubygems.org"
gem "rails", path: "."
Expand Down
6 changes: 3 additions & 3 deletions spec/runtime/inline_spec.rb
Expand Up @@ -2,7 +2,7 @@

RSpec.describe "bundler/inline#gemfile" do
def script(code, options = {})
requires = ["bundler/inline"]
requires = ["#{lib}/bundler/inline"]
requires.unshift File.expand_path("../../support/artifice/" + options.delete(:artifice) + ".rb", __FILE__) if options.key?(:artifice)
requires = requires.map {|r| "require '#{r}'" }.join("\n")
@out = ruby("#{requires}\n\n" + code, options)
Expand Down Expand Up @@ -96,7 +96,7 @@ def script(code, options = {})

it "lets me use my own ui object" do
script <<-RUBY, :artifice => "endpoint"
require 'bundler'
require '#{lib}/bundler'
class MyBundlerUI < Bundler::UI::Silent
def confirm(msg, newline = nil)
puts "CONFIRMED!"
Expand Down Expand Up @@ -140,7 +140,7 @@ def confirm(msg, newline = nil)

it "does not mutate the option argument" do
script <<-RUBY
require 'bundler'
require '#{lib}/bundler'
options = { :ui => Bundler::UI::Shell.new }
gemfile(false, options) do
path "#{lib_path}" do
Expand Down
2 changes: 1 addition & 1 deletion spec/runtime/load_spec.rb
Expand Up @@ -80,7 +80,7 @@
G

ruby! <<-RUBY
require "bundler"
require "#{lib}/bundler"
Bundler.setup :default
Bundler.require :default
puts RACK
Expand Down
2 changes: 1 addition & 1 deletion spec/runtime/require_spec.rb
Expand Up @@ -193,7 +193,7 @@
G

cmd = <<-RUBY
require 'bundler'
require '#{lib}/bundler'
Bundler.require
RUBY
ruby(cmd)
Expand Down

0 comments on commit 6125cb3

Please sign in to comment.