Re: [gentoo-portage-dev] [PATCH] repoman: replace Fuse with Future

[Brian Dolbec]

On Sat, 16 Apr 2016 13:49:40 -0700
Zac Medico <zmedico@g.o> wrote:

> Replace Fuse with Future, which is similar more generic. The
> code ends up being slightly more verbose, but more flexible.
> The Future class will be useful elsewhere, including the
> EventLoop class.
> ---
> Applies to the *repoman* branch.
>
>  pym/portage/util/futures.py                 | 118
> ++++++++++++++++++++++++++++
> pym/repoman/fuse.py                         |  68 ----------------
> pym/repoman/main.py                         |  12 ++-
> pym/repoman/modules/scan/ebuild/ebuild.py   |  10 ++-
> pym/repoman/modules/scan/ebuild/isebuild.py |  25 ++++--
> pym/repoman/modules/scan/metadata/unused.py |  10 ++-
> pym/repoman/scanner.py                      |   4 +- 7 files changed,
> 164 insertions(+), 83 deletions(-) create mode 100644
> pym/portage/util/futures.py delete mode 100644 pym/repoman/fuse.py
>
> diff --git a/pym/portage/util/futures.py b/pym/portage/util/futures.py
> new file mode 100644
> index 0000000..c648f10
> --- /dev/null
> +++ b/pym/portage/util/futures.py
> @@ -0,0 +1,118 @@
> +# Copyright 2016 Gentoo Foundation
> +# Distributed under the terms of the GNU General Public License v2
> +#
> +# For compatibility with python versions which do not have the
> +# asyncio module (Python 3.3 and earlier), this module provides a
> +# subset of the asyncio.futures.Futures interface.
> +
> +from __future__ import unicode_literals
> +
> +__all__ = (
> +	'CancelledError',
> +	'Future',
> +	'InvalidStateError',
> +)
> +
> +try:
> +	from asyncio import (
> +		CancelledError,
> +		Future,
> +		InvalidStateError,
> +	)
> +except ImportError:
> +
> +	from portage.exception import PortageException
> +
> +	_PENDING = 'PENDING'
> +	_CANCELLED = 'CANCELLED'
> +	_FINISHED = 'FINISHED'
> +
> +	class Error(PortageException):
> +		pass
> +
> +	class CancelledError(Error):
> +		def __init__(self):
> +			Error.__init__(self, "cancelled")
> +
> +	class InvalidStateError(Error):
> +		pass
> +
> +	class Future(object):
> +
> +		# Class variables serving as defaults for instance
> variables.
> +		_state = _PENDING
> +		_result = None
> +		_exception = None
> +
> +		def cancel(self):
> +			"""Cancel the future and schedule callbacks.
> +
> +			If the future is already done or cancelled,
> return False.  Otherwise,
> +			change the future's state to cancelled,
> schedule the callbacks and
> +			return True.
> +			"""
> +			if self._state != _PENDING:
> +				return False
> +			self._state = _CANCELLED
> +			return True
> +
> +		def done(self):
> +			"""Return True if the future is done.
> +
> +			Done means either that a result / exception
> are available, or that the
> +			future was cancelled.
> +			"""
> +			return self._state != _PENDING
> +
> +		def result(self):
> +			"""Return the result this future represents.
> +
> +			If the future has been cancelled, raises
> CancelledError.  If the
> +			future's result isn't yet available, raises
> InvalidStateError.  If
> +			the future is done and has an exception set,
> this exception is raised.
> +			"""
> +			if self._state == _CANCELLED:
> +				raise CancelledError()
> +			if self._state != _FINISHED:
> +				raise InvalidStateError('Result is
> not ready.')
> +			if self._exception is not None:
> +				raise self._exception
> +			return self._result
> +
> +		def exception(self):
> +			"""Return the exception that was set on this
> future. +
> +			The exception (or None if no exception was
> set) is returned only if
> +			the future is done.  If the future has been
> cancelled, raises
> +			CancelledError.  If the future isn't done
> yet, raises
> +			InvalidStateError.
> +			"""
> +			if self._state == _CANCELLED:
> +				raise CancelledError
> +			if self._state != _FINISHED:
> +				raise InvalidStateError('Exception
> is not set.')
> +			return self._exception
> +
> +		def set_result(self, result):
> +			"""Mark the future done and set its result.
> +
> +			If the future is already done when this
> method is called, raises
> +			InvalidStateError.
> +			"""
> +			if self._state != _PENDING:
> +				raise InvalidStateError('{}:
> {!r}'.format(self._state, self))
> +			self._result = result
> +			self._state = _FINISHED
> +
> +		def set_exception(self, exception):
> +			"""Mark the future done and set an exception.
> +
> +			If the future is already done when this
> method is called, raises
> +			InvalidStateError.
> +			"""
> +			if self._state != _PENDING:
> +				raise InvalidStateError('{}:
> {!r}'.format(self._state, self))
> +			if isinstance(exception, type):
> +				exception = exception()
> +			self._exception = exception
> +			self._state = _FINISHED
> diff --git a/pym/repoman/fuse.py b/pym/repoman/fuse.py
> deleted file mode 100644
> index ac864fd..0000000
> --- a/pym/repoman/fuse.py
> +++ /dev/null
> @@ -1,68 +0,0 @@
> -
> -'''
> -fuse.py
> -
> -A tiny one-time-fuse class that uses a boolean to mimic the property
> of -an electrical fuse.  IT's good (True) until it is popped (bad,
> False). -It is not resetable.
> -'''
> -
> -
> -class Fuse(object):
> -	'''A One time fuse style boolean instance'''
> -
> -	__slots__ = ('_state')
> -
> -	def __init__(self):
> -		self._state = True
> -
> -	def pop(self):
> -		'''Blow's the fuse state (makes it False)'''
> -		self._state = False
> -
> -	def __repr__(self):
> -		'''x.__repr__() <==> repr(x)'''
> -		return repr(self._state>0)
> -
> -	def __str__(self):
> -		'''x.__str__() <==> str(x)'''
> -		return ['False', 'True'][self._state]
> -
> -	def __bool__(self):
> -		'''self != 0'''
> -		return self._state != 0
> -
> -	def __nonzero__(self):
> -		'''self != 0'''
> -		return self._state != 0
> -
> -	def __abs__(self):
> -		'''x.__abs__() <==> abs(x)'''
> -		return [0, 1] [self._state]
> -
> -	def __int__(self):
> -		'''int(self)'''
> -		return [0, 1][self._state]
> -
> -	def __eq__(self, value):
> -		'''Return self==value.'''
> -		return self._state == value
> -
> -	def __ne__(self, value):
> -		'''Return self!=value.'''
> -		return self._state != value
> -
> -	def __ge__(self, value):
> -		'''Return self>=value.'''
> -		return self._state >= value
> -
> -	def __gt__(self, value):
> -		return self._state > value
> -
> -	def __le__(self, value):
> -		'''Return self<=value.'''
> -		return self._state <= value
> -
> -	def __lt__(self, value):
> -		'''Return self<value.'''
> -		return self._state < value
> diff --git a/pym/repoman/main.py b/pym/repoman/main.py
> index 2ccda99..62c3c2c 100755
> --- a/pym/repoman/main.py
> +++ b/pym/repoman/main.py
> @@ -22,10 +22,13 @@ import portage.repository.config
>  from portage.output import create_color_func, nocolor
>  from portage.output import ConsoleStyleFile, StyleWriter
>  from portage.util import formatter
> +from portage.util.futures import (
> +	Future,
> +	InvalidStateError,
> +)
>
>  from repoman.actions import Actions
>  from repoman.argparser import parse_args
> -from repoman.fuse import Fuse
>  from repoman.qa_data import (
>  	format_qa_output, format_qa_output_column, qahelp,
>  	qawarnings, qacats)
> @@ -76,7 +79,7 @@ def repoman_main(argv):
>  	# Set this to False when an extraordinary issue (generally
>  	# something other than a QA issue) makes it impossible to
>  	# commit (like if Manifest generation fails).
> -	can_force = Fuse()
> +	can_force = Future()
>
>  	portdir, portdir_overlay, mydir =
> utilities.FindPortdir(repoman_settings) if portdir is None:
> @@ -171,6 +174,11 @@ def repoman_main(argv):
>  	qa_output = qa_output.getvalue()
>  	qa_output = qa_output.splitlines(True)
>
> +	try:
> +		can_force = can_force.result()
> +	except InvalidStateError:
> +		can_force = True
> +
>  	# output the results
>  	actions = Actions(repo_settings, options, scanner,
> vcs_settings) if actions.inform(can_force, result):
> diff --git a/pym/repoman/modules/scan/ebuild/ebuild.py
> b/pym/repoman/modules/scan/ebuild/ebuild.py index 540411f..67eee3f
> 100644 --- a/pym/repoman/modules/scan/ebuild/ebuild.py
> +++ b/pym/repoman/modules/scan/ebuild/ebuild.py
> @@ -8,6 +8,7 @@ from repoman.modules.scan.scanbase import ScanBase
>  # import our initialized portage instance
>  from repoman._portage import portage
>  from portage import os
> +from portage.util.futures import InvalidStateError
>
>  pv_toolong_re = re.compile(r'[0-9]{19,}')
>
> @@ -127,15 +128,18 @@ class Ebuild(ScanBase):
>  	def pkg_invalid(self, **kwargs):
>  		'''Sets some pkg info and checks for invalid packages
>
> -		@param validity_fuse: Fuse instance
> +		@param validity_future: Future instance
>  		@returns: dictionary, including {pkg object}
>  		'''
> -		fuse = kwargs.get('validity_fuse')
> +		fuse = kwargs.get('validity_future')
>  		if self.pkg.invalid:
>  			for k, msgs in self.pkg.invalid.items():
>  				for msg in msgs:
>  					self.qatracker.add_error(k,
> "%s: %s" % (self.relative_path, msg))
> -			fuse.pop()
> +			try:
> +				fuse.set_result(False)
> +			except InvalidStateError:
> +				pass
>  			return {'continue': True, 'pkg': self.pkg}
>  		return {'continue': False, 'pkg': self.pkg}
>
> diff --git a/pym/repoman/modules/scan/ebuild/isebuild.py
> b/pym/repoman/modules/scan/ebuild/isebuild.py index 514d23e..a8870c7
> 100644 --- a/pym/repoman/modules/scan/ebuild/isebuild.py
> +++ b/pym/repoman/modules/scan/ebuild/isebuild.py
> @@ -9,6 +9,7 @@ from _emerge.RootConfig import RootConfig
>  from repoman._portage import portage
>
>  from portage import os
> +from portage.util.futures import InvalidStateError
>
>  from repoman.qa_data import no_exec, allvars
>  from repoman.modules.scan.scanbase import ScanBase
> @@ -35,13 +36,13 @@ class IsEbuild(ScanBase):
>  		@param checkdirlist: list of files in the current
> package directory @param checkdir: current package directory path
>  		@param xpkg: current package directory being checked
> -		@param validity_fuse: Fuse instance
> +		@param validity_future: Future instance
>  		@returns: dictionary, including {pkgs, can_force}
>  		'''
>  		checkdirlist = kwargs.get('checkdirlist')
>  		checkdir = kwargs.get('checkdir')
>  		xpkg = kwargs.get('xpkg')
> -		fuse = kwargs.get('validity_fuse')
> +		fuse = kwargs.get('validity_future')
>  		can_force = kwargs.get('can_force')
>  		self.continue_ = False
>  		ebuildlist = []
> @@ -64,15 +65,24 @@ class IsEbuild(ScanBase):
>  				try:
>  					myaux = dict(zip(allvars,
> self.portdb.aux_get(cpv, allvars))) except KeyError:
> -					fuse.pop()
> +					try:
> +
> fuse.set_result(False)
> +					except InvalidStateError:
> +						pass
>  					self.qatracker.add_error("ebuild.syntax",
> os.path.join(xpkg, y)) continue
>  				except IOError:
> -					fuse.pop()
> +					try:
> +
> fuse.set_result(False)
> +					except InvalidStateError:
> +						pass
>  					self.qatracker.add_error("ebuild.output",
> os.path.join(xpkg, y)) continue
>  				if not
> portage.eapi_is_supported(myaux["EAPI"]):
> -					fuse.pop()
> +					try:
> +
> fuse.set_result(False)
> +					except InvalidStateError:
> +						pass
>  					self.qatracker.add_error("EAPI.unsupported",
> os.path.join(xpkg, y)) continue
>  				pkgs[pf] = Package(
> @@ -86,7 +96,10 @@ class IsEbuild(ScanBase):
>  			# metadata leads to false positives for
> several checks, and false # positives confuse users.
>  			self.continue_ = True
> -			can_force.pop()
> +			try:
> +				fuse.set_result(False)
> +			except InvalidStateError:
> +				pass
>
>  		return {'continue': self.continue_, 'pkgs': pkgs}
>
> diff --git a/pym/repoman/modules/scan/metadata/unused.py
> b/pym/repoman/modules/scan/metadata/unused.py index 6def48f..114c1f1
> 100644 --- a/pym/repoman/modules/scan/metadata/unused.py
> +++ b/pym/repoman/modules/scan/metadata/unused.py
> @@ -1,4 +1,6 @@
>
> +from portage.util.futures import InvalidStateError
> +
>
>  class UnusedCheck(object):
>  	'''Checks and reports any un-used metadata.xml use flag
> descriptions''' @@ -16,14 +18,18 @@ class UnusedCheck(object):
>  		@param xpkg: the pacakge being checked
>  		@param muselist: use flag list
>  		@param used_useflags: use flag list
> -		@param validity_fuse: Fuse instance
> +		@param validity_future: Future instance
>  		'''
>  		xpkg = kwargs.get('xpkg')
>  		muselist = kwargs.get('muselist')
>  		used_useflags = kwargs.get('used_useflags')
> +		try:
> +			valid_state =
> kwargs['validity_future'].result()
> +		except InvalidStateError:
> +			valid_state = True
>  		# check if there are unused local USE-descriptions
> in metadata.xml # (unless there are any invalids, to avoid noise)
> -		if kwargs.get('validity_fuse'):
> +		if valid_state:
>  			for myflag in
> muselist.difference(used_useflags): self.qatracker.add_error(
>  					"metadata.warning",
> diff --git a/pym/repoman/scanner.py b/pym/repoman/scanner.py
> index a9f56e9..e9a8e20 100644
> --- a/pym/repoman/scanner.py
> +++ b/pym/repoman/scanner.py
> @@ -9,7 +9,7 @@ import portage
>  from portage import normalize_path
>  from portage import os
>  from portage.output import green
> -from repoman.fuse import Fuse
> +from portage.util.futures import Future
>  from repoman.modules.commit import repochecks
>  from repoman.profile import check_profiles, dev_profile_keywords,
> setup_profile from repoman.repos import repo_metadata
> @@ -232,7 +232,7 @@ class Scanner(object):
>  				'repolevel': self.repolevel,
>  				'catdir': catdir,
>  				'pkgdir': pkgdir,
> -				'validity_fuse': Fuse()
> +				'validity_future': Future()
>  				}
>  			# need to set it up for ==> self.modules or
> some other ordered list for mod in ['Manifests', 'IsEbuild',
> 'KeywordChecks', 'FileChecks',


Yeah, it's sat long enough with me to be fine with it this way, it is
more flexible to handle both default states but only in the code it is
used in. It also can work for strings and any other data that isn't
normally an instance pointer.

Push it and I'll use Future() for those where it is needed in the
module_spec func_kwargs.

-- 
Brian Dolbec <dolsen>