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.

Sign up for GitHub

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

Jump to bottom

notmuch: add separate emacs output #84663

Merged
merged 2 commits into from Apr 8, 2020
Merged

notmuch: add separate emacs output #84663

merged 2 commits into from Apr 8, 2020

Conversation

flokli
Copy link
Contributor

@flokli flokli commented Apr 7, 2020

Motivation for this change

Emacs takes around 150Mb in closure size. neomutt is a package which uses notmuch as a library and users that don't use Emacs don't need this feature enabled. Hence to reduce closure size, while still supporting Emacs users of notmuch the emacs-related binary and emacs lisp files were moved to a notmuch.emacs 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 nixpkgs-review --run "nixpkgs-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.

Supersedes #84610.

flokli and others added 2 commits April 7, 2020 22:02
@flokli
notmuch: make emacs a separate output
d0dd8e6 
This puts all emacs-related binaries and code to a separate output,
removing emacs from the runtime closure of neomutt.
@doronbehar @flokli
notmuch: remove bash-completion and just specify path
028fcf9
@flokli flokli changed the title Notmuch multiple output notmuch: add separate emacs output Apr 7, 2020
@flokli flokli mentioned this pull request Apr 7, 2020
Closed
10 tasks
doronbehar
doronbehar approved these changes Apr 7, 2020
View reviewed changes
Copy link
Contributor

@doronbehar doronbehar left a comment

Choose a reason for hiding this comment

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

I tested the build of it and nix-store -q --references $(nix-build -A notmuch) doesn't mention emacs. Great!

Do you know @flokli why it works? This kind of issues have been bothering me since my early days in NixOS so I'm very curios.

@flokli
Copy link
Contributor Author

flokli commented Apr 7, 2020

So, there's some documentation in doc/stdenv/multiple-output.xml, but I have to admin, it's very sparse.

I think in your previous iteration, the code moving manpages etc. was triggering a bug in multiple-outputs.sh:127, because there still was something else in $out/share/… ($out/share/emacs, in our case). Can you push the ref containing the example somewhere? Then we could open an issue to narrow it down and fix it proprly.

By moving things into the emacs output before multiple-outputs.sh, we might have avoided triggering this edgecase, as $out/share was empty after everything else was moved away by the default handlers.

@doronbehar
Copy link
Contributor

Can you push the ref containing the example somewhere?

TBH @flokli I don't care enough in order to narrow it down, sorry :/. You can trigger the bug yourself by taking the derivation as in master and add a "lib" output.

@flokli
Copy link
Contributor Author

flokli commented Apr 8, 2020

Yeah, I think we should move the discussion into that discourse post :-)

@flokli flokli merged commit 68d37fe into NixOS:master Apr 8, 2020
@flokli flokli deleted the notmuch-multiple-output branch April 8, 2020 11:31
@doronbehar doronbehar mentioned this pull request Apr 16, 2020
Closed
20 tasks
@lheckemann
Copy link
Member

This breaks emacs-notmuch (surprise!) :/ opinions on adding a release notes entry?

@doronbehar
Copy link
Contributor

Added ? https://github.com/NixOS/nixpkgs/pull/84663/files#diff-43de86228df91216c5cfc9446cb041fe

@lheckemann
Copy link
Member

🙈 sorry, missed that

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@doronbehar doronbehar doronbehar approved these changes

@the-kenny the-kenny Awaiting requested review from the-kenny

@puckipedia puckipedia Awaiting requested review from puckipedia

Assignees
No one assigned
Labels
6.topic: nixos 8.has: changelog 8.has: documentation 10.rebuild-darwin: 11-100 10.rebuild-linux: 11-100 11.by: package-maintainer
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@flokli @doronbehar @lheckemann