Skip to content
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

nix-daemon.sh in zsh gives priority to system binaries on macOS #4169

Closed
YorikSar opened this issue Oct 20, 2020 · 14 comments
Closed

nix-daemon.sh in zsh gives priority to system binaries on macOS #4169

YorikSar opened this issue Oct 20, 2020 · 14 comments
Labels
Milestone

Comments

@YorikSar
Copy link
Contributor

Describe the bug

After #3608 nix-daemon is being sourced from /etc/zshenv instead of /etc/zshrc which leads to system binaries getting priority over ones installed in Nix profile.

Steps To Reproduce

  1. Install Nix in daemon mode
  2. Open zsh shell
  3. Install git nix-env -iA nixpkgs.git
  4. Run git
  5. Observe request from the system to download and install Xcode utilities.

Expected behavior

Git should run from Nix.

nix-env --version output

nix-env (Nix) 2.3.7

Additional context

The order of files run by zsh is:

  • /etc/zshenv and ~/.zshenv
  • /etc/zprofile and ~/.zprofile
  • /etc/zshrc and ~/.zshrc

/etc/zprofile on macOS contains a call to /usr/libexec/path_helper that overrides PATH variable prepending all system paths from /etc/paths file and /etc/paths.d dir. Before #3608 nix-daemon.sh was sourced in /etc/zshrc after this call and Nix paths were in the front of the resulting PATH variable. After #3608 it is sourced in /etc/zshenv which leads to Nix paths ending up in the very end of PATH.

@abathur
Copy link
Member

abathur commented Nov 10, 2020

I don't use ZSH so I'm not super familiar with the details here and am just speaking from what I recall of other issues, but this sounds like a pickle.

IIRC ZSH users frequently complain that macOS updates overwrite zshrc and "break" Nix. I'm not sure how often this actually happens. The hope in the change to zshenv was that Apple would leave it alone since it doesn't exist.

I can think of at least 2 more options here, but I'm not sure what the consequences of either would be:

  1. Prepend nix to /etc/paths. This seems simple enough, but I don't know what else uses it, and if it also gets overwritten?
  2. It's a little dirty, but we could define a function in /etc/zshenv (directly, or via nix-daemon.sh?), that would bump us back up to the front whenever it gets evaled. Again, I don't know where all path_helper is used, so I don't know what the splash damage would be. For reference, the function might look like:
/usr/libexec/path_helper(){
  command /usr/libexec/path_helper -s; echo 'PATH=[nix paths]:${PATH}'
}

@LnL7 @matthewbauer @lilyball anyone know more about these, or have other ideas?

@YorikSar
Copy link
Contributor Author

@abathur

I don't use ZSH so I'm not super familiar with the details here and am just speaking from what I recall of other issues, but this sounds like a pickle.

IIRC ZSH users frequently complain that macOS updates overwrite zshrc and "break" Nix. I'm not sure how often this actually happens. The hope in the change to zshenv was that Apple would leave it alone since it doesn't exist.

It actually seems like all of /etc/zshenv, /etc/zprofile and /etc/zshrc are being overwritten by the system upgrades. The only "safe" files would be /etc/zshrc_$TERM_PROGRAM that is sourced at the end of /etc/zshrc.

I can think of at least 2 more options here, but I'm not sure what the consequences of either would be:

  1. Prepend nix to /etc/paths. This seems simple enough, but I don't know what else uses it, and if it also gets overwritten?

/etc/paths itself will most likely be overwritten. There is /etc/paths.d dir where all system-local paths reside, but it's system-wide, and nix-daemon.sh adds both default and current user profile to PATH.

  1. It's a little dirty, but we could define a function in /etc/zshenv (directly, or via nix-daemon.sh?), that would bump us back up to the front whenever it gets evaled. Again, I don't know where all path_helper is used, so I don't know what the splash damage would be. For reference, the function might look like:
/usr/libexec/path_helper(){
  command /usr/libexec/path_helper -s; echo 'PATH=[nix paths]:${PATH}'
}

By default path_helper is used from /etc/zprofile, but it for sure can be used in any scores of scripts...

@abathur
Copy link
Member

abathur commented Nov 10, 2020

It actually seems like all of /etc/zshenv, /etc/zprofile and /etc/zshrc are being overwritten by the system upgrades. The only "safe" files would be /etc/zshrc_$TERM_PROGRAM that is sourced at the end of /etc/zshrc.

I don't know if they're literally overwriting these files with the unmodified contents on each update (or if these changes only happen when Apple actually updates the zsh files, but are happening more regularly lately because Apple is still consolidating issues with making it the default shell?) but:

  • I doubt zsh_Apple_Terminal is safe from this. This file is 911 bytes in Catalina, and 9296 in Big Sur RC1, for example.
  • When you say you think /etc/zshenv is getting overwritten, do you mean removed or truncated to 0 bytes? I don't have it at all in Catalina or Big Sur, so I don't know what else it could be getting overwritten to (unless something else is actually to blame on that front).

/etc/paths itself will most likely be overwritten. There is /etc/paths.d dir where all system-local paths reside, but it's system-wide,

