[gentoo-portage-dev] [PATCH] postsync: skip hooks and metadata-transfer when appropriate (bug 564988)

[gentoo-portage-dev] [PATCH] postsync: skip hooks and metadata-transfer when appropriate (bug 564988)

[Zac Medico]

Fix flaws in logic involving the updatecache_flg variable. Use this
variable to skip hooks and metadata-transfer when sync fails (or the
server timestamp has not changed).

X-Gentoo-Bug: 564988
X-Gentoo-Bug-URL: https://bugs.gentoo.org/show_bug.cgi?id=564988
---
 pym/portage/emaint/modules/sync/sync.py | 15 ++++++++++-----
 pym/portage/sync/controller.py          |  3 ++-
 pym/portage/sync/modules/rsync/rsync.py | 17 ++++++++++-------
 3 files changed, 22 insertions(+), 13 deletions(-)

diff --git a/pym/portage/emaint/modules/sync/sync.py b/pym/portage/emaint/modules/sync/sync.py
index 57c779d..faa20d9 100644
--- a/pym/portage/emaint/modules/sync/sync.py
+++ b/pym/portage/emaint/modules/sync/sync.py
@@ -232,16 +232,19 @@ class SyncRepos(object):
 		sync_scheduler.wait()
 		retvals = sync_scheduler.retvals
 		msgs.extend(sync_scheduler.msgs)
+		updatecache_flg = sync_scheduler.updatecache_flg
 
-		# 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'))
+
+		if updatecache_flg:
+			# run the post_sync_hook one last time for
+			# run only at sync completion hooks
+			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
@@ -321,6 +324,7 @@ class SyncScheduler(AsyncScheduler):
 		self._init_graph()
 		self.retvals = []
 		self.msgs = []
