chruby is not `set -u` friendly.

postmodern / chruby

Changes the current Ruby

MIT License

2.85k stars 190 forks source link

chruby is not `set -u` friendly. #417

Open zenspider opened 4 years ago

zenspider commented 4 years ago

Needs to have a default value for PREFIX in share/chruby/chruby.sh at the very least:

PREFIX="${PREFIX:-/usr/local}"

example:

9983 % set -u
9984 % . chruby.sh 
bash: PREFIX: unbound variable

I'm running out of a checkout now and trying to clean these up but I'm not sure if this is going to be accepted...

postmodern commented 4 years ago

Would this be to fix set -u issues in just scripts/setup.sh, or throughout the project? chruby's code (as well as most bash scripts) makes frequent usage of unbound variables in string interpolation. I'm guessing set -u is an environmental requirement you're needing to comply with? If it makes chruby more correct without adding more code, then I'll accept it. I suppose all unbound variables could be replaced with ${FOO:-}?

kvz commented 3 years ago

Yes, set -u is a best practice and I'm running into the same thing where I have to explicitly disable seat belts now before I source chruby. It would be a nice improvement to run chruby with set -o nounset (a.k.a. set -u) and then where chruby explicitly allows unset variables you use refer to them by ${name:-} (returning "") or ${name:-postmodern} (returning a default of "postmodern" in case the var was not set). Slightly more hassle from the get go, but catches a lot of bash bugs.