Skip to content
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

--regenerate-binstubs argument to setup.rb does not respect --destdir #2370

Closed
Roguelazer opened this issue Jul 28, 2018 · 10 comments · Fixed by #5053
Closed

--regenerate-binstubs argument to setup.rb does not respect --destdir #2370

Roguelazer opened this issue Jul 28, 2018 · 10 comments · Fixed by #5053

Comments

@Roguelazer
Copy link

I'm packing a new versions of rubygems for a distribution and it appears that some time between the last time I did this (2.6.14) and now (2.7.7), the --regenerate-binstubs argument became the default; this argument does not respect the --destdir argument and always attempts to write into the actual prefix. This makes it difficult to package rubygems into an RPM. Packaging works correctly if --no-regenerate-binstubs is passed, but I feel like I ought to be regenerating binstubs.

I will abide by the code of conduct.

@segiddins
Copy link
Member

I believe the --destdir argument is only meant to control where rubygems itself gets installed, and not where the underlying gems get put?

@Roguelazer
Copy link
Author

Yes, this is an issue occuring while installing rubygems itself with /path/to/ruby setup.rb --prefix=/path/to/prefix --destdir=$HOME/rpmbuild/BUILDROOT/blah/

@ghost
Copy link

ghost commented Jul 30, 2018

I'm packing a new versions of rubygems for a distribution and it appears that some time between the last time I did this (2.6.14) and now (2.7.7), the --regenerate-binstubs argument became the default; this argument does not respect the --destdir argument and always attempts to write into the actual prefix.

It was added in v2.7.0 (909b5fb), but at the time the option itself was
dead code: using --no-regenerate-binstubs had no effect, and the related code
was always executed. Also if I recall correctly, there was another issue
breaking --destdir option when Gem::USE_BUNDLER_FOR_GEMDEPS was true and
it was fixed in v2.7.1 (not 100 % sure, but 5d5f0c3 and 7694518 might be
related).

In the original patch I used, I had changed the default value to false
because I think this should be the default behavior. But since this feature was
already released, in the end I only submitted a patch to make
--no-regenerate-binstubs work: #2099. So starting with v2.7.3, we must
specify --no-regenerate-binstubs when using --destdir. Alternatively,
patching Gem::USE_BUNDLER_FOR_GEMDEPS to be false allows using --destdir
alone if I'm correct (which would be done when building a package).

Then in v2.7.5 you can just set DONT_USE_BUNDLER_FOR_GEMDEPS=yes
environment variable instead of patching, and again --destdir might work
without --no-regenerate-binstubs.

I attempted to fix this issue here: #2106. But attempting to support Windows
was too difficult for me (lack of knowledge and no direct access to this
platform), I didn't like the code I wrote for Windows support and closed the PR.
There are a few questions in the last comment but I received no feedback.

Later, you reported the same issue: #2134, where I suggested some patches,
but intended only for your port, not upstream (knowing they didn't support
Windows). I failed to participate at the time, which resulted on changes in
RubyGems based on my (incorrect) patches: #2169, #2178 (last one fix the first
one).

This makes it difficult to package rubygems into an RPM.

I agree, in my opinion this feature should not have been enabled by default,
or maybe wait for RubyGems 3. But I think it might be too late now.

However, does setting DONT_USE_BUNDLER_FOR_GEMDEPS=yes and not using
--[no-]regenerate-binstubs at all solves your issue? Because if you package
into RPM, I guess bundler would be a separate package
anyway.

Packaging works correctly if --no-regenerate-binstubs is passed, but I feel like I ought to be regenerating binstubs.

I don't really know which use case needs "regenerating binstubs", but when
packaging, I don't think we want/need this. I found my original patch, and
realize that I removed some context before submitting the one that got merged:

commit a430e12d91c9207b2ae1c8e2586a68e6a4ff19ef
Author: Thibault Jouan
Date:   Wed Nov 8 20:32:41 2017 +0000

    Fix setup command --regenerate-binstubs option flag

      The new --[no-]regenerate-binstubs option should probably not be
    enabled by default, as we cannot install any gems before installing
    rubygems itself. In most case the setup command will not be used by end
    users, but by maintainers of OS or distributions when building a
    "package" for rubygems. Developers and some users may need to run this
    command when working on the code or for specific install methods, it
    might be more safe to require explicit --regenerate-binstubs usage if
    needed.
