[gentoo-portage-dev] Re: [PATCH V2] emaint: exit with non-zero status code when module fails (bug 567478)

[Alexandru Elisei]

Module functions currently return a message to emaint after invocation.
Emaint prints this message then exits normally (with a success return
code) even if the module encountered an error. This patch aims to
change this by having each module public function return a tuple of
(returncode, message), where returncode is boolean True if the function
was successful or False otherwise. Emaint will inspect the return codes
and exit unsuccessfully if necessary.
---
#
# I've modified the patch as per Brian's suggestions:
# - the modules return True on success and False on failure.
# - TaskHandler.run_tasks() returns a list of all the module return
# codes, then emaint_main() exits.
# - the portage/pym/portage/tests/emerge/test_simple.py change wasn't
# present in the first version of the patch because of a subtle bug that
# I introduced: rval was None when the variable PORT_LOGDIR wasn't set,
# it was different from os.EX_OK so TaskHandler.run_tasks() was exiting
# with sys.exit(None), which is zero - success.
#
 pym/portage/emaint/main.py                    | 12 +++++++++---
 pym/portage/emaint/modules/binhost/binhost.py |  6 ++++--
 pym/portage/emaint/modules/config/config.py   |  6 ++++--
 pym/portage/emaint/modules/logs/logs.py       |  9 +++++----
 pym/portage/emaint/modules/merges/merges.py   | 18 +++++++++++-------
 pym/portage/emaint/modules/move/move.py       |  9 +++++++--
 pym/portage/emaint/modules/resume/resume.py   |  4 +++-
 pym/portage/emaint/modules/sync/sync.py       | 20 +++++++++++++-------
 pym/portage/emaint/modules/world/world.py     |  8 ++++++--
 pym/portage/tests/emerge/test_simple.py       |  1 +
 10 files changed, 63 insertions(+), 30 deletions(-)

diff --git a/pym/portage/emaint/main.py b/pym/portage/emaint/main.py
index 65e3545..f448d6b 100644
--- a/pym/portage/emaint/main.py
+++ b/pym/portage/emaint/main.py
@@ -115,6 +115,7 @@ class TaskHandler(object):
 		"""Runs the module tasks"""
 		if tasks is None or func is None:
 			return
+		returncodes = []
 		for task in tasks:
 			inst = task()
 			show_progress = self.show_progress_bar and self.isatty
@@ -135,14 +136,17 @@ class TaskHandler(object):
 				# them for other tasks if there is more to do.
 				'options': options.copy()
 				}
-			result = getattr(inst, func)(**kwargs)
+			returncode, msgs = getattr(inst, func)(**kwargs)
+			returncodes.append(returncode)
 			if show_progress:
 				# make sure the final progress is displayed
 				self.progress_bar.display()
 				print()
 				self.progress_bar.stop()
 			if self.callback:
-				self.callback(result)
+				self.callback(msgs)
+
+		return returncodes


 def print_results(results):
@@ -237,4 +241,6 @@ def emaint_main(myargv):
 	task_opts = options.__dict__
 	task_opts['return-messages'] = True
 	taskmaster = TaskHandler(callback=print_results, module_output=sys.stdout)
-	taskmaster.run_tasks(tasks, func, status, options=task_opts)
+	returncodes = taskmaster.run_tasks(tasks, func, status, options=task_opts)
+
+	sys.exit(False in returncodes)
diff --git a/pym/portage/emaint/modules/binhost/binhost.py
b/pym/portage/emaint/modules/binhost/binhost.py
index cf1213e..527b02f 100644
--- a/pym/portage/emaint/modules/binhost/binhost.py
+++ b/pym/portage/emaint/modules/binhost/binhost.py
@@ -86,7 +86,9 @@ class BinhostHandler(object):
 		stale = set(metadata).difference(cpv_all)
 		for cpv in stale:
 			errors.append("'%s' is not in the repository" % cpv)
-		return errors
+		if errors:
+			return (False, errors)
+		return (True, None)

 	def fix(self,  **kwargs):
 		onProgress = kwargs.get('onProgress', None)
@@ -177,4 +179,4 @@ class BinhostHandler(object):
 			if maxval == 0:
 				maxval = 1
 			onProgress(maxval, maxval)
