search

nix-community / home-manager

Manage a user environment using Nix [maintainer=@rycee]
https://nix-community.github.io/home-manager/
MIT License
6.84k stars 1.79k forks source link

bug: home.sessionPath is broken with ZSH #2991

Open sagikazarmark opened 2 years ago

sagikazarmark commented 2 years ago

Is there an existing issue for this?

Issue description

2708 moved session variable loading from .zshrc to .zshenv.

While it works with most session variables, it breaks home.sessionPath that modifies PATH.

It breaks, because zshenv is loaded first and subsequent PATH modifications (like Nix loading) override variables set in zshenv.

Prior to #2708 I submitted #2445 assessing the situation. In that (based on #183) I concluded that .zshenv is not the best place for session variables. Quoting the example .zshenv file:

# .zshenv is sourced on ALL invocations of the shell, unless the -f option is
# set.  It should NOT normally contain commands to set the command search path,
# or other common environment variables unless you really know what you're
# doing.  E.g. running "PATH=/custom/path gdb program" sources this file (when
# gdb runs the program via $SHELL), so you want to be sure not to override a
# custom environment in such cases.  Note also that .zshenv should not contain
# commands that produce output or assume the shell is attached to a tty.

Quoting why it was still added to .zshenv from #2708:

The reason why these session variables are not moved to .zprofile is that programs started by systemd user instances are not able to get variables defined in that file. For example, GNOME Terminal (gnome-terminal-server.service) is one of these programs and doesn't get variables defined in .zprofile. As a result, the shells it starts, which are interactive and non-login, do not get those variables.

Based on the available information, I believe .zshenv is not the right place for session variables by default. I understand that it's problematic for some graphical sessions, but the current solution still goes against how ZSH startup scripts are supposed to be used and requires workarounds for "normal" use cases.

