Allow optional installation of rust-docs component #253
Conversation
|
I'm vendoring cpcloud's fork because it makes my CI builds stop crying due to unnecessary doc installation adding 10 minutes or so to each run. Wish I had more upvotes to give. Please merge. |
|
@miangraham You may want to look into using |
There was a problem hiding this comment.
Thanks, this change looks good except for the fact we should parametrize installDoc at the package level instead of the manifest file. (see comment below)
Also, I think most users of this overlay are likely feeling the same burden. Thus making the documentation copy default to false would be a good idea.
rust-overlay.nix
Outdated
| @@ -253,7 +253,7 @@ let | |||
| # All extensions in this list will be installed for the target architectures. | |||
| # *Attention* If you want to install an extension like rust-src, that has no fixed architecture (arch *), | |||
| # you will need to specify this extension in the extensions options or it will not be installed! | |||
| fromManifestFile = manifest: { stdenv, fetchurl, patchelf }: | |||
| fromManifestFile = manifest: { stdenv, fetchurl, patchelf, installDoc }: | |||
There was a problem hiding this comment.
note: installDoc has no reason to bubble up to the processing of the Manifest file. A few lines below, there is makeOverridable ({extensions, targets, targetExtensions}: …) which is use to wrap each package in a similar way as callPackage which makes it usable with rust.override { extensions = ["rust-src"]; }.
This would be the place where to add installDoc ? false, such that it can be enabled by user who wants the doc to be installed.
|
@nbp I believe I have addressed your comments. |
| { inherit (self) stdenv lib fetchurl patchelf; } | ||
| ; | ||
| { | ||
| inherit (self) stdenv fetchurl patchelf; | ||
| }; |
There was a problem hiding this comment.
This revert a change from a previous patch which should cause this patch to not work as expected.
| installComponents = stdenv: namesAndSrcs: | ||
| installComponents = { stdenv, namesAndSrcs, installDoc }: | ||
| let | ||
| inherit (builtins) map; | ||
| installComponent = name: src: | ||
| installComponent = installDoc: { name, src }: | ||
| stdenv.mkDerivation { |
There was a problem hiding this comment.
nit: There is no need to add installDoc as argument of installComponent (no s), as it would be captured like stdenv is.
|
Closing this out. Feel free to revisit elsewhere. The review process took way too long and I don't have time to work on this anymore. |
This PR allows opting out of installing the
rust-docscomponent.
It can be used (for example) as follows, to create :
Closes #252.