-		return None
+		return (True, None)
diff --git a/pym/portage/emaint/modules/config/config.py
b/pym/portage/emaint/modules/config/config.py
index dad024b..a05a3c2 100644
--- a/pym/portage/emaint/modules/config/config.py
+++ b/pym/portage/emaint/modules/config/config.py
@@ -36,7 +36,8 @@ class CleanConfig(object):
 			if onProgress:
 				onProgress(maxval, i+1)
 				i += 1
-		return self._format_output(messages)
+		msgs = self._format_output(messages)
+		return (True, msgs)

 	def fix(self, **kwargs):
 		onProgress = kwargs.get('onProgress', None)
@@ -65,7 +66,8 @@ class CleanConfig(object):
 				i += 1
 		if modified:
 			writedict(configs, self.target)
-		return self._format_output(messages, True)
+		msgs = self._format_output(messages, True)
+		return (True, msgs)

 	def _format_output(self, messages=[], cleaned=False):
 		output = []
diff --git a/pym/portage/emaint/modules/logs/logs.py
b/pym/portage/emaint/modules/logs/logs.py
index fe65cf5..028084a 100644
--- a/pym/portage/emaint/modules/logs/logs.py
+++ b/pym/portage/emaint/modules/logs/logs.py
@@ -40,7 +40,6 @@ class CleanLogs(object):
 				'NUM': int: number of days
 				'pretend': boolean
 		"""
-		messages = []
 		num_of_days = None
 		pretend = False
 		if kwargs:
@@ -70,10 +69,12 @@ class CleanLogs(object):
 					clean_cmd.remove("-delete")

 		if not clean_cmd:
-			return []
+			return (True, None)
 		rval = self._clean_logs(clean_cmd, settings)
-		messages += self._convert_errors(rval)
-		return messages
+		errors = self._convert_errors(rval)
+		if errors:
+			return (False, errors)
+		return (True, None)


 	@staticmethod
diff --git a/pym/portage/emaint/modules/merges/merges.py
b/pym/portage/emaint/modules/merges/merges.py
index 8f677c2..416a725 100644
--- a/pym/portage/emaint/modules/merges/merges.py
+++ b/pym/portage/emaint/modules/merges/merges.py
@@ -239,7 +239,9 @@ class MergesHandler(object):
 		for pkg, mtime in failed_pkgs.items():
 			mtime_str = time.ctime(int(mtime))
 			errors.append("'%s' failed to merge on '%s'" % (pkg, mtime_str))
-		return errors
+		if errors:
+			return (False, errors)
+		return (True, None)


 	def fix(self, **kwargs):
@@ -247,13 +249,13 @@ class MergesHandler(object):
 		module_output = kwargs.get('module_output', None)
 		failed_pkgs = self._failed_pkgs()
 		if not failed_pkgs:
-			return ['No failed merges found.']
+			return (True, ['No failed merges found.'])

 		pkg_invalid_entries = set()
 		pkg_atoms = set()
 		self._get_pkg_atoms(failed_pkgs, pkg_atoms, pkg_invalid_entries)
 		if pkg_invalid_entries:
-			return pkg_invalid_entries
+			return (False, pkg_invalid_entries)

 		try:
 			self._tracking_file.save(failed_pkgs)
@@ -261,7 +263,7 @@ class MergesHandler(object):
 			errors = ['Unable to save failed merges to tracking file: %s\n'
 				% str(ex)]
 			errors.append(', '.join(sorted(failed_pkgs)))
-			return errors
+			return (False, errors)
 		self._remove_failed_dirs(failed_pkgs)
 		results = self._emerge_pkg_atoms(module_output, pkg_atoms)
 		# list any new failed merges
@@ -276,12 +278,14 @@ class MergesHandler(object):
 			if not vardb.cpv_exists(pkg_name):
 				still_failed_pkgs[pkg] = mtime
 		self._tracking_file.save(still_failed_pkgs)
-		return results
+		if still_failed_pkgs:
+			return (False, results)
+		return (True, results)


 	def purge(self, **kwargs):
 		"""Attempt to remove previously saved tracking file."""
 		if not self._tracking_file.exists():
-			return ['Tracking file not found.']
+			return (True, ['Tracking file not found.'])
 		self._tracking_file.purge()
-		return ['Removed tracking file.']
+		return (True, ['Removed tracking file.'])
diff --git a/pym/portage/emaint/modules/move/move.py
b/pym/portage/emaint/modules/move/move.py
index 41ca167..1c2636d 100644
--- a/pym/portage/emaint/modules/move/move.py
+++ b/pym/portage/emaint/modules/move/move.py
@@ -123,7 +123,10 @@ class MoveHandler(object):
 				errors.append("'%s' has outdated metadata" % cpv)
 			if onProgress:
 				onProgress(maxval, i+1)
-		return errors
+
+		if errors:
+			return (False, errors)
+		return (True, None)

 	def fix(self,  **kwargs):
 		onProgress = kwargs.get('onProgress', None)
@@ -156,7 +159,9 @@ class MoveHandler(object):
 		# Searching for updates in all the metadata is relatively slow, so this
 		# is where the progress bar comes out of indeterminate mode.
 		self._tree.dbapi.update_ents(allupdates, onProgress=onProgress)
-		return errors
+		if errors:
+			return (False, errors)
+		return (True, None)

 class MoveInstalled(MoveHandler):

diff --git a/pym/portage/emaint/modules/resume/resume.py
b/pym/portage/emaint/modules/resume/resume.py
index 1bada52..1d14275 100644
--- a/pym/portage/emaint/modules/resume/resume.py
+++ b/pym/portage/emaint/modules/resume/resume.py
@@ -2,6 +2,7 @@
 # Distributed under the terms of the GNU General Public License v2

 import portage
+import os


 class CleanResume(object):
@@ -37,7 +38,7 @@ class CleanResume(object):
 			finally:
 				if onProgress:
 					onProgress(maxval, i+1)
-		return messages
+		return (True, messages)

 	def fix(self,  **kwargs):
 		onProgress = kwargs.get('onProgress', None)
@@ -56,3 +57,4 @@ class CleanResume(object):
 					onProgress(maxval, i+1)
 		if delete_count:
 			mtimedb.commit()
+		return (True, None)
diff --git a/pym/portage/emaint/modules/sync/sync.py
b/pym/portage/emaint/modules/sync/sync.py
index 15d63e2..d867699 100644
--- a/pym/portage/emaint/modules/sync/sync.py
+++ b/pym/portage/emaint/modules/sync/sync.py
@@ -127,8 +127,8 @@ class SyncRepos(object):
 				% (bold(", ".join(repos))) + "\n   ...returning"
 				]
 			if return_messages:
-				return msgs
-			return
+				return (False, msgs)
+			return (False, None)
 		return self._sync(selected, return_messages,
 			emaint_opts=options)

@@ -211,8 +211,8 @@ class SyncRepos(object):
 			msgs.append("Emaint sync, nothing to sync... returning")
 			if return_messages:
 				msgs.extend(self.rmessage([('None', os.EX_OK)], 'sync'))
-				return msgs
-			return
+				return (True, msgs)
+			return (True, None)
 		# Portage needs to ensure a sane umask for the files it creates.
 		os.umask(0o22)

@@ -232,9 +232,14 @@ class SyncRepos(object):
 		sync_scheduler.wait()
 		retvals = sync_scheduler.retvals
 		msgs.extend(sync_scheduler.msgs)
+		returncode = True

 		if retvals:
 			msgs.extend(self.rmessage(retvals, 'sync'))
+			for repo, returncode in retvals:
+				if returncode != os.EX_OK:
+					returncode = False
+					break
 		else:
 			msgs.extend(self.rmessage([('None', os.EX_OK)], 'sync'))

@@ -244,6 +249,8 @@ class SyncRepos(object):
 			rcode = sync_manager.perform_post_sync_hook('')
 			if rcode:
 				msgs.extend(self.rmessage([('None', rcode)], 'post-sync'))
+				if rcode != os.EX_OK:
+					returncode = False

 		# Reload the whole config.
 		portage._sync_mode = False
@@ -254,8 +261,8 @@ class SyncRepos(object):
 			self.emerge_config.opts)

 		if return_messages:
-			return msgs
-		return
+			return (returncode, msgs)
+		return (returncode, None)


 	def _do_pkg_moves(self):
@@ -355,7 +362,6 @@ class SyncScheduler(AsyncScheduler):
 		# 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, hooks_enabled = task.result
 			if message:
diff --git a/pym/portage/emaint/modules/world/world.py
b/pym/portage/emaint/modules/world/world.py
index 2c9dbff..ebc3adc 100644
--- a/pym/portage/emaint/modules/world/world.py
+++ b/pym/portage/emaint/modules/world/world.py
@@ -65,7 +65,9 @@ class WorldHandler(object):
 			errors += ["'%s' is not installed" % x for x in self.not_installed]
 		else:
 			errors.append(self.world_file + " could not be opened for reading")
-		return errors
+		if errors:
+			return (False, errors)
+		return (True, None)

 	def fix(self, **kwargs):
 		onProgress = kwargs.get('onProgress', None)
@@ -83,7 +85,9 @@ class WorldHandler(object):
 				except portage.exception.PortageException:
 					errors.append("%s could not be opened for writing" % \
 						self.world_file)
-			return errors
+			if errors:
+				return (False, errors)
+			return (True, None)
 		finally:
 			world_set.unlock()

diff --git a/pym/portage/tests/emerge/test_simple.py
b/pym/portage/tests/emerge/test_simple.py
index b1a2af5..5930f6c 100644
--- a/pym/portage/tests/emerge/test_simple.py
+++ b/pym/portage/tests/emerge/test_simple.py
@@ -382,6 +382,7 @@ pkg_preinst() {
 			"PORTAGE_PYTHON" : portage_python,
 			"PORTAGE_REPOSITORIES" : settings.repositories.config_string(),
 			"PORTAGE_TMPDIR" : portage_tmpdir,
+			"PORT_LOGDIR" : portage_tmpdir,
 			"PYTHONDONTWRITEBYTECODE" : os.environ.get("PYTHONDONTWRITEBYTECODE", ""),
 			"PYTHONPATH" : pythonpath,
 			"__PORTAGE_TEST_PATH_OVERRIDE" : fake_bin,
-- 
2.10.2

On 1/17/17, Alexandru Elisei <alexandru.elisei@×××××.com> wrote:
> Currently module functions return a message to emaint after invocation.
> Emaint prints this message then exits normally (with a success return
> code) even if the module encountered an error. This patch aims to
> change this by having each module public function return a tuple of
> (returncode, message). Emaint will inspect the return code and will
> exit unsuccessfully if necessary.
> ---
>  pym/portage/emaint/main.py                    | 11 +++++++++--
>  pym/portage/emaint/modules/binhost/binhost.py |  6 ++++--
>  pym/portage/emaint/modules/config/config.py   |  8 ++++++--
>  pym/portage/emaint/modules/logs/logs.py       |  9 +++++----
>  pym/portage/emaint/modules/merges/merges.py   | 18 +++++++++++-------
>  pym/portage/emaint/modules/move/move.py       |  9 +++++++--
>  pym/portage/emaint/modules/resume/resume.py   |  4 +++-
>  pym/portage/emaint/modules/sync/sync.py       | 19 +++++++++++++------
>  pym/portage/emaint/modules/world/world.py     |  8 ++++++--
>  9 files changed, 64 insertions(+), 28 deletions(-)
>
> diff --git a/pym/portage/emaint/main.py b/pym/portage/emaint/main.py
> index 65e3545..ef4383a 100644
> --- a/pym/portage/emaint/main.py
> +++ b/pym/portage/emaint/main.py
> @@ -115,6 +115,7 @@ class TaskHandler(object):
>  		"""Runs the module tasks"""
>  		if tasks is None or func is None:
>  			return
> +		returncode = os.EX_OK
>  		for task in tasks:
>  			inst = task()
>  			show_progress = self.show_progress_bar and self.isatty
> @@ -135,14 +136,20 @@ class TaskHandler(object):
>  				# them for other tasks if there is more to do.
>  				'options': options.copy()
>  				}
> -			result = getattr(inst, func)(**kwargs)
> +			rcode, msgs = getattr(inst, func)(**kwargs)
>  			if show_progress:
>  				# make sure the final progress is displayed
>  				self.progress_bar.display()
>  				print()
>  				self.progress_bar.stop()
>  			if self.callback:
> -				self.callback(result)
> +				self.callback(msgs)
> +			# Keep the last error code when using the 'all' command.
> +			if rcode != os.EX_OK:
> +				returncode = rcode
> +
> +		if returncode != os.EX_OK:
> +			sys.exit(returncode)
>
>
>  def print_results(results):
> diff --git a/pym/portage/emaint/modules/binhost/binhost.py
> b/pym/portage/emaint/modules/binhost/binhost.py
> index cf1213e..8cf3da6 100644
> --- a/pym/portage/emaint/modules/binhost/binhost.py
> +++ b/pym/portage/emaint/modules/binhost/binhost.py
> @@ -86,7 +86,9 @@ class BinhostHandler(object):
>  		stale = set(metadata).difference(cpv_all)
>  		for cpv in stale:
>  			errors.append("'%s' is not in the repository" % cpv)
> -		return errors
> +		if errors:
> +			return (1, errors)
> +		return (os.EX_OK, None)
>
>  	def fix(self,  **kwargs):
>  		onProgress = kwargs.get('onProgress', None)
> @@ -177,4 +179,4 @@ class BinhostHandler(object):
>  			if maxval == 0:
>  				maxval = 1
>  			onProgress(maxval, maxval)
> -		return None
> +		return (os.EX_OK, None)
> diff --git a/pym/portage/emaint/modules/config/config.py
> b/pym/portage/emaint/modules/config/config.py
> index dad024b..a4a58d9 100644
> --- a/pym/portage/emaint/modules/config/config.py
> +++ b/pym/portage/emaint/modules/config/config.py
> @@ -36,7 +36,10 @@ class CleanConfig(object):
>  			if onProgress:
>  				onProgress(maxval, i+1)
>  				i += 1
> -		return self._format_output(messages)
> +		msgs = self._format_output(messages)
> +		if msgs:
> +			return (1, msgs)
> +		return (os.EX_OK, None)
>
>  	def fix(self, **kwargs):
>  		onProgress = kwargs.get('onProgress', None)
> @@ -65,7 +68,8 @@ class CleanConfig(object):
>  				i += 1
>  		if modified:
>  			writedict(configs, self.target)
> -		return self._format_output(messages, True)
> +		msgs = self._format_output(messages, True)
> +		return (os.EX_OK, msgs)
>
>  	def _format_output(self, messages=[], cleaned=False):
>  		output = []
> diff --git a/pym/portage/emaint/modules/logs/logs.py
> b/pym/portage/emaint/modules/logs/logs.py
> index fe65cf5..8c638d7 100644
> --- a/pym/portage/emaint/modules/logs/logs.py
> +++ b/pym/portage/emaint/modules/logs/logs.py
> @@ -40,7 +40,6 @@ class CleanLogs(object):
>  				'NUM': int: number of days
>  				'pretend': boolean
>  		"""
> -		messages = []
>  		num_of_days = None
>  		pretend = False
>  		if kwargs:
> @@ -70,10 +69,12 @@ class CleanLogs(object):
>  					clean_cmd.remove("-delete")
>
>  		if not clean_cmd:
> -			return []
> +			return (os.EX_OK, None)
>  		rval = self._clean_logs(clean_cmd, settings)
> -		messages += self._convert_errors(rval)
> -		return messages
> +		errors = self._convert_errors(rval)
> +		if errors:
> +			return (rval, errors)
> +		return (os.EX_OK, None)
>
>
>  	@staticmethod
> diff --git a/pym/portage/emaint/modules/merges/merges.py
> b/pym/portage/emaint/modules/merges/merges.py
> index 8f677c2..9a87d87 100644
> --- a/pym/portage/emaint/modules/merges/merges.py
> +++ b/pym/portage/emaint/modules/merges/merges.py
> @@ -239,7 +239,9 @@ class MergesHandler(object):
>  		for pkg, mtime in failed_pkgs.items():
>  			mtime_str = time.ctime(int(mtime))
>  			errors.append("'%s' failed to merge on '%s'" % (pkg, mtime_str))
> -		return errors
> +		if errors:
> +			return (1, errors)
> +		return (os.EX_OK, None)
>
>
>  	def fix(self, **kwargs):
> @@ -247,13 +249,13 @@ class MergesHandler(object):
>  		module_output = kwargs.get('module_output', None)
>  		failed_pkgs = self._failed_pkgs()
>  		if not failed_pkgs:
> -			return ['No failed merges found.']
> +			return (os.EX_OK, ['No failed merges found.'])
>
>  		pkg_invalid_entries = set()
>  		pkg_atoms = set()
>  		self._get_pkg_atoms(failed_pkgs, pkg_atoms, pkg_invalid_entries)
>  		if pkg_invalid_entries:
> -			return pkg_invalid_entries
> +			return (1, pkg_invalid_entries)
>
>  		try:
>  			self._tracking_file.save(failed_pkgs)
> @@ -261,7 +263,7 @@ class MergesHandler(object):
>  			errors = ['Unable to save failed merges to tracking file: %s\n'
>  				% str(ex)]
>  			errors.append(', '.join(sorted(failed_pkgs)))
> -			return errors
> +			return (1, errors)
>  		self._remove_failed_dirs(failed_pkgs)
>  		results = self._emerge_pkg_atoms(module_output, pkg_atoms)
>  		# list any new failed merges
> @@ -276,12 +278,14 @@ class MergesHandler(object):
>  			if not vardb.cpv_exists(pkg_name):
>  				still_failed_pkgs[pkg] = mtime
>  		self._tracking_file.save(still_failed_pkgs)
> -		return results
> +		if still_failed_pkgs:
> +			return (1, results)
> +		return (os.EX_OK, results)
>
>
>  	def purge(self, **kwargs):
>  		"""Attempt to remove previously saved tracking file."""
>  		if not self._tracking_file.exists():
> -			return ['Tracking file not found.']
> +			return (os.EX_OK, ['Tracking file not found.'])
>  		self._tracking_file.purge()
> -		return ['Removed tracking file.']
> +		return (os.EX_OK, ['Removed tracking file.'])
> diff --git a/pym/portage/emaint/modules/move/move.py
> b/pym/portage/emaint/modules/move/move.py
> index 41ca167..1a566c3 100644
> --- a/pym/portage/emaint/modules/move/move.py
> +++ b/pym/portage/emaint/modules/move/move.py
> @@ -123,7 +123,10 @@ class MoveHandler(object):
>  				errors.append("'%s' has outdated metadata" % cpv)
>  			if onProgress:
>  				onProgress(maxval, i+1)
> -		return errors
> +
> +		if errors:
> +			return (1, errors)
> +		return (os.EX_OK, None)
>
>  	def fix(self,  **kwargs):
>  		onProgress = kwargs.get('onProgress', None)
> @@ -156,7 +159,9 @@ class MoveHandler(object):
>  		# Searching for updates in all the metadata is relatively slow, so this
>  		# is where the progress bar comes out of indeterminate mode.
>  		self._tree.dbapi.update_ents(allupdates, onProgress=onProgress)
> -		return errors
> +		if errors:
> +			return(1, errors)
> +		return (os.EX_OK, None)
>
>  class MoveInstalled(MoveHandler):
>
> diff --git a/pym/portage/emaint/modules/resume/resume.py
> b/pym/portage/emaint/modules/resume/resume.py
> index 1bada52..648e8c1 100644
> --- a/pym/portage/emaint/modules/resume/resume.py
> +++ b/pym/portage/emaint/modules/resume/resume.py
> @@ -2,6 +2,7 @@
>  # Distributed under the terms of the GNU General Public License v2
>
>  import portage
> +import os
>
>
>  class CleanResume(object):
> @@ -37,7 +38,7 @@ class CleanResume(object):
>  			finally:
>  				if onProgress:
>  					onProgress(maxval, i+1)
> -		return messages
> +		return (os.EX_OK, messages)
>
>  	def fix(self,  **kwargs):
>  		onProgress = kwargs.get('onProgress', None)
> @@ -56,3 +57,4 @@ class CleanResume(object):
>  					onProgress(maxval, i+1)
>  		if delete_count:
>  			mtimedb.commit()
> +		return (os.EX_OK, None)
> diff --git a/pym/portage/emaint/modules/sync/sync.py
> b/pym/portage/emaint/modules/sync/sync.py
> index 15d63e2..03a684b 100644
> --- a/pym/portage/emaint/modules/sync/sync.py
> +++ b/pym/portage/emaint/modules/sync/sync.py
> @@ -127,8 +127,8 @@ class SyncRepos(object):
>  				% (bold(", ".join(repos))) + "\n   ...returning"
>  				]
>  			if return_messages:
> -				return msgs
> -			return
> +				return (1, msgs)
> +			return (1, None)
>  		return self._sync(selected, return_messages,
>  			emaint_opts=options)
>
> @@ -211,8 +211,8 @@ class SyncRepos(object):
>  			msgs.append("Emaint sync, nothing to sync... returning")
>  			if return_messages:
>  				msgs.extend(self.rmessage([('None', os.EX_OK)], 'sync'))
> -				return msgs
> -			return
> +				return (os.EX_OK, msgs)
> +			return (os.EX_OK, None)
>  		# Portage needs to ensure a sane umask for the files it creates.
>  		os.umask(0o22)
>
> @@ -232,6 +232,7 @@ class SyncRepos(object):
>  		sync_scheduler.wait()
>  		retvals = sync_scheduler.retvals
>  		msgs.extend(sync_scheduler.msgs)
> +		returncode = sync_scheduler.returncode
>
>  		if retvals:
>  			msgs.extend(self.rmessage(retvals, 'sync'))
> @@ -244,6 +245,8 @@ class SyncRepos(object):
>  			rcode = sync_manager.perform_post_sync_hook('')
>  			if rcode:
>  				msgs.extend(self.rmessage([('None', rcode)], 'post-sync'))
> +				if rcode != os.EX_OK:
> +					returncode = rcode
>
>  		# Reload the whole config.
>  		portage._sync_mode = False
> @@ -254,8 +257,8 @@ class SyncRepos(object):
>  			self.emerge_config.opts)
>
>  		if return_messages:
> -			return msgs
> -		return
> +			return (returncode, msgs)
> +		return (returncode, None)
>
>
>  	def _do_pkg_moves(self):
> @@ -323,6 +326,7 @@ class SyncScheduler(AsyncScheduler):
>  		self._init_graph()
>  		self.retvals = []
>  		self.msgs = []
> +		self.returncode = os.EX_OK
>
>  	def _init_graph(self):
>  		'''
> @@ -360,6 +364,9 @@ class SyncScheduler(AsyncScheduler):
>  			returncode, message, updatecache_flg, hooks_enabled = task.result
>  			if message:
>  				self.msgs.append(message)
> +		else:
> +			# Keep the last unsuccessful sync return code.
> +			self.returncode = returncode
>  		repo = task.kwargs['repo'].name
>  		self._running_repos.remove(repo)
>  		self.retvals.append((repo, returncode))
> diff --git a/pym/portage/emaint/modules/world/world.py
> b/pym/portage/emaint/modules/world/world.py
> index 2c9dbff..d8a4e69 100644
> --- a/pym/portage/emaint/modules/world/world.py
> +++ b/pym/portage/emaint/modules/world/world.py
> @@ -65,7 +65,9 @@ class WorldHandler(object):
>  			errors += ["'%s' is not installed" % x for x in self.not_installed]
>  		else:
>  			errors.append(self.world_file + " could not be opened for reading")
> -		return errors
> +		if errors:
> +			return (1, errors)
> +		return (os.EX_OK, None)
>
>  	def fix(self, **kwargs):
>  		onProgress = kwargs.get('onProgress', None)
> @@ -83,7 +85,9 @@ class WorldHandler(object):
>  				except portage.exception.PortageException:
>  					errors.append("%s could not be opened for writing" % \
>  						self.world_file)
> -			return errors
> +			if errors:
> +				return (1, errors)
> +			return (os.EX_OK, None)
>  		finally:
>  			world_set.unlock()
>
> --
> 2.10.2
>