Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use set -eu, fix all unbound variables and properly catch exit codes. #121

Open
yoosefi opened this issue Feb 10, 2017 · 2 comments
Open

Comments

@yoosefi
Copy link

yoosefi commented Feb 10, 2017

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
@colinmollenhour
Copy link
Owner

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.

@yoosefi
Copy link
Author

yoosefi commented Feb 12, 2017

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants