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

Fallback to a temp dir when the home directory is not usable #4951

Merged
merged 5 commits into from
Sep 30, 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
38 changes: 37 additions & 1 deletion lib/bundler.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@
require "pathname"
require "rbconfig"
require "thread"
require "tmpdir"

require "bundler/errors"
require "bundler/environment_preserver"
require "bundler/gem_remote_fetcher"
Expand Down Expand Up @@ -143,8 +145,41 @@ def ruby_scope
"#{Bundler.rubygems.ruby_engine}/#{Bundler.rubygems.config_map[:ruby_version]}"
end

def user_home
@user_home ||= begin
home = Bundler.rubygems.user_home
warning = "Your home directory is not set properly:"
if home.nil?
warning += "\n * It is not set at all"
elsif !File.directory?(home)
warning += "\n * `#{home}` is not a directory"
elsif !File.writable?(home)
warning += "\n * `#{home}` is not writable"
else
return @user_home = Pathname.new(home)
end

login = Etc.getlogin || "unknown"

tmp_home = Pathname.new(Dir.tmpdir).join("bundler", "home", login)
begin
SharedHelpers.filesystem_access(tmp_home, :write) do |p|
FileUtils.mkdir_p(p)
end
rescue => e
warning += "\n\nBundler also failed to create a temporary home directory at `#{tmp_home}`:\n#{e}"
raise warning
end

warning += "\n\nBundler will use `#{tmp_home}` as your home directory temporarily"

Bundler.ui.warn(warning)
tmp_home
end
end

def user_bundle_path
Pathname.new(Bundler.rubygems.user_home).join(".bundle")
Pathname.new(user_home).join(".bundle")
Copy link
Contributor

Choose a reason for hiding this comment

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

user_name is Pathname already. So it should be safe to leave user_home.join(".bundle")

end

def home
Expand Down Expand Up @@ -403,6 +438,7 @@ def reset!
@locked_gems = nil
@bundle_path = nil
@bin_path = nil
@user_home = nil

Plugin.reset!

Expand Down
2 changes: 2 additions & 0 deletions lib/bundler/cli.rb
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,10 @@ def initialize(*args)
ensure
self.options ||= {}
Bundler.settings.cli_flags_given = !options.empty?
unprinted_warnings = Bundler.ui.unprinted_warnings
Bundler.ui = UI::Shell.new(options)
Bundler.ui.level = "debug" if options["verbose"]
unprinted_warnings.each {|w| Bundler.ui.warn(w) }

