fish: Fix regression due to changes in nixpkgs.

[meck]

Description

Fix for changes in nixpkgs fish plugin organisation on unstable:

NixOS/nixpkgs@d94921d

fixes #1701 #1702

Checklist

• Change is backwards compatible.

• Code formatted with ./format.

• Code tested through nix-shell --pure tests -A run.all.

• Test cases updated/added. See example.

• Commit messages are formatted like

  {component}: {description}
  
  {long description}
  

[berbiche]

Would this break users not tracking the latest unstable?
If so, how about using pkgs.fish-foreign-env or pkgs.fishPlugins.foreign-env?

[cole-h]

Then it would need to be if pkgs ? fish-foreign-env then "${pkgs.fish-foreign-env}/share/fish-foreign-env/functions" else "${pkgs.fishPlugins.foreign-env}/share/fish/vendor_functions.d", since the directory providing the function changed as well.

[cohei]

pkgs.fish-foreign-env still exists (NixOS/nixpkgs@d94921d#diff-29ed0f5fda84c91253503c1664d60a194a324e29518d472adf846f30b8db52e3R161), so branching by pkgs ? fish-foreign-env seems not to work.

[cole-h]

Then:

Suggested change

	set --prepend fish_function_path ${pkgs.fishPlugins.foreign-env}/share/fish/vendor_functions.d
	set --prepend fish_function_path ${if pkgs ? fishPlugins && pkgs.fishPlugins ? foreign-env then "${pkgs.fishPlugins.foreign-env}/share/fish/vendor_functions.d" else "${pkgs.fish-foreign-env}/share/fish-foreign-env/functions"}

[rycee]

Did anybody try the diff proposed by @cole-h? If it works then we can include it but otherwise I think it should be OK to merge as-is since I think it is reasonable to assume that HM follows Nixpkgs master as closely as possible.

[toonn]

I'll report on it later today. I agree simply tracking master is ok but if it's this simple to keep backwards compatibility, maybe it's fine.

[toonn]

Wanted to try this out but I realized there's no home.imports or similar which is how I replace nixos modules. What's the most straightforward way for me to test this patch?

[berbiche]

You can set programs.home-manager.path = ~/path/to/cloned/home-manager and apply the modification locally.
This won't work when using flake (I think).

[toonn]

Thanks to rycee's help I can now confirm that cole-h's suggestion works. The h-m manual makes a point of mentioning backward-compatibility for modules in this case the price for that is a branch : )

[rycee]

Force-pushed commit with updated commit message. I can't judge if this is mergeable or not.

[cole-h]

It's mergeable if we don't mind breaking users who don't update nix{os,pkgs}-unstable and h-m unstable in lockstep.

[toonn]

Isn't the current situation broken for anyone who tracks nixpkgs-unstable close enough, regardless of how often they update h-m?

[meck]

Isn't the current situation broken for anyone who tracks nixpkgs-unstable close enough, regardless of how often they update h-m?

I think so, the issue is someone updating HM without updating nixpkgs...

[toonn]

It's not clear to me why this would be backwards-incompatible for users who don't update channels in lock-step, could someone explain the problem? @cole-h's test covers the case where h-m is updated but nixpkgs isn't, doesn't it?

[rycee]

Thanks all! Merged to master with @cole-h's proposed change included.