FS#13767 - Use errexit and nounset options in all bash scripts from pacman source tree

Attached to Project: Pacman
Opened by Xavier (shining) - Thursday, 12 March 2009, 10:35 GMT
Last edited by Allan McRae (Allan) - Friday, 09 March 2012, 05:41 GMT
Task Type Feature Request
Category Scripts & Tools
Status Closed
Assigned To Aaron Griffin (phrakture)
Xavier (shining)
Dan McGee (toofishes)
Allan McRae (Allan)
Architecture All
Severity Low
Priority Normal
Reported Version 3.2.1
Due in Version Undecided
Due Date Undecided
Percent Complete 100%
Votes 3
Private No

Details

The following page recommends using set -u and set -e, which both seem like a good idea to me :
http://www.davidpashley.com/articles/writing-robust-shell-scripts.html

Should we try to be consistent using these options in all the bash scripts in pacman repo?
Several already use set -e, but not all of them.
And none seem to use set -u. This setting will very likely require several changes (declaring all variables we use, even when we just check for emptiness).

But I am not sure if it is worth the trouble adding these to the existing mature scripts. It might require a hell lot of testing, maybe even several releases before the scripts are used in all sort of corner cases and all problems are found.
In this case, it could still be worth adding them to the most basic scripts, and/or to the new ones that are added.
This task depends upon

Closed by  Allan McRae (Allan)
Friday, 09 March 2012, 05:41 GMT
Reason for closing:  Won't implement
Additional comments about closing:  See final comment
Comment by Brendan (watricky) - Sunday, 12 April 2009, 02:06 GMT
I've been using -u in all my bash scripts ever since I came across that same blog entry. Its helped uncover some serious bugs that would have otherwise gone unnoticed until things were *really* broken. ;)

I agree that maybe it isn't a good idea to put it in there without lots of testing. At first I set it when I was making changes and when I was sure my new code was good I unset it. But now I leave it set everywhere anyway.
Comment by Allan McRae (Allan) - Friday, 07 August 2009, 12:59 GMT
I started looking at this for makepkg but it is going to be a big job and it will be really easy to miss some of the places needing adjusted for this to work. This should not be attempted until makepkg has a good test suite ( FS#15645 ).

As a reminder for later.... ${foo:-""} prevents nounset erroring during the [ -z "$foo" ] style tests.
Comment by Xavier (shining) - Tuesday, 18 August 2009, 09:51 GMT
I think I have decided it is not worth.

Even for the simple and recent rankmirrors bash script, it was written with the default bash behavior in mind, and well tested. I thought about adding errexit and nounset but I gave up, it really looked counter-productive.

I still think these options are good, but only if you start from scratch with them in mind.
So I would just close my bug report. It was just a FYI :)
Comment by Xavier (shining) - Wednesday, 19 August 2009, 08:47 GMT
I just found a recent and good illustration for the utility of either option :
http://archlinux.me/brain0/2009/08/16/shit-happens-when-you-party-naked-or-use-crappy-shell-scripts/
Comment by Brendan (watricky) - Saturday, 05 September 2009, 01:56 GMT
I've found that errexit is difficult to integrate into an existing script (sometimes I've even changed my mind due to a lack of time) but nounset is usually easy to add, even in a very large script.
Comment by Allan McRae (Allan) - Friday, 09 March 2012, 05:41 GMT
We are now moving towards adding error checking into the relevant places in makepkg and removing errexit. We have experience programs that "succeed" (in that they do what we wnat them to do...) but return non-zero exit codes, and programs that changed their exit codes to non-zero completely breaking that section of makepkg.

So this is going to be "Won"t Implement"

Loading...