[PATCH 05 of 10] chgserver: add a context manager to learn what to include for confighash

Jun Wu quark at fb.com
Thu Jul 7 09:24:14 EDT 2016


Excerpts from Yuya Nishihara's message of 2016-07-07 21:49:01 +0900:
> For this particular example, wrapping manifest.rev can be moved to reposetup().
> 
>  1. extend repo.__class__ to hook repo.manifest
>  2. extend manifest.__class__ to hook manifest.rev
 
The problem will become detecting the unnecessary hg serve's "reposetup" and
unwrap if the config changes. But this discussion is less important here.

> > It basically keeps a ui object in all kinds of its objects, and down in the
> > stack the ui object is used to do debugflag test or so. As said, I believed
> > this is not a good pattern and tried to fix them but the codebase is so
> > large and I gave up after a few hours.
> > 
> > The developer wants to use --debug to toggle debug output so they should be
> > able to tell chg to hash ui.debugflag, but not other ui config items.
> 
> Hmm, but you can't track the access to ui.debugflag without an ugly hack. Only
> ui.config*() calls are recorded.

That would be covered by my ui refactoring plan. I will make config the
source of truth, without sacrificing performance much.

> > > > About whitelist vs blacklist, I think it's less important. I prefer the
> > > > current situation.  
> > > 
> > > It defines the default behavior of chg, whether or not chg is permissive for
> > > third-party extensions which process their config values in undesired way.
> > > IMHO it's important property of chg.  
> > 
> > But as I said, my main concern about listing what *not* to hash is
> > unnecessary processes. The list will also be unfriendly for extensions to
> > change (if they need to) since all extensions must agree together, only that
> > can a section be added, which is practically impossible.
> 
> As long as the section is specific to the extension, the extension can add
> its own section to the white/black list.

It's assuming an extension won't read other extensions' config sections.
However, our recommended practice is to reuse existing config sections (I'm
sure I read it in the wiki but unable to find it now).

> It sounds like you're urged to reduce the number of chg processes, though
> I don't know why it's such important. In that case, I agree the not-to-hash
> list isn't acceptable. Also, I don't think your auto-learner can be the
> permanent solution since we'll have to fix chg-unfriendly extensions anyway.

Haha, not that "urged". Our internal users have all kinds of setups and
requirements and I try to satisfy as many reasonable requirements as I can.

Yes, I agree the leaner approach can solve most issues but does not handle
100% cases. Having some chg-specific API seems inevitable.

I think it's time to discuss about the formal confighash API, as changing
private variables is hacky. I will start a new thread.
 
> Instead, I think you can make the auto-learner extension and enable it as
> a temporary way around. The patch 3, [(section, name)] list, seems good to
> include in the core.

That's a good idea. I'll do it.


More information about the Mercurial-devel mailing list