[…]

@ghost
Copy link

ghost commented Jul 30, 2018

I believe the --destdir argument is only meant to control where rubygems itself gets installed, and not where the underlying gems get put?

I think that people who use this option expect that nothing outside the
given directory will be created, modified or removed. When packaging,
everything must happen in a single "staging" directory, which in a way is
equivalent to the file system root (/) for this package.

If something happens outside this directory, then the package would probably
be broken, and some build system will detect the issue and fail immediately.

@Roguelazer
Copy link
Author

@tjouan thanks for the comprehensive reply; I am already setting DONT_USE_BUNDLER_FOR_GEMDEPS to avoid installing bundler; however, with DONT_USE_BUNDLER_FOR_GEMDEPS, install still fails without --no-regenerate-binstubs

Installing gem executable
install -c -m 755 /tmp/gem.1978 /home/build/rpmbuild/BUILDROOT/ruby25-2.5.1-1.el6.x86_64/opt/ruby2.5/bin/gem
rm /tmp/gem.1978
RubyGems 2.7.7 installed
Regenerating binstubs
ERROR:  While executing gem ... (Gem::Exception)
    Failed to find gems [] >= 0
error: Bad exit status from /var/tmp/rpm-tmp.RjCkS9 (%install)

It's frustrating that every time I build a new version of rubygems it seems to be broken in some new novel way. :-(

@ghost
Copy link

ghost commented Jul 30, 2018

I was testing this again (on v2.7.7), and I just noticed too that
DONT_USE_BUNDLER_FOR_GEMDEPS has no effect in this particular case.

setup command worked well before v2.7, but it's true that since changes
related to "bundler" (which is called a "default gem" in this case I think),
setup command became more complex and a bit "brittle". In addition, installing
"bundler" with RubyGems seems wrong for me, but maybe there are reasons for
this, not sure if it was announced or explained somewhere.


--no-regenerate_binstubs

With --no-regenerate_binstubs everything seems to install into the
directory given to --destdir. There is something strange about "bundler"
executable however (installed into /tmp).

$ env -i GEM_HOME=/tmp/install/gem_home \
  ruby setup.rb \
  --destdir=/tmp/install/stage \
  --no-regenerate_binstubs
Bundler 1.16.2 installed
RubyGems 2.7.7 installed
Parsing documentation for rubygems-2.7.7
Installing ri documentation for rubygems-2.7.7
[…]
$ find /tmp/install/stage/tmp -type f
/tmp/install/stage/tmp/install/stage/usr/local/lib/ruby/gems/2.5/specifications/gems/bundler-1.16.2/exe/bundle

--no-regenerate_binstubs and DONT_USE_BUNDLER_FOR_GEMDEPS=yes

Same command, but with DONT_USE_BUNDLER_FOR_GEMDEPS environment variable
set. "bundler" executable issue is gone.

$ env -i DONT_USE_BUNDLER_FOR_GEMDEPS=yes \
  GEM_HOME=/tmp/install/gem_home \
  ruby setup.rb \
  --destdir=/tmp/install/stage \
  --no-regenerate_binstubs
RubyGems 2.7.7 installed
Parsing documentation for rubygems-2.7.7
Installing ri documentation for rubygems-2.7.7
[…]
$ \ls /tmp/install/stage
usr

--regenerate_binstubs (default)

First command, but without --no-regenerate_binstubs option, and adding
--backtrace --verbose. --destdir is ignored here. By luck I don't have
write permissions, and when I build RubyGems in poudriere it happens in a jail,
so not a big deal in my case but users in other build environment might have
issues: "messing" with directories/files that should be managed by a package
manager, overwriting data etc…

$ env -i GEM_HOME=/tmp/install/gem_home \
  ruby setup.rb \
  --destdir=/tmp/install/stage \
  --backtrace --verbose
