fish: use babelfish for hm-session-vars.sh

[emilazy]

Description

See NixOS/nixpkgs#108947, nix-darwin/nix-darwin#270. When enabled this brings down fish startup time from "noticeable delay" to "imperceptibly fast" for me.

The NixOS and nix-darwin versions of this construct derivations for the bash sources and place the results in /etc/fish. I couldn't figure out if there was any point to that kind of thing so I just used the derivation directly and embedded the bash in the build script. I also didn't abstract it out at all because there's nothing else to translate at present.

I wanted to port programs.nix-index.enableFishIntegration to use this and forego the additional shell when enabled, but babelfish isn't quite ready for it yet.

I didn't add a test because I'm not sure what kind of test case would be useful within the Home Manager test framework. I guess you could check the file contains correct code for setting a specific variable, but that might break if babelfish translates it differently (but equally correctly) in the future.

I'd be tempted to forego the option and just use babelfish unconditionally, since it should handle this basic variable-setting use case perfectly, but theoretically people could have added arbitrarily complicated bash code to home.sessionVariablesExtra and in that case this could break their configurations if they use edge-case features that babelfish doesn't yet correctly translate. If that's an acceptable tradeoff I'd be happy to amend the PR to default the option on or skip it altogether.

(Also maybe home.sessionVariablesInit should be marked as internal?)

Checklist

• Change is backwards compatible.

• Code formatted with ./format.

• Code tested through nix-shell --pure tests -A run.all or nix develop --ignore-environment .#all using Flakes.

• Test cases updated/added. See example.

• Commit messages are formatted like

  {component}: {description}
  
  {long description}
  

  See CONTRIBUTING for more information and recent commit messages for examples.

• If this PR adds a new module

  • Added myself as module maintainer. See example.

  • Added myself and the module files to .github/CODEOWNERS.

[ncfavier]

Maybe we can make this the default conditionally based on stateVersion?

[emilazy]

Yes, I had that thought. I would be happy to implement that if defaulting to it outright is a bridge too far. It'd be nice if NixOS and nix-darwin did that too; the speedup is quite considerable...

[emilazy]

Oh wait, home.sessionVariablesExtra is internal anyway; the only way anyone could cause any issue here would be home.sessionVariables.EVIL = "$(extremely complex bash code)", which seems deeply unlikely. (Nothing stops you doing home.sessionVariables.EVEN_MORE_EVIL = "\"; echo hi \"", either, which it presumably wouldn't be considered a valid compatibility break if it stopped working.)

I have the home.stateVersion logic implemented but maybe worrying about breakage here is overkill for an option that I seriously doubt anyone is doing anything fancier than simple variable and command substitutions with?

[ncfavier]

Can babelfish deal with the code that we know may be present in sessionVariablesExtra? (At a glance,

home-manager/modules/services/gpg-agent.nix

Lines 240 to 244 in 58eb968

	home.sessionVariablesExtra = optionalString cfg.enableSshSupport ''
	if [[ -z "$SSH_AUTH_SOCK" ]]; then
	export SSH_AUTH_SOCK="$(${gpgPkg}/bin/gpgconf --list-dirs agent-ssh-socket)"
	fi
	'';

and

home-manager/modules/targets/generic-linux.nix

Lines 59 to 64 in 58eb968

	home.sessionVariablesExtra = ''
	. "${nixPkg}/etc/profile.d/nix.sh"
	# reset TERM with new TERMINFO available (if any)
	export TERM="$TERM"
	'';

)

[emilazy]

Yeah, it gets both of these correct:

if test -z "$SSH_AUTH_SOCK"
  set -gx SSH_AUTH_SOCK (/path/to/gnupg/bin/gpgconf --list-dirs agent-ssh-socket | string collect; or echo)
end

and

/nix/store/lnggzp27m91d1niswv65jdr2kkg61my0-babelfish-1.1.0/bin/babelfish < '/path/to/nix/etc/profile.d/nix.sh' | source
# reset TERM with new TERMINFO available (if any)
set -gx TERM "$TERM"

respectively.

In general babelfish is quite sophisticated; e.g. its author said "I'd say babelfish will work fine for 100% of the use case we have in Nixpkgs. At some point I manually went though everything to make sure it does." on the NixOS PR and the only mistranslation I ran into on this nontrivial script from nix-index was this comment parsing infelicity: bouk/babelfish#23. It looks like nothing else adds to home.sessionVariablesExtra at present but anything added in the future would presumably have to be something more elaborate than anything in the NixOS or nix-darwin initialization scripts for it to trip up babelfish.

[ncfavier]

Let's give it a try then!

[emilazy]

Alright, yolo version pushed :)

For what it's worth, Nix ships with an /etc/profile/nix.fish which would avoid a conversion step, but the babelfish-translated version is functionally identical other than not using $fish_user_paths (which doesn't happen with the foreign-env version either). Still, it could be worth conditionalizing and porting to programs.fish.shellInit, but not in this PR.

[ncfavier]

Looks good. Since this is a potentially breaking change, I'll think we'll have to wait until the 23.05 branchoff (which is happening now for NixOS).

[emilazy]

Sure; does that mean this will land in the release after 23.05 and I should move the changelog entry to that one after branch-off happens?

[emilazy]

Just checking on this since I'm not familiar with the HM release process - it seems #4013 is done but the version on master hasn't been rolled over yet; I guess this should land after the next equivalent of #3440?

[rycee]

@emilazy HM master is now 23.11 so I guess you are good to go 🙂

[emilazy]

@ncfavier Thanks for the merge! I was just about to rebase this for the release turnover - you'll want to move the release notes to rl-2311.adoc :)

[ncfavier]

Fixed