+		self.updatecache_flg = False
 
 	def _init_graph(self):
 		'''
@@ -350,6 +354,7 @@ class SyncScheduler(AsyncScheduler):
 		returncode = task.returncode
 		if task.returncode == os.EX_OK:
 			returncode, message, updatecache_flg = task.result
+			self.updatecache_flg |= updatecache_flg
 			if message:
 				self.msgs.append(message)
 		repo = task.kwargs['repo'].name
diff --git a/pym/portage/sync/controller.py b/pym/portage/sync/controller.py
index e8132c2..1beb545 100644
--- a/pym/portage/sync/controller.py
+++ b/pym/portage/sync/controller.py
@@ -156,7 +156,8 @@ 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)
+		if self.updatecache_flg:
+			self.perform_post_sync_hook(repo.name, repo.sync_uri, repo.location)
 
 		return self.exitcode, None, self.updatecache_flg
 
diff --git a/pym/portage/sync/modules/rsync/rsync.py b/pym/portage/sync/modules/rsync/rsync.py
index 8ae8a5c..e0f76b3 100644
--- a/pym/portage/sync/modules/rsync/rsync.py
+++ b/pym/portage/sync/modules/rsync/rsync.py
@@ -112,10 +112,10 @@ class RsyncSync(NewBase):
 		if syncuri.startswith("file://"):
 			self.proto = "file"
 			dosyncuri = syncuri[6:]
-			is_synced, exitcode = self._do_rsync(
+			is_synced, exitcode, updatecache_flg = self._do_rsync(
 				dosyncuri, timestamp, opts)
 			self._process_exitcode(exitcode, dosyncuri, out, 1)
-			return (exitcode, exitcode == os.EX_OK)
+			return (exitcode, updatecache_flg)
 
 		retries=0
 		try:
@@ -138,7 +138,7 @@ class RsyncSync(NewBase):
 		else:
 			# getaddrinfo needs the brackets stripped
 			getaddrinfo_host = hostname[1:-1]
-		updatecache_flg=True
+		updatecache_flg = False
 		all_rsync_opts = set(self.rsync_opts)
 		all_rsync_opts.update(self.extra_rsync_opts)
 
@@ -240,7 +240,8 @@ class RsyncSync(NewBase):
 			if dosyncuri.startswith('ssh://'):
 				dosyncuri = dosyncuri[6:].replace('/', ':/', 1)
 
-			is_synced, exitcode = self._do_rsync(dosyncuri, timestamp, opts)
+			is_synced, exitcode, updatecache_flg = self._do_rsync(
+				dosyncuri, timestamp, opts)
 			if is_synced:
 				break
 
@@ -251,7 +252,6 @@ class RsyncSync(NewBase):
 			else:
 				# over retries
 				# exit loop
-				updatecache_flg=False
 				exitcode = EXCEEDED_MAX_RETRIES
 				break
 		self._process_exitcode(exitcode, dosyncuri, out, maxretries)
@@ -382,6 +382,7 @@ class RsyncSync(NewBase):
 
 
 	def _do_rsync(self, syncuri, timestamp, opts):
+		updatecache_flg = False
 		is_synced = False
 		if timestamp != 0 and "--quiet" not in opts:
 			print(">>> Checking server timestamp ...")
@@ -489,7 +490,7 @@ class RsyncSync(NewBase):
 				print(">>> In order to force sync, remove '%s'." % self.servertimestampfile)
 				print(">>>")
 				print()
-				return is_synced, exitcode
+				return is_synced, exitcode, updatecache_flg
 			elif (servertimestamp != 0) and (servertimestamp < timestamp):
 				self.logger(self.xterm_titles,
 					">>> Server out of date: %s" % syncuri)
@@ -543,6 +544,8 @@ class RsyncSync(NewBase):
 							os.unlink(self.servertimestampfile)
 						except OSError:
 							pass
+					else:
+						updatecache_flg = True
 
 				if exitcode in [0,1,3,4,11,14,20,21]:
 					is_synced = True
@@ -554,4 +557,4 @@ class RsyncSync(NewBase):
 			# --prune-empty-directories.  Retry for a server that supports
 			# at least rsync protocol version 29 (>=rsync-2.6.4).
 			pass
-		return is_synced, exitcode
+		return is_synced, exitcode, updatecache_flg
-- 
2.4.9

Re: [gentoo-portage-dev] [PATCH] postsync: skip hooks and metadata-transfer when appropriate (bug 564988)

[Alexander Berntsen]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA512

On 06/11/15 08:44, Zac Medico wrote:
> Fix flaws in logic involving the updatecache_flg variable. Use this
> variable to skip hooks and metadata-transfer when sync fails (or the
> server timestamp has not changed).
> 
> X-Gentoo-Bug: 564988
> X-Gentoo-Bug-URL: https://bugs.gentoo.org/show_bug.cgi?id=564988
> ---
>  pym/portage/emaint/modules/sync/sync.py | 15 ++++++++++-----
>  pym/portage/sync/controller.py          |  3 ++-
>  pym/portage/sync/modules/rsync/rsync.py | 17 ++++++++++-------
>  3 files changed, 22 insertions(+), 13 deletions(-)
> 
> diff --git a/pym/portage/emaint/modules/sync/sync.py b/pym/portage/emaint/modules/sync/sync.py
> index 57c779d..faa20d9 100644
> --- a/pym/portage/emaint/modules/sync/sync.py
> +++ b/pym/portage/emaint/modules/sync/sync.py
> @@ -232,16 +232,19 @@ class SyncRepos(object):
>  		sync_scheduler.wait()
>  		retvals = sync_scheduler.retvals
>  		msgs.extend(sync_scheduler.msgs)
> +		updatecache_flg = sync_scheduler.updatecache_flg
>  
> -		# 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'))
> +
> +		if updatecache_flg:
> +			# run the post_sync_hook one last time for
> +			# run only at sync completion hooks
> +			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
> @@ -321,6 +324,7 @@ class SyncScheduler(AsyncScheduler):
>  		self._init_graph()
>  		self.retvals = []
>  		self.msgs = []
> +		self.updatecache_flg = False
>  
>  	def _init_graph(self):
>  		'''
> @@ -350,6 +354,7 @@ class SyncScheduler(AsyncScheduler):
>  		returncode = task.returncode
>  		if task.returncode == os.EX_OK:
>  			returncode, message, updatecache_flg = task.result
> +			self.updatecache_flg |= updatecache_flg
>  			if message:
>  				self.msgs.append(message)
>  		repo = task.kwargs['repo'].name
> diff --git a/pym/portage/sync/controller.py b/pym/portage/sync/controller.py
> index e8132c2..1beb545 100644
> --- a/pym/portage/sync/controller.py
> +++ b/pym/portage/sync/controller.py
> @@ -156,7 +156,8 @@ 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)
> +		if self.updatecache_flg:
> +			self.perform_post_sync_hook(repo.name, repo.sync_uri, repo.location)
>  
>  		return self.exitcode, None, self.updatecache_flg
>  
> diff --git a/pym/portage/sync/modules/rsync/rsync.py b/pym/portage/sync/modules/rsync/rsync.py
> index 8ae8a5c..e0f76b3 100644
> --- a/pym/portage/sync/modules/rsync/rsync.py
> +++ b/pym/portage/sync/modules/rsync/rsync.py
> @@ -112,10 +112,10 @@ class RsyncSync(NewBase):
>  		if syncuri.startswith("file://"):
>  			self.proto = "file"
>  			dosyncuri = syncuri[6:]
> -			is_synced, exitcode = self._do_rsync(
> +			is_synced, exitcode, updatecache_flg = self._do_rsync(
>  				dosyncuri, timestamp, opts)
>  			self._process_exitcode(exitcode, dosyncuri, out, 1)
> -			return (exitcode, exitcode == os.EX_OK)
> +			return (exitcode, updatecache_flg)
>  
>  		retries=0
>  		try:
> @@ -138,7 +138,7 @@ class RsyncSync(NewBase):
>  		else:
>  			# getaddrinfo needs the brackets stripped
>  			getaddrinfo_host = hostname[1:-1]
> -		updatecache_flg=True
> +		updatecache_flg = False
>  		all_rsync_opts = set(self.rsync_opts)
>  		all_rsync_opts.update(self.extra_rsync_opts)
>  
> @@ -240,7 +240,8 @@ class RsyncSync(NewBase):
>  			if dosyncuri.startswith('ssh://'):
>  				dosyncuri = dosyncuri[6:].replace('/', ':/', 1)
>  
> -			is_synced, exitcode = self._do_rsync(dosyncuri, timestamp, opts)
> +			is_synced, exitcode, updatecache_flg = self._do_rsync(
> +				dosyncuri, timestamp, opts)
>  			if is_synced:
>  				break
>  
> @@ -251,7 +252,6 @@ class RsyncSync(NewBase):
>  			else:
>  				# over retries
>  				# exit loop
> -				updatecache_flg=False
>  				exitcode = EXCEEDED_MAX_RETRIES
>  				break
>  		self._process_exitcode(exitcode, dosyncuri, out, maxretries)
> @@ -382,6 +382,7 @@ class RsyncSync(NewBase):
>  
>  
>  	def _do_rsync(self, syncuri, timestamp, opts):
> +		updatecache_flg = False
>  		is_synced = False
>  		if timestamp != 0 and "--quiet" not in opts:
>  			print(">>> Checking server timestamp ...")
> @@ -489,7 +490,7 @@ class RsyncSync(NewBase):
>  				print(">>> In order to force sync, remove '%s'." % self.servertimestampfile)
>  				print(">>>")
>  				print()
> -				return is_synced, exitcode
> +				return is_synced, exitcode, updatecache_flg
>  			elif (servertimestamp != 0) and (servertimestamp < timestamp):
>  				self.logger(self.xterm_titles,
>  					">>> Server out of date: %s" % syncuri)
> @@ -543,6 +544,8 @@ class RsyncSync(NewBase):
>  							os.unlink(self.servertimestampfile)
>  						except OSError:
>  							pass
> +					else:
> +						updatecache_flg = True
>  
>  				if exitcode in [0,1,3,4,11,14,20,21]:
>  					is_synced = True
> @@ -554,4 +557,4 @@ class RsyncSync(NewBase):
>  			# --prune-empty-directories.  Retry for a server that supports
>  			# at least rsync protocol version 29 (>=rsync-2.6.4).
>  			pass
> -		return is_synced, exitcode
> +		return is_synced, exitcode, updatecache_flg
> 
I know nothing about the egencache stuff. Maybe Michał can comment?

- -- 
Alexander
bernalex@gentoo.org
https://secure.plaimi.net/~alexander
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2

iQIcBAEBCgAGBQJWPFvCAAoJENQqWdRUGk8Bv9IP/2SZHOXOycQkX46/cJT9ELr/
3yofUPa9fwwUFYOaM/QaTTKdAbJBy9UnTbGl3ho500p9xzB5uUq2+8OZQ9f9Nsdn
8hKDDdm0LGUCnGW+QmzsT0Mqa0fRaG9WUTF4ysUQ818mLeq02MedjLzxDOibyz4/
B+ExdkyhiQs9x0pfIQl7IezMvS5qZYwtMqTlX4Vk1dOENPHTSpkCfiAU576qfZK7
GKxaDQ9zI8cN2Twjl588JU9e5feA+/SOaMCcESFIp5Q4/rh0vDcKr3CQfcjJEVkO
keOVT87+evVAwc1bCXqE30Q+ezbYmrXvFN95bkxqw55jmWJA4daywx29p8S2NWHF
qO5etd3KEkaskbBlsvr3i47YRX7raFVtuIlxTAcsJi801fCTqq+iC32Ex1BY6hdW
HDUjrZBjWljhsAxhrVE9H6QnBKg344rpj9ELMiJeD+T173lfqxYiRRe+LQJdCN4K
olReMMmWzCILqq+mW+BOKn749q8rXAsa4bPv0tIGb0WZ7AgGdd2bOxQF0T3fhshJ
eO0yzn/vsfSE4KmjbKoqzqwksVh9UOhY1p7j0QO9RZEfgNNiwJcAotQ4yPRHtdxh
NtYfZBsrFcVFn2PY87hr38mxxb21RX7ls6SD816b6aYh1Tc0NvX+kJyf98xzchd/
4sHH3ibM9UwBVRbT4ItL
=BMQo
-----END PGP SIGNATURE-----

Re: [gentoo-portage-dev] [PATCH] postsync: skip hooks and metadata-transfer when appropriate (bug 564988)

[Michał Górny]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA512



Dnia 6 listopada 2015 08:50:26 CET, Alexander Berntsen <bernalex@gentoo.org> napisał(a):
>-----BEGIN PGP SIGNED MESSAGE-----
>Hash: SHA512
>
>On 06/11/15 08:44, Zac Medico wrote:
>> Fix flaws in logic involving the updatecache_flg variable. Use this
>> variable to skip hooks and metadata-transfer when sync fails (or the
>> server timestamp has not changed).
>>
>> X-Gentoo-Bug: 564988
>> X-Gentoo-Bug-URL: https://bugs.gentoo.org/show_bug.cgi?id=564988
>> ---
>>  pym/portage/emaint/modules/sync/sync.py | 15 ++++++++++-----
>>  pym/portage/sync/controller.py          |  3 ++-
>>  pym/portage/sync/modules/rsync/rsync.py | 17 ++++++++++-------
>>  3 files changed, 22 insertions(+), 13 deletions(-)
>>
>> diff --git a/pym/portage/emaint/modules/sync/sync.py
>b/pym/portage/emaint/modules/sync/sync.py
>> index 57c779d..faa20d9 100644
>> --- a/pym/portage/emaint/modules/sync/sync.py
>> +++ b/pym/portage/emaint/modules/sync/sync.py
>> @@ -232,16 +232,19 @@ class SyncRepos(object):
>>  		sync_scheduler.wait()
>>  		retvals = sync_scheduler.retvals
>>  		msgs.extend(sync_scheduler.msgs)
>> +		updatecache_flg = sync_scheduler.updatecache_flg
>>
>> -		# 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'))
>> +
>> +		if updatecache_flg:
>> +			# run the post_sync_hook one last time for
>> +			# run only at sync completion hooks
>> +			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
>> @@ -321,6 +324,7 @@ class SyncScheduler(AsyncScheduler):
>>  		self._init_graph()
>>  		self.retvals = []
>>  		self.msgs = []
>> +		self.updatecache_flg = False
>>
>>  	def _init_graph(self):
>>  		'''
>> @@ -350,6 +354,7 @@ class SyncScheduler(AsyncScheduler):
>>  		returncode = task.returncode
>>  		if task.returncode == os.EX_OK:
>>  			returncode, message, updatecache_flg = task.result
>> +			self.updatecache_flg |= updatecache_flg
>>  			if message:
>>  				self.msgs.append(message)
>>  		repo = task.kwargs['repo'].name
>> diff --git a/pym/portage/sync/controller.py
>b/pym/portage/sync/controller.py
>> index e8132c2..1beb545 100644
>> --- a/pym/portage/sync/controller.py
>> +++ b/pym/portage/sync/controller.py
>> @@ -156,7 +156,8 @@ 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)
>> +		if self.updatecache_flg:
>> +			self.perform_post_sync_hook(repo.name, repo.sync_uri,
>repo.location)
>>
>>  		return self.exitcode, None, self.updatecache_flg
>>
>> diff --git a/pym/portage/sync/modules/rsync/rsync.py
>b/pym/portage/sync/modules/rsync/rsync.py
>> index 8ae8a5c..e0f76b3 100644
>> --- a/pym/portage/sync/modules/rsync/rsync.py
>> +++ b/pym/portage/sync/modules/rsync/rsync.py
>> @@ -112,10 +112,10 @@ class RsyncSync(NewBase):
>>  		if syncuri.startswith("file://"):
>>  			self.proto = "file"
>>  			dosyncuri = syncuri[6:]
>> -			is_synced, exitcode = self._do_rsync(
>> +			is_synced, exitcode, updatecache_flg = self._do_rsync(
>>  				dosyncuri, timestamp, opts)
>>  			self._process_exitcode(exitcode, dosyncuri, out, 1)
>> -			return (exitcode, exitcode == os.EX_OK)
>> +			return (exitcode, updatecache_flg)
>>
>>  		retries=0
>>  		try:
>> @@ -138,7 +138,7 @@ class RsyncSync(NewBase):
>>  		else:
>>  			# getaddrinfo needs the brackets stripped
>>  			getaddrinfo_host = hostname[1:-1]
>> -		updatecache_flg=True
>> +		updatecache_flg = False
>>  		all_rsync_opts = set(self.rsync_opts)
>>  		all_rsync_opts.update(self.extra_rsync_opts)
>>
>> @@ -240,7 +240,8 @@ class RsyncSync(NewBase):
>>  			if dosyncuri.startswith('ssh://'):
>>  				dosyncuri = dosyncuri[6:].replace('/', ':/', 1)
>>
>> -			is_synced, exitcode = self._do_rsync(dosyncuri, timestamp, opts)
>> +			is_synced, exitcode, updatecache_flg = self._do_rsync(
>> +				dosyncuri, timestamp, opts)
>>  			if is_synced:
>>  				break
>>
>> @@ -251,7 +252,6 @@ class RsyncSync(NewBase):
>>  			else:
>>  				# over retries
>>  				# exit loop
>> -				updatecache_flg=False
>>  				exitcode = EXCEEDED_MAX_RETRIES
>>  				break
>>  		self._process_exitcode(exitcode, dosyncuri, out, maxretries)
>> @@ -382,6 +382,7 @@ class RsyncSync(NewBase):
>>
>>
>>  	def _do_rsync(self, syncuri, timestamp, opts):
>> +		updatecache_flg = False
>>  		is_synced = False
>>  		if timestamp != 0 and "--quiet" not in opts:
>>  			print(">>> Checking server timestamp ...")
>> @@ -489,7 +490,7 @@ class RsyncSync(NewBase):
>>  				print(">>> In order to force sync, remove '%s'." %
>self.servertimestampfile)
>>  				print(">>>")
>>  				print()
>> -				return is_synced, exitcode
>> +				return is_synced, exitcode, updatecache_flg
>>  			elif (servertimestamp != 0) and (servertimestamp < timestamp):
>>  				self.logger(self.xterm_titles,
>>  					">>> Server out of date: %s" % syncuri)
>> @@ -543,6 +544,8 @@ class RsyncSync(NewBase):
>>  							os.unlink(self.servertimestampfile)
>>  						except OSError:
>>  							pass
>> +					else:
>> +						updatecache_flg = True
>>
>>  				if exitcode in [0,1,3,4,11,14,20,21]:
>>  					is_synced = True
>> @@ -554,4 +557,4 @@ class RsyncSync(NewBase):
>>  			# --prune-empty-directories.  Retry for a server that supports
>>  			# at least rsync protocol version 29 (>=rsync-2.6.4).
>>  			pass
>> -		return is_synced, exitcode
>> +		return is_synced, exitcode, updatecache_flg
>>
>I know nothing about the egencache stuff. Maybe Michał can comment?

Michał finds this black magic. Trusts zmedico.

>
>- --
>Alexander
>bernalex@gentoo.org
>https://secure.plaimi.net/~alexander
>-----BEGIN PGP SIGNATURE-----
>Version: GnuPG v2
>
>iQIcBAEBCgAGBQJWPFvCAAoJENQqWdRUGk8Bv9IP/2SZHOXOycQkX46/cJT9ELr/
>3yofUPa9fwwUFYOaM/QaTTKdAbJBy9UnTbGl3ho500p9xzB5uUq2+8OZQ9f9Nsdn
>8hKDDdm0LGUCnGW+QmzsT0Mqa0fRaG9WUTF4ysUQ818mLeq02MedjLzxDOibyz4/
>B+ExdkyhiQs9x0pfIQl7IezMvS5qZYwtMqTlX4Vk1dOENPHTSpkCfiAU576qfZK7
>GKxaDQ9zI8cN2Twjl588JU9e5feA+/SOaMCcESFIp5Q4/rh0vDcKr3CQfcjJEVkO
>keOVT87+evVAwc1bCXqE30Q+ezbYmrXvFN95bkxqw55jmWJA4daywx29p8S2NWHF
>qO5etd3KEkaskbBlsvr3i47YRX7raFVtuIlxTAcsJi801fCTqq+iC32Ex1BY6hdW
>HDUjrZBjWljhsAxhrVE9H6QnBKg344rpj9ELMiJeD+T173lfqxYiRRe+LQJdCN4K
>olReMMmWzCILqq+mW+BOKn749q8rXAsa4bPv0tIGb0WZ7AgGdd2bOxQF0T3fhshJ
>eO0yzn/vsfSE4KmjbKoqzqwksVh9UOhY1p7j0QO9RZEfgNNiwJcAotQ4yPRHtdxh
>NtYfZBsrFcVFn2PY87hr38mxxb21RX7ls6SD816b6aYh1Tc0NvX+kJyf98xzchd/
>4sHH3ibM9UwBVRbT4ItL
>=BMQo
>-----END PGP SIGNATURE-----

- --
Best regards,
Michał Górny
-----BEGIN PGP SIGNATURE-----

iQJJBAEBCgAzLBxNaWNoYcWCIEfDs3JueSAoR2VudG9vKSA8bWdvcm55QGdlbnRv
by5vcmc+BQJWPF9RAAoJELB6GurvtEZOoGUP/1NyldF5Cknu72Lu2BR3XgIXB9/u
/pVTtRkUjntYDbMWikIZzFTuy95xsdUESW7tbzFGa+O81toivaUkHjrcT3b6Nx4s
O89paUYZkfNRMDCCMJvxwa7da34s4hEy96u+JDoFSLElFOzfb/oQnRxreeWidmGX
g/DbPgY6AkMjNydUb8GtMScNL0fzpw0jhWf9HPaBM9n4CvxAq38T099Q2XyFL1zw
HJi4ZvEafkmTb1TjIcXlAKSVjbExOgmVHtP3V+XFS3A0u3/UhuKIl3xOqQIe27Vc
gVSmpArStwP6pWSKqAMP/sK9n73r+jdm8yfAbsTmzIn0eDj9oVoF5rHE6MuhpvuU
huVqoGNIi8Cs6xF5ZKCpsfsPPgSD+ZRUEpJeMlNoQeGG6QFU5WBlvkXBw/rsMeJ1
4z71tl5ub7tOie7ZHJidloPbmKYyrYF/1Gb2AIr7IczsYP+Qr+yZBic2n37HEGoB
5DLApzhw+/nx+zMFLaPDHsXcHYPtYqvqsnkR210Zy2pQVyY0t9xpTogznRif4N1k
NwrJ/Yk59u83ve5+REyd9Mkahw4eVkS4rwrNwrK0QPPewF7xCT2GlKAipxiSKsiC
hl55BG2GZeMWOUWeH0UMbSmqnOE/myo8V1D8ii1apNLvx75RCJFmwrBgMGR9qe/u
0U4PYlrEGmEJw9HS
=ibNs
-----END PGP SIGNATURE-----

Re: [gentoo-portage-dev] [PATCH] postsync: skip hooks and metadata-transfer when appropriate (bug 564988)

[Alexander Berntsen]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA512

On 06/11/15 09:05, Michał Górny wrote:
>>> I know nothing about the egencache stuff. Maybe Michał can
>>> comment?
> Michał finds this black magic. Trusts zmedico.
I think it looks like it's probably supposed to be reasonable, perhaps.

Maybe Brian can look at it. At least that way we'll have a lot of
people that attempted understanding what's going on.

Maybe we need a "Trusted-by:" line.
- -- 
Alexander
bernalex@gentoo.org
https://secure.plaimi.net/~alexander
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2

iQIbBAEBCgAGBQJWPGLcAAoJENQqWdRUGk8BJUIP9Ry5Nzx02cPpr9Tpdj63V95q
0gnSVsjr63a1ZDscuNFqYe9cq4DXQiMIl1623Rs6+PoDT0vsnjZe5DsBvIOoQIt3
FSv8hn5/DbnheY/ZkaT79lm6NQW9yAyX7M+0ob/po82SuiE1bLW2xhwa9z/iAoR+
wtswMMzf52OaSp0dPi3Xt6YAZ+KhHFz92ihJv0saAtkEhqOduCn/jIX6nxL49+2V
qQkz8jvqdifETVtIC+1WbrjyA25XN/448KqWCnPTXPHPZbOwhrkIioodo19OLSQx
amq4piWb0XtEC/e5+R9sE7eIhhZPdfdbVQlTR+9b9LwF+jouKdJNyrd98EbpUf7l
CqdXDtFy0zmoBhXGqp4QrQ7sKUL+CvtGga5YKZSvKzvgBL1V2if7xpQu1JG+hmkV
mTkhwE8X5OclYbKs676zOXXyV5X9QyEZgqar6PIaHlPmnQqm8EpoV6AXowpefyaK
Fo72mHwb4kyfmTdyB9a3UcfCm9CGNggKp4m2pZ9WFpHmkh5RiOWXCZUOb0jw4WY0
5YjIsw7sOd2IhiZ8DkJaB1bkPiFeZ0Aw/ruw5U6vR+xObo5RAjnfN1Zs+tiYLxkd
0OUIPDdtspgGvyI6WuSIYCN0EF24gwI0HY+Ug2qNzzANfAjcBWSV+ReoVW15foMO
ywKSvuKNtDHK5aEhERw=
=4raC
-----END PGP SIGNATURE-----

Re: [gentoo-portage-dev] [PATCH] postsync: skip hooks and metadata-transfer when appropriate (bug 564988)

[Zac Medico]

On 11/06/2015 12:20 AM, Alexander Berntsen wrote:
> On 06/11/15 09:05, Michał Górny wrote:
>>>> I know nothing about the egencache stuff. Maybe Michał can
>>>> comment?
>> Michał finds this black magic. Trusts zmedico.
> I think it looks like it's probably supposed to be reasonable, perhaps.
> 
> Maybe Brian can look at it. At least that way we'll have a lot of
> people that attempted understanding what's going on.
> 
> Maybe we need a "Trusted-by:" line.
> 

Maybe it helps if I give some more context. At my workplace, we have
lots of scripts that call `emerge --sync private-work-repo` to ensure
that the current system has the latest changes from private-work-repo.
It can be annoying if it spends the bulk of its time calling hooks, even
though private-work-repo was already up-to-date:

>>> Timestamps on the server and in the local repository are the same.
>>> Cancelling all further sync action. You are already up to date.

So, we want to skip the hooks when repos are already up-to-date. In this
case, there's no point in calling hooks or updating the metadata cache.
-- 
Thanks,
Zac

Re: [gentoo-portage-dev] [PATCH] postsync: skip hooks and metadata-transfer when appropriate (bug 564988)

[Michał Górny]

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

On Fri, 6 Nov 2015 09:24:15 -0800
Zac Medico <zmedico@gentoo.org> wrote:

> On 11/06/2015 12:20 AM, Alexander Berntsen wrote:
> > On 06/11/15 09:05, Michał Górny wrote:  
> >>>> I know nothing about the egencache stuff. Maybe Michał can
> >>>> comment?  
> >> Michał finds this black magic. Trusts zmedico.  
> > I think it looks like it's probably supposed to be reasonable, perhaps.
> > 
> > Maybe Brian can look at it. At least that way we'll have a lot of
> > people that attempted understanding what's going on.
> > 
> > Maybe we need a "Trusted-by:" line.
> >   
> 
> Maybe it helps if I give some more context. At my workplace, we have
> lots of scripts that call `emerge --sync private-work-repo` to ensure
> that the current system has the latest changes from private-work-repo.
> It can be annoying if it spends the bulk of its time calling hooks, even
> though private-work-repo was already up-to-date:
> 
> >>> Timestamps on the server and in the local repository are the same.
> >>> Cancelling all further sync action. You are already up to date.  
> 
> So, we want to skip the hooks when repos are already up-to-date. In this
> case, there's no point in calling hooks or updating the metadata cache.

This is incorrect assumption. A change in master repo may trigger
metadata cache update in slaved repo.

-- 
Best regards,
Michał Górny
<http://dev.gentoo.org/~mgorny/>

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 949 bytes --]

Re: [gentoo-portage-dev] [PATCH] postsync: skip hooks and metadata-transfer when appropriate (bug 564988)

[Brian Dolbec]

On Fri, 6 Nov 2015 09:20:45 +0100
Alexander Berntsen <bernalex@gentoo.org> wrote:

> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA512
> 
> On 06/11/15 09:05, Michał Górny wrote:
> >>> I know nothing about the egencache stuff. Maybe Michał can
> >>> comment?  
> > Michał finds this black magic. Trusts zmedico.  
> I think it looks like it's probably supposed to be reasonable,
> perhaps.
> 
> Maybe Brian can look at it. At least that way we'll have a lot of
> people that attempted understanding what's going on.
> 
> Maybe we need a "Trusted-by:" line.
> - -- 
> Alexander
> bernalex@gentoo.org


Well, the changes look pretty straight forward.  I thought we had
handled not updating the caches/ running postsync hooks if it didn't
sync, but Zac obviously found one that slipped through.

Seeing the patch only doesn't give an entirely clear picture of the
changes.   For that you would have to read the code and the changes in
the code to figure them all out.  But I too trust Zac, and I don't
feel this is complex and critical enough to warrant the time needed to
study all the changes.  I know he's tested the code ;)

But I thought I saw a typo when you used BOOLEAN1 |= BOOLEAN2.  I'd
never seen the | used like that.  It is nearly impossible to find any
docs/examples of it.  But I did find it as a bitwise or on integers
which boolean is a special form of.  Interesting that it is equivalent
to a = max(a,b) 



-- 
Brian Dolbec <dolsen>

Re: [gentoo-portage-dev] [PATCH] postsync: skip hooks and metadata-transfer when appropriate (bug 564988)

[Zac Medico]

On 11/06/2015 10:39 AM, Michał Górny wrote:
> On Fri, 6 Nov 2015 09:24:15 -0800
> Zac Medico <zmedico@gentoo.org> wrote:
> 
>> On 11/06/2015 12:20 AM, Alexander Berntsen wrote:
>>> On 06/11/15 09:05, Michał Górny wrote:  
>>>>>> I know nothing about the egencache stuff. Maybe Michał can
>>>>>> comment?  
>>>> Michał finds this black magic. Trusts zmedico.  
>>> I think it looks like it's probably supposed to be reasonable, perhaps.
>>>
>>> Maybe Brian can look at it. At least that way we'll have a lot of
>>> people that attempted understanding what's going on.
>>>
>>> Maybe we need a "Trusted-by:" line.
>>>   
>>
>> Maybe it helps if I give some more context. At my workplace, we have
>> lots of scripts that call `emerge --sync private-work-repo` to ensure
>> that the current system has the latest changes from private-work-repo.
>> It can be annoying if it spends the bulk of its time calling hooks, even
>> though private-work-repo was already up-to-date:
>>
>>>>> Timestamps on the server and in the local repository are the same.
>>>>> Cancelling all further sync action. You are already up to date.  
>>
>> So, we want to skip the hooks when repos are already up-to-date. In this
>> case, there's no point in calling hooks or updating the metadata cache.
> 
> This is incorrect assumption. A change in master repo may trigger
> metadata cache update in slaved repo.
> 

Good point. I'll update it to account for this.
-- 
Thanks,
Zac

Re: [gentoo-portage-dev] [PATCH] postsync: skip hooks and metadata-transfer when appropriate (bug 564988)

[Michał Górny]

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

On Fri, 6 Nov 2015 10:58:23 -0800
Zac Medico <zmedico@gentoo.org> wrote:

> On 11/06/2015 10:39 AM, Michał Górny wrote:
> > On Fri, 6 Nov 2015 09:24:15 -0800
> > Zac Medico <zmedico@gentoo.org> wrote:
> >   
> >> On 11/06/2015 12:20 AM, Alexander Berntsen wrote:  
> >>> On 06/11/15 09:05, Michał Górny wrote:    
> >>>>>> I know nothing about the egencache stuff. Maybe Michał can
> >>>>>> comment?    
> >>>> Michał finds this black magic. Trusts zmedico.    
> >>> I think it looks like it's probably supposed to be reasonable, perhaps.
> >>>
> >>> Maybe Brian can look at it. At least that way we'll have a lot of
> >>> people that attempted understanding what's going on.
> >>>
> >>> Maybe we need a "Trusted-by:" line.
> >>>     
> >>
> >> Maybe it helps if I give some more context. At my workplace, we have
> >> lots of scripts that call `emerge --sync private-work-repo` to ensure
> >> that the current system has the latest changes from private-work-repo.
> >> It can be annoying if it spends the bulk of its time calling hooks, even
> >> though private-work-repo was already up-to-date:
> >>  
> >>>>> Timestamps on the server and in the local repository are the same.
> >>>>> Cancelling all further sync action. You are already up to date.    
> >>
> >> So, we want to skip the hooks when repos are already up-to-date. In this
> >> case, there's no point in calling hooks or updating the metadata cache.  
> > 
> > This is incorrect assumption. A change in master repo may trigger
> > metadata cache update in slaved repo.
> >   
> 
> Good point. I'll update it to account for this.

Please don't. This is just one of the corner cases when it will fail.
You can't assume any post-sync hook can be skipped if X or Y didn't
change.

-- 
Best regards,
Michał Górny
<http://dev.gentoo.org/~mgorny/>

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 949 bytes --]

Re: [gentoo-portage-dev] [PATCH] postsync: skip hooks and metadata-transfer when appropriate (bug 564988)

[Zac Medico]

On 11/06/2015 11:34 AM, Michał Górny wrote:
> On Fri, 6 Nov 2015 10:58:23 -0800
> Zac Medico <zmedico@gentoo.org> wrote:
> 
>> On 11/06/2015 10:39 AM, Michał Górny wrote:
>>> On Fri, 6 Nov 2015 09:24:15 -0800
>>> Zac Medico <zmedico@gentoo.org> wrote:
>>>   
>>>> On 11/06/2015 12:20 AM, Alexander Berntsen wrote:  
>>>>> On 06/11/15 09:05, Michał Górny wrote:    
>>>>>>>> I know nothing about the egencache stuff. Maybe Michał can
>>>>>>>> comment?    
>>>>>> Michał finds this black magic. Trusts zmedico.    
>>>>> I think it looks like it's probably supposed to be reasonable, perhaps.
>>>>>
>>>>> Maybe Brian can look at it. At least that way we'll have a lot of
>>>>> people that attempted understanding what's going on.
>>>>>
>>>>> Maybe we need a "Trusted-by:" line.
>>>>>     
>>>>
>>>> Maybe it helps if I give some more context. At my workplace, we have
>>>> lots of scripts that call `emerge --sync private-work-repo` to ensure
>>>> that the current system has the latest changes from private-work-repo.
>>>> It can be annoying if it spends the bulk of its time calling hooks, even
>>>> though private-work-repo was already up-to-date:
>>>>  
>>>>>>> Timestamps on the server and in the local repository are the same.
>>>>>>> Cancelling all further sync action. You are already up to date.    
>>>>
>>>> So, we want to skip the hooks when repos are already up-to-date. In this
>>>> case, there's no point in calling hooks or updating the metadata cache.  
>>>
>>> This is incorrect assumption. A change in master repo may trigger
>>> metadata cache update in slaved repo.
>>>   
>>
>> Good point. I'll update it to account for this.
> 
> Please don't. This is just one of the corner cases when it will fail.
> You can't assume any post-sync hook can be skipped if X or Y didn't
> change.

Can you give an example use case? It the sync operation did not change
anything, then how is it useful to run hooks?
-- 
Thanks,
Zac

Re: [gentoo-portage-dev] [PATCH] postsync: skip hooks and metadata-transfer when appropriate (bug 564988)

[Michał Górny]

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

On Fri, 6 Nov 2015 11:49:44 -0800
Zac Medico <zmedico@gentoo.org> wrote:

> On 11/06/2015 11:34 AM, Michał Górny wrote:
> > On Fri, 6 Nov 2015 10:58:23 -0800
> > Zac Medico <zmedico@gentoo.org> wrote:
> >   
> >> On 11/06/2015 10:39 AM, Michał Górny wrote:  
> >>> On Fri, 6 Nov 2015 09:24:15 -0800
> >>> Zac Medico <zmedico@gentoo.org> wrote:
> >>>     
> >>>> On 11/06/2015 12:20 AM, Alexander Berntsen wrote:    
> >>>>> On 06/11/15 09:05, Michał Górny wrote:      
> >>>>>>>> I know nothing about the egencache stuff. Maybe Michał can
> >>>>>>>> comment?      
> >>>>>> Michał finds this black magic. Trusts zmedico.      
> >>>>> I think it looks like it's probably supposed to be reasonable, perhaps.
> >>>>>
> >>>>> Maybe Brian can look at it. At least that way we'll have a lot of
> >>>>> people that attempted understanding what's going on.
> >>>>>
> >>>>> Maybe we need a "Trusted-by:" line.
> >>>>>       
> >>>>
> >>>> Maybe it helps if I give some more context. At my workplace, we have
> >>>> lots of scripts that call `emerge --sync private-work-repo` to ensure
> >>>> that the current system has the latest changes from private-work-repo.
> >>>> It can be annoying if it spends the bulk of its time calling hooks, even
> >>>> though private-work-repo was already up-to-date:
> >>>>    
> >>>>>>> Timestamps on the server and in the local repository are the same.
> >>>>>>> Cancelling all further sync action. You are already up to date.      
> >>>>
> >>>> So, we want to skip the hooks when repos are already up-to-date. In this
> >>>> case, there's no point in calling hooks or updating the metadata cache.    
> >>>
> >>> This is incorrect assumption. A change in master repo may trigger
> >>> metadata cache update in slaved repo.
> >>>     
> >>
> >> Good point. I'll update it to account for this.  
> > 
> > Please don't. This is just one of the corner cases when it will fail.
> > You can't assume any post-sync hook can be skipped if X or Y didn't
> > change.  
> 
> Can you give an example use case? It the sync operation did not change
> anything, then how is it useful to run hooks?

I've already given you one example. I'm afraid there may be more
customized scripts where not being run is at least unexpected. The gain
is minor compared to the potential damage and confusion.

-- 
Best regards,
Michał Górny
<http://dev.gentoo.org/~mgorny/>

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 949 bytes --]

Re: [gentoo-portage-dev] [PATCH] postsync: skip hooks and metadata-transfer when appropriate (bug 564988)

[Zac Medico]

On 11/06/2015 12:19 PM, Michał Górny wrote:
> On Fri, 6 Nov 2015 11:49:44 -0800
> Zac Medico <zmedico@gentoo.org> wrote:
> 
>> On 11/06/2015 11:34 AM, Michał Górny wrote:
>>> On Fri, 6 Nov 2015 10:58:23 -0800
>>> Zac Medico <zmedico@gentoo.org> wrote:
>>>   
>>>> On 11/06/2015 10:39 AM, Michał Górny wrote:  
>>>>> On Fri, 6 Nov 2015 09:24:15 -0800
>>>>> Zac Medico <zmedico@gentoo.org> wrote:
>>>>>     
>>>>>> On 11/06/2015 12:20 AM, Alexander Berntsen wrote:    
>>>>>>> On 06/11/15 09:05, Michał Górny wrote:      
>>>>>>>>>> I know nothing about the egencache stuff. Maybe Michał can
>>>>>>>>>> comment?      
>>>>>>>> Michał finds this black magic. Trusts zmedico.      
>>>>>>> I think it looks like it's probably supposed to be reasonable, perhaps.
>>>>>>>
>>>>>>> Maybe Brian can look at it. At least that way we'll have a lot of
>>>>>>> people that attempted understanding what's going on.
>>>>>>>
>>>>>>> Maybe we need a "Trusted-by:" line.
>>>>>>>       
>>>>>>
>>>>>> Maybe it helps if I give some more context. At my workplace, we have
>>>>>> lots of scripts that call `emerge --sync private-work-repo` to ensure
>>>>>> that the current system has the latest changes from private-work-repo.
>>>>>> It can be annoying if it spends the bulk of its time calling hooks, even
>>>>>> though private-work-repo was already up-to-date:
>>>>>>    
>>>>>>>>> Timestamps on the server and in the local repository are the same.
>>>>>>>>> Cancelling all further sync action. You are already up to date.      
>>>>>>
>>>>>> So, we want to skip the hooks when repos are already up-to-date. In this
>>>>>> case, there's no point in calling hooks or updating the metadata cache.    
>>>>>
>>>>> This is incorrect assumption. A change in master repo may trigger
>>>>> metadata cache update in slaved repo.
>>>>>     
>>>>
>>>> Good point. I'll update it to account for this.  
>>>
>>> Please don't. This is just one of the corner cases when it will fail.
>>> You can't assume any post-sync hook can be skipped if X or Y didn't
>>> change.  
>>
>> Can you give an example use case? It the sync operation did not change
>> anything, then how is it useful to run hooks?
> 
> I've already given you one example. I'm afraid there may be more
> customized scripts where not being run is at least unexpected. The gain
> is minor compared to the potential damage and confusion.
> 

Yeah, I guess some people might be working under the assumption that
emerge --sync will *always* call their hook, in order to trigger some
system administration tasks that have little or nothing to do with the
sync operation itself. It's impossible to speculate how many people
might be relying on this sort of behavior.

So, I'll think about exposing an environment variable to the hooks which
will allow them to decide if anything relevant has changed.
-- 
Thanks,
Zac