[PATCH] extensions: ignore invalid extensions completely when listing disabled commands

[Mads Kiilerich]

Brodie Rao wrote, On 12/19/2010 01:57 AM:
> On Dec 19, 2010, at 7:00 AM, Mads Kiilerich wrote:
>
>> # HG changeset patch
>> # User Mads Kiilerich <mads at kiilerich.com>
>> # Date 1292697155 -3600
>> # Node ID ad1938c4715bdc6601ab35902fde2db4aec0aa2d
>> # Parent  115a9760c38222ee01fabea6904c6bf1956d6f12
>> extensions: ignore invalid extensions completely when listing 
>> disabled commands
>>
>> Invalid extensions in hgext/ could in some cases cause a crash when 
>> searching
>> for unknown commands in disabled extensions.
>>
>> This makes us silently ignore all errors when trying to extract 
>> commands from
>> extensions. Extensions might have been disabled for a reason and we 
>> don't care
>> bout any errors until they are enabled.
>>
>> Reported on https://bugzilla.redhat.com/show_bug.cgi?id=663183 with 
>> forest.py.
>
> My rationale for not silencing exceptions in this feature was that 
> hgext should never have extensions that can't be imported or can't 
> have commands safely extracted.
>
> Installing third party extensions into hgext seems like the wrong 
> thing to do, in my opinion. 

Unfortunately we have no guidelines for that, only opinions and no 
alternatives ;-)

It was also discussed on 
http://selenic.com/pipermail/mercurial-devel/2010-November/thread.html#25956 
.

> In spite of this, swallowing those exceptions is probably okay as long 
> as hg tells us what extension failed and gives us the traceback if we 
> specify --traceback.
>
>> diff --git a/mercurial/extensions.py b/mercurial/extensions.py
>> --- a/mercurial/extensions.py
>> +++ b/mercurial/extensions.py
>> @@ -264,7 +264,7 @@
>>         try:
>>             aliases, entry = cmdutil.findcmd(cmd,
>>                 getattr(mod, 'cmdtable', {}), strict)
>> -        except (error.AmbiguousCommand, error.UnknownCommand):
>> +        except:
>>             return
>
> We should probably use except Exception: instead of except:. I don't 
> think we want to catch SystemExit or KeyboardInterrupt here.
>
> I'd leave the original except clause in, add a second one for 
> Exception that prints a debug message or a warning that we couldn't 
> introspect the disabled extension, and call ui.traceback() after that.

Ok, thanks. Pushed to crew as 1aea66b71f4f .

/Mads