Skip to content

haskellPackages.Agda: Split outputs to reduce closure size#302351

Merged
ncfavier merged 1 commit intoNixOS:masterfrom
phijor:agda-separate-bin-output
Apr 14, 2024
Merged

haskellPackages.Agda: Split outputs to reduce closure size#302351
ncfavier merged 1 commit intoNixOS:masterfrom
phijor:agda-separate-bin-output

Conversation

@phijor
Copy link
Contributor

@phijor phijor commented Apr 7, 2024

After enabling a separate binary output for the Agda Haskell package, the new bin output measures ~100MiB, compared to the ~4.5GiB before. Using it in agdaWithPackages reduces the closure size of an Agda installation from ~5GiB to ~3GiB. The remaining space is taken up mostly by the GHC backend.

With this change, derivations depending on haskellPackages.Agda directly need to pick the right (binary) output. This concerns in particular emacsPackages.agda2-mode.

This is an attempt to resolve #285261.

Description of changes

Things done

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
    • sandbox = relaxed
    • sandbox = true
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 24.05 Release Notes (or backporting 23.05 and 23.11 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md.

Add a 👍 reaction to pull requests you find important.

After enabling a separate binary output for the `Agda` Haskell package,
the new `bin` output measures ~100MiB, compared to the ~4.5GiB before.
Using it in `agdaWithPackages` reduces the closure size of an Agda
installation from ~5GiB to ~3GiB.  The remaining space is taken up
mostly by the GHC backend.

With this change, derivations depending on `haskellPackages.Agda`
directly need to pick the right (binary) output.  This concerns in
particular `emacsPackages.agda2-mode`.
@github-actions github-actions bot added 6.topic: haskell General-purpose, statically typed, purely functional programming language 6.topic: emacs Text editor 6.topic: agda A dependently typed programming language / interactive theorem prover labels Apr 7, 2024
@ofborg ofborg bot requested review from abbradar, iblech and turion April 7, 2024 14:05
@ofborg ofborg bot added 10.rebuild-darwin: 11-100 This PR causes between 11 and 100 packages to rebuild on Darwin. 10.rebuild-linux: 11-100 This PR causes between 11 and 100 packages to rebuild on Linux. labels Apr 7, 2024
@github-actions github-actions bot added the 6.topic: vim Advanced text editor label Apr 7, 2024
@phijor phijor force-pushed the agda-separate-bin-output branch from 7f6859c to 294245f Compare April 7, 2024 17:18
@github-actions github-actions bot removed the 6.topic: vim Advanced text editor label Apr 7, 2024
@turion
Copy link
Contributor

turion commented Apr 8, 2024

@ofborg build agda.passthru.tests.allPackages

@iblech
Copy link
Contributor

iblech commented Apr 8, 2024

I believe I very much like this, thank you for your work :-), but can probably only test on Saturday.

@turion
Copy link
Contributor

turion commented Apr 8, 2024

CC @alexarice @neosimsim

@phijor
Copy link
Contributor Author

phijor commented Apr 8, 2024

All tests seem to pass; I'll mark this PR as ready for review. This is my first time working on code that a lot of packages depend on, so any feedback and comments are appreciated.

I have used this successfully locally for Agda development with Neovim + cornelis. Can anyone who uses emacsPackages.agda-mode and/or vimPackages.vim-agda confirm that I didn't break editor integration?

I believe I very much like this, thank you for your work :-), but can probably only test on Saturday.

Thank you, I'd appreciate that a lot!

@phijor phijor marked this pull request as ready for review April 8, 2024 17:15
Comment on lines -43 to +44
inherit (Agda) meta;
# Agda is a split package with multiple outputs; do not inherit them here.
meta = removeAttrs Agda.meta [ "outputsToInstall" ];
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The interaction of multiple-output packages and outputsToInstall was very surprising to me (Why does changing a meta attribute cause an evaluation error?? I believe I ran into #255947). Is removing outputsToInstall for the derived package the right thing to do here?

@wegank wegank added 12.approvals: 1 This PR was reviewed and approved by one person. 12.approved-by: package-maintainer This PR was reviewed and approved by a maintainer listed in any of the changed packages. labels Apr 9, 2024
Copy link
Member

@ncfavier ncfavier left a comment

Choose a reason for hiding this comment

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

LGTM, haven't tested.

@wegank wegank added 12.approvals: 2 This PR was reviewed and approved by two persons. and removed 12.approvals: 1 This PR was reviewed and approved by one person. labels Apr 9, 2024
@iblech
Copy link
Contributor

iblech commented Apr 13, 2024

I have now tested this with my workflow -- everything works as it should. (I'm using the agda-mode for Emacs, however I'm not using emacsPackages.agda-mode.) Thank you for your work! Unfortunately I cannot comment on your question regading outputsToInstall.

I propose to handle the issue of unconditional dependency on GHC at some later point and don't hold off this pull request just for that reaon.

@ncfavier ncfavier merged commit 25f42f0 into NixOS:master Apr 14, 2024
@phijor phijor deleted the agda-separate-bin-output branch April 14, 2024 13:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

6.topic: agda A dependently typed programming language / interactive theorem prover 6.topic: emacs Text editor 6.topic: haskell General-purpose, statically typed, purely functional programming language 10.rebuild-darwin: 11-100 This PR causes between 11 and 100 packages to rebuild on Darwin. 10.rebuild-linux: 11-100 This PR causes between 11 and 100 packages to rebuild on Linux. 12.approvals: 2 This PR was reviewed and approved by two persons. 12.approved-by: package-maintainer This PR was reviewed and approved by a maintainer listed in any of the changed packages.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Should we reduce Agda closure size by 4.4 GiB by unbundling libHSAgda.a and GHC?

5 participants