Binstub: set GEM_PATH to '', not nil.

[krallin]

Appbundler currently sets GEM_PATH to nil (except when running with APPBUNDLER_ALLOW_RVM), but this does not fully achieve Appbundler's isolation goal.

Indeed, when GEM_PATH is set to nil, Rubygems will use a default path consisting of two locations:

• The user gem path
• The gem home (under the Omnibus path)

But, in an Omnibus context, the former shouldn't be there: we'd like to have full isolation of the Omnibus gems from the rest of the system, so only those gems from the gem home are relevant.

You can repro this using the following script (clone the ruby/rubygems repo, then put the script at the root of the repo):

require_relative 'lib/rubygems/path_support.rb'

[
  { 'GEM_HOME' => nil, 'GEM_PATH' => nil },
  { 'GEM_HOME' => nil, 'GEM_PATH' => '' }
].each do |test_case|
  subject = ::Gem::PathSupport.new(test_case)
  puts "case: #{test_case}"
  puts "home: #{subject.home}"
  puts "path: #{subject.path.join(':')}"
  puts
end

Using Ruby from an Omnibus context, the difference is fairly clear:

case: {"GEM_HOME"=>nil, "GEM_PATH"=>nil}
home: /opt/aptible-toolbelt/embedded/lib/ruby/gems/2.3.0
path: /Users/thomas/.gem/ruby/2.3.0:/opt/aptible-toolbelt/embedded/lib/ruby/gems/2.3.0

case: {"GEM_HOME"=>nil, "GEM_PATH"=>""}
home: /opt/aptible-toolbelt/embedded/lib/ruby/gems/2.3.0
path: /opt/aptible-toolbelt/embedded/lib/ruby/gems/2.3.0

That said, this typically doesn't cause issues: the gems on the user's gem path shouldn't ever be required precisely because Appbundler pre-activates every gem that the Gemfile.lock predicts will be required.

But, unfortunately, the user gem path can leak through even if no gems are ever required from it! Indeed, consider an Omnibus package with "optional" dependencies of this form (potentially themselves located in a dependency of the Gem being Omnibus-packaged):

begin
  require "readline"
rescue LoadError
end

Assuming readline is not installed in the Omnibus package (presumably because it's not used there), require will fail, but it won't be the normal Ruby require.

Instead, it'll use Rubygems' stubbed require, which will scan the Gem path to try activate a suitable gem for what is being required. But, if the Gem path includes the user gem path (i.e. what this PR seeks to fix), there will be 3 problems with this:

• It's going to be slower.
• The behavior of the Omnibus package will depends on whether the Gem being required happens to be installed in the user gem path.
• With native gems, it might generate a bunch of warnings / errors coming from Ruby gems. This seems to happens because Rubygems logs a warning when it notices a gem on the path whose native dependencies haven't been built, which it seems may be the case when it finds a gem on the user path but Ruby is run from the Omnibus context. The error in question looks like this: Ignoring GEM-VERSION because its extensions are not built. Try: gem pristine GEM --version VERSION (this doesn't break anything, but it's fairly terrible from a cosmetic standpoint).

Setting GEM_PATH to an empty string rather than nil appears to solve all these issues. Alternatively, the binstub could also set both GEM_HOME and GEM_PATH to Gem.default_dir if that's a preferred approach.

[krallin]

I should mention that this is likely to break binstubs for users who currently expect Appbundler to load gems from the user gem path but don't use APPBUNDLER_ALLOW_RVM, but I'm not sure whether that is actually a supported use case for Appbundler.

[krallin]

Note that this comment i made earlier turned out to be a little optimistic:

That said, this typically doesn't cause issues: the gems on the user's gem path shouldn't ever be required precisely because Appbundler pre-activates every gem that the Gemfile.lock predicts will be required.

For example, the json gem might cause a crash here if it's not in Gemfile.lock: if the user has a Ruby installed that is more different from the omnibus'd Ruby (likely), loading the native extension for the json gem might cause a crash (the json gem might not be in the Gemfile since it's built in).

[skull-squadron]

This issue is causing inspec and chef omnibus to break completely on macOS. I had to uninstall them.