Re: [gentoo-portage-dev] [PATCH] emirrordist: Clean dangling symlinks up

[Michał Górny]

On Mon, 2019-10-21 at 00:44 -0700, Zac Medico wrote:
> On 10/20/19 3:23 AM, Michał Górny wrote:
> > Bug: https://bugs.gentoo.org/697906
> > Signed-off-by: Michał Górny <mgorny@g.o>
> > ---
> >  lib/portage/_emirrordist/DeletionIterator.py | 21 +++++++++++++++++---
> >  1 file changed, 18 insertions(+), 3 deletions(-)
> >
> > diff --git a/lib/portage/_emirrordist/DeletionIterator.py b/lib/portage/_emirrordist/DeletionIterator.py
> > index dab6eaea2..6bc0fd09f 100644
> > --- a/lib/portage/_emirrordist/DeletionIterator.py
> > +++ b/lib/portage/_emirrordist/DeletionIterator.py
> > @@ -27,16 +27,31 @@ class DeletionIterator(object):
> >  			# require at least one successful stat()
> >  			exceptions = []
> >  			for layout in reversed(self._config.layouts):
> > +				path = os.path.join(distdir, layout.get_path(filename))
> >  				try:
> > -					st = os.stat(
> > -							os.path.join(distdir, layout.get_path(filename)))
> > +					st = os.stat(path)
> >  				except OSError as e:
> > -					exceptions.append(e)
> > +					# is it a dangling symlink?
> > +					try:
> > +						if os.path.islink(path):
> > +							os.unlink(path)
> > +					except OSError as e:
> > +						exceptions.append(e)
> >  				else:
> >  					if stat.S_ISREG(st.st_mode):
> >  						break
>
> How about if we remove the above break so that we can eliminate the
> lstat loop below?

Oops, I forgot to remove it from my patch.  The 'break' is fine since we
need this dangling symlink elimination only if none of the files exist. 
If they do, then symlink removal post distfile removal will take care of
them.

>
>
> >  			else:
> >  				if exceptions:
> > +					# check for dangling symlinks
> > +					for layout in self._config.layouts:
> > +						path = os.path.join(distdir, layout.get_path(filename))
> > +						try:
> > +							st = os.lstat(path)
> > +							if stat.S_ISLNK(st.st_mode):
> > +								os.unlink(path)
> > +						except OSError as e:
> > +							pass
> > +
> >  					logging.error("stat failed on '%s' in distfiles: %s\n" %
> >  						(filename, '; '.join(str(x) for x in exceptions)))
> >  				continue
> >
>
> Looks good except that it would be nice to eliminate the second loop.

-- 
Best regards,
Michał Górny