On Wed, 18 Jan 2017 19:15:54 +0200
Alexandru Elisei <alexandru.eli...@gmail.com> wrote:
> 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,
merged :) thank you
--
Brian Dolbec <dolsen>
