Skip to content
This repository has been archived by the owner on Jul 4, 2023. It is now read-only.

Zsh etcdir shouldn't be disabled by default #25719

Closed
allait opened this issue Jan 7, 2014 · 18 comments
Closed

Zsh etcdir shouldn't be disabled by default #25719

allait opened this issue Jan 7, 2014 · 18 comments

Comments

@allait
Copy link

allait commented Jan 7, 2014

A recent change #24538 disabled the reading of Zsh rc files in /etc by default, which means that paths added to /etc/paths.d by application installers (like MacTeX and XQuartz) don't get added to $PATH at all (unlike bash, which still reads /etc/profile).

Caveat added in #11056 about renaming /etc/zshenv to /etc/zprofile means that path_helper will be executed only for login shells, it doesn't disable it completely. As far as I can tell nothing has changed in Mavericks and if the user doesn't want path_helper to run for non-login shells renaming the file still is the way to go.

@MikeMcQuaid
Copy link
Member

I'd suggest fixing this in your local configuration. We did this to avoid users needing to change system files that are harder to restore than Homebrew ones.

@allait
Copy link
Author

allait commented Jan 7, 2014

There's no real need to change system files — renaming /etc/zshenv means that path_helper won't be run when it's not needed, I am not aware of any actual current issues with running it each time though. Disabling /etc means that folders that probably should be in the $PATH (used to be in zsh, still are in bash) aren't anymore.

I think the actual problem here is that everyone updating zsh to 5.0.5 right now is going to end up with a different environment than the one they had before (e.g. MacTeX suddenly disappears from $PATH, which is exactly why I found out about this change in the first place).

@goxberry
Copy link
Contributor

goxberry commented Jan 7, 2014

@allait The MacTeX and BasicTeX casks in homebrew-cask were updated with caveats for this use case. (See Homebrew/homebrew-cask#2311, Homebrew/homebrew-cask#2284, and Homebrew/homebrew-cask#2277.)

@allait
Copy link
Author

allait commented Jan 8, 2014

@goxberry I think it's worth noting that MacTeX installer does add itself to system PATH: it creates /etc/paths.d/TeX which is then read by path_helper. path_helper is called in /etc/profile for bash and /etc/zshenv for zsh, except zsh one is now turned off by default.

@goxberry
Copy link
Contributor

goxberry commented Jan 8, 2014

@allait I already noted that MacTeX uses path_helper in the relevant issue in homebrew-cask. An issue was raised of latex no longer being on the path, so we felt it was prudent to include a recommendation to prepend /usr/texbin to the PATH in case of difficulties rather than say nothing, regardless of how your current issue is resolved.

@FlorianFranzen
Copy link
Contributor

I agree. OS X adds system wide paths to /etc/paths.d which can then be retrieved using /usr/libexec/path_helper. Do to the patch for #24538, system wide paths are no longer added to zsh sessions by default.

Therefore please either revert this fix or add a caveat that users need to add eval $(/usr/libexec/path_helper -s) to their .zshrc to add the default system wide path to their environment.

@MikeMcQuaid
Copy link
Member

@FloFra If you can submit a PR that makes zsh use path-helper we'll review it. This comment is the first in which you're not being rude, please continue to be constructive and we'll get on fine.

@mistydemeo
Copy link
Member

Both Homebrew bash and fish read from their respective /etc/ files; I'd say having a login shell read from there is a reasonable assumption. I'd be fine with making those read by default in zsh.

@MikeMcQuaid
Copy link
Member

The problem in this case was that you needed to modify system files.

@FlorianFranzen
Copy link
Contributor

No, you only have to modify the file system, if you do not want path_helper to run for non-login shells.

@MikeMcQuaid
Copy link
Member

Read the caveats of the commit that changed this behaviour and you'll see what I mean.

@mistydemeo
Copy link
Member

OK, yeah, I see why now. /etc/bashrc doesn't execute path_helper, /etc/zshrc does. We should document this in the formula itself or we'll definitely keep having this discussion every few months.

Assuming zsh does look in /usr/local/etc, we could disable the systemwide /etc/ directory and copy /etc/profile to /usr/local/etc/zprofile. Looking at the formula, we could set --enable-etcdir=/usr/local/etc by default along with copying that one file.

@MikeMcQuaid
Copy link
Member

Sounds reasonable to me @mistydemeo. I don't object to us using the stuff in etc; we just shouldn't need to recommend people modify it and ideally put as much in /usr/local as makes sense.

@MikeMcQuaid
Copy link
Member

@nuoymit Thanks for the reasonable reply.

If the two are unrelated I don't have a problem with us reading from /etc but I have a major problem with us adding the caveat. If @adamv and @mistydemeo are OK with this we can revert the /etc change but leave the caveat omitted.

@ghost
Copy link

ghost commented Jan 20, 2014

@MikeMcQuaid you're right, as a project owner you should have an issue with the caveat. After giving it some more thought, I think in this particular issue, we're asking homebrew to do too much. It makes sense to keep the caveat removed but also restore the default of reading from /etc

Understanding shell initialization is not the goal of the homebrew project. That should be the goal of the user. It is unfortunate default OS X setup makes this much more difficult than it needs to be, but it is also unfortunate that the common knowledge out there about shell initialization tends to be subpar at best because it is made for convenience, not correctness.

@nuoymit Thanks for the reasonable reply.

If the two are unrelated I don't have a problem with us reading from /etc but I have a major problem with us adding the caveat. If @adamv and @mistydemeo are OK with this we can revert the /etc change but leave the caveat omitted.

MikeMcQuaid added a commit that referenced this issue Jan 21, 2014
This reverts commit 52fe004.

Conflicts:
	Library/Formula/zsh.rb

References #24538.
References #25719.
@MikeMcQuaid
Copy link
Member

Reverted in 4f990ea and caveats changed in 0e8bc06.

For anyone else viewing this conversation: @nuoymit's very reasonable reply explaining this well made a big difference. Thanks for it.

@mislav
Copy link
Sponsor Contributor

mislav commented Jan 29, 2014

@MikeMcQuaid So wait, why did you remove the caveats in the end? I think @nuoymit was arguing that you leave them in. The OS X bug where /etc/zshenv should actually be /etc/zprofile is very real.

@MikeMcQuaid
Copy link
Member

@mislav Sure. We don't want to advise people to modify system files though (hence why we originally pulled this change to the etcdir). It's a bug, sure, but it's Apple's bug rather than Homebrew's.

rolandwalker added a commit to rolandwalker/homebrew-cask that referenced this issue Mar 27, 2014
ehershey pushed a commit to ehershey/homebrew that referenced this issue Apr 4, 2014
This reverts commit 52fe004.

Conflicts:
	Library/Formula/zsh.rb

References Homebrew#24538.
References Homebrew#25719.
@Homebrew Homebrew locked and limited conversation to collaborators Jul 6, 2014
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants