[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