buildRubyGem: fix to support bundler install --redownload #104977

fzakaria · 2020-11-26T05:51:23Z

Motivation for this change

The way in which Nixpks builds Ruby gems means that certain operations by bundler will not work, namely bundle install --redownload.

According to the source the cache/ directory should have been kept, however it seems through revisions to the file it has been purged.

Here was the comment from the original commit that introduced buildRubyGem (still present in the source)

nixpkgs/pkgs/development/ruby-modules/gem/default.nix

Lines 156 to 160 in 8585991

    
           # Note: 
        
           #   We really do need to keep the $out/${ruby.gemPath}/cache. 
        
           #   This is very important in order for many parts of RubyGems/Bundler to not blow up. 
        
           #   See https://github.com/bundler/bundler/issues/3327 
        
           installPhase = attrs.installPhase or ''

Why is the /cache directory needed?

Bundler and RubyGems uses the cache as a source of truth. When bundler executes bundler install --redownload, any gems it
discovers in the GEM_PATH it assumes must have their .gem file present in the cache (unaware it was installed from Nix).

Rather than downloading the gem from RubyGems the bundler code forcibly re-installs the gem from the cache directory instead and fails if it does not exist.

I've opened rubygems/rubygems#4088 to see if this failure should be soft and not so explicit; or fallback to fetching the gem from scratch.

Without this change the following is the error:

[nix-shell:~/code/nix/playground/jruby-bundler-rake]$ bundle install --redownload
WARNING: An illegal reflective access operation has occurred
WARNING: Illegal reflective access by org.jruby.ext.openssl.SecurityHelper (file:/nix/store/fis6nzrpw9pmcivr84qh5byfgm07qn10-jruby-9.2.13.0/lib/ruby/stdlib/jopenssl.jar) to field java.security.MessageDigest.provider
WARNING: Please consider reporting this to the maintainers of org.jruby.ext.openssl.SecurityHelper
WARNING: Use --illegal-access=warn to enable warnings of further illegal reflective access operations
WARNING: All illegal access operations will be denied in a future release
Fetching gem metadata from https://rubygems.org/.
Using bundler 2.1.4
Installing hello-world 1.2.0
Bundler::GemNotFound: Could not find hello-world-1.2.0.gem for installation
An error occurred while installing hello-world (1.2.0), and Bundler
cannot continue.
Make sure that `gem install hello-world -v '1.2.0' --source
'https://rubygems.org/'` succeeds before bundling.

Wth the fix the following no error occurs:

[nix-shell:~/code/nix/playground/jruby-bundler-rake]$ bundle install --redownload
WARNING: An illegal reflective access operation has occurred
WARNING: Illegal reflective access by org.jruby.ext.openssl.SecurityHelper (file:/nix/store/69wjlj4yirp48rv1q03zxgd4xvf0150d-jruby-9.2.13.0/lib/ruby/stdlib/jopenssl.jar) to field java.security.MessageDigest.provider
WARNING: Please consider reporting this to the maintainers of org.jruby.ext.openssl.SecurityHelper
WARNING: Use --illegal-access=warn to enable warnings of further illegal reflective access operations
WARNING: All illegal access operations will be denied in a future release
Fetching gem metadata from https://rubygems.org/.
Using bundler 2.1.4
Installing hello-world 1.2.0
Bundle complete! 1 Gemfile dependency, 2 gems now installed.
Use `bundle info [gemname]` to see where a bundled gem is installed.

Here we can see the /cache directory hydrated.

[nix-shell:~/code/nix/playground/jruby-bundler-rake]$ ls -l /nix/store/cwl9n5073hqgpfhnw4wic13nrrgg9dn8-gem-env/lib/jruby/gems/2.5.0/cache/
total 8
lrwxrwxrwx 1 fmzakari primarygroup 102 Dec 31  1969 bundler-2.1.4.gem -> /nix/store/ifc8a0gsfkrhkv953rd4rz8bcspahi8y-bundler-2.1.4/lib/jruby/gems/2.5.0/cache/bundler-2.1.4.gem
lrwxrwxrwx 1 fmzakari primarygroup 110 Dec 31  1969 hello-world-1.2.0.gem -> /nix/store/xi9ln6n1mz2is5ppykjxqhhkpjq9zm6i-hello-world-1.2.0/lib/jruby/gems/2.5.0/cache/hello-world-1.2.0.gem

