Re: [PATCH] sanity.bbclass: check SSTATE_DIR, DL_DIR and *MIRROR for broken symlinks

[<Mikko.Rapeli@bmw.de>]

Hi,

On Mon, Aug 10, 2015 at 11:10:58AM +0200, Mike Looijmans wrote:
> On 10-08-15 10:18, Mikko.Rapeli@bmw.de wrote:
> >On Fri, Jul 31, 2015 at 12:00:15PM +0300, Mikko Rapeli wrote:
> ...
> >>+# Check that path isn't a broken symlink
> >>+def check_symlink(lnk):
> >>+    if os.path.islink(lnk) and not os.path.exists(lnk):
> >>+       return False
> >>+    return True
> 
> - Bad coding style "if (x) return false".
> - Naming a method "check..." suggests to me that it will raise an exception
> on failure.

Thanks. I tried to follow existing patterns from the functions there.

> alternatives:
> 
> def is_broken_symlink(lnk):
>     return os.path.islink(lnk) and not os.path.exists(lnk)
> 
> def check_symlink(lnk, data):
>     if os.path.islink(lnk) and not os.path.exists(lnk):
> 	raise_sanity_error("%s is a broken symlink." % lnk, data)

This feels better and reduces the log statements too. Hopefully the generic
error message is enough to figure out what is wrong. I'll test and send a
new version.

Regarding testing, I guess selftests are not generally used to test error
paths like this.

-Mikko