Closed Un1q32 closed 1 year ago
So you SSHed into a machine, within its interactive shell, you run topgrade
, then you get the error, right?
The command
zsh -c 'source /home/joey/.zshrc > /dev/null && export -p | grep ZSH > /dev/null && echo $ZSH'
that is being run exits with code 1
I think this is the root cause, it is propagated by the grep
command (export -p | grep ZSH > /dev/null
) as there is no ZSH
in its input.
When I implemented #528, I recalled I had tested that this command won't give an error if the ZSH
variable is not present when using remote execution (login shell, not an interactive shell)
It seems like they have different behaviors here, needs investigation
I am having the same issue, but it is happening when I run it with remote execution.
I also noticed that oh-my-zsh is not a listed step to disable in the config file.
0: Command failed: `zsh -c 'source /home/violet/.zshrc > /dev/null && export -p | grep ZSH > /dev/null && echo $ZSH'`
1: `zsh` failed: exit status: 1
Location:
src/steps/zsh.rs:190```
What is the purpose of greping export -p anyway? Why not just check if the variable is set with [ -n $ZSH ]
or test -n $ZSH
Realized immediately after writing it wouldn't check if it's exported.
You can at least do grep -q
instead of redirecting output to /dev/null, I'm pretty sure every major grep implementation supports -q
I also noticed that oh-my-zsh is not a listed step to disable in the config file.
Yeah, oh-my-zsh
is not a step, shell
is, see #515
Here is a build for x86_64-unknown-linux-musl
, please test it
Do you guys need builds for other targets?
Code review for #592 is also enacouraged
Here is a build for
x86_64-unknown-linux-musl
, please test itDo you guys need builds for other targets?
Code review for #592 is also enacouraged
that seems to fix it
Can confirm that this addresses it. Wonderful!
Thanks for testing it!
I will give it a test to see whether it will work when oh-my-zsh
is actually installed, after that, I will merge that PR
Thank you so much for looking into it.
Somewhat unrelatedly, is there no configuration option to disable oh-my-zsh checks, or did I just guess the name of the step incorrectly? If the former, it would be nice to blacklist that as I do use zsh on nearly all my hosts, but I do not use Oh-My-Zsh!
Somewhat unrelatedly, is there no configuration option to disable oh-my-zsh checks, or did I just guess the name of the step incorrectly?
Unfortunately, there is currently no way to disable oh-my-zsh
cause it is not a step, Topgrade groups all the shell-related things into a step shell
:
$ topgrade --only shell --dry-run
── 10:06:36 - Sudo ─────────────────────────────────────────────────────────────
Dry running: /usr/bin/sudo -v
── 10:06:36 - oh-my-zsh ────────────────────────────────────────────────────────
Pulling custom plugins and themes
Would pull /home/steve/.oh-my-zsh/custom/plugins/zsh-autosuggestions
Would pull /home/steve/.oh-my-zsh/custom/plugins/zsh-syntax-highlighting
Dry running: zsh /home/steve/.oh-my-zsh/tools/upgrade.sh
── 10:06:36 - Summary ──────────────────────────────────────────────────────────
oh-my-zsh: OK
I have a plan to enhance the step granularity, see #515, though I do think it needs more evaluation and discussion
If the former, it would be nice to blacklist that as I do use zsh on nearly all my hosts, but I do not use Oh-My-Zsh!
If this issue is fixed, then Topgrade won't try the oh-my-zsh
update when it finds that the tool is not installed on the host, so no worries about this
Just a thought, why run grep at all when you can just parse the output of export -p in rust to get the $ZSH environment variable? Means you depend on less external binaries on top of being a bit faster.
Yeah, you are right
Just a thought, why run grep at all when you can just parse the output of export -p in rust to get the $ZSH environment variable? Means you depend on less external binaries on top of being a bit faster.
I just updated the binary here, would you like to give it a test?
Erroneous Behavior
oh-my-zsh step is tried and fails when topgrade is run over ssh, even though oh-my-zsh is not installed.
The failing command seems to run right at the end of another step, instead of being a separate step with its own banner with the timestamp and name like normal. On my Pi running Debian 12 it runs at the end of the system upgrade step, on my main Fedora machine it runs at the end of the flatpak system packages step, on my macbook pro running MacOS 14 with OCLP it runs at the end of the brew cask step.
Expected Behavior
oh-my-zsh step should not run
Steps to reproduce
https://github.com/OldWorldOrdr/dotfiles has my full zsh config
Possible Cause (Optional)
I know something similar used to happen in older versions, but that was fixed my not having the $ZSH variable set, not anymore.
I think the issue is the check for if oh-my-zsh should be run at all is failing.
Problem persists without calling from topgrade
The command
zsh -c 'source /home/joey/.zshrc > /dev/null && export -p | grep ZSH > /dev/null && echo $ZSH'
that is being run exits with code 1 even when not called from topgrade, but the bug is that it produces an errorDid you run topgrade through
Remote Execution
Configuration file (Optional)
Additional Details
Operating System/Version fedora 38, macos 14, debian 12
Installation github releases
Topgrade version (
topgrade -V
) Topgrade 13.0.0Verbose Output (
topgrade -v
)