Patchwork [3,of,3] hg: explicitly check that peer lookup object has instance() if call failed

login
register
mail settings
Submitter Yuya Nishihara
Date May 31, 2015, 6:19 a.m.
Message ID <7b372cd3be9caf49a2b4.1433053183@mimosa>
Download mbox | patch
Permalink /patch/9406/
State Accepted
Headers show

Comments

Yuya Nishihara - May 31, 2015, 6:19 a.m.
# HG changeset patch
# User Yuya Nishihara <yuya@tcha.org>
# Date 1432957590 -32400
#      Sat May 30 12:46:30 2015 +0900
# Node ID 7b372cd3be9caf49a2b4ecf14d7fc96f1d0f4fe8
# Parent  37bd00295cac9a7144dc7ca685a4fe201b95a860
hg: explicitly check that peer lookup object has instance() if call failed

If a "thing" is callable but raises TypeError for some reason, a callable
object would be returned. Thereafter, unfriendly traceback would be displayed:

  Traceback (most recent call last):
    ...
    File "mercurial/hg.pyc", line 119, in _peerorrepo
      obj = _peerlookup(path).instance(ui, path, create)
  AttributeError: 'function' object has no attribute 'instance'

Instead, we should show the reason why "thing(path)" didn't work:

  Traceback (most recent call last):
    ...
    File "hggit/__init__.py", line 89, in _local
      p = urlcls(path).localpath()
  TypeError: 'NoneType' object is not callable

If a "thing" is not callable, it must be a module or an object that implements
instance(). If that module didn't have instance(), the error message would be
"<unloaded module 'foo'> object is not callable". It doesn't make perfect sense,
but it isn't so bad as it can blame which module went wrong.
Pierre-Yves David - May 31, 2015, 7:05 a.m.
On 05/30/2015 11:19 PM, Yuya Nishihara wrote:
> # HG changeset patch
> # User Yuya Nishihara <yuya@tcha.org>
> # Date 1432957590 -32400
> #      Sat May 30 12:46:30 2015 +0900
> # Node ID 7b372cd3be9caf49a2b4ecf14d7fc96f1d0f4fe8
> # Parent  37bd00295cac9a7144dc7ca685a4fe201b95a860
> hg: explicitly check that peer lookup object has instance() if call failed

These seems great thanks. Pushed to the clowncopter.

Patch

diff --git a/mercurial/hg.py b/mercurial/hg.py
--- a/mercurial/hg.py
+++ b/mercurial/hg.py
@@ -92,6 +92,10 @@  def _peerlookup(path):
     try:
         return thing(path)
     except TypeError:
+        # we can't test callable(thing) because 'thing' can be an unloaded
+        # module that implements __call__
+        if not util.safehasattr(thing, 'instance'):
+            raise
         return thing
 
 def islocal(repo):