mkdir -p /tmp/install/stage/usr/local/lib/ruby/site_ruby/2.5
mkdir -p /tmp/install/stage/usr/local/bin
Installing RubyGems
install -c -m 644 rubygems.rb /tmp/install/stage/usr/local/lib/ruby/site_ruby/2.5/rubygems.rb
[…]
install -c -m 644 bundler/ssl_certs/index.rubygems.org/GlobalSignRootCA.pem /tmp/install/stage/usr/local/lib/ruby/site_ruby/2.5/bundler/ssl_certs/index.rubygems.org/GlobalSignRootCA.pem
Installing gem executable
install -c -m 755 /tmp/gem.2610 /tmp/install/stage/usr/local/bin/gem25
rm /tmp/gem.2610
Installing bundler executable
install -c -m 755 /tmp/bundle.2610 /tmp/install/stage/usr/local/bin/bundle25
rm /tmp/bundle.2610
mkdir -p /tmp/install/stage/usr/local/lib/ruby/gems/2.5/specifications/default
mkdir -p /tmp/install/stage/tmp/install/stage/usr/local/lib/ruby/gems/2.5/specifications/gems/bundler-1.16.2/exe
cp bundler/exe/bundle /tmp/install/stage/tmp/install/stage/usr/local/lib/ruby/gems/2.5/specifications/gems/bundler-1.16.2/exe/bundle
Bundler 1.16.2 installed
RubyGems 2.7.7 installed
Regenerating binstubs
ERROR:  While executing gem ... (Gem::FilePermissionError)
    You don't have write permissions for the /usr/local/bin directory.
	lib/rubygems/installer.rb:476:in `generate_bin'
	lib/rubygems/commands/pristine_command.rb:170:in `block in execute'
	lib/rubygems/specification.rb:1027:in `block in each'
	lib/rubygems/specification.rb:1026:in `each'
	lib/rubygems/specification.rb:1026:in `each'
	lib/rubygems/commands/pristine_command.rb:112:in `map'
	lib/rubygems/commands/pristine_command.rb:112:in `each'
	lib/rubygems/commands/pristine_command.rb:112:in `execute'
	lib/rubygems/command.rb:313:in `invoke_with_build_args'
	lib/rubygems/command.rb:291:in `invoke'
	lib/rubygems/commands/setup_command.rb:586:in `regenerate_binstubs'
	lib/rubygems/commands/setup_command.rb:155:in `execute'
	lib/rubygems/command.rb:313: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>'
zsh: exit 1     env -i GEM_HOME=/tmp/install/gem_home ruby setup.rb  --backtrace --verbose

I repeated last command with DONT_USE_BUNDLER_FOR_GEMDEPS=yes, because I
think it also "solved" this issue in a previous version, but I now obtain the
same result (attempting to write outside directory given to --destdir).

@segiddins
Copy link
Member

When packaging,
everything must happen in a single "staging" directory, which in a way is
equivalent to the file system root (/) for this package.

In that case, it seems like explicitly not regenerating binstubs is what you want?

@ghost
Copy link

ghost commented Aug 6, 2018

In that case, it seems like explicitly not regenerating binstubs is what you want?

I'd say yes, but only because I spent some time debugging and understanding the
problem, and because I "guessed" what I think is the intent of this feature
(--regenerate_binstubs). My conclusion was that only RubyGems developers (not
end-users) would want --regenerate_binstubs as default. Except maybe users
who would build/maintain their own OS without package manager like Linux From
Scratch maybe?

Could the default be changed to --regenerate_binstubs instead of
--no-regenerate_binstubs for RubyGems 3 maybe?

Now if we ignore the default value aspect, do we agree that there is a bug when
--regenerate_binstubs is used in combination with --destdir? That the first
one breaks the second one?

@ghost
Copy link

ghost commented Aug 6, 2018

Could the default be changed to --regenerate_binstubs instead of
--no-regenerate_binstubs for RubyGems 3 maybe?

Or RubyGems 4 if it's too late for 3 (#2191).

@segiddins
Copy link
Member

The idea was to regenerate them so changes/improvements in the bin stub contents would be rolled out to users

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants