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

Ensure global startup RCs see the right ZDOTDIR #4

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

strugee
Copy link

@strugee strugee commented Aug 5, 2022

We do this by disabling zsh's builtin behavior to read these startup files and instead doing them ourselves. This allow us to exert control over what ZDOTDIR is set to (we cannot just set ZDOTDIR to the right thing before global startup scripts are executed because this would cause zsh to not execute the custom startup scripts we wrote out to the temporary directory).

In order to maintain compatibility with existing zsh behavior, we stash the state of the GLOBAL_RCS option at the end of our custom startup scripts but before disabling the option. We then restore its state at the beginning of the next startup script (so immediately after zsh would have executed the relevant global startup script).

Note that I didn't test this exhaustively (I basically just tested that the behavior described in #3 was fixed).

Fixes #3

We do this by disabling zsh's builtin behavior to read these startup
files and instead doing them ourselves. This allow us to exert control
over what ZDOTDIR is set to (we cannot just set ZDOTDIR to the right
thing before global startup scripts are executed because this would
cause zsh to not execute the custom startup scripts we wrote out to
the temporary directory).

In order to maintain compatibility with existing zsh behavior, we
stash the state of the GLOBAL_RCS option at the end of our custom
startup scripts but before disabling the option. We then restore its
state at the beginning of the next startup script (so immediately
after zsh *would* have executed the relevant global startup script).

Fixes romkatv#3
@romkatv
Copy link
Owner

romkatv commented Aug 6, 2022

The directory with global rcs is specified when compiling zsh and it's not guaranteed to be /etc. On many systems it's /etc/zsh and on some systems it's disabled altogether. Unfortunately, there is no mechanism to ask zsh for the location of this directory.

I don't see a way to properly fix #3. If you just want it to work on your system, you can modify your ~/.zshrc to set HISTFILE and load it with fc -R.

@strugee
Copy link
Author

strugee commented Aug 7, 2022

What if we gate the code in this patch on /etc/zsh* existing? The idea being that if those files exist zsh was probably compiled with global config files in /etc.

Obviously it wouldn't fix things for systems with a different global config directory, but at least it would be fixed for many systems and would not introduce regressions for the rest.

@romkatv
Copy link
Owner

romkatv commented Aug 7, 2022

It would cause a regression for users of https://github.com/romkatv/zsh-bin (including myself). Normally it does not source any global rc files.

The new bugs look worse, or at least more weird, than the old bugs: sourcing global rc files with wrong ZDOTDIR isn't as bad as sourcing files that aren't supposed to be sourced.

If you just want it to work on your system, you can modify your ~/.zshrc to set HISTFILE and load it with fc -R.

Could you reply to this ☝️ Does this workaround work for you?

@strugee
Copy link
Author

strugee commented Aug 7, 2022

It would cause a regression for users of https://github.com/romkatv/zsh-bin (including myself). Normally it does not source any global rc files.

But it seems to read them, though. Is the intended behavior for zshi to not read these files? Why don't we just set -o no_global_rcs then? That would fix #3 and seems to align zshi with (IIUC) the intended behavior too.

Could you reply to this ☝️ Does this workaround work for you?

We depend on this script at work so we could probably use this workaround in the wrapper, yeah. I would just prefer not to since it's kind of a hack and means that our script is even more complicated.

@romkatv
Copy link
Owner

romkatv commented Aug 7, 2022

It would cause a regression for users of https://github.com/romkatv/zsh-bin (including myself). Normally it does not source any global rc files.

But it seems to read them, though.

How are you testing this?

Is the intended behavior for zshi to not read these files?

The intended behavior is described in https://github.com/romkatv/zshi/blob/master/README.md:

Usage: zshi <init-command> [zsh-flag]...

The same as plain zsh [zsh-flag]... except that an additional <init-command> gets executed after all standard Zsh startup files have been sourced.

The word "standard" is somewhat fuzzy. I'm trying to say here that zshi by itself doesn't affect which startup files are sourced.

@romkatv
Copy link
Owner

romkatv commented Aug 7, 2022

I would just prefer not to since it's kind of a hack and means that our script is even more complicated.

Any solution will be a hack. I don't see an option for a proper fix. zshi is a tiny script and it's released under a permissive license. You can simply fork it and modify if that's easier than modifying zshrc.

@strugee
Copy link
Author

strugee commented Aug 8, 2022

How are you testing this?

Oh shoot, I mixed up zsh-bin and zshi, sorry.

Am I understanding correctly that zsh-bin disables reading of all startup files, but that does not affect the GLOBAL_RCS option (at least AFAICT from the upstream documentation)? And that's why this would be a backwards-incompatible change for zsh-bin and anything like it?

@romkatv
Copy link
Owner

romkatv commented Feb 13, 2023

I missed your question here, sorry.

Am I understanding correctly that zsh-bin disables reading of all startup files, but that does not affect the GLOBAL_RCS option (at least AFAICT from the upstream documentation)?

zsh-bin never reads global startup files.

And that's why this would be a backwards-incompatible change for zsh-bin and anything like it?

On my system there is /etc/zsh with a bunch of global startup files. They are a part of Debian package for zsh. However, since I'm using zsh from zsh-bin as my shell, these startup files are not being read. This is the desired and intended behavior. If zshi were to be modified so so that it would manually source global startup files from /etc/zsh, it would be a backward-incompatible change.

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

Successfully merging this pull request may close these issues.

$HISTFILE is set to a file in the temporary directory on macOS zsh
2 participants