This is an archive of the discontinued Mercurial Phabricator instance.

wrapfunction: use functools.partial if possible
ClosedPublic

Authored by quark on Sep 5 2017, 7:48 PM.

Details

Summary

Every extensions.bind call inserts a frame in traceback:

... in closure
  return func(*(args + a), **kw)

which makes traceback noisy.

The Python stdlib has a functools.partial which is backed by C code and
does not pollute traceback. However it does not support instancemethod and
sets args attribute which could be problematic for alias handling.

This patch makes wrapfunction use functools.partial if we are wrapping a
function directly exported by a module (so it's impossible to be a class or
instance method), and special handles wrapfunction results so alias
handling code could handle args just fine.

As an example, hg rebase -s . -d . --traceback got 6 lines removed in my
setup:

 File "hg/mercurial/dispatch.py", line 898, in _dispatch
   cmdpats, cmdoptions)
-File "hg/mercurial/extensions.py", line 333, in closure
-  return func(*(args + a), **kw)
 File "hg/hgext/journal.py", line 84, in runcommand
   return orig(lui, repo, cmd, fullargs, *args)
-File "hg/mercurial/extensions.py", line 333, in closure
-  return func(*(args + a), **kw)
 File "fb-hgext/hgext3rd/fbamend/hiddenoverride.py", line 119, in runcommand
   result = orig(lui, repo, cmd, fullargs, *args)
 File "hg/mercurial/dispatch.py", line 660, in runcommand
   ret = _runcommand(ui, options, cmd, d)
-File "hg/mercurial/extensions.py", line 333, in closure
-  return func(*(args + a), **kw)
 File "hg/hgext/pager.py", line 69, in pagecmd
   return orig(ui, options, cmd, cmdfunc)
 ....

Diff Detail

Repository
rHG Mercurial
Lint
Lint Skipped
Unit
Unit Tests Skipped

Event Timeline

quark created this revision.Sep 5 2017, 7:48 PM
quark edited the summary of this revision. (Show Details)Sep 5 2017, 7:51 PM
phillco accepted this revision.Sep 6 2017, 6:03 PM
phillco added a subscriber: phillco.

Nice.

martinvonz added inline comments.
mercurial/extensions.py
464

This patch makes test-py3-commands.t fail. I suspect it's because py3 has no "im_self".

quark edited the summary of this revision. (Show Details)Sep 7 2017, 6:08 PM
quark updated this revision to Diff 1678.
quark marked an inline comment as done.EditedSep 7 2017, 6:10 PM

Fixed. It's tricker than I thought - Python 3 does not have "unbound function" concept and there is no easy way to detect "unbound function". Python 3 descriptor protocol seems to treat function and partial differently. So I changed approach to check ismodule instead, which seems to be more conservative and correct.

This revision was automatically updated to reflect the committed changes.