Use set -eu, fix all unbound variables and properly catch exit codes.
yoosefi opened this issue · 2 comments
Modman is relying on a lot of loose behaviors in bash, which appears to be the source of many bugs.
Using potentially unbound variables in a script that alters the filesystem is dangerous and unacceptable.
The script needs to be put into strict mode to protect against this.
Furthermore, if a subcommand fails with an unexpected non-zero exit code, the entire script needs to halt.
At the top of the script:
set -eu
I'm not a bash expert now because I didn't even know about "set -u", but I definitely wasn't an expert when I wrote the first version 8 years ago.. :)
Feel free to submit PRs or point out specific issues but I'm sorry to say I don't have time to reinspect every line of code right now. As far as set -e goes there is a lot of error checking already but I'm guessing that if that was turned on right now it would break. So again, just not something I have time for at the moment.
Correct, the script refuses to run in this stricter mode.
Many error checks could be removed if -eu was enabled, potentially allowing a significant cleanup.
The script would indeed have to be checked pretty much line by line, but this can be done in segments by enabling -eu in each modman command, one-by-one, and slowly conforming the script to global -eu
I would be glad to help in this effort. If anyone sees this issue and wants to pitch in, that'd be great, too.