chg and uisetup

Jun Wu quark at fb.com
Wed Jul 13 11:46:54 EDT 2016


Excerpts from Yuya Nishihara's message of 2016-07-14 00:26:47 +0900:
> On Wed, 13 Jul 2016 16:09:32 +0100, Jun Wu wrote:
> > Excerpts from Yuya Nishihara's message of 2016-07-13 23:32:50 +0900:
> > >  4. run ui/extsetup and register commands, templates, etc. if there are
> > >     preloaded and enabled extensions  
> > 
> > This is problematic, there are too many decorators to patch already and I
> > think there are also non-decorator side-effect patterns used in the wild.
> 
> Huh? @command and @template* are carefully designed to not have import-time
> side effect.

This is from templatekw.py:

  keywords = {}
  templatekeyword = registrar.templatekeyword(keywords)

Using @templatekeyword will change the global (or module) variable
templatekw.keywords.

> > The Python language is too free and developers can easily write code with
> > all kinds of side-effects. We will never be confident enough: we cannot
> > devel-warn them. And I can imagine developers yell at chg.
> > 
> > Just an example, I think this is a legit:
> > 
> >   from hgext import histedit
> >   @histedit.action
> >   ...
> 
> IMHO, that's invalid. Registering things while importing would leave None
> object on import failure, result in cryptic error message. If @histedit.action
> has a side effect, it should be done by ui/extsetup().

The point is, if developers write bad code which just runs, and we cannot
warn them, they will blame us.

In general, I think the difference of two approaches are:

  1. Allow side-effects outside uisetup, have more server processes handling
     different [extensions] set
  2. Assume nobody has side-effects outside uisetup, patch decorators
     needed, have less server processes (but still need > 1 if, for
     example,LD_PRELOAD changes)

Now I think 1 is much better than 2 (and easier given the current code).
I didn't think so as I didn't realize the side-effect issue can be very
tricky.


More information about the Mercurial-devel mailing list