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

notmuch: set emacs off by default #84610

Closed
wants to merge 6 commits into from

Conversation

doronbehar
Copy link
Contributor

@doronbehar doronbehar 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 I turned that feature to off by default and added a notmuch-emacs package targeting these users of notmuch. I created a notmuch-lib package that is used by neomutt which has a much lower closure size - 560Kb because it is compiled with less features enabled.

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.

Add a build named notmuch-emacs which adds emacs support for users who
want it.
@pbogdan
Copy link
Member

pbogdan commented Apr 7, 2020

Would it be possible instead to add a lib output to notmuch? In theory (I haven't tried) it should remove Emacs from the runtime closure for consumers that only use the library component itself.

@flokli
Copy link
Contributor

flokli commented Apr 7, 2020

This has been discussed in #84008, and should probably still be built, but a separate output of some sort. That way, it'd be outside the runtime closure.

@doronbehar
Copy link
Contributor Author

I see, then allow me to take this 1 step even further. I hope you'll agree the next commits are better. Notably, there's no attribute rename now - notmuch is the derivation which has both the ruby script and the emacs stuff.

API docs are not installed even with them available during build.
Add notmuch-lib and use it for neomutt.
perl & python are build inputs only, emacs & ruby are runtime dependency
if you want to use this feature.
splitted outputs don't work now from some reason, but it's not that
important as user who wishes not to have the documentation installed can
use the attribute `notmuch-lib`.
@flokli
Copy link
Contributor

flokli commented Apr 7, 2020

Well, this will not pull in emacs two times when I configure environment.systemPackages = [ pkgs.nemoutt pkgs.notmuch ];. I still think multiple outputs are the way to go here.

@doronbehar
Copy link
Contributor Author

Well, this will not pull in emacs two times when I configure environment.systemPackages = [ pkgs.nemoutt pkgs.notmuch ];

No, but it will pull two different version of notmuch - the 1st notmuch is essentially notmuch-lib which has a size of 560Kb while the other notmuch will have a size of 1.4M.

@doronbehar
Copy link
Contributor Author

I really don't know why the split outputs got broke during my build trials, as a maintainer of this package, if you feel this is a game changer for you I'd be ready to dive deeper into this. The issue was that I got rmdir: directory not empty errors for $out/share - no idea how that worked before.

My suspicion is that the emacs files weren't moved and as a consequence multiple-outputs.sh failed. That's why personally I like to avoid splitting outputs because Nixpkgs' multiple-outputs.sh is not that reliable.

BTW, just for the record, even with the current state of the derivation, the split outputs don't help removing the derivation's reference to emacs, unless you build it yourself with withEmacs = false; I just wanted that to be the default for neomutt because adding an overlay for that means you build both notmuch and neomutt in order to save 150Mb of Emacs.

@flokli
Copy link
Contributor

flokli commented Apr 7, 2020

@doronbehar I think multiple outputs were made exactly for these usecases. We should get notmuch.lib to work, instead of reinventing notmuch-lib.

I assume bin/notmuch-emacs-mua and share/emacs would both need to be moved to the emacs output - How did the mutli-output approach fail? In the case of circular references, it should show where.

Do you have the commits somewhere that tried to move emacs to a separate output? I could then take a look.

@doronbehar
Copy link
Contributor Author

@doronbehar I think multiple outputs were made exactly for these usecases. We should get notmuch.lib to work, instead of reinventing notmuch-lib.

Technically I agree it's just that I have some opinions regarding the current implementation of this idea in Nixpkgs. This is out of scope for this PR.
This is what I got when I went back to master and added "lib" to outputs:

post-installation fixup
Moving /nix/store/9cjz23y381a66daq0m95749159p1h1bx-notmuch-0.29.3/share/man to /nix/store/ffbfvbh6p854887dx8yc8dwy066x4zmx-notmuch-0.29.3-man/share/man
rmdir: failed to remove '/nix/store/9cjz23y381a66daq0m95749159p1h1bx-notmuch-0.29.3/share': Directory not empty
shrinking RPATHs of ELF executables and libraries in /nix/store/9cjz23y381a66daq0m95749159p1h1bx-notmuch-0.29.3
shrinking /nix/store/9cjz23y381a66daq0m95749159p1h1bx-notmuch-0.29.3/lib/libnotmuch.so.5.2.0
shrinking /nix/store/9cjz23y381a66daq0m95749159p1h1bx-notmuch-0.29.3/bin/notmuch
strip is /nix/store/sq2b0dqlq243mqn4ql5h36xmpplyy20k-binutils-2.31.1/bin/strip
stripping (with command strip and flags -S) in /nix/store/9cjz23y381a66daq0m95749159p1h1bx-notmuch-0.29.3/lib  /nix/store/9cjz23y381a66daq0m95749159p1h1bx-notmuch-0.29.3/bin
patching script interpreter paths in /nix/store/9cjz23y381a66daq0m95749159p1h1bx-notmuch-0.29.3
checking for references to /build/ in /nix/store/9cjz23y381a66daq0m95749159p1h1bx-notmuch-0.29.3...
shrinking RPATHs of ELF executables and libraries in /nix/store/ffbfvbh6p854887dx8yc8dwy066x4zmx-notmuch-0.29.3-man
strip is /nix/store/sq2b0dqlq243mqn4ql5h36xmpplyy20k-binutils-2.31.1/bin/strip
patching script interpreter paths in /nix/store/ffbfvbh6p854887dx8yc8dwy066x4zmx-notmuch-0.29.3-man
checking for references to /build/ in /nix/store/ffbfvbh6p854887dx8yc8dwy066x4zmx-notmuch-0.29.3-man...
shrinking RPATHs of ELF executables and libraries in /nix/store/aqh84bqs0zrfmcm5fddybi7al57j7rv9-notmuch-0.29.3-info
strip is /nix/store/sq2b0dqlq243mqn4ql5h36xmpplyy20k-binutils-2.31.1/bin/strip
patching script interpreter paths in /nix/store/aqh84bqs0zrfmcm5fddybi7al57j7rv9-notmuch-0.29.3-info
checking for references to /build/ in /nix/store/aqh84bqs0zrfmcm5fddybi7al57j7rv9-notmuch-0.29.3-info...
find: '/nix/store/5qv7m2ma4pzi80zj3prmpfl0bhfblf4x-notmuch-0.29.3-lib': No such file or directory
strip is /nix/store/sq2b0dqlq243mqn4ql5h36xmpplyy20k-binutils-2.31.1/bin/strip
builder for '/nix/store/gsl65x0fax54v402b384asisfflxzyg6-notmuch-0.29.3.drv' failed to produce output path '/nix/store/5qv7m2ma4pzi80zj3prmpfl0bhfblf4x-notmuch-0.29.3-lib'
error: build of '/nix/store/gsl65x0fax54v402b384asisfflxzyg6-notmuch-0.29.3.drv' failed

@doronbehar
Copy link
Contributor Author

Do you have the commits somewhere that tried to move emacs to a separate output? I could then take a look.

I didn't try to put emacs in it's own output, I only tried to keep the outputs attribute as is while I prepared the PR and at some point I started to get very similar errors to this one.

@flokli
Copy link
Contributor

flokli commented Apr 7, 2020

@doronbehar in your case, the build still failed because share/ wasn't empty after moving docs - as the emacs lisp stuff still was inside.

In fact, you can tell notmuch to put emacs lisp files into another location by the --emacslispdir configureFlag. If you then also move notmuch-emacs-mua, you end up eliminating all references to emacs in the out derivation.

I pushed a commit doing that, and cherry-picking the bash completion commit to https://github.com/flokli/nixpkgs/tree/notmuch-multiple-output.

@flokli flokli mentioned this pull request Apr 7, 2020
10 tasks
@flokli
Copy link
Contributor

flokli commented Apr 7, 2020

I added some release notes and opened #84663, PTAL.

@doronbehar
Copy link
Contributor Author

Closed in favor of #84663 .

@doronbehar doronbehar closed this Apr 7, 2020
@doronbehar doronbehar deleted the notmuch-emacs branch April 7, 2020 20:37
@nixos-discourse
Copy link

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/could-weve-implemented-multi-output-packages-better/6597/1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants