[gentoo-portage-dev] plugin-sync progress report

[gentoo-portage-dev] plugin-sync progress report

[Brian Dolbec]

I've started working on the repository/config.py changes needed for the
plugin-sync system.

First:
  The following class performed checks on the
repos.conf entries for the sync variables regardless if they were being
used or not.  I would like to remove that code block completely from
this loader/parser.  I would prefer to have the sync module perform
it's own checks and only when syncing or from a future emaint module
with that specific 'check-config' option.

I've sectioned off the code I'd like to remove below. 
Is there any reason they must be done here and during initial parsing?

Advantages:
  - No need to import and initialize the sync module and the plugin system 
    for other emerge/portage operations.
  - Should speed things up slightly with less to do.
  - Makes the sync module responsible the validation of any sync specific variables.


Second:
  - Should all the repos.conf entires to be synced be validated prior to syncing and fail
    if there are any errors?
  - Or, just call each sync module's sync() and let each fail individually 
    and continue syncing remaining repos?

Third:
  - I would like to add a new config item for all repos.
    It is a variable to define which sources of sync operations which
    the repo will be allowed to be synced from.  It could be a space separated list
    which could be used to prevent it from being synced via certain commands.

    i.e:

    [repo1] 
    sync-from = emerge emaint

    [repo2]
    sync-from = emerge emaint layman

    [repo3]
    sync-from = None

  - Thoughts?  
  - Opinions?

############

class RepoConfigLoader(object):
	"""Loads and store config of several repositories, loaded from PORTDIR_OVERLAY or repos.conf"""

	@staticmethod
	def _parse(paths, prepos, ignored_map, ignored_location_map, local_config, portdir):
		"""Parse files in paths to load config"""
		parser = SafeConfigParser()

		# use read_file/readfp in order to control decoding of unicode
		try:
			# Python >=3.2
			read_file = parser.read_file
			source_kwarg = 'source'
		except AttributeError:
			read_file = parser.readfp
			source_kwarg = 'filename'

		recursive_paths = []
		for p in paths:
			if isinstance(p, basestring):
				recursive_paths.extend(_recursive_file_list(p))
			else:
				recursive_paths.append(p)

		for p in recursive_paths:
			if isinstance(p, basestring):
				f = None
				try:
					f = io.open(_unicode_encode(p,
						encoding=_encodings['fs'], errors='strict'),
						mode='r', encoding=_encodings['repo.content'],
						errors='replace')
				except EnvironmentError:
					pass
				else:
					# The 'source' keyword argument is needed since otherwise
					# ConfigParser in Python <3.3.3 may throw a TypeError
					# because it assumes that f.name is a native string rather
					# than binary when constructing error messages.
					kwargs = {source_kwarg: p}
					read_file(f, **portage._native_kwargs(kwargs))
				finally:
					if f is not None:
						f.close()
			elif isinstance(p, io.StringIO):
				kwargs = {source_kwarg: "<io.StringIO>"}
				read_file(p, **portage._native_kwargs(kwargs))
			else:
				raise TypeError("Unsupported type %r of element %r of 'paths' argument" % (type(p), p))

		prepos['DEFAULT'] = RepoConfig("DEFAULT",
			parser.defaults(), local_config=local_config)

		for sname in parser.sections():
			optdict = {}
			for oname in parser.options(sname):
				optdict[oname] = parser.get(sname, oname)

			repo = RepoConfig(sname, optdict, local_config=local_config)

###################################
# remove these checks
###################################
			if repo.sync_type is not None and repo.sync_uri is None:
				writemsg_level("!!! %s\n" % _("Repository '%s' has sync-type attribute, but is missing sync-uri attribute") %
					sname, level=logging.ERROR, noiselevel=-1)
				continue

			if repo.sync_uri is not None and repo.sync_type is None:
				writemsg_level("!!! %s\n" % _("Repository '%s' has sync-uri attribute, but is missing sync-type attribute") %
					sname, level=logging.ERROR, noiselevel=-1)
				continue

			if repo.sync_type not in portage.sync.module_names + [None]:
				writemsg_level("!!! %s\n" % _("Repository '%s' has sync-type attribute set to unsupported value: '%s'") %
					(sname, repo.sync_type), level=logging.ERROR, noiselevel=-1)
				continue

			if repo.sync_type == "cvs" and repo.sync_cvs_repo is None:
				writemsg_level("!!! %s\n" % _("Repository '%s' has sync-type=cvs, but is missing sync-cvs-repo attribute") %
					sname, level=logging.ERROR, noiselevel=-1)
				continue
###################################
# end remove
###################################

			# For backward compatibility with locations set via PORTDIR and
			# PORTDIR_OVERLAY, delay validation of the location and repo.name
			# until after PORTDIR and PORTDIR_OVERLAY have been processed.
			prepos[sname] = repo


-- 
Brian Dolbec <dolsen>