It looked like everything in paths.d is also appended to the end of the PATH list, so I think /etc/paths is the only way to the front without either disabling path_helper or modifying the setting afterwards.

There's probably also some way to use a hook/trap to set the PATH later, but I'm not familiar with the ZSH options here.

@abathur
Copy link
Member

abathur commented Nov 10, 2020

@surajbarkale may also have some idea of what's going on here.

@lilyball
Copy link
Member

I believe I've suggested this in a different ticket before, but I think nix-daemon should just check both /etc/bashrc and /etc/zshrc on launch for the Nix setup and restore it if it's missing. These files are only modified in a multi-user install so we don't have to worry about the case where nix-daemon isn't running. This particular ticket would then be solved just by going back to /etc/zshrc instead of /etc/zshenv.

The risk is if I have a shell environment that's initialized before nix-daemon. Normally nix-daemon is run before the GUI logs in. However, right now there's a real possibility of hitting this if I'm using an encrypted Nix volume as the volume isn't mounted until login, and the nix-daemon LaunchDaemon uses wait4path to avoid running nix-daemon until the volume mounts, meaning it's possible to restore my GUI state including Terminal.app sessions prior to this. That said, #4181 is an attempt to fix this and allow the volume to mount prior to GUI login (though merging this PR won't fix the setup for existing users who set up an encrypted volume by hand).

It actually seems like all of /etc/zshenv, /etc/zprofile and /etc/zshrc are being overwritten by the system upgrades. The only "safe" files would be /etc/zshrc_$TERM_PROGRAM that is sourced at the end of /etc/zshrc.

I expect /etc/zshrc_Apple_Terminal isn't safe either, and installing there wouldn't work for anyone using iTerm 2, or other third-party terminals.

I also think /etc/bashrc isn't necessarily a safe file to be installing into, for the same reasons.

Having said that, when was the last time Apple actually moved these files aside? If Apple is still overwriting these files with new updates, as opposed to merely having done so in the past, then really the best solution is to file a ticket with Apple to get them to stop blowing away changes to these files.

@YorikSar
Copy link
Contributor Author

I don't know if they're literally overwriting these files with the unmodified contents on each update (or if these changes only happen when Apple actually updates the zsh files, but are happening more regularly lately because Apple is still consolidating issues with making it the default shell?)

I never checked whether /etc/zshrc was getting new content from Apple, but it has been overwritten with every single macOS update since I've installed Nix. I would guess we can't rely on next update not breaking this.

I think nix-daemon should just check both /etc/bashrc and /etc/zshrc on launch for the Nix setup and restore it if it's missing.

I very much agree. In fact, I think everything in Nix install script should be enforced on every boot or even more often, so that we can:

  • make sure that OS or user didn't break something for Nix like shell extensions or custom users;
  • be able to upgrade configuration from previous versions of Nix, for example if we decide to move those lines from zshrc to zshenv or modify launchd plist, user should get that change not only when they install Nix, but also after upgrade or at least on next boot;
  • just have more uniform configuration for all installations.

NixOS is great at pushing configuration management to every aspect of every machine. I think Nix of Mac should at least take care of its own configuration in similar way.

bcc32 added a commit to bcc32/prezto that referenced this issue May 27, 2021
This is a workaround for macOS's path_helper stuff in /etc/zprofile:

- NixOS/nix#4169
- LnL7/nix-darwin#122
@stale
Copy link

stale bot commented Jun 2, 2021

I marked this as stale due to inactivity. → More info

@stale stale bot added the stale label Jun 2, 2021
@robbins
Copy link

robbins commented Jul 5, 2021

Not stale, same issue here. Although, I do have nix-daemon being sourced in zshrc, and am still having the issue.

@matthewbauer
Copy link
Member

matthewbauer commented Aug 26, 2021

Could we just restore it to /etc/zshrc in the meantime?

Try: #5179

matthewbauer added a commit to matthewbauer/nix that referenced this issue Aug 26, 2021
This reverts commit 909d8cb.

This messes up PATH priority since /etc/profile gets sourced AFTER
/etc/zshenv and it sets the system paths so
$HOME/.nix-profile/bin:/nix/var/nix/profiles/default/bin is behind
/usr/local/bin:/usr/bin:/bin:/usr/sbin:/sbin. See discussion in
NixOS#4169.
@surajbarkale
Copy link
Contributor

+1 for the #5179 from me as well. I was unable to find another suitable fix for the issue.

@abathur
Copy link
Member

abathur commented Aug 26, 2021

Could we just restore it to /etc/zshrc in the meantime?

No strong reason not to, I guess.

I worry a little that reverting to the old status quo will make it easier to drop it from the 2.4 milestone and kick the can on the underlying problem of macOS clobbering these files. But as I meditate on it, I guess a simple reversion is also the most-likely result if this actually ended up blocking release...

@lilyball
Copy link
Member

I think the proper long-term solution to the files being clobbered is for nix-daemon to check for this and repair it on launch.

@edolstra
Copy link
Member

Fixed in #5179.

@nixos-discourse
Copy link

This issue has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/system-version-of-command-in-path-until-i-reload-my-shell/16677/7

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

9 participants