[gentoo-portage-dev] [PATCH v2] repos.conf: support sync-hooks-only-on-change attribute (bug 565172)

[Zac Medico]

If sync-hooks-only-on-change 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-only-on-change 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
---
[PATCH v2] renames sync-hooks-lazy to sync-hooks-only-on-change

 man/portage.5                           |  7 +++-
 pym/portage/emaint/modules/sync/sync.py | 57 +++++++++++++++++++++++++++------
 pym/portage/repository/config.py        |  6 ++--
 pym/portage/sync/controller.py          | 18 +++++++----
 4 files changed, 70 insertions(+), 18 deletions(-)

diff --git a/man/portage.5 b/man/portage.5
index 8e2be4f..c9e70a0 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\-only\-on\-change
+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..ffb4544 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_only_on_change',
 		'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_only_on_change = repo_opts.get(
+			'sync-hooks-only-on-change', '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_only_on_change',
 							'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..4595293 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,14 @@ 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 self.updatecache_flg or
+			not repo.sync_hooks_only_on_change):
+			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 +334,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
-- 
2.4.9