Skip to content

Support setup --destdir option for bundler install #2106

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from
Closed

Support setup --destdir option for bundler install #2106

wants to merge 1 commit into from

Conversation

ghost
Copy link

@ghost ghost commented Nov 29, 2017

No description provided.

mkdir_p bundler_spec.bin_dir
bundler_spec.executables.each {|e| cp File.join("bundler", bundler_spec.bindir, e), File.join(bundler_spec.bin_dir, e) }
bundler_bin_dir = File.join(
options[:destdir], Gem.default_dir, 'gems',
Copy link
Author

Choose a reason for hiding this comment

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

I searched in RubyGems source code, but could not find a method or
constant that returns this gems directory, it seems hardcoded at other
places too.
In Ruby source code a literal string is used too if I'm correct:
https://svn.ruby-lang.org/cgi-bin/viewvc.cgi/tags/v2_4_2/tool/rbinstall.rb?revision=59899&view=markup#l724

@@ -143,6 +143,22 @@ def test_install_default_bundler_gem
assert_path_exists 'default/gems/bundler-audit-1.0.0'
end if Gem::USE_BUNDLER_FOR_GEMDEPS

def test_install_default_bundler_gem_with_destdir
Gem.instance_variable_set :@default_dir, nil
Copy link
Author

Choose a reason for hiding this comment

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

I'm not fan of this code, but in Gem::TestCase.setup we have the
following:

@default_dir = File.join @tempdir, 'default'
Gem.instance_variable_set :@default_dir, @default_dir

I understand why it's done this way and don't think we should change
it. This makes testing --destdir complicated, and unless there is a
nicer solution, I concluded it might be acceptable to resort to using
Gem.instance_variable_set and use @tempdir as the value for
--destdir option.

@ghost
Copy link
Author

ghost commented Nov 30, 2017

Unfortunately I was too slow to finish this patch and missed 2.7.3
release!
It tooks me a bit of time to understand the code.

Without this, --destdir is ignored for bundler installation as
default gem, example:

env -i GEM_HOME=/tmp/install/gem_home \
  ruby setup.rb \
  --destdir=/tmp/install/stage \
  --no-regenerate_binstubs \
  --backtrace --verbose
[…]
Installing gem executable
install -c -m 755 /tmp/gem.7938 /tmp/install/stage/usr/local/bin/gem24
rm /tmp/gem.7938
Installing bundler executable
install -c -m 755 /tmp/bundle.7938 /tmp/install/stage/usr/local/bin/bundle24
rm /tmp/bundle.7938
mkdir -p /usr/local/lib/ruby/gems/2.4/specifications/default
ERROR:  While executing gem ... (Errno::EACCES)
    Permission denied @ unlink_internal - /usr/local/lib/ruby/gems/2.4/specifications/default/bundler-1.16.0.gemspec
  lib/rubygems/commands/setup_command.rb:364:in `delete'
  lib/rubygems/commands/setup_command.rb:364:in `block in install_default_bundler_gem'
  lib/rubygems/commands/setup_command.rb:364:in `each'
  lib/rubygems/commands/setup_command.rb:364:in `install_default_bundler_gem'
  lib/rubygems/commands/setup_command.rb:151:in `execute'
  lib/rubygems/command.rb:310:in `invoke_with_build_args'
  lib/rubygems/command_manager.rb:171:in `process_args'
  lib/rubygems/command_manager.rb:141:in `run'
  lib/rubygems/gem_runner.rb:59:in `run'
  setup.rb:46:in `<main>'
exit 1

If you fix the problem for the gem specification, then you would get
similar error for the gem executable.

@segiddins
Copy link
Member

What issue is this trying to address?

@ghost
Copy link
Author

ghost commented Dec 4, 2017

What issue is this trying to address?

Restoring --destdir option usage with setup command, specifically
for bundler default gem. If I'm correct since d9eb4f0 and 19a4b10
two files (gemspec and bundler executable) are installed at fixed
locations, destdir option seems ignored in:
Gem::Commands::SetupCommand#install_default_bundler_gem

Please see this comment for a simple example of an error we may get:
#2106 (comment)
In this case the error was triggered because of permissions, but it
may also happen in a packaging system even when you are root: it would
complain that filesystem was modified outside destination directory,
and some files would be missing.

Since then I was able to make the build pass on windows:
master...tjouan:commands-setup-install_default_bundler_gem-destdir-windows-rg

But it was laborious and I'm even less satisfied with the new code
than the one in this PR. So I'll close this PR for now, especially
considering I'm not actually using this code since I now apply the
following patch in my port:

--- lib/rubygems/commands/setup_command.rb.orig
+++ lib/rubygems/commands/setup_command.rb
@@ -148,8 +148,6 @@ By default, this RubyGems will install g

     remove_old_lib_files lib_dir

-    install_default_bundler_gem
-
     say "RubyGems #{Gem::VERSION} installed"

     regenerate_binstubs if options[:regenerate_binstubs]
@@ -217,7 +215,6 @@ By default, this RubyGems will install g
     @bin_file_names = []

     executables = { 'gem' => 'bin' }
-    executables['bundler'] = 'bundler/exe' if Gem::USE_BUNDLER_FOR_GEMDEPS
     executables.each do |tool, path|
       say "Installing #{tool} executable" if @verbose

Do you want me to open an issue about the --destdir option problem?

Would a feature to disable installing bundler default gem and bundle
executable be considered? I'm thinking about a
--[no-]install-bundler for example, which would default to true, but
of course always install bundler lib, which is required.

This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants