[PATCH stable] hooks: only disable/re-enable demandimport when it's already enabled
Brodie Rao
brodie at sf.io
Tue Feb 11 14:08:10 CST 2014
On Mon, Feb 10, 2014 at 3:51 PM, Simon Heimberg <Simohe at besonet.ch> wrote:
> Am 11.02.2014 00:00, schrieb Brodie Rao:
>
>> # HG changeset patch
>> # User Brodie Rao <brodie at sf.io>
>> # Date 1392072666 28800
>> # Mon Feb 10 14:51:06 2014 -0800
>> # Branch stable
>> # Node ID aac87f70f38e1561c98057fc845f333b185b6d5d
>> # Parent e4d7cbc94219e54f5e73df9c2f88eca3d46d7f90
>> hooks: only disable/re-enable demandimport when it's already enabled
>>
>> This fixes an issue introduced in d7c28954d901 where, when disabling
>> demandimport while running hooks, it's inadvertently re-enabled even when
>> it was never enabled in the first place.
>>
>> This doesn't affect normal command line usage of Mercurial; it only
>> matters
>> when Mercurial is run with demandimport intentionally disabled.
>>
>> diff --git a/mercurial/demandimport.py b/mercurial/demandimport.py
>> --- a/mercurial/demandimport.py
>> +++ b/mercurial/demandimport.py
>> @@ -162,6 +162,9 @@ ignore = [
>> 'mimetools',
>> ]
>>
>> +def isenabled():
>> + return __builtin__.__import__ == _demandimport
>> +
>> def enable():
>> "enable global demand-loading of modules"
>> __builtin__.__import__ = _demandimport
>> diff --git a/mercurial/hook.py b/mercurial/hook.py
>> --- a/mercurial/hook.py
>> +++ b/mercurial/hook.py
>> @@ -36,28 +36,33 @@ def _pythonhook(ui, repo, name, hname, f
>> if modpath and modfile:
>> sys.path = sys.path[:] + [modpath]
>> modname = modfile
>> + demandimportenabled = demandimport.isenabled()
>> + if demandimportenabled:
>> + demandimport.disable()
>> try:
>> - demandimport.disable()
>> - obj = __import__(modname)
>
>
> Why did we disable demandimport here? __import__ is never delayed. (See
> comment at start of mercurial/demandimport.py) Let's only remove disable and
> enable.
I believe that change was made to disable demandimport for import
statements made inside the extension. Using __import__() here only
bypasses demandimport when importing the extension itself;
demandimport will still be enabled for the extension's own imports.
So I don't think removing disable/enable is the right thing to do. I
think my patch as it stands is OK.
>
>
>> - demandimport.enable()
>> - except ImportError:
>> - e1 = sys.exc_type, sys.exc_value, sys.exc_traceback
>> try:
>> - # extensions are loaded with hgext_ prefix
>> - obj = __import__("hgext_%s" % modname)
>> + obj = __import__(modname)
>> + except ImportError:
>> + e1 = sys.exc_type, sys.exc_value, sys.exc_traceback
>> + try:
>> + # extensions are loaded with hgext_ prefix
>> + obj = __import__("hgext_%s" % modname)
>> + except ImportError:
>> + e2 = sys.exc_type, sys.exc_value, sys.exc_traceback
>> + if ui.tracebackflag:
>> + ui.warn(_('exception from first failed import '
>> + 'attempt:\n'))
>> + ui.traceback(e1)
>> + if ui.tracebackflag:
>> + ui.warn(_('exception from second failed import '
>> + 'attempt:\n'))
>> + ui.traceback(e2)
>> + raise util.Abort(_('%s hook is invalid '
>> + '(import of "%s" failed)') %
>> + (hname, modname))
>> + finally:
>> + if demandimportenabled:
>> demandimport.enable()
>> - except ImportError:
>> - demandimport.enable()
>> - e2 = sys.exc_type, sys.exc_value, sys.exc_traceback
>> - if ui.tracebackflag:
>> - ui.warn(_('exception from first failed import
>> attempt:\n'))
>> - ui.traceback(e1)
>> - if ui.tracebackflag:
>> - ui.warn(_('exception from second failed import
>> attempt:\n'))
>> - ui.traceback(e2)
>> - raise util.Abort(_('%s hook is invalid '
>> - '(import of "%s" failed)') %
>> - (hname, modname))
>> sys.path = oldpaths
>> try:
>> for p in funcname.split('.')[1:]:
>>
>
More information about the Mercurial-devel
mailing list