Re: [gentoo-portage-dev] plugin-sync progress report

[Sebastian Luther]

Am 15.03.2014 21:32, schrieb Brian Dolbec:
> I've started working on the repository/config.py changes needed for the
> plugin-sync system.
> 
> First:
>   The following class performed checks on the
> repos.conf entries for the sync variables regardless if they were being
> used or not. [...]
> 
> Second:
>   - Should all the repos.conf entires to be synced be validated prior to syncing and fail
>     if there are any errors?
>   - Or, just call each sync module's sync() and let each fail individually 
>     and continue syncing remaining repos?
> 

Maybe that's just me, but I don't like things that sometimes work and
sometimes not. Having some emerge invocations fail with "broken config"
and some not, seems rather ugly to me.

(This also implies that syncing a repo with working config is not
possible as long as there are repos with broken config.)

> Third:
>   - I would like to add a new config item for all repos.
>     It is a variable to define which sources of sync operations which
>     the repo will be allowed to be synced from.  It could be a space separated list
>     which could be used to prevent it from being synced via certain commands.

What exactly does this variable mean? What are emaint or layman doing here?

Re: [gentoo-portage-dev] plugin-sync progress report

[Alec Warner]

[-- Attachment #1: Type: text/plain, Size: 7328 bytes --]

On Sat, Mar 15, 2014 at 1:32 PM, Brian Dolbec <dolsen@gentoo.org> wrote:

> I've started working on the repository/config.py changes needed for the
> plugin-sync system.
>
> First:
>   The following class performed checks on the
> repos.conf entries for the sync variables regardless if they were being
> used or not.  I would like to remove that code block completely from
> this loader/parser.  I would prefer to have the sync module perform
> it's own checks and only when syncing or from a future emaint module
> with that specific 'check-config' option.
>
> I've sectioned off the code I'd like to remove below.
> Is there any reason they must be done here and during initial parsing?
>
> Advantages:
>   - No need to import and initialize the sync module and the plugin system
>     for other emerge/portage operations.
>   - Should speed things up slightly with less to do.
>   - Makes the sync module responsible the validation of any sync specific
> variables.
>
>
> Second:
>   - Should all the repos.conf entires to be synced be validated prior to
> syncing and fail
>     if there are any errors?
>   - Or, just call each sync module's sync() and let each fail individually
>     and continue syncing remaining repos?
>
> Third:
>   - I would like to add a new config item for all repos.
>     It is a variable to define which sources of sync operations which
>     the repo will be allowed to be synced from.  It could be a space
> separated list
>     which could be used to prevent it from being synced via certain
> commands.
>

As we discussed on IRC, the use case here seemed to be to have some repos
that are not synced automatically (eg: via emerge --sync or its equivalent.)

I'd propose something more obvious:

sync=manual # user has to call emaint sync <reponame>
sync=auto # emerge --sync will sync this repo.

-A


>     i.e:
>
>     [repo1]
>     sync-from = emerge emaint
>
>     [repo2]
>     sync-from = emerge emaint layman
>
>     [repo3]
>     sync-from = None
>
>   - Thoughts?
>   - Opinions?
>
> ############
>
> class RepoConfigLoader(object):
>         """Loads and store config of several repositories, loaded from
> PORTDIR_OVERLAY or repos.conf"""
>
>         @staticmethod
>         def _parse(paths, prepos, ignored_map, ignored_location_map,
> local_config, portdir):
>                 """Parse files in paths to load config"""
>                 parser = SafeConfigParser()
>
>                 # use read_file/readfp in order to control decoding of
> unicode
>                 try:
>                         # Python >=3.2
>                         read_file = parser.read_file
>                         source_kwarg = 'source'
>                 except AttributeError:
>                         read_file = parser.readfp
>                         source_kwarg = 'filename'
>
>                 recursive_paths = []
>                 for p in paths:
>                         if isinstance(p, basestring):
>
> recursive_paths.extend(_recursive_file_list(p))
>                         else:
>                                 recursive_paths.append(p)
>
>                 for p in recursive_paths:
>                         if isinstance(p, basestring):
>                                 f = None
>                                 try:
>                                         f = io.open(_unicode_encode(p,
>                                                 encoding=_encodings['fs'],
> errors='strict'),
>                                                 mode='r',
> encoding=_encodings['repo.content'],
>                                                 errors='replace')
>                                 except EnvironmentError:
>                                         pass
>                                 else:
>                                         # The 'source' keyword argument is
> needed since otherwise
>                                         # ConfigParser in Python <3.3.3
> may throw a TypeError
>                                         # because it assumes that f.nameis a native string rather
>                                         # than binary when constructing
> error messages.
>                                         kwargs = {source_kwarg: p}
>                                         read_file(f,
> **portage._native_kwargs(kwargs))
>                                 finally:
>                                         if f is not None:
>                                                 f.close()
>                         elif isinstance(p, io.StringIO):
>                                 kwargs = {source_kwarg: "<io.StringIO>"}
>                                 read_file(p,
> **portage._native_kwargs(kwargs))
>                         else:
>                                 raise TypeError("Unsupported type %r of
> element %r of 'paths' argument" % (type(p), p))
>
>                 prepos['DEFAULT'] = RepoConfig("DEFAULT",
>                         parser.defaults(), local_config=local_config)
>
>                 for sname in parser.sections():
>                         optdict = {}
>                         for oname in parser.options(sname):
>                                 optdict[oname] = parser.get(sname, oname)
>
>                         repo = RepoConfig(sname, optdict,
> local_config=local_config)
>
> ###################################
> # remove these checks
> ###################################
>                         if repo.sync_type is not None and repo.sync_uri is
> None:
>                                 writemsg_level("!!! %s\n" % _("Repository
> '%s' has sync-type attribute, but is missing sync-uri attribute") %
>                                         sname, level=logging.ERROR,
> noiselevel=-1)
>                                 continue
>
>                         if repo.sync_uri is not None and repo.sync_type is
> None:
>                                 writemsg_level("!!! %s\n" % _("Repository
> '%s' has sync-uri attribute, but is missing sync-type attribute") %
>                                         sname, level=logging.ERROR,
> noiselevel=-1)
>                                 continue
>
>                         if repo.sync_type not in portage.sync.module_names
> + [None]:
>                                 writemsg_level("!!! %s\n" % _("Repository
> '%s' has sync-type attribute set to unsupported value: '%s'") %
>                                         (sname, repo.sync_type),
> level=logging.ERROR, noiselevel=-1)
>                                 continue
>
>                         if repo.sync_type == "cvs" and repo.sync_cvs_repo
> is None:
>                                 writemsg_level("!!! %s\n" % _("Repository
> '%s' has sync-type=cvs, but is missing sync-cvs-repo attribute") %
>                                         sname, level=logging.ERROR,
> noiselevel=-1)
>                                 continue
> ###################################
> # end remove
> ###################################
>
>                         # For backward compatibility with locations set
> via PORTDIR and
>                         # PORTDIR_OVERLAY, delay validation of the
> location and repo.name
>                         # until after PORTDIR and PORTDIR_OVERLAY have
> been processed.
>                         prepos[sname] = repo
>
>
> --
> Brian Dolbec <dolsen>
>
>
>

[-- Attachment #2: Type: text/html, Size: 9857 bytes --]

Re: [gentoo-portage-dev] plugin-sync progress report

[Brian Dolbec]

On Sat, 15 Mar 2014 21:48:59 +0100
Sebastian Luther <SebastianLuther@gmx.de> wrote:

> Am 15.03.2014 21:32, schrieb Brian Dolbec:
> > I've started working on the repository/config.py changes needed for
> > the plugin-sync system.
> > 
> > First:
> >   The following class performed checks on the
> > repos.conf entries for the sync variables regardless if they were
> > being used or not. [...]
> > 
> > Second:
> >   - Should all the repos.conf entires to be synced be validated
> > prior to syncing and fail if there are any errors?
> >   - Or, just call each sync module's sync() and let each fail
> > individually and continue syncing remaining repos?
> > 
> 
> Maybe that's just me, but I don't like things that sometimes work and
> sometimes not. Having some emerge invocations fail with "broken
> config" and some not, seems rather ugly to me.
> 
> (This also implies that syncing a repo with working config is not
> possible as long as there are repos with broken config.)
> 

So, what your saying is EVERY time emerge is run or portage is imported
for api use.  It _MUST_ load the sync modules and  verify the sync
parameters regardless if those setting will be used or NOT.  That is a
waste of resources and time to me.  Portage is already far too slow.  I
admit, that this will not be much of a time/resource consumer for the
majority of gentoo systems.  But what about low power systems, where
resources including memory are at a premium?  After all, how many times
do you edit your repos.conf files?  Why test them for every possible
error every time portage code is started up?

I was intending on adding a check-config option to the emaint sync
module I planned on adding.  I think that is a good compromise between
over checking and no checking at all.


> > Third:
> >   - I would like to add a new config item for all repos.
> >     It is a variable to define which sources of sync operations
> > which the repo will be allowed to be synced from.  It could be a
> > space separated list which could be used to prevent it from being
> > synced via certain commands.
> 
> What exactly does this variable mean? What are emaint or layman doing
> here?
> 
> 
> 

emaint is here because I intend on making an emaint sync module for
doing more detailed sync operations.  Like syncing individual repos. If
a user had an overlay that they did not want to be synced via emerge
then not putting emerge there, when the emerge --sync command was
started. It would skip syncing that repo, even though it had a
sync-type and sync-url.  Layman was an entry there since most overlays
are installed & managed by it.  So, would have been a possible entry.
 

Alex's solution seems like a good fit in place of this.  See his email
reply.


-- 
Brian Dolbec <dolsen>