Re: [gentoo-portage-dev] [PATCH] conf: Enable to set rsync extra opts per repository

[Brian Dolbec <dolsen@gentoo.org>]

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

On Mon, 6 Jul 2015 11:28:36 -0700
Brian Dolbec <dolsen@gentoo.org> wrote:

 
> hmm, this is the cvs module's module_spec, I was thinking we might be
> able to add the module_specific_opts there after the validate_config
> definition, like so:
> 
> module_spec = {
> 	'name': 'cvs',
> 	'description': doc,
> 	'provides':{
> 		'cvs-module': {
> 			'name': "cvs",
> 			'class': "CVSSync",
> 			'description': doc,
> 			'functions': ['sync', 'new', 'exists'],
> 			'func_desc': {
> 				'sync': 'Performs a cvs up on the
> repository', 'new': 'Creates the new repository at the specified
> location', 'exists': 'Returns a boolean of whether the specified dir
> ' + 'exists and is a valid CVS
> repository', },
> 			'validate_config': CheckCVSConfig,
>                         'module_specific_opts': None,
> 		}
> 	}
> }
> 
> 
> Also, the config validation code should probably be exteneded for any
> module specific options as well.  
> 
>  I'll look into whether that will work.
> I'll add your patch and see about moving things around.
> 


OK, I think I got it, I haven't tested your extra-options field, but a
normal rsync operation worked for me.  I simplified the naming some,
the "sync_cvs_repo" name was old and oblsolete (replaced by the
universal "auto-sync" setting).  So for rsync it is simply
"extra-options".  Also since they are being stored in a dictionary,
there is no need to sub the - for _. 

I think this should work out well.  Now it should be easier to add
branch support to the git module.

Try it out, see what you think.  You can squash my commits back into
yours with just a small mention of my contribution.  No big deal for
me ;)

I thank you for your work on this.

I've attached the 2 patches of changes.  Please test and check it for
errors (there is bound to be some) I haven't tested it thoroughly yet.


-- 
Brian Dolbec <dolsen>


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Rework-PATCH-v2-1-2-sync-allow-sync-modules-to-have-.patch --]
[-- Type: text/x-patch, Size: 9174 bytes --]

From e540b25c133aa5df8c13ae9b6f57506ec2cb07ba Mon Sep 17 00:00:00 2001
From: Brian Dolbec <dolsen@gentoo.org>
Date: Mon, 6 Jul 2015 14:51:13 -0700
Subject: [PATCH 1/2] Rework [PATCH v2 1/2] sync: allow sync modules to have
 specific options

Move things around so they are defined in the module_spec instead.
Simplify the code a little as portage has no other use for these settings.

Signed-off-by: Brian Dolbec <dolsen@gentoo.org>
---
 pym/portage/repository/config.py              | 31 +++++++++++----------------
 pym/portage/sync/__init__.py                  | 18 +++++++++-------
 pym/portage/sync/modules/cvs/__init__.py      |  1 +
 pym/portage/sync/modules/cvs/cvs.py           |  7 +-----
 pym/portage/sync/modules/git/__init__.py      |  1 +
 pym/portage/sync/modules/svn/__init__.py      |  1 +
 pym/portage/sync/modules/webrsync/__init__.py |  1 +
 pym/portage/sync/syncbase.py                  |  5 -----
 8 files changed, 28 insertions(+), 37 deletions(-)

diff --git a/pym/portage/repository/config.py b/pym/portage/repository/config.py
index 8c16cb3..6626beb 100644
--- a/pym/portage/repository/config.py
+++ b/pym/portage/repository/config.py
@@ -90,8 +90,8 @@ class RepoConfig(object):
 		'sync_depth',
 		'sync_type', 'sync_umask', 'sync_uri', 'sync_user', 'thin_manifest',
 		'update_changelog', 'user_location', '_eapis_banned',