I have a minimal project that demonstrates this issue at https://github.com/fzakaria/jruby-bundler-nix-failure however it boils down to the following shell.nix file

let nixpkgs = import <nixpkgs> {};
in with nixpkgs;
with stdenv;
with stdenv.lib;
let gems = buildEnv {
  name = "gem-env";
  pathsToLink = ["/lib" "/bin" "nix-support"];
  paths = [
    (bundler.override {
      ruby = jruby;
    })
    (buildRubyGem rec {
      ruby = jruby;
      name = "${gemName}-${version}";
      gemName = "hello-world";
      version = "1.2.0";
      source.sha256 = "141r6pafbwjf8aczsilxxhdrdbbmdhimgbsq8m9qsvjm522ln15p";
    })

  ];
};
in
mkShell {
  name = "jruby-bundler-shell";
  buildInputs = [ jruby ];
  shellHook = ''
      export GEM_NIX_ENV=${gems}/${jruby.gemPath}
      export GEM_USER_DIR=$(pwd)/.gem
      export GEM_DEFAULT_DIR=$(${jruby}/bin/ruby -e 'puts Gem.default_dir')
      export GEM_PATH=$GEM_DEFAULT_DIR:$GEM_PATH:$GEM_NIX_ENV
      export GEM_HOME=$GEM_USER_DIR
      export PATH=$GEM_HOME/bin:$GEM_NIX_ENV/bin:$PATH
  '';
}

source "https://rubygems.org"

gem 'hello-world', '1.2.0'

Things done

CC @cstrahan @zimbatm @alyssais @manveru

RaghavSood · 2020-11-26T06:18:14Z

Please base this on staging instead of master, mass rebuilds should target staging so that they don't hold up channel builds

fzakaria · 2020-11-26T06:27:33Z

@RaghavSood thanks for the feedback; changed the target branch to staging

Others looking at the PR may wonder the motivation:
At my employer I use a mix of installing some gems via Nix through buildEnv however we still rely on bundler for many gems.
(incremental transition)

Gems which exist in the Gemfile (direct or transitive) that bundler wants to install that were also installed via Nix succumb to the error described in the description above.

zimbatm · 2020-11-27T10:21:51Z

I had a quick look and it makes sense. The main downside is that it will increase all the program's closure sizes, even those that are not being used for development. Can this be enabled with a flag instead?

fzakaria · 2020-11-29T04:42:36Z

@zimbatm -- I can add a flag.
I feel inclined to make the case that the flag however should default to "yes".
The original comments and goal of buildRubyGem is to build the gems in a way that is consistent with Bundler.
Having it default yes aligns closer to that goal.

fzakaria

@zimbatm thank you for looking at this!

I've taken your advice and added a comment.
In the spirit of getting this merged faster I've kept the default "false" although my personal opinion is it should largely be true by default.

🙏

fzakaria · 2020-11-29T04:55:41Z

pkgs/development/ruby-modules/gem/default.nix

+# such as `bundler install --redownload`.
+# At the cost of increasing the store size, you can keep the gems to have closer
+# alignment with what Bundler expects.
+, keepGemCache ? false


@zimbatm actually decided to keep it default false largely because I rather see this merged sooner
(trying to upstream my personal fixes).

Sounds good

zimbatm · 2020-11-29T14:14:58Z

pkgs/development/ruby-modules/gem/default.nix

+
+    # Do not remove the cache as this is needed to make some additional Bundler commands work
+    # such as `bundler install --redownload`
+    # https://github.com/rubygems/rubygems/issues/4088
+    # https://github.com/bundler/bundler/issues/3327
+    # rm -fvr cache


If you move this comment out of the bash script, it won't cause a mass rebuild and can target the master branch directly.

Thanks -- took your suggestion.
I'm curious, how does staging branch make it to master?

The way in which Nixpks builds Ruby gems means that certain operations by bundler *will not work*, namely `bundle install --redownload`. According to the source the _cache/_ directory should have been kept, however it seems through revisions to the file it has been purged. Here was the comment from the original commit that introduced buildRubyGem: ``` # Note: # We really do need to keep the $out/${ruby.gemPath}/cache. # This is very important in order for many parts of RubyGems/Bundler to not blow up. # See rubygems/bundler#3327 ``` Why is the _cache_ directory needed? Bundler and RubyGems uses the cache as a source of truth. When bundler executes `bundler install --redownload`, any gems it discovers in the _GEM_PATH_ it assums must have their _.gem_ file present in the cache (unaware it was installed from Nix). Rather than downloading the gem from RubyGems the bundler code forcibly re-installs the gem from the cache directory instead and **fails** if it does not exist. I've opened rubygems/rubygems#4088 to see if this failure should be soft and not so explicit; or fallback to fetching the gem from scratch. Without this change the following is the error: ```bash > [nix-shell:~/code/nix/playground/jruby-bundler-rake]$ bundle install --force [DEPRECATED] The `--force` option has been renamed to `--redownload` WARNING: An illegal reflective access operation has occurred WARNING: Illegal reflective access by org.jruby.ext.openssl.SecurityHelper (file:/nix/store/fis6nzrpw9pmcivr84qh5byfgm07qn10-jruby-9.2.13.0/lib/ruby/stdlib/jopenssl.jar) to field java.security.MessageDigest.provider WARNING: Please consider reporting this to the maintainers of org.jruby.ext.openssl.SecurityHelper WARNING: Use --illegal-access=warn to enable warnings of further illegal reflective access operations WARNING: All illegal access operations will be denied in a future release Fetching gem metadata from https://rubygems.org/. Using bundler 2.1.4 Installing hello-world 1.2.0 Bundler::GemNotFound: Could not find hello-world-1.2.0.gem for installation An error occurred while installing hello-world (1.2.0), and Bundler cannot continue. Make sure that `gem install hello-world -v '1.2.0' --source 'https://rubygems.org/'` succeeds before bundling. ``` Wth the fix the following no woccurs: ```bash [nix-shell:~/code/nix/playground/jruby-bundler-rake]$ bundle install --redownload WARNING: An illegal reflective access operation has occurred WARNING: Illegal reflective access by org.jruby.ext.openssl.SecurityHelper (file:/nix/store/69wjlj4yirp48rv1q03zxgd4xvf0150d-jruby-9.2.13.0/lib/ruby/stdlib/jopenssl.jar) to field java.security.MessageDigest.provider WARNING: Please consider reporting this to the maintainers of org.jruby.ext.openssl.SecurityHelper WARNING: Use --illegal-access=warn to enable warnings of further illegal reflective access operations WARNING: All illegal access operations will be denied in a future release Fetching gem metadata from https://rubygems.org/. Using bundler 2.1.4 Installing hello-world 1.2.0 Bundle complete! 1 Gemfile dependency, 2 gems now installed. Use `bundle info [gemname]` to see where a bundled gem is installed. ``` ``` [nix-shell:~/code/nix/playground/jruby-bundler-rake]$ ls -l /nix/store/cwl9n5073hqgpfhnw4wic13nrrgg9dn8-gem-env/lib/jruby/gems/2.5.0/cache/ total 8 lrwxrwxrwx 1 fmzakari primarygroup 102 Dec 31 1969 bundler-2.1.4.gem -> /nix/store/ifc8a0gsfkrhkv953rd4rz8bcspahi8y-bundler-2.1.4/lib/jruby/gems/2.5.0/cache/bundler-2.1.4.gem lrwxrwxrwx 1 fmzakari primarygroup 110 Dec 31 1969 hello-world-1.2.0.gem -> /nix/store/xi9ln6n1mz2is5ppykjxqhhkpjq9zm6i-hello-world-1.2.0/lib/jruby/gems/2.5.0/cache/hello-world-1.2.0.gem ``` I have a minimal project that demonstrates this issue at https://github.com/fzakaria/jruby-bundler-nix-failure

