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

default to including "man" in outputsToInstall #35884

Merged
merged 3 commits into from
Jan 31, 2019

Conversation

dtzWill
Copy link
Member

@dtzWill dtzWill commented Feb 27, 2018

Fixes #24717

In particular, man tmux works after installing tmux with nix-env.

This likely breaks things or is bad for some reason, but should work as a starting point.

TODO:

  • Ensure NixOS programs.man.enable = false; works (as well as = true).

@Ericson2314
Copy link
Member

  1. If this indeed is sufficient, this is super important to merge.

  2. What about info outputs?

@dezgeg
Copy link
Contributor

dezgeg commented Feb 27, 2018

I think if you do this, programs.man.enable = false; in a NixOS configuration doesn't stop installing the manpages anymore.

I don't know of an easier solution right now, though. Maybe nix-env itself should call into some nixpkgs function to build user environments (which could then look in config.enableMan or something to see whether manpages should be installed or not).

@edolstra ?

@jokogr
Copy link
Contributor

jokogr commented Mar 4, 2018

I do have some installations where I prefer not to have man pages installed to save some storage space, so please do not merge this right away.

@Ericson2314
Copy link
Member

@dezgeg How about we control this with a config option?

@dezgeg
Copy link
Contributor

dezgeg commented Mar 5, 2018

The problem is that it's use-case dependant of whether the man output needs to be installed in a particular buildEnv or not, that would be a global solution to a local problem.

@Ericson2314
Copy link
Member

@dezgeg excellent point. Can we give buildEnv an additionalOutputsToInstall parameter, which is a whitelist or predicte on output names?

@fpletz
Copy link
Member

fpletz commented Mar 12, 2018

Does anybody know what broke this? Did multiple outputs break this? Or Nix 2.0?

I'm not using nix-env much so I'm not sure. If this didn't work in 17.09 or 17.03, this isn't a release blocker imho.

@Ericson2314
Copy link
Member

@fpletz I'm not sure it ever worked, but I don't really know.

@matthewbauer
Copy link
Member

matthewbauer commented Sep 8, 2018

Nix only started looking at outputsToInstall Nix 2.0, IIRC. Previously it would just install everything. See NixOS/nix#815

@Ericson2314
Copy link
Member

Good sleuthing! Can we get a nix-level fix for 18.09??

@matthewbauer matthewbauer mentioned this pull request Sep 19, 2018
9 tasks
@matthewbauer matthewbauer modified the milestones: 18.03, 18.09 Oct 1, 2018
@nixos-discourse
Copy link

This pull request has been mentioned on Nix community. There might be relevant details there:

https://discourse.nixos.org/t/nix-env-not-linking-man-pages-into-environment/1468/2

@GrahamcOfBorg GrahamcOfBorg added the 6.topic: stdenv Standard environment label Jan 26, 2019
@matthewbauer
Copy link
Member

matthewbauer commented Jan 27, 2019

I'd like to get this merged for 19.03 but I can't figure out what the error is coming from...

@dtzWill
Copy link
Member Author

dtzWill commented Jan 27, 2019

I'd like to get this merged for 19.03 but I can't figure out what the error is coming from...

weechat error seems to due to copying meta (including meta.outputsToInstall) to the wrapper expression which doesn't have a man output. Not sure how to best solve but seems the problem is 'best' not whether it can be solved at all :). Maybe wrapper should be propagating the man pages too? Or filter outputsToInstall when copying meta? Dunno.

@matthewbauer matthewbauer merged commit 8020bd6 into NixOS:master Jan 31, 2019
@dtzWill
Copy link
Member Author

dtzWill commented Jan 31, 2019

!!!! 🎊 👍 😸

@LnL7
Copy link
Member

LnL7 commented Jan 31, 2019

@dezgeg I'm pretty sure programs.man.enable is not affected by this.

If I'm not mistaken the outputs of system packages are determined by environment.extraOutputsToInstall, not individual package metadata. So if that's correct this only affects nix-env -i.

EDIT: Looks like buildEnv does look at both, but /share/man paths are not included by default so it should still be ignored.

@masaeedu
Copy link
Contributor

masaeedu commented May 5, 2019

Awesome, thank you so much!

@lilyball
Copy link
Member

We should probably extend this to include info by default.

@matthewbauer
Copy link
Member

We should probably extend this to include info by default.

Perhaps, although at some point it makes sense to just drop outputsToInstall altogether and use nix-env's default behavior of installing everything. #44168

@lilyball
Copy link
Member

The downside to just installing everything is this means downloading more artifacts that I may not want. For example, if a given package vends a binary, a library, and dev headers, and all I care about is the binary (which is why I'm installing it directly), I certainly don't need to download and install the dev headers.

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.

Missing man pages for packages with multiple outputs