I propose moving session variables to zprofile instead of zshenv. Anyone in need of loading them in zshenv as well can workaround the issue (or we can introduce a variable if that's absolutely necessary).

I'd also like to point out that bash also uses the equivalent of zprofile at the moment.

Temporary workaround:

{
    zsh.profileExtra = lib.optionalString (config.home.sessionPath != [ ]) ''
      export PATH="$PATH''${PATH:+:}${lib.concatStringsSep ":" config.home.sessionPath}"
    '';
}

Maintainer CC

cc @jian-lin

System information

- system: `"x86_64-linux"`
 - host os: `Linux 5.15.43, NixOS, 22.05 (Quokka)`
 - multi-user?: `yes`
 - sandbox: `yes`
 - version: `nix-env (Nix) 2.8.1`
 - nixpkgs: `/nix/var/nix/profiles/per-user/root/channels/nixos`
jian-lin commented 2 years ago

While it works with most session variables, it breaks home.sessionPath that modifies PATH. It breaks, because zshenv is loaded first and subsequent PATH modifications (like Nix loading) override variables set in zshenv.

I still don't understand why it breaks home.sessionPath. Can you elaborate on that?

Also, what do you mean by "Nix loading"?

E.g. running "PATH=/custom/path gdb program" sources this file (when gdb runs the program via $SHELL), so you want to be sure not to override a custom environment in such cases.

The change in #2708 doesn't have this issue because setting session variables is guarded by a variable to not run more than once.

sagikazarmark commented 2 years ago

I still don't understand why it breaks home.sessionPath. Can you elaborate on that?

Also, what do you mean by "Nix loading"?

nix.sh is sourced after zshenv and it overrides PATH. Since session vars are only sourced once, nix.sh's override stays in effect.

The change in #2708 doesn't have this issue because setting session variables is guarded by a variable to not run more than once.

That's exactly the problem: they are sourced too early and only once. Path modifications are not supposed to be in zshenv.

jian-lin commented 2 years ago

nix.sh is sourced after zshenv and it overrides PATH. Since session vars are only sourced once, nix.sh's override stays in effect.

So you use nix on a foreign system given that you say nix.sh, right? I have one machine running gentoo with nix. Its nix.sh sets PATH like this: export PATH="$NIX_LINK/bin:$PATH". Is it what you mean by override? If so, I still don't understand why home.sessionPath is broken since it remains in PATH after nix.sh's change.

jian-lin commented 2 years ago

That's exactly the problem: they are sourced too early and only once.

No, they are two different situation.

The example .zshenv you quote says .zshenv should not contain PATH setting because if so your custom PATH (PATH=/custom/path gdb program) will be overridden by the PATH set in .zshenv. Home-manager's zsh module doesn't have this issue, i.e. your custom PATH will not be overridden by the PATH set in .zshenv.

What you say is the contrary: custom PATH is not overridden by the PATH in .zshenv. I am confused since what you want and what the example .zshenv you quote tries to avoid is the same thing.

Path modifications are not supposed to be in zshenv.

Please show us what exactly your issue is and provide a minimal reproducible environment.

sagikazarmark commented 2 years ago

I use both NixOS and macOS. Haven't tested it on macOS yet, but on NixOS sessionPath is missing from the PATH after upgrading to 22.05.

It does contain the nix profile PATH though, so my only explanation is that something overrides the PATH after zshenv is loaded for the first time. I went through my own scripts and nothing sets the PATH, I only use sessionPath and the ZSH module.

I'll try to reproduce it in an isolated environment later.

sagikazarmark commented 2 years ago

Steps to reproduce:

  1. Install NixOS 22.05 in a VM
  2. Create a user with ZSH as its shell
  3. Install Home Manager using the standalone installation
  4. Set sessionPath and enable ZSH module
  5. Logout and login to reload environment
  6. Check the value of PATH

configuration.nix:

  # ...

  users.users.mark = {
    hashedPassword = "...";
    description = "User";
    isNormalUser = true;
    shell = pkgs.zsh;
    extraGroups = [
      "wheel"
      "networkmanager"
      "audio"
      "video"
      "docker"
      "input"
    ];
    openssh.authorizedKeys.keys = [
      "..."
    ];
  };

home.nix:

  # ...

  home.sessionPath = ["$HOME/.local/bin"];

  programs.zsh = {
    enable = true;

    sessionVariables = { FOO = "bar"; };
  };
echo $PATH
/run/wrappers/bin:/home/mark/.nix-profile/bin:/etc/profiles/per-user/mark/bin:/nix/var/nix/profiles/default/bin:/run/current-system/sw/bin

echo $FOO
bar

The set-environment script sourced in /etc/profile contains the following line:

export PATH="$HOME/.nix-profile/bin:/etc/profiles/per-user/$USER/bin:/nix/var/nix/profiles/default/bin:/run/current-system/sw/bin"

It completely overrides PATH. This script is sourced after zshenv is loaded for the first time. The session variables script is sourced only once and it's sourced before profile (in zshenv).

jian-lin commented 2 years ago

Do you have system leval programs.zsh.enable enabled?

sagikazarmark commented 2 years ago

I don't. The problem is reproducible with the above minimal config.

jian-lin commented 2 years ago

Please enable that and set-environment will be sourced in /etc/zshenv, which is the right way for zsh.

see https://github.com/nix-community/home-manager/issues/2751#issuecomment-1048682643

sagikazarmark commented 2 years ago

Thanks, I'll try that later. I wonder if that's going to work on Mac though.

Any ideas how we can make it easier for users to figure out the ZSH module depends on a global setting?

jian-lin commented 2 years ago

I think a mention in the manual and release note will help. Will you make a PR for that?

jian-lin commented 2 years ago

I wonder if that's going to work on Mac though

As long as mac's /etc/{zprofile,zshrc} are sane, i.e. not removing items from PATH, it should be ok.

stale[bot] commented 2 years ago

Thank you for your contribution! I marked this issue as stale due to inactivity. Please be considerate of people watching this issue and receiving notifications before commenting 'I have this issue too'. We welcome additional information that will help resolve this issue. Please read the relevant sections below before commenting.

If you are the original author of the issue

* If this is resolved, please consider closing it so that the maintainers know not to focus on this. * If this might still be an issue, but you are not interested in promoting its resolution, please consider closing it while encouraging others to take over and reopen an issue if they care enough. * If you know how to solve the issue, please consider submitting a Pull Request that addresses this issue.

If you are not the original author of the issue

* If you are also experiencing this issue, please add details of your situation to help with the debugging process. * If you know how to solve the issue, please consider submitting a Pull Request that addresses this issue.

Memorandum on closing issues

Don't be afraid to manually close an issue, even if it holds valuable information. Closed issues stay in the system for people to search, read, cross-reference, or even reopen – nothing is lost! Closing obsolete issues is an important way to help maintainers focus their time and effort.