On Wed, Feb 15, 2012 at 7:05 AM, Allan McRae <[email protected]> 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 <[email protected]> > > --- > > 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@gnu to see if this is really the case, though. d
