-
-
Notifications
You must be signed in to change notification settings - Fork 14.7k
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
binutils and gcc manpages hidden by wrappers #43547
Comments
@Ericson2314, perhaps you know of an easy way to resolve this? Perhaps just symlink the manpages into the wrappers' output? |
It seems to only propagate man pages via nixpkgs/pkgs/build-support/cc-wrapper/default.nix Lines 261 to 269 in bf2a009
This has the problem of only working when you actually install it into a profile, see #43049. The fix would be to add something like ln -s ${cc.man}/share $man |
@bgamari For now you can do:
We definitely need a more versatile solution for this though. A lot of people have setups to collect all of the manpages to install:
Then you can just do |
Yes this is just a mistake @oxij and I forgot to correct. It is done correctly in |
@Ericson2314 I see, yep, that's a bug. Do you want me to fix this or do you want to do this yourself?
|
To be clear it's really my fault, not yours :). I added the binutils one and didn't update the original, so I just didn't finish the job. You didn't touch the propagation part at al, so that would have been an out out of scope change. That said, if you could do this I'd really appreciate it. I have a few other things to tackle. |
See #44516. |
I think #44558 actually fixes this. At least I can see both `man` and `info` for both `gcc` and `ld` in a minimal NixOS VM.
|
Does that solve it for readelf too ? I couldn't get access to the man either (https://linux.die.net/man/1/readelf) |
Yes, readelf too. But the patches are in staging, so it would take awhile till they hit the channels.
|
Patches hit master recently. I think this should now be closed.
|
Was reverted in c981787 |
:/ |
In c981787 and fa41297, to be precise.
@edolstra, what exactly was not to your liking there? Reverting bugfixes without explanation isn't nice.
Do you want `binutils-wrapper` to put docs into `propagated-user-env-packages` like `cc-wrapper` did (and does again after these reverts) or what? If yes, then why? I think symlinking is a cleaner solution, as, I think, wrappers should just reexport outputs of their inputs when possible.
If Nixpkgs was SLNOS and SLNOS had centralized committer list like Nixpkgs I'd seriously think about removing you from that list after this and 4d1332e.
Did either of those break anything?
|
Correction. There's also fde7296, so `bintools` already does the `propagated-user-env-packages` thing on master.
I still don't understand why these changes were made, though. Please explain, @edolstra -sensei!
I made #46115 with an edited revert. I think the first patch over master there needs to be reapplied again either way (unless it breaks something I'm not aware of) as that change did a useful and, I'd hope, non-controversial thing. The symlinking change is now a separate patch (which I still like).
|
@oxij Don't worry, I have no desire to contribute to SLNOS. Turning this around, I don't understand why these changes (moving from |
@edolstra so |
|
Don't worry, I have no desire to contribute to SLNOS.
My point is that a polite way do a revert of a recently discussed and accepted change is, at the very least, to make a PR and mention the author of the changes in question.
I think that shooing PRs with "this needs mode discussion, hence this needs to go through RFC process" while commiting reverts straight to master without discussion is hypocrisy.
so `nix-shell -p stdenv.cc.man` works
That, and it also allows to control things directly via `outputsToInstall` which is used by `documentation` module of NixOS. Without the symlinking change that module can't do its thing for `environment.systemPackages = [ binutils gcc ];`
|
I can't get even |
Without the symlinking change that module can't do its thing for `environment.systemPackages = [ binutils gcc ];`
Correction. I checked and `buildEnv` does handle this indirection properly, so this argument is moot.
So, I only have the following pros of symlinking left:
- simpler,
- uses 4 inodes less (two intermediate directories x2 for man and info),
- direct references like "export MANPATH=${stdenv.cc.man}/share/man" simply work (which, I think, @Ericson2314 meant in the above).
|
I opened a documentation request issue #46141. If we document these, tooling can add support to understand them (like |
So, a day and several hours of experiments later, I still think the correct way to proceed on the issue is to revert the reverts, i.e. merge #46115 as is (I also edited the reverted reverts a tiny bit there). I wrote an explanation why the first patch of #46115 is the correct way to proceed on the issue in #46119 (comment) (also see the changes in #46119 to see the problem I didn't know I accidentally fixed it in #44558). Since I don't see any actual arguments against using symlinks, I think the second patch of #46115 should be applied too. Hence, I would merge #46115 as is. |
02c09e0 (NixOS#44558) was reverted in c981787 but, as it turns out, it fixed an issue I didn't know about at the time: the values of `propagateDoc` options were (and now again are) inconsistent with the underlying things those wrappers wrap (see NixOS#46119), which was (and now is) likely to produce more instances of NixOS#43547, if not now, then eventually as stdenv changes. This patch (which is a simplified version of the original reverted patch) is the simplest solution to this whole thing: it forces wrappers to directly inspect the outputs of the things they are wrapping instead of making stdenv guess the correct values.
This was fixed with #46115.
|
Issue description
The
gcc
andbin-utils
wrappers don't expose the underlying derivations'man
outputs.Steps to reproduce
nix run nixpkgs.binutils.man -c man ld
Technical details
The text was updated successfully, but these errors were encountered: