@therealpxc, thanks for your PR! By analyzing the history of the files in this pull request, we identified @jgillich, @rnhmjoj and @Profpatsch to be potential reviewers.

This looks awesome.
Does that mean I can have fish as my nix-shell shell now? If so, how do I enable it with that patch?

It sure does, my friend!

~/Playground  cat ./fishie-shell.nix                                                 3698ms  Sat 25 Mar 2017 08:54:34 AM MST
with import <nixpkgs> {};
stdenv.mkDerivation {
  name = "whatever";
  buildInputs = [
    pkgs.psmisc # for killall
  ];
}
~/Playground  which killall                                                                   Sat 25 Mar 2017 08:54:37 AM MST
which: no killall in (/home/pxc/bin:/run/wrappers/bin:/run/wrappers/bin:/home/pxc/.nix-profile/bin:/home/pxc/.nix-profile/sbin:
/home/pxc/.nix-profile/lib/kde4/libexec:/nix/var/nix/profiles/default/bin:/nix/var/nix/profiles/default/sbin:/nix/var/nix/profi
les/default/lib/kde4/libexec:/run/current-system/sw/bin:/run/current-system/sw/sbin:/run/current-system/sw/lib/kde4/libexec:/ni
x/store/sq1q54snjvwk5ka7iwdv04nn42h84r55-dbus-1.10.16/bin:/nix/store/zks6v9wdc7jp8wg0yd74b48sdsavgcgw-qttools-5.7.1/bin:/nix/st
ore/g4kkh32q8rj390fyz1fwqqlpy2vbz3qj-socat-1.7.3.2/bin:/nix/store/avmxym1w34sc17nrilsmgrk469l3ml0z-gnugrep-3.0/bin:/nix/store/0
xwrn1p8fp8h3cynszpgbmhmydbzhns5-gnused-4.4/bin:/nix/store/zlqp9kb8399hvihidlnzqsad3q1ivbxz-kconfig-5.31.0/bin:/nix/store/zlqp9k
b8399hvihidlnzqsad3q1ivbxz-kconfig-5.31.0/lib/libexec:/nix/store/zlqp9kb8399hvihidlnzqsad3q1ivbxz-kconfig-5.31.0/lib/libexec/kf
5:/nix/store/40b3czpgrs222fadr11y22phxxj6nm1d-kinit-5.31.0/bin:/nix/store/40b3czpgrs222fadr11y22phxxj6nm1d-kinit-5.31.0/lib/lib
exec:/nix/store/40b3czpgrs222fadr11y22phxxj6nm1d-kinit-5.31.0/lib/libexec/kf5:/nix/store/mfran48hv83bmi9qiiklm0952zq45bf3-kserv
ice-5.31.0/bin:/nix/store/cmypgamykrfys6aa1mz8zdzfqz3vcjv5-plasma-workspace-5.8.6/bin:/nix/store/cmypgamykrfys6aa1mz8zdzfqz3vcj
v5-plasma-workspace-5.8.6/lib/libexec:/nix/store/1qc099pc7iydmj7ly2b1l73k3rs7vpn9-xmessage-1.0.4/bin:/nix/store/bkjji24z8prs7kp
4ckddfzbz8h8j02m0-xprop-1.2.2/bin:/nix/store/n52jjwm629mbfk5k6lcp7p065139wqwv-xsetroot-1.1.0/bin:/nix/store/pxwylyb489aliqql7mz
58xshh2cs8m52-yakuake-3.0.2/bin)

! %  ~/Playground  nix-shell ./fishie-shell.nix                                              Sat 25 Mar 2017 08:54:43 AM MST

[nix-shell:~/Playground]$ killall
Usage: killall [OPTION]... [--] NAME...
      killall -l, --list
      killall -V, --version

  -e,--exact          require exact match for very long names
  -I,--ignore-case    case insensitive process name match
  -g,--process-group  kill process group instead of process
  -y,--younger-than   kill processes younger than TIME
  -o,--older-than     kill processes older than TIME
  -i,--interactive    ask for confirmation before killing
  -l,--list           list all known signal names
  -q,--quiet          don't print complaints
  -r,--regexp         interpret NAME as an extended regular expression
  -s,--signal SIGNAL  send this signal instead of SIGTERM
  -u,--user USER      kill only process(es) running as USER
  -v,--verbose        report if the signal was successfully sent
  -V,--version        display version information
  -w,--wait           wait for processes to die

