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

make-derivation: install all outputs #44168

Closed
wants to merge 3 commits into from

Conversation

matthewbauer
Copy link
Member

This confuses users too much and the old behavior to me seems
to have very little benefit. Since it only effect “declarative” installation,
it doesn’t seem helpful to not give every output available. In this case, users
want what would normally be included in the /zlib/ package, not just
one output. Individual packages can still specify what outputs to install
with meta.outputsToInstall.

Fixes #44144
Fixes #24717

This confuses users too much and to me seems to have very little
benefit. Since it only effect “declarative” installation, it doesn’t
seem helpful to not give every output available. In this case, users
want what would normally be included in the /zlib/ package, not just
one output.

Fixes NixOS#44144
Fixes NixOS#24717
@matthewbauer
Copy link
Member Author

@GrahamcOfBorg eval

@FRidh
Copy link
Member

FRidh commented Jul 28, 2018

I think this should be brought up on the mailing list.

@dezgeg
Copy link
Contributor

dezgeg commented Jul 28, 2018

This will regress closure sizes since now anything in systemPackages will install the dev outputs and other pointless stuff.

Regarding #44144, compiling outside nix-shell isn't really supported anyway.

As for having manpages installed by nix-env, I think nix-env should call into Nixpkgs buildEnv (or some new dedicated custom function) to decide what outputs to install. That way you could e.g. disable installation of manpages or info pages based on ~/.nixpkgs/.config.nix.

@matthewbauer
Copy link
Member Author

matthewbauer commented Jul 28, 2018

This will regress closure sizes since now anything in systemPackages will install the dev outputs and other pointless stuff.

Doesn't systemPackages use pathsToLink though? I'd bet there is some increase in size but not a huge amount. We can definitely provide a global outputsToInstall in buildEnv that the NixOS environment can set to [ "out" ] or something else for this.

As for having manpages installed by nix-env, I think nix-env should call into Nixpkgs buildEnv (or some new dedicated custom function) to decide what outputs to install. That way you could e.g. disable installation of manpages or info pages based on ~/.nixpkgs/.config.nix.

Yes, I think for most of us this would be the ideal solution. I still think we would want the default behavior of that function to be install everything. Most beginners to Nix will not understand multiple outputs, or pathsToLink, etc. They will just assume Nix(pkgs) does not provide info pages, man pages, headers, libraries, etc. I want to make sure we can still provide good support for the imperative package installation style, even if most NixOS/Nixpkgs core contributors are using something else.

@matthewbauer
Copy link
Member Author

@GrahamcOfBorg eval

@GrahamcOfBorg GrahamcOfBorg added the 6.topic: stdenv Standard environment label Jul 28, 2018
@dezgeg
Copy link
Contributor

dezgeg commented Jul 28, 2018

Doesn't systemPackages use pathsToLink though?

It uses buildEnv does respect outputsToInstall.

I'd bet there is some increase in size but not a huge amount.

No, it's going to be hundreds of megs if some package's .dev pulls in GCC or glibc headers et al.

I want to make sure we can still provide good support for the imperative package installation style,

For development nearly nothing currently supports that since setup hooks won't be run. And most likely isn't going to be fixed without a major redesign.

This restores the behavior in buildEnv to before
ddf58a1, where outputsToInstall only
pointed to one output. ddf58a1
expanded this to include all outputs.

Installing all outputs is desired in ‘nix-env -iA’ where there is no
way to configure what is installed, but not so much in buildEnv where
it is possible to configure extraOutputsToInstall to add additional
outputs (this is already done in NixOS).
@matthewbauer
Copy link
Member Author

matthewbauer commented Jul 29, 2018

For development nearly nothing currently supports that since setup hooks won't be run. And most likely isn't going to be fixed without a major redesign.

This doesn't just apply to headers & libraries. Things like man pages are now frequently missing when someone decides to add a separate output for it. Not everyone is installing their packages through systemPackages. I think it's a bad idea to expect everyone to be doing this, especially when it only is possible when you have root privileges, or have set up something else like home-manager.

I have added a commit to restore the buildEnv behavior. I agree that it makes sense to only use one output in buildEnv's case because it can be configured to add more outputs by the user. nix-env provides no equivalent configuration, however.

@vcunat
Copy link
Member

vcunat commented Jul 29, 2018

Ah, cross-link: 3342f71#commitcomment-29875048

@dezgeg
Copy link
Contributor

dezgeg commented Jul 29, 2018

If we need to do this without any changes to nix-env we could perhaps retcon outputsToInstall to make it only apply to nix-env (i.e. replace all other usages of that in nixpkgs with say, propagatedUserEnvPackages except the one in mk-derivation that sets it). But #44168 (comment) is still more preferable.

@matthewbauer
Copy link
Member Author

matthewbauer commented Jul 29, 2018

If we need to do this without any changes to nix-env we could perhaps retcon outputsToInstall to make it only apply to nix-env (i.e. replace all other usages of that in nixpkgs with say, propagatedUserEnvPackages except the one in mk-derivation that sets it).

s/propagatedUserEnvPackages/propagatedUserEnvPkgs/

The only usage I found was in buildenv.pl. I would be interested in whether there are any others. Do you think it makes sense to leave the outputsToInstall uses in packages the same?

But #44168 (comment) is still more preferable.

It's definitely preferable, but could take a long time. The goal for me is to find some solution for #24717 before 18.09 release. If someone wants to step up and create a PR to Nix to use a custom buildEnv, that would be great but I am not counting on it.

Because @vcunat is assigned #24717, I kind of want to see what he thinks of this PR. After the update to have buildEnv calculate outputsToInstall itself, I am hoping it addresses some concerns. This whole thing has been an outstanding issue for too long IMO. I definitely get that things like dev don't really belong in the profile, but it doesn't seem to hurt too much either if they are (in fact they are downloaded anyway).

@vcunat
Copy link
Member

vcunat commented Jul 30, 2018

I haven't noticed extraneous (unused) outputs being downloaded; that would be a bug. (Unless e.g. the usual case that .bin is installed and it depends on .out, but that's basically intentional. I meant mainly .dev, i.e. the main motivator behind splitting.)

@matthewbauer matthewbauer mentioned this pull request Sep 19, 2018
9 tasks
@matthewbauer
Copy link
Member Author

Closing in favor of #35884 for now.

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.

zlib headers are not linked in ~/.nix-profile/include. Missing man pages for packages with multiple outputs
5 participants