-		'_eapis_deprecated', '_masters_orig') + \
-		tuple(portage.sync.module_specific_options)
+		'_eapis_deprecated', '_masters_orig', 'module_specific_options',
+		)
 
 	def __init__(self, name, repo_opts, local_config=True):
 		"""Build a RepoConfig with options in repo_opts
@@ -176,10 +176,7 @@ class RepoConfig(object):
 
 		self.sync_depth = repo_opts.get('sync-depth')
 
-		for o in portage.sync.module_specific_options:
-			odash = o.replace('_', '-')
-			if odash in repo_opts:
-				setattr(self, o, repo_opts[odash])
+		self.module_specific_options = {}
 
 		# Not implemented.
 		format = repo_opts.get('format')
@@ -281,6 +278,10 @@ class RepoConfig(object):
 			self._eapis_banned = frozenset(layout_data['eapis-banned'])
 			self._eapis_deprecated = frozenset(layout_data['eapis-deprecated'])
 
+	def set_module_specific_opts(self, opts):
+		for o in opts:
+			self.module_specific_options[o] = repo_opts.get(o, None)
+
 	def eapi_is_banned(self, eapi):
 		return eapi in self._eapis_banned
 
@@ -425,11 +426,6 @@ class RepoConfig(object):
 		if self.eclass_overrides:
 			repo_msg.append(indent + "eclass-overrides: " + \
 				" ".join(self.eclass_overrides))
-		if self.sync_type is not None:
-			prefix = "sync_" + self.sync_type + "_"
-			for o in portage.sync.module_specific_options:
-				if hasattr(self, o) and o.startswith(prefix) and getattr(self, o):
-					repo_msg.append(indent + o.replace('_', '-') + ": " + getattr(self, o))
 		repo_msg.append("")
 		return "\n".join(repo_msg)
 
@@ -481,9 +477,6 @@ class RepoConfigLoader(object):
 		if prepos['DEFAULT'].masters is not None:
 			default_repo_opts['masters'] = \
 				' '.join(prepos['DEFAULT'].masters)
-		for o in portage.sync.module_specific_options:
-			if hasattr(prepos['DEFAULT'], o):
-				default_repo_opts[o.replace('_', '-')] = getattr(prepos['DEFAULT'], o)
 
 		if overlays:
 			# We need a copy of the original repos.conf data, since we're
@@ -513,7 +506,7 @@ class RepoConfigLoader(object):
 							'force', 'masters', 'priority',
 							'sync_depth',
 							'sync_type', 'sync_umask', 'sync_uri', 'sync_user',
-							) + tuple(portage.sync.module_specific_options):
+							) + portage.sync.module_specific_options(repo, logging):
 							v = getattr(repos_conf_opts, k, None)
 							if v is not None:
 								setattr(repo, k, v)
@@ -605,6 +598,8 @@ class RepoConfigLoader(object):
 				optdict[oname] = parser.get(sname, oname)
 
 			repo = RepoConfig(sname, optdict, local_config=local_config)
+			repo.set_module_specific_opts(
+				portage.sync.module_specific_options(repo, logging))
 
 			# Perform repos.conf sync variable validation
 			portage.sync.validate_config(repo, logging)
@@ -635,8 +630,8 @@ class RepoConfigLoader(object):
 			# deprecated portdir_sync
 			portdir_sync = settings.get("SYNC", "")
 
-		default_opts['sync-rsync-extra-opts'] = \
-			settings.get("PORTAGE_RSYNC_EXTRA_OPTS", None)
+		#default_opts['sync-rsync-extra-opts'] = \
+		#	settings.get("PORTAGE_RSYNC_EXTRA_OPTS", None)
 
 		try:
 			self._parse(paths, prepos, ignored_map,
@@ -974,7 +969,7 @@ class RepoConfigLoader(object):
 		str_or_int_keys = ("auto_sync", "format", "location",
 			"main_repo", "priority",
 			"sync_type", "sync_umask", "sync_uri", 'sync_user')
-		str_or_int_keys += tuple(portage.sync.module_specific_options)
+		#str_or_int_keys += self.module_specific_options()
 		str_tuple_keys = ("aliases", "eclass_overrides", "force")
 		repo_config_tuple_keys = ("masters",)
 		keys = str_or_int_keys + str_tuple_keys + repo_config_tuple_keys
diff --git a/pym/portage/sync/__init__.py b/pym/portage/sync/__init__.py
index 11059eb..4edb402 100644
--- a/pym/portage/sync/__init__.py
+++ b/pym/portage/sync/__init__.py
@@ -25,15 +25,17 @@ module_controller = Modules(path=path, namepath="portage.sync.modules")
 module_names = module_controller.module_names[:]
 
 
-def _build_module_specific_options_list():
-	modules = set()
-	for (mn, m) in [(mn, module_controller.get_class(mn)) for mn in module_names]:
-		modules.update(["sync_" + mn + "_" + opt.replace('-', '_') for opt in m.specific_options()])
-	return modules
-
-
-module_specific_options = frozenset(_build_module_specific_options_list())
+def module_specific_options(repo, logger):
+	'''Get the authorized module specific options set for
+	the repos.conf settings for the repo'''
+	global module_controller
 
+	#print(repo)
+	if repo.sync_type:
+		opts = frozenset([opt for opt in
+			module_controller.modules[repo.sync_type]['module_specific_options']])
+		return opts
+	return frozenset()
 
 def validate_config(repo, logger):
 	'''Validate the repos.conf settings for the repo'''
diff --git a/pym/portage/sync/modules/cvs/__init__.py b/pym/portage/sync/modules/cvs/__init__.py
index 0f4a029..5da0781 100644
--- a/pym/portage/sync/modules/cvs/__init__.py
+++ b/pym/portage/sync/modules/cvs/__init__.py
@@ -40,6 +40,7 @@ module_spec = {
 					'exists and is a valid CVS repository',
 			},
 			'validate_config': CheckCVSConfig,
+			'module_specific_options': ("sync_cvs_repo",),
 		}
 	}
 }
diff --git a/pym/portage/sync/modules/cvs/cvs.py b/pym/portage/sync/modules/cvs/cvs.py
index 90e256b..4654ca4 100644
--- a/pym/portage/sync/modules/cvs/cvs.py
+++ b/pym/portage/sync/modules/cvs/cvs.py
@@ -19,11 +19,6 @@ class CVSSync(NewBase):
 		return "CVSSync"
 
 
-	@staticmethod
-	def specific_options():
-		return ("repo",)
-
-
 	def __init__(self):
 		NewBase.__init__(self, "cvs", portage.const.CVS_PACKAGE_ATOM)
 
@@ -42,7 +37,7 @@ class CVSSync(NewBase):
 			"cd %s; exec cvs -z0 -d %s co -P -d %s %s" %
 			(portage._shell_quote(os.path.dirname(self.repo.location)), portage._shell_quote(cvs_root),
 				portage._shell_quote(os.path.basename(self.repo.location)),
-				portage._shell_quote(self.repo.sync_cvs_repo)),
+				portage._shell_quote(self.repo.module_specific_options["sync_cvs_repo"])),
 				**portage._native_kwargs(self.spawn_kwargs)) != os.EX_OK:
 			msg = "!!! cvs checkout error; exiting."
 			self.logger(self.xterm_titles, msg)
diff --git a/pym/portage/sync/modules/git/__init__.py b/pym/portage/sync/modules/git/__init__.py
index a372881..cd28253 100644
--- a/pym/portage/sync/modules/git/__init__.py
+++ b/pym/portage/sync/modules/git/__init__.py
@@ -50,6 +50,7 @@ module_spec = {
 					'exists and is a valid Git repository',
 			},
 			'validate_config': CheckGitConfig,
+			'module_specific_options': (,),
 		}
 	}
 }
diff --git a/pym/portage/sync/modules/svn/__init__.py b/pym/portage/sync/modules/svn/__init__.py
index 59ab950..4189298 100644
--- a/pym/portage/sync/modules/svn/__init__.py
+++ b/pym/portage/sync/modules/svn/__init__.py
@@ -26,6 +26,7 @@ module_spec = {
 					'exists and is a valid SVN repository',
 			},
 			'validate_config': CheckSyncConfig,
+			'module_specific_options': (,),
 		}
 	}
 }
diff --git a/pym/portage/sync/modules/webrsync/__init__.py b/pym/portage/sync/modules/webrsync/__init__.py
index 5a92066..f3c40d4 100644
--- a/pym/portage/sync/modules/webrsync/__init__.py
+++ b/pym/portage/sync/modules/webrsync/__init__.py
@@ -44,6 +44,7 @@ module_spec = {
 					'exists and is a valid repository',
 			},
 			'validate_config': CheckSyncConfig,
+			'module_specific_options': (,),
 		},
 	}
 }
diff --git a/pym/portage/sync/syncbase.py b/pym/portage/sync/syncbase.py
index 4d75f69..d30d69d 100644
--- a/pym/portage/sync/syncbase.py
+++ b/pym/portage/sync/syncbase.py
@@ -24,11 +24,6 @@ class SyncBase(object):
 		return "BlankSync"
 
 
-	@staticmethod
-	def specific_options():
-		return ()
-
-
 	def can_progressbar(self, func):
 		return False
 
-- 
2.4.5


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #3: 0002-Rework-PATCH-v2-2-2-sync-Enable-to-set-rsync-extra-o.patch --]
[-- Type: text/x-patch, Size: 1948 bytes --]

From 5909c1d7e290961eafe6802ff385719e08f4ff5a Mon Sep 17 00:00:00 2001
From: Brian Dolbec <dolsen@gentoo.org>
Date: Mon, 6 Jul 2015 14:53:34 -0700
Subject: [PATCH 2/2] Rework [PATCH v2 2/2] sync: Enable to set rsync extra
 opts per repository

Update for changes made in previous commit

Signed-off-by: Brian Dolbec <dolsen@gentoo.org>
---
 pym/portage/sync/modules/rsync/__init__.py |  1 +
 pym/portage/sync/modules/rsync/rsync.py    | 10 +++-------
 2 files changed, 4 insertions(+), 7 deletions(-)

diff --git a/pym/portage/sync/modules/rsync/__init__.py b/pym/portage/sync/modules/rsync/__init__.py
index 9adc4c8..c717cd6 100644
--- a/pym/portage/sync/modules/rsync/__init__.py
+++ b/pym/portage/sync/modules/rsync/__init__.py
@@ -23,6 +23,7 @@ module_spec = {
 				'exists': 'Returns a boolean if the specified directory exists',
 				},
 			'validate_config': CheckSyncConfig,
+			'module_specific_options': ("extra-opts",),
 			}
 		}
 	}
diff --git a/pym/portage/sync/modules/rsync/rsync.py b/pym/portage/sync/modules/rsync/rsync.py
index 2237901..27e60eb 100644
--- a/pym/portage/sync/modules/rsync/rsync.py
+++ b/pym/portage/sync/modules/rsync/rsync.py
@@ -44,11 +44,6 @@ class RsyncSync(NewBase):
 		return "RsyncSync"
 
 
-	@staticmethod
-	def specific_options():
-		return ("extra-opts",)
-
-
 	def __init__(self):
 		NewBase.__init__(self, "rsync", RSYNC_PACKAGE_ATOM)
 
@@ -78,9 +73,10 @@ class RsyncSync(NewBase):
 		self.rsync_opts = self._rsync_opts_extend(opts, rsync_opts)
 
 		self.extra_rsync_opts = list()
-		if self.repo.sync_rsync_extra_opts is not None:
+		extra_opts = self.repo.module_specific_options['sync_rsync_extra_opts']
+		if extra_opts is not None:
 			self.extra_rsync_opts.extend(portage.util.shlex_split(
-				self.repo.sync_rsync_extra_opts))
+				extra_opts))
 
 		# Real local timestamp file.
 		self.servertimestampfile = os.path.join(
-- 
2.4.5