[PATCH] chgserver: wrap ui without calling its constructor

Jun Wu quark at fb.com
Sat Mar 12 00:32:19 EST 2016


On 03/12/2016 05:04 AM, Yuya Nishihara wrote:
> I really don't like this sort of hacks that just hide bugs. If it is a bug
> of blackboxui, it must be fixed in blackbox.

Why is it a hack? I see the pattern "x.__class__ = anotherclass" being
commonly used in other places (dispatch.py, extensions.py, eol.py, keyword.py,
largefiles/reposetup.py, histedit.py, blackbox.py, color.py and fb extensions).

It's not only just about blackbox. It can be any other extension that has a
broken wrapui logic. The chg server will enter a bad state that never recovers
automatically and the client won't print useful error message. The only way to
get actual error message is to use strace.

I think it's severe enough and should be fixed. An extra benefit of skipping
ui.__init__ is to save some time, good for perf.

> It complicates the scope of csystem variable more. I'd rather remove csystem
> from __init__.

I tried setting srcui._csystem directly but due to the current bad ui object
logic it does not work well if others replace our ui object and forgot to
copy _csystem.

I think this is a separate issue about ui/config object design. I have some
ideas on it. If I have free time I plan to get rid of all ui.copy()s and make
config read from files immutabler.


More information about the Mercurial-devel mailing list