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

tmux: install manpages #86296

Closed
wants to merge 2 commits into from
Closed

tmux: install manpages #86296

wants to merge 2 commits into from

Conversation

@iblech
Copy link
Contributor

iblech commented Apr 29, 2020

Motivation for this change

On current master, man tmux comes empty after in a nix-shell -p tmux. This pull request fixes that.

However, I'm a little bit unsure whether this is the correct procedure. I looked at other packages for guidance, but didn't spot a consistent pattern. The packages feh and vdr both use multiple outputs, as tmux did before this pull request; the manpages for feh are installed correctly and these for vdr are not.

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.
@@ -23,7 +23,7 @@ stdenv.mkDerivation rec {
pname = "tmux";
version = "3.1";

outputs = [ "out" "man" ];
outputs = [ "out" ];

This comment has been minimized.

Copy link
@prusnak

prusnak Apr 29, 2020

Member

I think this line is redundant if you only want to have the "out" output.

This comment has been minimized.

Copy link
@teto

teto Apr 29, 2020

Contributor

I dom't believe removing the man output is the solution.
I am not sure how nix-shell and MANPATH interact
nix-shell --pure -p tmux.man man does not find the man but my installed tmux brings up the manpage

This comment has been minimized.

Copy link
@iblech

iblech Apr 29, 2020

Author Contributor

I think this line is redundant if you only want to have the "out" output.

Oh yes, I agree. I'll change this in a second.

This comment has been minimized.

Copy link
@iblech

iblech Apr 29, 2020

Author Contributor

I dom't believe removing the man output is the solution.
I am not sure how nix-shell and MANPATH interact
nix-shell --pure -p tmux.man man does not find the man but my installed tmux brings up the manpage

Okay, weird, it doesn't work for me. But it's not terribly important. I'll await further guidance how to proceed. :-) Thank you two of your review!

@Ma27
Copy link
Member

Ma27 commented Apr 29, 2020

👎 the multiple-output feature exists for a reason: https://nixos.org/nixpkgs/manual/#chap-multiple-output

I'm not 100% sure why the MANPATH isn't properly configured on nix-shell, you may want to file an issue here instead since a lot more packages are utilizing this pattern.

The packages feh and vdr both use multiple outputs, as tmux did before this pull request; the manpages for feh are installed correctly and these for vdr are not.

Is it possible that feh is installed in your local profile or your system and that's why the man-pages are found?

@doronbehar
Copy link
Contributor

doronbehar commented May 1, 2020

This should be closed in favor of either of:

@doronbehar doronbehar closed this May 2, 2020
@iblech
Copy link
Contributor Author

iblech commented May 4, 2020

Thank you @doronbehar, I agree this should be closed in favor of the other issues/pull requests. I did not notice these before. Thank you for your work on our documentation system!

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

Successfully merging this pull request may close these issues.

None yet

5 participants
You can’t perform that action at this time.