fzakaria requested a review from alyssais as a code owner November 26, 2020 05:51

fzakaria changed the title ~~buildRubyGem: fix to support bundler cmds~~ buildRubyGem: fix to support bundler install --redownload Nov 26, 2020

fzakaria mentioned this pull request Nov 26, 2020

Bundler should not try to force install StubSpecifications rubygems/rubygems#4088

Open

ofborg bot added the 6.topic: ruby label Nov 26, 2020

ofborg bot requested review from Ma27, manveru, zimbatm, marsam and nixy November 26, 2020 06:14

ofborg bot added 10.rebuild-darwin: 501+ 10.rebuild-darwin: 1001-2500 10.rebuild-linux: 501+ 10.rebuild-linux: 5001+ labels Nov 26, 2020

fzakaria changed the base branch from master to staging November 26, 2020 06:25

fzakaria force-pushed the nixpkgs-ruby-bundler-fix branch from 38b68cd to 2b00a49 Compare November 29, 2020 04:55

fzakaria commented Nov 29, 2020

View reviewed changes

zimbatm reviewed Nov 29, 2020

View reviewed changes

fzakaria force-pushed the nixpkgs-ruby-bundler-fix branch from 2b00a49 to cac20fb Compare November 30, 2020 04:30

fzakaria changed the base branch from staging to master November 30, 2020 04:31

fzakaria requested a review from zimbatm November 30, 2020 04:32

ofborg bot added 10.rebuild-darwin: 0 10.rebuild-linux: 0 and removed 10.rebuild-darwin: 501+ 10.rebuild-darwin: 1001-2500 10.rebuild-linux: 501+ labels Nov 30, 2020

ofborg bot removed the 10.rebuild-linux: 5001+ label Nov 30, 2020

zimbatm merged commit 4af8bc0 into NixOS:master Nov 30, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

buildRubyGem: fix to support bundler install --redownload #104977

buildRubyGem: fix to support bundler install --redownload #104977

fzakaria commented Nov 26, 2020 •

edited

RaghavSood commented Nov 26, 2020

fzakaria commented Nov 26, 2020 •

edited

zimbatm commented Nov 27, 2020

fzakaria commented Nov 29, 2020

fzakaria left a comment

fzakaria Nov 29, 2020

zimbatm Nov 29, 2020

zimbatm Nov 29, 2020

fzakaria Nov 30, 2020

	# Note:
	# We really do need to keep the $out/${ruby.gemPath}/cache.
	# This is very important in order for many parts of RubyGems/Bundler to not blow up.
	# See https://github.com/bundler/bundler/issues/3327
	installPhase = attrs.installPhase or ''

buildRubyGem: fix to support bundler install --redownload #104977

buildRubyGem: fix to support bundler install --redownload #104977

Conversation

fzakaria commented Nov 26, 2020 • edited

Motivation for this change

Things done

RaghavSood commented Nov 26, 2020

fzakaria commented Nov 26, 2020 • edited

zimbatm commented Nov 27, 2020

fzakaria commented Nov 29, 2020

fzakaria left a comment

Choose a reason for hiding this comment

fzakaria Nov 29, 2020

Choose a reason for hiding this comment

zimbatm Nov 29, 2020

Choose a reason for hiding this comment

zimbatm Nov 29, 2020

Choose a reason for hiding this comment

fzakaria Nov 30, 2020

Choose a reason for hiding this comment

fzakaria commented Nov 26, 2020 •

edited

fzakaria commented Nov 26, 2020 •

edited