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

@Profpatsch Profpatsch commented Dec 12, 2019

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.*
''}

This comment has been minimized.

@zimbatm

zimbatm Dec 13, 2019
Member

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

This comment has been minimized.

@Mic92

Mic92 Dec 13, 2019
Contributor

Does this break jit compiling?

This comment has been minimized.

@Profpatsch

Profpatsch Dec 16, 2019
Author Member

$ 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.

This comment has been minimized.

@alyssais

alyssais Dec 21, 2019
Member

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.

This comment has been minimized.

@Profpatsch

Profpatsch Dec 21, 2019
Author Member

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;

This comment has been minimized.

@Mic92

Mic92 Dec 13, 2019
Contributor

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

This comment has been minimized.

@Profpatsch

Profpatsch Dec 16, 2019
Author Member

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

This comment has been minimized.

@zimbatm

zimbatm Dec 16, 2019
Member

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;

This comment has been minimized.

@Mic92

Mic92 Dec 13, 2019
Contributor

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

This comment has been minimized.

@Profpatsch

Profpatsch Dec 21, 2019
Author Member

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

This comment has been minimized.

@Profpatsch

Profpatsch Dec 21, 2019
Author Member

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

@Profpatsch Profpatsch commented Dec 16, 2019

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

@Profpatsch
Copy link
Member Author

@Profpatsch Profpatsch commented Dec 16, 2019

@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

@Profpatsch Profpatsch commented Dec 21, 2019

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 alyssais commented Dec 22, 2019

@Profpatsch
Copy link
Member Author

@Profpatsch Profpatsch commented Dec 22, 2019

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 Profpatsch force-pushed the Profpatsch:add-rubyMinimal branch from 05912b0 to 2866ada Dec 22, 2019
@Profpatsch
Copy link
Member Author

@Profpatsch Profpatsch commented Dec 22, 2019

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

@alyssais
Copy link
Member

@alyssais alyssais commented Dec 22, 2019

@Profpatsch
Copy link
Member Author

@Profpatsch 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

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

@Profpatsch Profpatsch commented Dec 24, 2019

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

@Profpatsch Profpatsch commented Dec 24, 2019

Well, once I figure out the build failure …

@Profpatsch Profpatsch force-pushed the Profpatsch:add-rubyMinimal branch from b53c260 to e3aa2be Dec 25, 2019
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 force-pushed the Profpatsch:add-rubyMinimal branch from e3aa2be to c3babde Dec 25, 2019
@Profpatsch Profpatsch merged commit 7d4c1ae into NixOS:master Dec 25, 2019
16 checks passed
16 checks passed
Evaluation Performance Report Evaluator Performance Report
Details
grahamcofborg-eval ^.^!
Details
grahamcofborg-eval-check-maintainers matching changed paths to changed attrs...
Details
grahamcofborg-eval-check-meta config.nix: checkMeta = true
Details
grahamcofborg-eval-darwin nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./pkgs/top-level/release.nix -A darwin-tested
Details
grahamcofborg-eval-nixos nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./nixos/release-combined.nix -A tested
Details
grahamcofborg-eval-nixos-manual nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./nixos/release.nix -A manual
Details
grahamcofborg-eval-nixos-options nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./nixos/release.nix -A options
Details
grahamcofborg-eval-nixpkgs-manual nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./pkgs/top-level/release.nix -A manual
Details
grahamcofborg-eval-nixpkgs-tarball nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./pkgs/top-level/release.nix -A tarball
Details
grahamcofborg-eval-nixpkgs-unstable-jobset nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./pkgs/top-level/release.nix -A unstable
Details
grahamcofborg-eval-package-list nix-env -qa --json --file .
Details
grahamcofborg-eval-package-list-no-aliases nix-env -qa --json --file . --arg config { allowAliases = false; }
Details
ruby, rubyMinimal on aarch64-linux Success
Details
ruby, rubyMinimal on x86_64-darwin Success
Details
ruby, rubyMinimal on x86_64-linux Success
Details
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
Linked issues

Successfully merging this pull request may close these issues.

None yet

4 participants
You can’t perform that action at this time.