Fix auto upgrade failure from non-exported ZSH env var

[ghost]

Fixes #549. Specify ZSH=$ZSH explicitly when invoking
the auto update scripts.

Other pull requests for this issue are incomplete. ZSH needs to be manually propagated in two places.

[ghost]

Related pull requests are #544 & #533

[cleishm]

Actually, the second propagation (in check_for_upgrade.sh) is only necessary if "check_for_upgrade.sh" is invoked directly (not via oh-my-zsh.sh), which isn't really related to the auto-upgrade failing issue. Still, it's a good idea.

[ghost]

@chrisleishman Ahh. I was alternating between invoking it directly and twiddling the epoch in .zsh-update and opening new terminal windows. Must have missed that it only failed when invoked directly.

[vguerci]

Several questions about this :

1. I never had this issue, $ZSH env var is propagated without this patch for me... how is that possible?
2. Why spawning a new zsh (/usr/bin/env zsh) while the check_for_upgrade.sh is just a sh script (#!/bin/sh) and afaik doesn't depend on any zsh or omz feature. (it just needs $ZSH and called upgrade.sh is also a basic shell script, does I miss something?)
   • Then couldn't that just be : /bin/sh $ZSH/tools/check_for_upgrade.sh
     (or even assuming it is chmod-ed +x just $ZSH/tools/check_for_upgrade.sh, that shebang is pretty useless w/o no?)
   • Or why no just sourcing it (=same context) : . $ZSH/tools/check_for_upgrade.sh ?
     (isnt faster than creating a new shell?)
3. Since a little fix for showing colors during upgrade #512, I have ugly -e before ZSH logo when upgrading on OSX, /bin/sh echo not handling -e... Sourcing and then staying in zsh sort this...

[ghost]

@vguerci you probably have a ~/.zshrc copied from templates/zshrc.zsh-template back when ZSH was exported. If you recopy templates/zshrc.zsh-template and merge in your local changes you'll probably have the same issue.

[vguerci]

@toolbear thanks for the clarification, I was exporting it.
And what do you think about sourcing ? see vguerci/oh-my-zsh@dbee12a67ceec1bb1e2a72eb300cd9cf21654a4a

[ghost]

Sourcing can pollute the current environment. I considered sourcing at first, then decided on explicitly passing ZSH to maintain isolation.

[vguerci]

I see, but looking at upgrade scripts, I don't see any potential env pollution, but maybe I'm wrong.
My concern is about performance, my guess is that there is an overhead of starting a new process.
Plus I noticed your request #555 which is also solved by sourcing...

[ghost]

It's not like we're spawning processes in a tight loop, here. Lol. I think perf is a non-issue in this case.

You got your pull request. I got mine. Either isolation is desirable, or it's unecessary. Now @robbyrussel gets to choose. Feel free to add a comment to your pull request indicating that it makes #555 unnecessary. If isolation isn't needed, that certainly makes a compelling case for your fix.

[pikeas]

I submitted issue #567 a couple of weeks ago. Today I realized the non-exported variable was the real issue and submitted a pull request today. Looks like you guys beat me to it!

[ruthiejames95]

Thanks for sharing this. I think it is very interesting and informative.
Ruth James | http://www.cbsautotech.com/autobody_repair.html