Fix the script when sourced from zsh

[wilkmar]

When the novarc file is sourced from the Zsh, it fails due to a few Bash specifics:

• $BASH_SOURCE - does not exist in Zsh
• readarray - does not exist in Zsh

Resolves issue #176

[dosaboy]

this looks like it might now work in bash?

[wilkmar]

this looks like it might now work in bash?

I tested it in both bash and zsh, both when sourced or executed directly (however don't see the use case for the latter one). Also ran ./configure with sources novarc and haven't noticed any issues

[pponnuvel]

There are a number of differences between zsh and bash. So it's going to be difficult for a script to work in both shells.

A simple workaround is (assuming user starts in zsh):

bash # switch to bash shell
source novarc # do it
zsh # new zsh shell; exported vars will be available in the subshell (zsh)
## will have to exit twice of course

If that's not acceptable, and zsh support is absolutely required, perhaps we can make two: novarc.bash and novarc.zsh?

[wilkmar]

There are a number of differences between zsh and bash. So it's going to be difficult for a script to work in both shells.

A simple workaround is (assuming user starts in zsh):

bash # switch to bash shell
source novarc # do it
zsh # new zsh shell; exported vars will be available in the subshell (zsh)
## will have to exit twice of course

If that's not acceptable, and zsh support is absolutely required, perhaps we can make two: novarc.bash and novarc.zsh?

IMHO only the changes I am proposing in the PR are needed to make it work on both bash and zsh. I have tested this for both shells and haven't noticed problems but if more testing is needed I am happy to do that.
Unfortunately, the proposed workaround is not an option, since the novarc is sourced from a number of other scripts.

[pponnuvel]

IMHO only the changes I am proposing in the PR are needed to make it work on both bash and zsh. I have tested this for both shells and haven't noticed problems but if more testing is needed I am happy to do that.

I don't see any issues - just saying that keeping script written in X working in Y is always going to be hard when X & Y aren't feature compatible. If we go with the PR as is, a comment might be useful so that future travellers know they should keep it compatible with zsh.

Unfortunately, the proposed workaround is not an option, since the novarc is sourced from a number of other scripts.

That shouldn't be an issue as every script that sources it is a bash script. The only issue here is that sourcing novarc from a user's shell is the problem because user's shell is zsh. So zsh -> bash -> zsh should work.

---

Alternatively, novarc can be modified to produce those variables and that can be sourced in any shell. Example:

bash_cmd='
list=(V1 V2 V3)

V1="111 A B C"
V2="222"
V3="333"

for v in "${list[@]}"; do
    printf "export $v=\"${!v}\"\n"
done
'

eval $(bash -c "$bash_cmd")

[wilkmar]

Hi @pponnuvel I got your point about the need to maintain the script for both shells. I have added the comment as you suggested.
Unfortunately, not all scripts use bash in the shebang. The .configure uses #!/bin/sh -u, and that is causing issues too. Maybe it's worth to change that to bash at the same time?

[pponnuvel]

Thanks @wilkmar . Yes, it makes sense to change the shebang in openstack/configure to bash. In practice, it doesn't cause any issues because when sourcing a script, the shebang has no meaning. But for consistency/clarity, it makes sense to change it to bash.

There's also one more in jaas/configure.

[wilkmar]

jaas/configure

the openstack/configure is meant to be executed directly (not sourced), which is an issue because indirectly it sources novarc. Not sure what about the jaas/configure (I haven't used it so far) but I suspect the same.

[pponnuvel]

jaas/configure

the openstack/configure is meant to be executed directly (not sourced), which is an issue because indirectly it sources novarc. Not sure what about the jaas/configure (I haven't used it so far) but I suspect the same.

Regardless, you can change the shebang in both.

[wilkmar]

ACK but rather I will make it a separate PR

[pponnuvel]

LGTM

[wilkmar]

Created PR #209

[raineszm]

I have noclobber set in my zsh config so this line fails for me since mktemp has already created the file. If Ichange the redirection to >| instead of > i.e.

juju status --format=json >| $juju_status_json_cache

everything works fine in both bash and zsh.

[wilkmar]

@raineszm thank you for spotting this issue and proposing the fix. This is unrelated to this PR, though. I created another PR: #323. Once it is merged, I will rebase this one.

[raineszm]

I've been using this on top of the main branch for a few weeks without issue. Any chance we could get it merged?

[wilkmar]

I rebased this branch onto main. It would be awesome if someone could merge it.

[nicolasbock]

Hi @wilkmar ! Could you rebase (again, sorry). Thanks!

[wilkmar]

Hey @nicolasbock , I just did. Please have a look whenever you are available. Thx!

[nicolasbock]

@wilkmar and @raineszm , I don't use zsh and am not sure whether I can test this properly. Could you agree on something and let me know when this is ready? Thanks!

[wilkmar]

@nicolasbock discussed it with @raineszm
This patch is still needed, but I just submitted the updated version - only the crucial bits.
Please have a look when you have a moment