chg and uisetup

[Yuya Nishihara]

On Wed, 13 Jul 2016 16:46:54 +0100, Jun Wu wrote:
> 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.

templatekw.py owns the keywords table. No problem to modify its module-level
variable. See the other use of @templatekeyword and read the docstring.

> > > 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.

IIRC, your proposal relies on there's no import-time side effect. If we can't
blame wonky third-party extensions, we can't preload extension modules at all.
Therefore, we would have to stick to the current confighash method.