Re: [gentoo-portage-dev] [PATCH 2/2] emirrordist: Pass path from DeletionIterator to DeletionTask

[Zac Medico]

On 10/21/19 1:43 AM, Michał Górny wrote:
> Since DeletionIterator needs to stat the distfile and therefore find
> one working path for it, pass it to DeletionTask instead of recomputing
> it there.  This also fixes wrongly assuming that first layout will
> always be correct.
>
> Bug: https://bugs.gentoo.org/697890
> Signed-off-by: Michał Górny <mgorny@g.o>
> ---
>  lib/portage/_emirrordist/DeletionIterator.py |  2 ++
>  lib/portage/_emirrordist/DeletionTask.py     | 14 +++++---------
>  2 files changed, 7 insertions(+), 9 deletions(-)
>
> diff --git a/lib/portage/_emirrordist/DeletionIterator.py b/lib/portage/_emirrordist/DeletionIterator.py
> index 5c193911a..3cbff2c3a 100644
> --- a/lib/portage/_emirrordist/DeletionIterator.py
> +++ b/lib/portage/_emirrordist/DeletionIterator.py
> @@ -72,6 +72,7 @@ class DeletionIterator(object):
>
>  					yield DeletionTask(background=True,
>  						distfile=filename,
> +						distfile_path=path,
>  						config=self._config)
>
>  				else:
> @@ -85,6 +86,7 @@ class DeletionIterator(object):
>
>  						yield DeletionTask(background=True,
>  							distfile=filename,
> +							distfile_path=path,
>  							config=self._config)
>
>  		if deletion_db is not None:
> diff --git a/lib/portage/_emirrordist/DeletionTask.py b/lib/portage/_emirrordist/DeletionTask.py
> index a4bb29419..4e9c26ca2 100644
> --- a/lib/portage/_emirrordist/DeletionTask.py
> +++ b/lib/portage/_emirrordist/DeletionTask.py
> @@ -10,14 +10,9 @@ from _emerge.CompositeTask import CompositeTask
>
>  class DeletionTask(CompositeTask):
>
> -	__slots__ = ('distfile', 'config')
> +	__slots__ = ('distfile', 'distfile_path', 'config')
>
>  	def _start(self):
> -
> -		distfile_path = os.path.join(
> -			self.config.options.distfiles,
> -			self.config.layouts[0].get_path(self.distfile))
> -
>  		if self.config.options.recycle_dir is not None:
>  			recycle_path = os.path.join(
>  				self.config.options.recycle_dir, self.distfile)
> @@ -29,7 +24,8 @@ class DeletionTask(CompositeTask):
>  					"distfiles to recycle") % self.distfile)
>  				try:
>  					# note: distfile_path can be a symlink here
> -					os.rename(os.path.realpath(distfile_path), recycle_path)
> +					os.rename(os.path.realpath(self.distfile_path),
> +							recycle_path)
>  				except OSError as e:
>  					if e.errno != errno.EXDEV:
>  						logging.error(("rename %s from distfiles to "
> @@ -40,7 +36,7 @@ class DeletionTask(CompositeTask):
>  					return
>
>  				self._start_task(
> -					FileCopier(src_path=distfile_path,
> +					FileCopier(src_path=self.distfile_path,
>  						dest_path=recycle_path,
>  						background=False),
>  					self._recycle_copier_exit)
> @@ -55,7 +51,7 @@ class DeletionTask(CompositeTask):
>  			logging.debug(("delete '%s' from "
>  				"distfiles") % self.distfile)
>  			try:
> -				os.unlink(distfile_path)
> +				os.unlink(self.distfile_path)
>  			except OSError as e:
>  				if e.errno not in (errno.ENOENT, errno.ESTALE):
>  					logging.error("%s unlink failed in distfiles: %s" %
>

Looks good. Please merge.
-- 
Thanks,
Zac