[gentoo-portage-dev] Re: [PATCH V3] 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.

The variable PORT_LOGDIR was added to the test environment to prevent
CleanLogs.clean() from failing when the variable isn't set or not a
directory.
---
 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   |  3 ++-
 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, 62 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..0d65d41 100644
--- a/pym/portage/emaint/modules/resume/resume.py
+++ b/pym/portage/emaint/modules/resume/resume.py
@@ -37,7 +37,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 +56,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
>