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 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" ];
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

@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
Development

Successfully merging this pull request may close these issues.

None yet

5 participants