if ENV["RUBYGEMS_GEMDEPS"] && !ENV["RUBYGEMS_GEMDEPS"].empty?
Bundler.ui.warn(
Expand Down
2 changes: 1 addition & 1 deletion lib/bundler/fetcher/compact_index.rb
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ def fetch_spec(spec)
compact_index_request :fetch_spec

def available?
user_home = Pathname.new(Bundler.rubygems.user_home)
user_home = Bundler.user_home
return nil unless user_home.directory? && user_home.writable?
# Read info file checksums out of /versions, so we can know if gems are up to date
fetch_uri.scheme != "file" && compact_index_client.update_and_parse_checksums!
Expand Down
2 changes: 1 addition & 1 deletion lib/bundler/gem_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ def rubygem_push(path)
allowed_push_host = @gemspec.metadata["allowed_push_host"]
gem_command += " --host #{allowed_push_host}" if allowed_push_host
end
unless allowed_push_host || Pathname.new("~/.gem/credentials").expand_path.file?
unless allowed_push_host || Bundler.user_home.join(".gem/credentials").file?
Copy link
Member

Choose a reason for hiding this comment

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

Does expand_path raise if HOME isn't set? If it doesn't raise, I think we shouldn't create or use the temporary directory when looking for gem creds.

Copy link
Member Author

Choose a reason for hiding this comment

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

inexpand_path': non-absolute home (ArgumentError)`

Copy link
Member

Choose a reason for hiding this comment

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

Okay, as I suspected. That means we don't want to use Bundler.user_home here. In fact, this makes me think that all this logic should be inside the CompactIndex cache writer, and not inside Bundler.user_home, because we actively don't want to use a temp directory for either writing a settings file or looking for .gem/credentials. We specifically and only want to use a temp directory for the compact index files.

Copy link
Member Author

Choose a reason for hiding this comment

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

Why don't we want to use a temp directory for writing a settings file? It would allow users without a home directory to have 'global' settings for at least a single usage session

raise "Your rubygems.org credentials aren't set. Run `gem push` to set them."
end
sh(gem_command)
Expand Down
13 changes: 10 additions & 3 deletions lib/bundler/shared_helpers.rb
Original file line number Diff line number Diff line change
Expand Up @@ -39,10 +39,12 @@ def default_bundle_dir
bundle_dir = find_directory(".bundle")
return nil unless bundle_dir

global_bundle_dir = File.join(Bundler.rubygems.user_home, ".bundle")
bundle_dir = Pathname.new(bundle_dir)

global_bundle_dir = Bundler.user_home.join(".bundle")
return nil if bundle_dir == global_bundle_dir

Pathname.new(bundle_dir)
bundle_dir
end

def in_bundle?
Expand Down Expand Up @@ -202,10 +204,15 @@ def set_rubyopt

def set_rubylib
rubylib = (ENV["RUBYLIB"] || "").split(File::PATH_SEPARATOR)
rubylib.unshift File.expand_path("../..", __FILE__)
rubylib.unshift bundler_ruby_lib
ENV["RUBYLIB"] = rubylib.uniq.join(File::PATH_SEPARATOR)
end

def bundler_ruby_lib
File.expand_path("../..", __FILE__)
end
private :bundler_ruby_lib

def clean_load_path
# handle 1.9 where system gems are always on the load path
if defined?(::Gem)
Expand Down
4 changes: 4 additions & 0 deletions lib/bundler/ui/shell.rb
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,10 @@ def silence(&blk)
with_level("silent", &blk)
end

def unprinted_warnings
[]
end

private

# valimism
Expand Down
9 changes: 9 additions & 0 deletions lib/bundler/ui/silent.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,10 @@
module Bundler
module UI
class Silent
def initialize
@warnings = []
end

def add_color(string, color)
string
end
Expand All @@ -13,6 +17,7 @@ def confirm(message, newline = nil)
end

def warn(message, newline = nil)
@warnings |= [message]
Copy link
Contributor

Choose a reason for hiding this comment

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

Sorry, I accidentally removed @indirect's comment. Original comment was saying that (recovering from memories):

Seems like this means message will never be garbage collected.

Copy link
Member Author

Choose a reason for hiding this comment

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

until the Silent instance is deallocated, yes

Copy link
Member

Choose a reason for hiding this comment

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

How do you feel about making this a Set instead of using the (esoteric and relatively confusing) |= operator?

Copy link
Member

Choose a reason for hiding this comment

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

How do you feel about using a Set instead of the (esoteric and relatively confusing) |= operator?

Copy link
Member Author

Choose a reason for hiding this comment

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

That's fine, I think I was just copying what the Shell class did

end

def error(message, newline = nil)
Expand Down Expand Up @@ -44,6 +49,10 @@ def trace(message, newline = nil)
def silence
yield
end

def unprinted_warnings
@warnings
end
end
end
end
5 changes: 3 additions & 2 deletions spec/bundler/shared_helpers_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -234,7 +234,9 @@
shared_examples_for "ENV['RUBYLIB'] gets set correctly" do
let(:ruby_lib_path) { "stubbed_ruby_lib_dir" }

before { allow(File).to receive(:expand_path).and_return(ruby_lib_path) }
before do
allow(Bundler::SharedHelpers).to receive(:bundler_ruby_lib).and_return(ruby_lib_path)
end

it "ensures bundler's ruby version lib path is in ENV['RUBYLIB']" do
subject.set_bundle_environment
Expand Down Expand Up @@ -324,7 +326,6 @@
let(:ruby_lib_path) { "stubbed_ruby_lib_dir" }

before do
allow(File).to receive(:expand_path).and_return(ruby_lib_path)
ENV["RUBYLIB"] = ruby_lib_path
end

Expand Down
20 changes: 20 additions & 0 deletions spec/runtime/inline_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -173,4 +173,24 @@ def confirm(msg, newline = nil)
expect(err).to be_empty
expect(exitstatus).to be_zero if exitstatus
end

it "allows calling gemfile twice" do
script <<-RUBY
gemfile do
path "#{lib_path}" do
gem "two"
end
end

gemfile do
path "#{lib_path}" do
gem "four"
end
end
RUBY

expect(out).to eq("two\nfour")
expect(err).to be_empty
expect(exitstatus).to be_zero if exitstatus
end
end