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
installShellFiles: init #65211
installShellFiles: init #65211
Conversation
This pull request has been mentioned on Nix community. There might be relevant details there: https://discourse.nixos.org/t/proposal-make-it-easier-to-define-manpages-shell-completions/3491/6 |
I haven't updated any packages to use this yet as that will require the package to be rebuilt. I wasn't sure if it was worth it when there's no functional difference. If this gets merged I'll try to start adopting it any time I update a package that has manpages or shell completions. |
1e57f2a
to
89ace0d
Compare
89ace0d
to
87e9d71
Compare
I just tested what happens with a multi-output derivation, and it appears to work just fine, because it seems that That said, I'm thinking I should probably just go ahead and use |
87e9d71
to
bf5d616
Compare
I've updated it to use |
FYI, since Nix 2.0 you can set __structuredAttrs to get what you want. It's only used two places in nixpkgs, but since nixpkgs requires nix 2.0 now anyways, I don't see why it shouldn't be used more. |
|
bf5d616
to
ca87133
Compare
Updated again so the |
This pull request has been mentioned on Nix community. There might be relevant details there: https://discourse.nixos.org/t/prs-ready-for-review-may-2019/3032/32 |
ca87133
to
3db2370
Compare
Updated again because I screwed up the handling of escaped characters in shell completion paths. |
dd2e403
to
c6c3321
Compare
Added support for renaming shell completions, including automatically renaming As an example I updated the diff --git a/pkgs/tools/misc/exa/default.nix b/pkgs/tools/misc/exa/default.nix
index 7c43638ea56..f51fe9e2982 100644
--- a/pkgs/tools/misc/exa/default.nix
+++ b/pkgs/tools/misc/exa/default.nix
@@ -1,5 +1,5 @@
{ stdenv, fetchFromGitHub, rustPlatform, cmake, perl, pkgconfig, zlib
-, darwin, libiconv
+, darwin, libiconv, installShellFiles
}:
with rustPlatform;
@@ -17,25 +17,18 @@ buildRustPackage rec {
sha256 = "14qlm9zb9v22hxbbi833xaq2b7qsxnmh15s317200vz5f1305hhw";
};
- nativeBuildInputs = [ cmake pkgconfig perl ];
+ nativeBuildInputs = [ cmake pkgconfig perl installShellFiles ];
buildInputs = [ zlib ]
++ stdenv.lib.optionals stdenv.isDarwin [
libiconv darwin.apple_sdk.frameworks.Security ]
;
- postInstall = ''
- mkdir -p $out/share/man/man1
- cp contrib/man/exa.1 $out/share/man/man1/
-
- mkdir -p $out/share/bash-completion/completions
- cp contrib/completions.bash $out/share/bash-completion/completions/exa
-
- mkdir -p $out/share/fish/vendor_completions.d
- cp contrib/completions.fish $out/share/fish/vendor_completions.d/exa.fish
-
- mkdir -p $out/share/zsh/site-functions
- cp contrib/completions.zsh $out/share/zsh/site-functions/_exa
- '';
+ manpages = [ "contrib/man/exa.1" ];
+ shellCompletions = installShellFiles {
+ bash = { name = "exa"; path = "contrib/completions.bash"; };
+ fish = { name = "exa.fish"; path = "contrib/completions.fish"; };
+ zsh = { name = "_exa"; path = "contrib/completions.zsh"; };
+ };
# Some tests fail, but Travis ensures a proper build
doCheck = false; I'm not committing this change because it requires rebuilding |
An alternative solution would be to have the hook introduce shell functions such that the installManPage contrib/man/exa.1
installShellCompletion --bash --name exa contrib/completions.bash
installShellCompletion --fish --name exa.fish contrib/completions.fish
installShellCompletion --zsh --name _exa contrib/completions.zsh I.e., without needing to introduce new attributes in the derivation. |
I rewrote this setup hook to vend |
With this new version the patch to diff --git a/pkgs/tools/misc/exa/default.nix b/pkgs/tools/misc/exa/default.nix
index 7c43638ea56..d3f27b9814e 100644
--- a/pkgs/tools/misc/exa/default.nix
+++ b/pkgs/tools/misc/exa/default.nix
@@ -1,5 +1,5 @@
{ stdenv, fetchFromGitHub, rustPlatform, cmake, perl, pkgconfig, zlib
-, darwin, libiconv
+, darwin, libiconv, installShellFiles
}:
with rustPlatform;
@@ -17,24 +17,17 @@ buildRustPackage rec {
sha256 = "14qlm9zb9v22hxbbi833xaq2b7qsxnmh15s317200vz5f1305hhw";
};
- nativeBuildInputs = [ cmake pkgconfig perl ];
+ nativeBuildInputs = [ cmake pkgconfig perl installShellFiles ];
buildInputs = [ zlib ]
++ stdenv.lib.optionals stdenv.isDarwin [
libiconv darwin.apple_sdk.frameworks.Security ]
;
postInstall = ''
- mkdir -p $out/share/man/man1
- cp contrib/man/exa.1 $out/share/man/man1/
-
- mkdir -p $out/share/bash-completion/completions
- cp contrib/completions.bash $out/share/bash-completion/completions/exa
-
- mkdir -p $out/share/fish/vendor_completions.d
- cp contrib/completions.fish $out/share/fish/vendor_completions.d/exa.fish
-
- mkdir -p $out/share/zsh/site-functions
- cp contrib/completions.zsh $out/share/zsh/site-functions/_exa
+ installManPage contrib/man/exa.1
+ installShellCompletion --name exa contrib/completions.bash
+ installShellCompletion --name exa.fish contrib/completions.fish
+ installShellCompletion --name _exa contrib/completions.zsh
'';
# Some tests fail, but Travis ensures a proper build |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like the functions better 🙂
Thanks! This hook seems very helpful.
This is a new package that provides a shell hook to make it easy to declare manpages and shell completions in a manner that doesn't require remembering where to actually install them. Basic usage looks like { stdenv, installShellFiles, ... }: stdenv.mkDerivation { # ... nativeBuildInputs = [ installShellFiles ]; postInstall = '' installManPage doc/foobar.1 installShellCompletion --bash share/completions/foobar.bash installShellCompletion --fish share/completions/foobar.fish installShellCompletion --zsh share/completions/_foobar ''; # ... } See source comments for more details on the functions.
57ee838
to
b9369d1
Compare
I went ahead and squashed the commits together and force-pushed. No file changes, just git history. |
This pull request has been mentioned on Nix community. There might be relevant details there: https://discourse.nixos.org/t/prs-ready-for-review-may-2019/3032/38 |
This pull request has been mentioned on Nix community. There might be relevant details there: https://discourse.nixos.org/t/prs-ready-for-review-may-2019/3032/45 |
the previous structured versions allowed to load completion while in nix-shell (which was great). The new one will work only after postInstall :( |
@teto What's your actual use-case here? You're running |
yes that's my usecase. Running installPhase tends to fail as it won't be able to write to $out unless you change it (and let's not mention multioutput scenarios). Running installPhase can take some time too. And the installPhase is written so that it works after running a buildPhase but when in a nix-shell, most of the time you don't necessarily run buildPhase. For instance python enters develop mode. |
@teto Is your goal to use this generically with arbitrary packages? Because if you're working on a specific package, surely you can just source the completion files from wherever you know they are. Also if you're not running In any case, I liked the fact that the structured approach let you find the shell completions outside of the install process, but I have to admit the approach this PR currently takes is probably simpler to get people to use, more of a drop-in replacement for existing solutions, and also supports something the structured approach didn't, which is shell globbing. |
If you have any ideas on how to regain the ability to source shell completions from within installShellFilesPhase = ''
installManPage doc/foobar.1
''; and we could configure this automatically to run after One downside to this approach is anyone who doesn't use the separate |
Something along these lines. |
The problem is anything complex, people aren't going to use. With the current approach, it works pretty much the way people normally expect, except instead of having to remember where exactly the files get installed, they just use this shell function to install it (but it's otherwise done at the same time, in I liked the structured approach because it preserved structured data in a way you could take advantage of e.g. with what you're proposing, but it was definitely more complicated and harder for people to understand exactly what it's doing. I'm not really sure how to get the best of both worlds. Like I said, we might possibly convince people to do something like write their shell script installations in a separate var |
I just had a look at __structuredAttrs, what a shame it breaks builders (but the future is bright). What's hard about defining : It would allow myself or another to add some code later on for the nix-shell case. |
Yes because the pname isn't necessarily the same thing as the executable name, and the package could install multiple executables.
How do you get people to write this
when they could just write
Especially since this approach doesn't even handle edge cases like paths that have spaces in them (Nix arrays get exposed to bash just as space-separated strings, and if you escape the elements of the array, you then need to pass the whole thing to |
making installShellFiles a default hook would make sense. Then you don't need to modify installPhase. modifying the installPhase can be tricky too (must remember to properly trigger the pre/post hooks) As for the space default, I believe it's so rare that we could leave it as an edgecase until __structuredAttrs becomes a thing. |
Given that Bash doesn't have any support for JSON I'm not expecting to be using |
At this point, what's stopping this PR from being merged? |
This pull request has been mentioned on NixOS Discourse. There might be relevant details there: https://discourse.nixos.org/t/prs-ready-for-review-may-2019/3032/53 |
This is a new package that provides a shell hook to make it easy to declare manpages and shell completions in a manner that doesn't require remembering where to actually install them. Basic usage looks like { stdenv, installShellFiles, ... }: stdenv.mkDerivation { # ... nativeBuildInputs = [ installShellFiles ]; postInstall = '' installManPage doc/foobar.1 installShellCompletion --bash share/completions/foobar.bash installShellCompletion --fish share/completions/foobar.fish installShellCompletion --zsh share/completions/_foobar ''; # ... } See source comments for more details on the functions.
Thanks for your patience 🙂 Rebased to master in 43dade2. |
This is a new package that provides a shell hook to make it easy to declare manpages and shell completions in a manner that doesn't require remembering where to actually install them. Basic usage looks like { stdenv, installShellFiles, ... }: stdenv.mkDerivation { # ... nativeBuildInputs = [ installShellFiles ]; postInstall = '' installManPage doc/foobar.1 installShellCompletion --bash share/completions/foobar.bash installShellCompletion --fish share/completions/foobar.fish installShellCompletion --zsh share/completions/_foobar ''; # ... } See source comments for more details on the functions. (cherry picked from commit 43dade2)
I have a use case for making mkdir -p "$out/share/"{bash-completion/completions,fish/vendor_completions.d,zsh/site-functions}
"$out/bin/kitty" + complete setup zsh > "$out/share/zsh/site-functions/_kitty"
"$out/bin/kitty" + complete setup fish > "$out/share/fish/vendor_completions.d/kitty.fish"
"$out/bin/kitty" + complete setup bash > "$out/share/bash-completion/completions/kitty.bash" It could currently be rewritten as mkdir 'completions'
"$out/bin/kitty" + complete setup zsh > 'completions/_kitty'
"$out/bin/kitty" + complete setup fish > 'completions/kitty.fish'
"$out/bin/kitty" + complete setup bash > 'completions/kitty.bash'
installShellCompletion --zsh 'completions/_kitty'
installShellCompletion --fish 'completions/kitty.fish'
installShellCompletion --bash 'completions/kitty.bash' to use installShellCompletion. This is much more verbose than before. I propose adding the possibility of making "$out/bin/kitty" + complete setup zsh | installShellCompletion --zsh -
"$out/bin/kitty" + complete setup fish | installShellCompletion --fish -
"$out/bin/kitty" + complete setup bash | installShellCompletion --bash - Is this a good place for this feature request or does it belong in a new issue or in the Nix community? |
I just ran into this same scenario myself. My thought was to do something like
but perhaps the stdin approach is better. Note that in both cases we actually need to throw in In any case, this should go in a separate issue. If you want to file it that would be great, and please tag me on it. |
This is a new package that provides a shell hook to make it easy to declare manpages and shell completions in a manner that doesn't require remembering where to actually install them. This is intended for packages that don't have a
make install
that does this already. Usage looks likeThere's detailed documentation in the header comments of the shell functions.
Motivation for this change
I got tired of having to look up the shell completion paths for the various shells when writing derivations. It's also a centralized location to teach about other shells if anyone cares about adding completions (e.g.
ffsend
has.elv
and.ps1
completions in its source tree but I have no idea where those get installed).Things done
sandbox
innix.conf
on non-NixOS)nix-shell -p nix-review --run "nix-review wip"
./result/bin/
)nix path-info -S
before and after)I tested this by writing a dummy derivation whose source contained manpages and shell completions and then invoked this shell hook.