[nix-shell:~/Playground]$ fish
Linux davetim 4.9.17 x86_64
08:54am  up   9:20,  1 user,  load average: 0.46, 0.35, 0.31
~/Playground  killall                                                                         Sat 25 Mar 2017 08:54:54 AM MST
Usage: killall [OPTION]... [--] NAME...
      killall -l, --list
      killall -V, --version

  -e,--exact          require exact match for very long names
  -I,--ignore-case    case insensitive process name match
  -g,--process-group  kill process group instead of process
  -y,--younger-than   kill processes younger than TIME
  -o,--older-than     kill processes older than TIME
  -i,--interactive    ask for confirmation before killing
  -l,--list           list all known signal names
  -q,--quiet          don't print complaints
  -r,--regexp         interpret NAME as an extended regular expression
  -s,--signal SIGNAL  send this signal instead of SIGTERM
  -u,--user USER      kill only process(es) running as USER
  -v,--verbose        report if the signal was successfully sent
  -V,--version        display version information
  -w,--wait           wait for processes to die

But my nix-shell-fu is weak. What's the most natural way to have the shell drop to fish instead of bash as soon as you enter it? Is it some neat little shebang trick?

nix-shell --command 'fish' should work. You can also use the nix integration of direnv, which provides fish shell support.

Hey, folks! I made some additional changes. The most important is the first, imo, but here they are

1. Fish on NixOS now finishes sourcing the NixOS environment before going on to source fish snippets (e.g., ~/.config/fish/conf.d/*). Previously, on NixOS 17.03 and newer, if you sourced Oh-My-Fish from such a snippet, your shell (and possibly login manager) was likely to break. I got some help on figuring out how to do this from some ‘swimmers’, particularly @faho. This required modifying the fish package as well as the module.

2. I added more support for vendor-provided fish scripts. Previously we had some support for incorporating vendor completions, but none for vendor configuration snippets or vendor functions. I added support for the latter two as options and converted the former to an option as well.

3. I added fish hooks to a couple of tools that I use, namely fzf and direnv, to test and demonstrate item A small set of package updates #2.

I would like to make the support for fish hooks a little more general/streamlined instead of just having packages that use them by copying their fish scripts to magic paths, but I'm not yet sure how.

I would also like to make something like a custom fish package that links in all the snippets, functions, and completions it wants so that we can take advantage of these features on non-NixOS. I have some ideas, but if you have any suggestions they would be much appreciated.

I've still got more cleaning up to do and I'm not yet sure that everything works perfectly right, but I would appreciate your feedback and thoughts.

I looked into #18946, I think the solution is rather simple: setEnvironment and shellInit should be wrapped in status --is-login. In the case of a non-login shell, the environment would be inherited from the parent. The bash module does the same, it puts them in /etc/profile - read only for login shells.

Although I have to admit, I have no idea why $__ETC_PROFILE_SOURCED exists, since /etc/profile would only be sourced in login shells and not subshells? Can login shells be nested? @edolstra bcbe2dc

If we're going to parse the initialization variable for bash and zsh in fish, that makes me wonder why we're not doing the same in all shell modules (or just use a common variable).

I think using a common variable for checking whether the NixOS environment has been sourced would be a good idea. Right now the behavior for other shells is to clobber environments set up by one another, which seems like an oversight.

I made some silly mistakes earlier. The most recent version fixes them and I can confirm that and vendor_functions.d, vendor_completions.d, vendor_conf.d all get linked in correctly now.

Btw, @jgillich , what were your plans for the promptInit option? It doesn't currently do anything and I'm not sure fish needs one in addition to interactiveShellInit.

ping pong bump bleep

Hey, @nagisa !

Sorry this thing hasn't seen much activity for a little while. I've been playing with some related changes on my personal Nixpkgs fork, and I currently lack clean commits for all the changes I wanted to make to this. Those changes will take a bit longer for me to work out in full and probably ought to be in another pull request anyway.

But I will try in the next few hours to clean up this PR by removing things related to my more experimental work and all commented-out code to get this into a suitable state for merging ASAP. I'll push the number of commits back down, too.

@Mic92 , @jgillich

Handling fish configuration in NixOS using the tee command and string interpolation in a NixOS-specific way seems to me to be not great. I'm working out doing it in a better way right now, but I'm still experimenting, which is why I haven't cherry-picked those changes over from my experimental branch just yet. But here's the state of the union:

Here's the state of the union:

I don't love this way of putting the config files together either, but changing it for this update seems out of scope. I'm working on other changes right now on my messy, personal Nixpkgs branch. I'd like to see those changes merged eventually some day soon too, but the basic idea is using Nix to represent a complete fish configuration (completions folder with files, snippets folder with files, functions folder with files, main config file, etc.) through buildEnv. Those fish-config paths in the Nix store can then be used in the following ways (1-3 are the important ones imo)

1. To specify a fish-config at build time to compile a particular configuration in with fish
   • note: currently fish on Nixpkgs is compiled to read all the same files as a fish-config represents from $out/etc where $out is the final store path of the fish package being built. All this integration would do is generate those files through the mechanisms used to generate a fish-config, then link them in. So right now we're building fishes that read from configs we can't specify through Nix. This would give us more flexibility for defining custom fish packages and not necessarily add anything to the binary caches.
2. To manage layered fish configurations by installing fish-configs to Nix profiles
   • note: to implement this, all we have to do is patch $__fish_datadir/config.fish ($out/share/config.fish in the fish package) to inject the appropriate sections of fish-config paths into wherever they are usually read. So snippets, for example, will be read from the paths in the list
     • ~/.config/fish/conf.d
     • /nix/var/nix/profiles/per-user/(whoami)/profile/etc/fish/conf.d or ~/.nix-profile/etc/fish/conf.d
     • /nix/var/nix/profiles/default/etc/fish/conf.d
     • /nix/var/nix/profiles/system/etc/fish/conf.d
   • Practically, this just means inserting a few entries in an array during fish initialization. Fish already includes platform-specific initialization hooks, and upstream fish developers have expressed (vaguely; I think we'd still owe them a working example and a clear, concise account of the motivations) a willingness to make Fish similarly Nix-aware upstream, so if the Nix community thinks this is a sound way to handle things, I'd like to see it integrated into fish and then we won't have to modify our fish packages to get fish to read from Nix profiles.
3. Have NixOS accommodate all fish configuration options by passing through its options to generate an appropriate fish-config, then simply adding it to environment.systemPackages. This gives Nix and NixOS users identical syntax and behavior for declaratively configuring fish through Nix.
4. Eventually fish-config can be enhanced to handle oh-my-fish and other fish framework packages and their system dependencies.
5. With further enhancements, one can use a modified fish-config to build a special fish package which doesn't read from /etc, /home, or anywhere not built-in to produce robust/portable fish scripts which install their own dependencies through Nix, use a fixed version of fish, and never access outside paths. (With an extended version of the same tricks necessary to prevent shell initialization breakage when user snippets depend on programs installed to Nix profile paths, this can be done so that all portable fish scripts which rely on the same upstream fish version can rely on a common package for fish, no rebuilding just to change the config)

I'm currently working on a solution that does all of that, and in doing it I'm generating the configuration files in a more Nix-native way (using builtins for writing files and so on, then pulling them all together with buildEnv) than using bash/tee hacks in a postPatchPhase). Right now I am testing to make sure that I generate those fish-config environments correctly, and I think that part is done except for oh-my-fish support (which I'll add later) but I haven't switched the fish package over to using them yet.

Does this seem like a reasonable approach? For users uninterested in use cases 2-4, these changes will have no effect except adding a Nix store path for the default fish configuration used in the default fish package. But that's just symlinks, it shouldn't even really take up more space.

Anyway, that effort is why I'm not super concerned about style issues related to the way config files are generated in the NixOS module for fish at this time.

I've cleaned thing up a little. Please let me know what you think.

I followed the suggestion of @jgillich with using status --is-login to determine whether to update the environment, but not for the actual NixOS shell configuration. This is just to keep parity with the way configuration for other shells is handled on NixOS. This means that if you run a login shell from within an interactive shell, it will not source the interactive shell configuration over again, but just the login shell configuration, and so on.

Thanks for your review @faho ! I'm embarrassed for not catching that earlier, but I'm glad you found it. :-)

FWIW: I've been running this version of the NixOS module for fish and the fish package on my two NixOS machines (my laptop on my multimedia machine) and it successfully initializes my omf config. Using fish as in nix-shell --fish works correctly (no clobbering), and the direnv and fzf fish hooks are loaded automatically (i can remove them from my config and they still work). (The fzf keybinds don't get assigned yet because those are implemented as a little oh-my-fish package and I haven't added oh-my-fish support yet, but the fzf_key_bindings function is made available according to its fish function path.)

@Mic92 Given that this PR is the source of the above warnings and the mosh failure above (both hitting me), shouldn't it be reverted out of 17.03?

The mosh issue is just the one listed just above #25789. It's not urgent for me, just might be inconvenient because I tend to use it on the train. @therealpxc The work is appreciated though, trust me, I'm not whinging about it! I just thought I should raise the possibility of a revert just because it may block others.

@therealpxc So I've just started seeing this with git annex:

Please make sure you have the correct access rights
and the repository exists.
fish: Unknown command 'git-receive-pack /home/sean/'
fish: git-receive-pack '/home/sean/'
      ^
fatal: Could not read from remote repository.

I'm surmising it's the same thing as what's going on with mosh, which makes me think it's a general issue with non interactive shells.

@therealpxc I'll have a looky see and give it a bash myself.

diff --git a/pkgs/shells/fish/default.nix b/pkgs/shells/fish/default.nix
index 32b7694ba3..35a548eaac 100644
--- a/pkgs/shells/fish/default.nix
+++ b/pkgs/shells/fish/default.nix
@@ -46,8 +46,7 @@ let
 
   fishPreInitHooks = ''
     # source nixos environment if we're a login shell
-    builtin status --is-login
-    and test -f /etc/fish/nixos-env-preinit.fish
+    test -f /etc/fish/nixos-env-preinit.fish
     and source /etc/fish/nixos-env-preinit.fish
 
     test -n "$NIX_PROFILES"

This apparently fix the problem with mosh.

Also note that stable branches are dead ends – normally the code only moves the other way (master -> stable) via cherry-picks.

@vcunat these commits are in both branches by now.

@Mic92 At the risk of asking a stupid question, that's only in your fork right?

@seanparsons No. Here it is on master, and here it is on 17.03.

The fixes for the issues related to this PR are available in two commits in a new PR. Feel free to cherry-pick them for your own use.

@therealpxc I think we've got our wires crossed, I'm referring to the commits that fix this issue that are in the PR. The stuff you're linking to is from March.

I was hoping someone could help me with my attempt to add pass autocompletions. By way of experiment I started by overriding the current pass derivation with a preInstall attribute that copies over the fish completions.

{
  pass_with_completions = lib.overrideDerivation pass (oldAttrs: {
    preInstall = ''
      # fish completions
      mkdir -p "$out/share/fish/vendor_completions.d"
      cp "src/completion/pass.fish-completion" "$out/share/fish/vendor_completions.d/pass.fish"
    '';
  });
}

In the above I have attempted following the same approach used for adding fzf in this PR, but I must be doing something wrong because fish is not picking up on the completions on its own. I do see the completions file appearing at /nix/store/<has>-password-store-1.7/share/fish/vendor_completions.d/pass.fish and when I source that file manually the completions start working. I'm running a fully updated version of channel 17.03. Any help would be greatly appreciated!

Are you on NixOS, or Nix with non-NixOS?

I'm on NixOS. I'm not using Fish as a login shell.

Ah! So this feature depends on the NixOS module for fish, which is only active when programs.fish.enable = true; is set in your configuration.nix. You don't have to use fish as your login shell, but fish support for NixOS has to be enabled for this to work, because that's what produces /etc/fish/nixos-env-preinit.fish.

The reason is that the Nix environment has to be sourced from the set-environment file which NixOS produces for each generation of the system-- it doesn't have a single, stable location; it lives in the Nix store. Part of the purpose of /etc/fish/nixos-preinit-env.fish is to have a stable path which can be sourced, as well as to contain the logic for sourcing the set-environment file (which is a sh script, thus the fenv wrapping).

However, the good news the pass package already installs its completions to the right place, so as soon as you set programs.fish.enable = true;, you'll get the completions you want:

> string replace ' ' '\n' $fish_complete_path |grep vendor_completions.d                                        
/home/pxc/.nix-profile/share/fish/vendor_completions.d
/nix/var/nix/profiles/default/share/fish/vendor_completions.d
/run/current-system/sw/share/fish/vendor_completions.d
/nix/store/n4vzxw57hp4xiff8hqh538jnfpz3zi51-fish-2.6.0/share/fish/vendor_completions.d


> ls /run/current-system/sw/share/fish/vendor_completions.d/                                                      
docker.fish  pass.fish

> pass                                                                                                            
cp                                            (Command: copy existing password) 
edit                                 (Command: edit password using text editor)
find              (Command: find a password file or directory matching pattern)
generate                                       (Command: generate new password)
…and 31 more rows

Thank you so much @therealpxc for solving my problem and taking the time to teach me something in the process. I appreciate it!

No problem at all! I'm happy to know concretely that this feature I added has a user besides me. ;-)

I don't contribute as often as I'd like to, but I am intermittently working on some more Nix-related fish goodies, so with any luck we'll see more of each other. :-)