Re: [PATCH] populate SDK: prepare calling of bb.utils for exceptions

[Benjamin Esquivel <benjamin.esquivel@linux.intel.com>]

Hello Mike, comments below:

On Fri, 2015-10-16 at 09:00 +0200, Mike Looijmans wrote:
> On 15-10-15 19:27, Benjamin Esquivel wrote:
> > bb.utils.remove, bb.utils.movefile and bb.utils.mkdirhier can throw
> > exceptions that need handling and proper error messages.
> > 
> > [YOCTO#8213]
> ...
> > +    def mkdirhier(self, dirpath):
> > +        try:
> > +            bb.utils.mkdirhier(dirpath)
> > +        except OSError as e:
> > +            bb.warn(str(e))
> > +            bb.error("cannot make dir for SDK:
> > {}".format(dirpath))
> ...
> 
> 
> I see two bad things happening here:
> 
> - This will print a message, but continue processing as if nothing
> bad 
> happened, wreaking havoc later on when the caller might expect the
> directory 
> to actually exist and raise exceptions that are much harder to solve.
The exception is not re-raised since this is the appropriate level of
handling and you made me realize that this specific case will only be
reached if the dir is not present and unable to be created so it should
be changed for a bb.fatal instead of bb.error. thanks.
> 
> - It loses the information in the original exception, and since it
> doesn't 
> throw a new exception, the user is now left in the dark as to where
> the 
> problem occured, since there's no stack trace now.
> 
I will throw the trace to the log
> 
> For what I see in this code, it will actually make thing worse by
> hiding 
> errors and obscuring information. You're not "handling" the exception
> here, 
> you're almost "ignoring" them, which isn't quite the same.
> 
not agreed, current code hides errors in return codes which are not
even checked.
> 
> 
> Kind regards,
> 
> Mike Looijmans
> System Expert
> 
> TOPIC Embedded Products
> Eindhovenseweg 32-C, NL-5683 KH Best
> Postbus 440, NL-5680 AK Best
> Telefoon: +31 (0) 499 33 69 79
> Telefax: +31 (0) 499 33 69 70
> E-mail: mike.looijmans@topicproducts.com
> Website: www.topicproducts.com
> 
> Please consider the environment before printing this e-mail
> 
> 
> 
> 
>