[PATCH evolve-ext] evolve: do not set experimental.evolution in uisetup

[Jun Wu]

On 04/06/2016 06:58 PM, Pierre-Yves David wrote:
> On 04/06/2016 10:27 AM, Jun Wu wrote:
>> On 04/03/2016 11:49 PM, Jun Wu wrote:
>>> On 04/03/2016 09:01 PM, Pierre-Yves David wrote:
>>>> I don't think this will work. There is third party code (especially…
>>>> mercurial core) that rely on the extension to set this when enabled.
>>>> And I assume this would break it all.
>>>
>>> Thanks. I didn't realize obsolete.py is reading it.
>>>
>>> How about doing these instead: - evolve.py: remove setconfig -
>>> evolve.py: set obsolete._enabled = True
>>
>> Gentle ping. If we want to prioritize evolve chg tests fixes, we need to
>> have a way to get rid of setconfig. I think it's okay to do the above.
>> Opinions?
>
> Unfortunately is a bit too short for me to understand what you are actually
>  proposing here. Can you elaborate?

Okay. So first let's talk about why the following pattern has issues with chg:

   def uisetup(ui):
       if not ui.getconfig('x', 'y'):
           ui.setconfig('x', 'y', 'z')

Currently, when reloading config per request, chg will scan all the old config
items not read from files by looking at source and apply them as is. This means
the side effect of setconfig will be applied as is without the "if" condition
(also because uisetup is executed only once).

It does not work with whitelisting the x.y config because:
1) chgserver.uisetup runs after all other extensions (i.e. other extensions
have done setconfig()s already), so the confighash chgserver gets is the one
after setconfig()s
2) there is no way to get the original value overrided by setconfig().

That can also be solved by my big ui/config refactoring plan because it would
allow accessing the values being overrided and calculating confighash in a more
clean way. But as the refactoring is not trivial., it's unlikely to happen soon.

Back to evolve, the attempt here is to get rid of the pattern above. Using
ui.setconfig and ui.getconfig sounds like global variables to me so I think
it's okay to move back to obsolete._enabled temporarily. Alternatively it may
make sense to blacklist the test for chg temporarily. By temporarily I mean
until I get the ui/config refactoring done but I don't have an ETA for the
refactoring now.