[PATCH 3 of 3 RFC] chgserve: preimport enabled extensions

Jun Wu quark at fb.com
Mon Oct 3 10:39:24 EDT 2016


Excerpts from Yuya Nishihara's message of 2016-10-03 21:59:02 +0900:
> Regarding this, I was thinking about 'extensions.<name>:enabled' syntax so
> users (or sysadmins) can define a set of conditionally-enabled extensions
> globally.
> 
>   [extensions]
>   rebase =
>   topic = /path/to/topic.py
>   topic:enabled = False
> 
> chg daemon will import all extensions listed in ~/.hgrc. And if :enabled = True
> is flagged by repo/.hg/hgrc, ui/reposetup() will be run.

This may make the implementation more complex and fragile. One area that chg
currently cannot handle 100% correct is things like "-r bundles", shared
repo, or inferrepo - confighash may mismatch forever because of differences
between chg's config loading logic and the non-chg one. If that happens, chg
may redirect forever. I have seen this but haven't got enough time to find
out what's going on exactly. In comparison, the new code can load a random
hgrc and do not worry about if the extensions listed will be enabled or
disabled because of the nice side-effect-free assumption.

Running "uisetup" is not safe for extensions reading configs from "ui". I
think controlling them via configs is not very friendly for average users.
They may mess up and get surprised.

Because of the above two reasons, I prefer no uisetup for all extensions. It
leads to a simpler and safer implementation.

That said, I understand "uisetup" can be fat and expensive. I think if that
happens, the extension author can do the slow logic at import-time. So
instead of:

  def uisetup(ui):
    x = expensive()
    ...

They can write:

  x = expensive()

  def uisetup(ui):
    global x
    ...

So uisetup would remain cheap. If "expensive()" relies on "ui", it cannot
be pre-calculated, since chg cannot provide a "correct" "ui" because of 
possible future config changes.

Regarding on reposetup, I'm aware that some repo state like the radix tree
index needs to be persistent in memory. In my opinion, it would be better
solved by other IPC means, like shared memory or a background daemon
speaking some protocol.

> 
> > +def _importext(orig, name, path=None, reportfunc=None):
> > +    mod = _preimported.get((name, path))
> > +    if mod:
> > +        return mod
> > +    else:
> > +        return orig(name, path, reportfunc)
> > +
> > +def _preimportextensions(ui):
> > +    for name, path in ui.configitems("extensions"):
> > +        # only enabled extensions are pre-imported - assuming importing an
> > +        # extension is side-effect free.
> > +        if path.startswith('!'):
> > +            continue
> > +        try:
> > +            mod = extensions._importext(name, path)
> > +        except ImportError:
> > +            pass
> > +        else:
> > +            _preimported[(name, path)] = mod
> 
> Perhaps path needs to be normalized.


More information about the Mercurial-devel mailing list