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

crystal: reduce closure size of compiled exes by splitting outputs #75719

Merged
merged 1 commit into from Dec 20, 2019

Conversation

@manveru
Copy link
Contributor

@manveru manveru commented Dec 15, 2019

Motivation for this change

So far, compiled crystal programs retained a reference to crystal, which in turn depends on llvm and a ton of other things.
This makes sure that the output paths of crystal are split up, so only lib is still referenced for runtime backtrace output.

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 @david50407 @peterhoeg

@manveru manveru force-pushed the manveru:crystal-compiled-closure-size branch from 40c7cc8 to dfd103b Dec 15, 2019
@ofborg ofborg bot requested review from david50407 and peterhoeg Dec 15, 2019
@manveru manveru force-pushed the manveru:crystal-compiled-closure-size branch from dfd103b to 55bdfde Dec 15, 2019
@@ -56,6 +56,8 @@ let
inherit sha256;
};

outputs = [ "out" "lib" "bin" "share" ];

This comment has been minimized.

@matthewbauer

matthewbauer Dec 16, 2019
Member

"share" is a little bit unusual. For instance, we don't have a lib.getShare, like we do for lib.getDev, lib.getLib, and lib.getBin. Unless the share & out outputs are intentionally separate, I would recommend just putting share in $out.

This comment has been minimized.

@matthewbauer

matthewbauer Dec 16, 2019
Member

"doc" would be a little bit better if you would like.

This comment has been minimized.

@manveru

manveru Dec 16, 2019
Author Contributor

share has stuff like shell completions, the manpage, a bunch of html docs, and the license... I'm not sure it matters since this gets all symlinked into $out anyway, but if you feel strongly about it i don't mind removing it either.

@manveru manveru force-pushed the manveru:crystal-compiled-closure-size branch from 55bdfde to 42cc742 Dec 16, 2019
@grahamc grahamc merged commit b0a78e2 into NixOS:master Dec 20, 2019
15 checks passed
15 checks passed
crystal on aarch64-linux No attempt
Details
Evaluation Performance Report Evaluator Performance Report
Details
crystal on x86_64-linux Success
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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

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