Re: [gentoo-portage-dev] [PATCH] repos.conf: support sync-hooks-lazy attribute (bug 565172)

[Brian Dolbec]

On Sun,  8 Nov 2015 15:20:01 -0800
Zac Medico <zmedico@g.o> wrote:

> If sync-hooks-lazy is set to true, do not trigger postsync hooks
> unless hooks would have executed for a master repository or the
> repository has changed since the previous sync operation.
>
> If the user has not explicitly enabled sync-hooks-lazy in repos.conf,
> then execute all hooks regardless of whether or not anything has
> changed (for backward compatibility).
>
> X-Gentoo-Bug: 565172
> X-Gentoo-Bug-URL: https://bugs.gentoo.org/show_bug.cgi?id=565172
> ---
>  man/portage.5                           |  7 +++-
>  pym/portage/emaint/modules/sync/sync.py | 57
> +++++++++++++++++++++++++++------
> pym/portage/repository/config.py        |  6 ++--
> pym/portage/sync/controller.py          | 17 ++++++---- 4 files
> changed, 69 insertions(+), 18 deletions(-)
>
> diff --git a/man/portage.5 b/man/portage.5
> index 8e2be4f..9ddbee8 100644
> --- a/man/portage.5
> +++ b/man/portage.5
> @@ -1,4 +1,4 @@
> -.TH "PORTAGE" "5" "Feb 2015" "Portage VERSION" "Portage"
> +.TH "PORTAGE" "5" "Nov 2015" "Portage VERSION" "Portage"
>  .SH NAME
>  portage \- the heart of Gentoo
>  .SH "DESCRIPTION"
> @@ -968,6 +968,11 @@ Specifies CVS repository.
>  Specifies clone depth to use for DVCS repositories. Defaults to 1
> (only the newest commit). If set to 0, the depth is unlimited.
>  .TP
> +.B sync\-hooks\-lazy
> +If set to true, then sync of a given repository will not trigger
> postsync +hooks unless hooks would have executed for a master
> repository or the +repository has changed since the previous sync
> operation. +.TP
>  .B sync\-type
>  Specifies type of synchronization performed by `emerge \-\-sync`.
>  .br
> diff --git a/pym/portage/emaint/modules/sync/sync.py
> b/pym/portage/emaint/modules/sync/sync.py index 57c779d..15d63e2
> 100644 --- a/pym/portage/emaint/modules/sync/sync.py
> +++ b/pym/portage/emaint/modules/sync/sync.py
> @@ -233,15 +233,17 @@ class SyncRepos(object):
>  		retvals = sync_scheduler.retvals
>  		msgs.extend(sync_scheduler.msgs)
>
> -		# run the post_sync_hook one last time for
> -		# run only at sync completion hooks
> -		rcode = sync_manager.perform_post_sync_hook('')
>  		if retvals:
>  			msgs.extend(self.rmessage(retvals, 'sync'))
>  		else:
>  			msgs.extend(self.rmessage([('None',
> os.EX_OK)], 'sync'))
> -		if rcode:
> -			msgs.extend(self.rmessage([('None', rcode)],
> 'post-sync')) +
> +		# run the post_sync_hook one last time for
> +		# run only at sync completion hooks
> +		if sync_scheduler.global_hooks_enabled:
> +			rcode =
> sync_manager.perform_post_sync_hook('')
> +			if rcode:
> +				msgs.extend(self.rmessage([('None',
> rcode)], 'post-sync'))
>  		# Reload the whole config.
>  		portage._sync_mode = False
> @@ -339,6 +341,8 @@ class SyncScheduler(AsyncScheduler):
>  				if master.name in
> selected_repo_names: self._repo_map[master.name] = master
>  					self._sync_graph.add(master.name,
> repo.name)
> +		self._complete_graph = self._sync_graph.copy()
> +		self._hooks_repos = set()
>  		self._update_leaf_nodes()
>
>  	def _task_exit(self, task):
> @@ -347,9 +351,13 @@ class SyncScheduler(AsyncScheduler):
>  		more leaf nodes.
>  		'''
>  		self._running_tasks.discard(task)
> +		# Set hooks_enabled = True by default, in order to
> ensure
> +		# that hooks will be called in a backward-compatible
> manner
> +		# even if all sync tasks have failed.
> +		hooks_enabled = True
>  		returncode = task.returncode
>  		if task.returncode == os.EX_OK:
> -			returncode, message, updatecache_flg =
> task.result
> +			returncode, message, updatecache_flg,
> hooks_enabled = task.result if message:
>  				self.msgs.append(message)
>  		repo = task.kwargs['repo'].name
> @@ -357,8 +365,38 @@ class SyncScheduler(AsyncScheduler):
>  		self.retvals.append((repo, returncode))
>  		self._sync_graph.remove(repo)
>  		self._update_leaf_nodes()
> +		if hooks_enabled:
> +			self._hooks_repos.add(repo)
>  		super(SyncScheduler, self)._task_exit(self)
>
> +	def _master_hooks(self, repo_name):
> +		"""
> +		@param repo_name: a repo name
> +		@type repo_name: str
> +		@return: True if hooks would have been executed for
> any master
> +			repositories of the given repo, False
> otherwise
> +		@rtype: bool
> +		"""
> +		traversed_nodes = set()
> +		node_stack = [repo_name]
> +		while node_stack:
> +			node = node_stack.pop()
> +			if node in self._hooks_repos:
> +				return True
> +			if node not in traversed_nodes:
> +				traversed_nodes.add(node)
> +
> node_stack.extend(self._complete_graph.child_nodes(node))
> +		return False
> +
> +	@property
> +	def global_hooks_enabled(self):
> +		"""
> +		@return: True if repo.postsync.d hooks would have
> been executed
> +			for any repositories.
> +		@rtype: bool
> +		"""
> +		return bool(self._hooks_repos)
> +
>  	def _update_leaf_nodes(self):
>  		'''
>  		Populate self._leaf_nodes with current leaves from
> @@ -389,9 +427,10 @@ class SyncScheduler(AsyncScheduler):
>  		self._running_repos.add(node)
>  		self._update_leaf_nodes()
>
> -		task = self._sync_manager.async(
> -			self._emerge_config, self._repo_map[node])
> -		return task
> +		return self._sync_manager.async(
> +			emerge_config=self._emerge_config,
> +			repo=self._repo_map[node],
> +			master_hooks=self._master_hooks(node))
>
>  	def _can_add_job(self):
>  		'''
> diff --git a/pym/portage/repository/config.py
> b/pym/portage/repository/config.py index 1060bc7..c7a1a1a 100644
> --- a/pym/portage/repository/config.py
> +++ b/pym/portage/repository/config.py
> @@ -87,7 +87,7 @@ class RepoConfig(object):
>  		'main_repo', 'manifest_hashes', 'masters',
> 'missing_repo_name', 'name', 'portage1_profiles',
> 'portage1_profiles_compat', 'priority', 'profile_formats',
> 'sign_commit', 'sign_manifest',
> -		'sync_depth',
> +		'sync_depth', 'sync_hooks_lazy',
>  		'sync_type', 'sync_umask', 'sync_uri', 'sync_user',
> 'thin_manifest', 'update_changelog', 'user_location', '_eapis_banned',
>  		'_eapis_deprecated', '_masters_orig',
> 'module_specific_options', @@ -175,6 +175,8 @@ class
> RepoConfig(object): self.auto_sync = auto_sync
>
>  		self.sync_depth = repo_opts.get('sync-depth')
> +		self.sync_hooks_lazy =
> repo_opts.get('sync-hooks-lazy',
> +			'false').lower() == 'true'
>
>  		self.module_specific_options = {}
>
> @@ -506,7 +508,7 @@ class RepoConfigLoader(object):
>  						# repos.conf is
> allowed to override. for k in ('aliases', 'auto_sync',
> 'eclass_overrides', 'force', 'masters', 'priority',
> -							'sync_depth',
> +							'sync_depth',
> 'sync_hooks_lazy', 'sync_type', 'sync_umask', 'sync_uri', 'sync_user',
>  							'module_specific_options'):
>  							v =
> getattr(repos_conf_opts, k, None) diff --git
> a/pym/portage/sync/controller.py b/pym/portage/sync/controller.py
> index e8132c2..57add91 100644 --- a/pym/portage/sync/controller.py
> +++ b/pym/portage/sync/controller.py
> @@ -114,16 +114,17 @@ class SyncManager(object):
>  			return desc
>  		return []
>
> -	def async(self, emerge_config=None, repo=None):
> +	def async(self, emerge_config=None, repo=None,
> master_hooks=True): self.emerge_config = emerge_config
>  		self.settings, self.trees, self.mtimedb =
> emerge_config self.xterm_titles = "notitles" not in
> self.settings.features self.portdb =
> self.trees[self.settings['EROOT']]['porttree'].dbapi return
> SyncRepo(sync_task=AsyncFunction(target=self.sync,
> -			kwargs=dict(emerge_config=emerge_config,
> repo=repo)),
> +			kwargs=dict(emerge_config=emerge_config,
> repo=repo,
> +			master_hooks=master_hooks)),
>  			sync_callback=self._sync_callback)
>
> -	def sync(self, emerge_config=None, repo=None):
> +	def sync(self, emerge_config=None, repo=None,
> master_hooks=True): self.callback = None
>  		self.repo = repo
>  		self.exitcode = 1
> @@ -156,9 +157,13 @@ class SyncManager(object):
>  		taskmaster = TaskHandler(callback=self.do_callback)
>  		taskmaster.run_tasks(tasks, func, status,
> options=task_opts)
> -		self.perform_post_sync_hook(repo.name,
> repo.sync_uri, repo.location)
> +		hooks_enabled = False
> +		if master_hooks or not repo.sync_hooks_lazy or
> self.updatecache_flg:
> +			hooks_enabled = True
> +			self.perform_post_sync_hook(
> +				repo.name, repo.sync_uri,
> repo.location)
> -		return self.exitcode, None, self.updatecache_flg
> +		return self.exitcode, None, self.updatecache_flg,
> hooks_enabled
>
>  	def do_callback(self, result):
> @@ -328,7 +333,7 @@ class SyncManager(object):
>  		exitcode = proc.returncode
>  		updatecache_flg = False
>  		if proc.returncode == os.EX_OK:
> -			exitcode, message, updatecache_flg =
> proc.result
> +			exitcode, message, updatecache_flg,
> hooks_enabled = proc.result
>  		if updatecache_flg and "metadata-transfer" not in
> self.settings.features: updatecache_flg = False


The code changes look good to me :)

-- 
Brian Dolbec <dolsen>