bash-preexec fails if user has "nounset" option set about bash-preexec HOT 8 CLOSED

It may be an anti-pattern, but I think it'd be best not to make that choice on behalf of the user. Shells are highly personal things and the less we make them break in mysterious ways for our users, the better, even if they've made strange choices to get there.

The main practical issue is when code that breaks "nounset" is reached from the debug trap that bash-preexec installs, which means the user can't even safely use nounset in contexts where it to all appearances is neatly scoped to their own code. However, it's easier to just make the entire script nounset-safe rather than to try to reason about what is reachable from where.

Supporting it is easy, the only difficulty is supporting it consistently. And given that this happens to be a bash script with a test suite, even that shouldn't be hard. I have the necessary code and test changes in a local client -- will try to put together a PR tomorrow.

from bash-preexec.

Any update on this -- decision to pull or not?

As mentioned this is an issue arising from actual usage. Maintaining a patch for this locally would be somewhat painful for me, since it's so easy to introduce an incompatibility if the tests upstream aren't covering it.

from bash-preexec.

Personally, I don't think supporting -u is worth the effort. If you can make it work great, but I think using it (in your shell) is generally an anti-pattern.

+1 for swapping to the bats-core fork though.

from bash-preexec.

@steinarvk Thanks for the PR and all the reasoning + logic behind it! On first glance, I agree with @dimo414 that setting -u in your shell is an anti-pattern and question if it's worth trying to maintain it in bash-preexec. Looking at the code, I feel it also lowers the readability by inserting {*:-} in tons of places.

Do you personally or are you running into users who are having this issue at Google because they set it in their shells? Or they're setting it as part of their preexec functions and it's causing bash-preexec to break?

from bash-preexec.

Can confirm that it's not a theoretical concern, but based on an actual user issue. (See the "main practical issue" paragraph: I've not seen anyone setting "set -u" for their entire shell, but I've seen reasonable-seeming usage bleeding into bash-preexec somehow.)

from bash-preexec.

@steinarvk Planning to merge this. Give me a day or two. I want to cut a release with it. Thanks for your patience and contributions.

from bash-preexec.

Awesome, thanks.

from bash-preexec.

Merged. Will try to cut a release later this evening.

from bash-preexec.