[PATCH 2 of 2 v2] extensions: catch uisetup and extsetup failures and don't let them break hg

Yuya Nishihara yuya at tcha.org
Thu Jun 8 12:00:44 EDT 2017


On Thu, 08 Jun 2017 11:04:04 -0400, Augie Fackler wrote:
> # HG changeset patch
> # User Augie Fackler <augie at google.com>
> # Date 1496758188 14400
> #      Tue Jun 06 10:09:48 2017 -0400
> # Node ID eed6a79c768cad00dcca30b150eb8cd65b3e0d67
> # Parent  bf999f47f7deff5a53e4eea18c22465ec738dd31
> extensions: catch uisetup and extsetup failures and don't let them break hg
> 
> Otherwise users of the patience diff extension will be unable to run
> anything at all in hg 4.3 until they figure out what's broken.
> 
> diff --git a/mercurial/extensions.py b/mercurial/extensions.py
> --- a/mercurial/extensions.py
> +++ b/mercurial/extensions.py
> @@ -10,6 +10,7 @@ from __future__ import absolute_import
>  import imp
>  import inspect
>  import os
> +import sys
>  
>  from .i18n import (
>      _,
> @@ -167,17 +168,31 @@ def load(ui, name, path):
>  def _runuisetup(name, ui):
>      uisetup = getattr(_extensions[name], 'uisetup', None)
>      if uisetup:
> -        uisetup(ui)
> +        try:
> +            uisetup(ui)
> +        except Exception as inst:
> +            ui.traceback(sys.exc_info())

Nit: no need to pass exc_info() explicitly. It's the default.

> @@ -203,15 +218,20 @@ def loadall(ui, whitelist=None):
>                  ui.warn(_("*** (%s)\n") % inst.hint)
>              ui.traceback()
>  
> +    broken = set()
>      for name in _order[newindex:]:
> -        _runuisetup(name, ui)
> +        if not _runuisetup(name, ui):
> +            broken.add(name)
>  
>      for name in _order[newindex:]:
> -        _runextsetup(name, ui)
> +        if name in broken:
> +            continue
> +        if not _runextsetup(name, ui):
> +            broken.add(name)

Still reposetup() would be called for broken extensions.
"_extensions[name] = None" looks ugly, but that's what we do right now.

>      # Call aftercallbacks that were never met.
>      for shortname in _aftercallbacks:
> -        if shortname in _extensions:
> +        if shortname in _extensions or shortname in broken:
>              continue

Perhaps _aftercallbacks had to check if _extensions[shortname] is None or not.
That's an old bug.


More information about the Mercurial-devel mailing list