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

Add rubyMinimal #75566

Merged
merged 2 commits into from Dec 25, 2019
Merged

Add rubyMinimal #75566

merged 2 commits into from Dec 25, 2019

Conversation

Profpatsch
Copy link
Member

For small scripts that don’t need the full interpreter with all dependencies (especially runtime dependencies)

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nix-review --run "nix-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.
Notify maintainers

cc @

${removeReferencesTo}/bin/remove-references-to \
-t ${stdenv.cc} \
$out/lib/libruby.so.*
''}
Copy link
Member

Choose a reason for hiding this comment

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

could this be done every time? sounds interesting to remove it if we can

Copy link
Member

Choose a reason for hiding this comment

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

Does this break jit compiling?

Copy link
Member Author

Choose a reason for hiding this comment

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

$ nix-shell -p ruby --run 'ruby -e "puts RbConfig::CONFIG['configure_args']"'
'--prefix=/nix/store/b3lhi1wlia02vyd94jk56bm3kvgf1yc0-ruby-2.6.5' '--bindir=/nix/store/b3lhi1wlia02vyd94jk56bm3kvgf1yc0-ruby-2.6.5/bin' '--sbindir=/nix/store/b3lhi1wlia02vyd94jk56bm3kvgf1yc0-ruby-2.6.5/sbin' '--includedir=/nix/store/b3lhi1wlia02vyd94jk56bm3kvgf1yc0-ruby-2.6.5/include' '--oldincludedir=/nix/store/b3lhi1wlia02vyd94jk56bm3kvgf1yc0-ruby-2.6.5/include' '--mandir=/nix/store/b3lhi1wlia02vyd94jk56bm3kvgf1yc0-ruby-2.6.5/share/man' '--infodir=/nix/store/b3lhi1wlia02vyd94jk56bm3kvgf1yc0-ruby-2.6.5/share/info' '--docdir=/nix/store/b3lhi1wlia02vyd94jk56bm3kvgf1yc0-ruby-2.6.5/share/doc/' '--libdir=/nix/store/b3lhi1wlia02vyd94jk56bm3kvgf1yc0-ruby-2.6.5/lib' '--libexecdir=/nix/store/b3lhi1wlia02vyd94jk56bm3kvgf1yc0-ruby-2.6.5/libexec' '--localedir=/nix/store/b3lhi1wlia02vyd94jk56bm3kvgf1yc0-ruby-2.6.5/share/locale' '--enable-shared' '--enable-pthread' 'CC=gcc' 'CXX=g++'

Dunno which gems need it at runtime, I wouldn’t risk it by default.

Copy link
Member

Choose a reason for hiding this comment

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

Any gem that does need this at runtime probably wouldn’t work on a pure Nix system anyway, so I’d be happy disabling this across the board.

Copy link
Member Author

Choose a reason for hiding this comment

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

Won’t do it in this PR, but feel free to open another one for the proper ruby package. :)
Though of course it would break systems which might be using that.

@@ -9312,6 +9312,20 @@ in
ruby_2_5
ruby_2_6;

rubyMinimal = ruby.override {
useRailsExpress = false;
Copy link
Member

@Mic92 Mic92 Dec 13, 2019

Choose a reason for hiding this comment

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

Why would you strip this one? It's just a small patchset.

Copy link
Member Author

Choose a reason for hiding this comment

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

We can enable it by default, I have no idea about what any of these options do to be honest 😅.

Copy link
Member

Choose a reason for hiding this comment

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

railsExpress is a patchset on top of ruby that was developed by some rails developer to make things run faster. It doesn't affect the closure size but creates a non-vanilla version of ruby. Might as well use the vanilla ruby for the minimal version.

useRailsExpress = false;
rubygemsSupport = false;
zlibSupport = false;
opensslSupport = false;
Copy link
Member

@Mic92 Mic92 Dec 13, 2019

Choose a reason for hiding this comment

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

What are you using ruby for that does not support HTTPS/HTTP with deflate support?

Copy link
Member Author

Choose a reason for hiding this comment

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

It’s about the minimal runtime closure, not so much about supporting openssl.

Copy link
Member Author

Choose a reason for hiding this comment

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

But if http clients are a thing most small ruby scripts would like to use, we should enable it.

For example the home-manager service activation script doesn’t need SSL.

@FRidh FRidh added this to Needs review in Staging Dec 15, 2019
@Profpatsch
Copy link
Member Author

Btw this is only a staging PR because of the whitespace change m(

@Profpatsch
Copy link
Member Author

@Mic92 Feel free to propose a set of options we should enable by default even for rubyMinimal. I’m not a ruby user, just a grumpy devops guy who wants minimal CI overhead. :)

@Profpatsch
Copy link
Member Author

Here’s a short breakdown:

useRailsExpress: unofficial patchset
rubygemsSupport: small scripts, no gems required
zlibSupport: zlib module (pretty specific)
opensslSupport: https/ssl; important for webclients, but imposes a runtime dependency openssl
gdbmSupport: age-old key-value database
cursesSupport: curses is pretty specific, usually not required for simple scripts
fiddleSupport: libffi runtime dependency
yamlSupport: libyaml runtime dependency
docSupport: not required for small scripts

@alyssais
Copy link
Member

alyssais commented Dec 22, 2019 via email

@Profpatsch
Copy link
Member Author

I think ever user of rubyMinimal will have at least one other package that uses OpenSSL anyway.

That doesn’t necessarily hold on restricted systems or on CI systems which get redeployed on every run.

I use gems a lot in small scripts -- how big a difference does
Rubygems make? I imagine it's pretty tiny.

Then let’s enable gems

@Profpatsch
Copy link
Member Author

Okay, from my side this is ready to merge. Should we push it to staging or is master fine?

@alyssais
Copy link
Member

alyssais commented Dec 22, 2019 via email

@Profpatsch
Copy link
Member Author

Profpatsch commented Dec 22, 2019

Do the machines you want to use this on otherwise have OpenSSL installed?

I don’t know, I also don’t know whether they have necessarily the same version of openssl that would come with rubyMinimal (projects are usually pinned to a wholly different pin of nixpkgs than the system closure).

Copy link
Member

@zimbatm zimbatm left a comment

Choose a reason for hiding this comment

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

Since it's just an addition, I am fine with not resolving the smaller details. There are some open questions which can be resolved later in time.

Staging automation moved this from Needs review to Ready Dec 23, 2019
Reduces the runtime closure by ~200MB if enabled.
@Profpatsch
Copy link
Member Author

Okay, I rebased and got rid of the space changes in the ruby script, so no mass rebuild required. Merging.

@Profpatsch
Copy link
Member Author

Well, once I figure out the build failure …

Similar to `gitMinimal` or `pythonMinimal`, this is useful for scripts
which don’t use anything but the standard library and want a small
footprint.
@Profpatsch Profpatsch merged commit 7d4c1ae into NixOS:master Dec 25, 2019
Staging automation moved this from Ready to Done Dec 25, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Staging
  
Done
Development

Successfully merging this pull request may close these issues.

None yet

4 participants