[pacman-dev] [PATCH 1/3] makepkg: restrict usage of errexit to user functions

[Dave Reisner]

On Wed, Feb 15, 2012 at 7:05 AM, Allan McRae <allan at archlinux.org> wrote:

> On 15/02/12 04:57, Dave Reisner wrote:
> > It's expected that this will lead to unwanted behavior, and needs
> > widespread testing. It's desirable to commit this for a few reasons:
> >
> > - there's no reason we can't do our own error checking for code that we
> >   write.
> > - it avoids the need for ||true hacks scattered about in the code.
> > - it makes us immune to upstream changes in exit codes (FS#28248)
> >
> > Signed-off-by: Dave Reisner <dreisner at archlinux.org>
> > ---
> > Allan, just making sure we're on the same page -- this _will_ cause
> breakage,
> > and the next patch in this series addresses one specific case. I figure
> getting
> > this patch in now gives us "ample time" to uncover and fix all these
> before the
> > next release.
>
>
> OK...  I like this idea somewhat as I have seen the issues this can
> cause.  But I have also seen it validly stop the scripts execution due
> to errors that would have been non-obvious.
>
>
> Here goes a few concerns you might help me alleviate:
>
> 1) does it really make us immune to upstream changes in exit codes?  In
> the particular example of mercurial, would we not be checking the exit
> code of the "hg pull" part anyway?
>

Yeah, this is probably accurate. I feel like there's always going to be
commands which can exit non-zero for which we still expect that they did
what we told them to, I just picked a bad example.


> 2) how much extra error checking are we going to need to do? e.g. if I
> look at create_srcpackage() would we only need to check the mkdir and ln
> lines?


So, the big stuff is: file/directory creation, directory changes, and
executing arbitrary code (e.g. sourcing files). There's no reason we
couldn't create a "lighter weight" ERR trap with errtrace if we expect to
go through an entire function that needs to succeed, rather than checking
everything. create_srcpackage() looks like a good candidate for this sort
of thing. We can do something like this:

somefunc() { true; false; true; echo 'shouldnt be here' }
set -E
trap -- 'return 1 2>/dev/null' ERR
somefunc
set +E
trap -- ERR
echo 'success'

The stderr redirect is due to what I suspect is a bug in bash (exists in
bash3 as well) -- it errors out saying you can only return from a function,
but it gives us the behavior we want (evidenced by the debug echo's). I'm
going to followup with bug-bash at gnu to see if this is really the case,
though.

d