emaint: exit with non-zero status code when module fails (bug 567478)

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.

10 files changed, 62 insertions, 30 deletions

diff --git a/pym/portage/emaint/main.py b/pym/portage/emaint/main.py
index 65e3545b4..f448d6baa 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 cf1213e94..527b02fb3 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 dad024b21..a05a3c2c7 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 fe65cf587..028084af5 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 8f677c2a8..416a725ff 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 41ca167b7..1c2636dac 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 1bada5288..0d65d4181 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 15d63e2c8..d867699fc 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 2c9dbffeb..ebc3adcbb 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 b1a2af52f..5930f6cc8 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,