[pacman-dev] Scripts bombing with no args

[Dave Reisner]

On Fri, Jul 29, 2011 at 07:01:29PM +1000, Allan McRae wrote:
> On 29/07/11 03:14, Dave Reisner wrote:
> >On Thu, Jul 28, 2011 at 11:18:15AM -0500, Dan McGee wrote:
> >>Allan or Dave, mind taking a look at this? We should show usage info,
> >>as rankmirrors and pacman-key do without args, instead of this silly
> >>behavior.
> >>
> >>-Dan
> >>
> >>dmcgee at galway ~/projects/pacman (master)
> >>$ ./scripts/repo-add
> >>==>  ERROR: '' does not have a valid archive extension.
> >
> >We never check for $1 being unset before proceeding. Probably my
> >mistake when I did some refactoring of repo-add.
> >
> >>dmcgee at galway ~/projects/pacman (master)
> >>$ ./scripts/pkgdelta
> >>./scripts/pkgdelta: line 147: $1: unbound variable
> >>
> >
> >'set -u' with 'set -e'? Ugly...
> 
> Care to explain why?  Using this in our cleanup scripts would have
> prevented the Arch repos disappearing on more than one occasion....
> 

Because the combination of these two flags causes scripts to fail in
mysterious and unexpected ways. Also, it won't save you from set but
empty vars, which I suspect would be just as dangerous as unset vars in
the situation you allude to. Just validate the user inputs. It makes
things more obvious to an outside reader of the code, as well as
consumers of the script.

dave

> >I've got fixes for both on my 'scripts-fixup' branch
> >
> >https://github.com/falconindy/pacman/commit/e99b6a131ea08829da26d62d498c33f3129683e3
> >https://github.com/falconindy/pacman/commit/e42d97b7370c9e30d9ae66a33f42d27460eaf137
> 
> why not:
> 
> case ${1:-""} in
> 
> which fixes the issue and leaves nounset.
> 
> Allan
>