[PATCH 1 of 8] transaction: introduce "changes" dictionary to precisely track updates

[Pierre-Yves David]

So, what is the status of this series?
I've important speedup work stuck behind it so I would like to see the 
topic move forward.

On 05/03/2017 09:09 PM, Pierre-Yves David wrote:
>
>
> On 05/03/2017 03:46 PM, Yuya Nishihara wrote:
>> On Wed, 3 May 2017 15:06:05 +0200, Pierre-Yves David wrote:
>>> On 05/03/2017 09:51 AM, Yuya Nishihara wrote:
>>>> On Wed, 03 May 2017 01:43:38 +0200, Pierre-Yves David wrote:
>>>>> # HG changeset patch
>>>>> # User Pierre-Yves David <pierre-yves.david at ens-lyon.org>
>>>>> # Date 1493742678 -7200
>>>>> #      Tue May 02 18:31:18 2017 +0200
>>>>> # Branch stable
>>>>> # Node ID 6697da7c4eab3fbe3588a2f91fa3f99b16f808ac
>>>>> # Parent  fbb5f4bf94928b98fa87871e84bb2ef972ec2d51
>>>>> # EXP-Topic obscache
>>>>> # Available At
>>>>> https://www.mercurial-scm.org/repo/users/marmoute/mercurial/
>>>>> #              hg pull
>>>>> https://www.mercurial-scm.org/repo/users/marmoute/mercurial/ -r
>>>>> 6697da7c4eab
>>>>> transaction: introduce "changes" dictionary to precisely track updates
>>>>>
>>>>> The transaction is already tracking some data intended for hooks (in
>>>>> 'hookargs'). However, that information is minimal as we optimise for
>>>>> passing data to other processes through environment variables.
>>>>> There are
>>>>> multiple places were we could use more complete and lower level
>>>>> information locally (eg: cache update, better report of changes to
>>>>> hooks, etc...).
>>>>>
>>>>> For this purpose we introduces a 'changes' dictionary on the
>>>>> transaction.  It is intended to track every changes happening to the
>>>>> repository (eg: new revs, bookmarks move, phases move, obs-markers,
>>>>> etc).
>>>>>
>>>>> For now we just adds the 'changes' dictionary. We'll adds more
>>>>> tracking
>>>>> and usages over time.
>>>>>
>>>>> diff --git a/mercurial/transaction.py b/mercurial/transaction.py
>>>>> --- a/mercurial/transaction.py
>>>>> +++ b/mercurial/transaction.py
>>>>> @@ -137,6 +137,10 @@ class transaction(object):
>>>>>              releasefn = lambda tr, success: None
>>>>>          self.releasefn = releasefn
>>>>>
>>>>> +        # A dict dedicated to precisely tracking the changes
>>>>> introduced in the
>>>>> +        # transaction.
>>>>> +        self.changes = {}
>>>>
>>>> I'm not sure if it's good idea to add more free-form dict to the
>>>> transaction
>>>> class, since that would make code less manageable in general.
>>>
>>> Can you elaborate on your worries here?
>>>
>>> The content of the dictionnary should be strictly defined by the
>>> localrepo object, and the data should be filled at low level by code
>>> that already requires a transaction to be present. So I think the result
>>> will be quite manageable.
>>>
>>> We -need- something else than "hookargs" to carry transaction related
>>> information. For example we needs to tracks bookmark movement, phases
>>> changes and tags movement. The "hookargs" dictionnary is not appropriate
>>> to track such data.
>>
>> My concern is unclear, sorry. The transaction code seems hard to
>> review because
>> it has arbitrary hooks, weakrefs in them, hookargs dict, etc. And now
>> 'changes'
>> dict. They all seem to make things too abstract.
>>
>> I believe the result looks better overall than before, but I can't say
>> the
>> transaction itself gets better.
>
> The net result at the end of my work should be clearer:
>
> - code tracking changes should be clearer and safer,
> - code reacting to changes would have more well defined spot to react to
> these changes.
>
> (as a bonus, we might be able to compute the hookargs, dict for the
> 'changes' one at some point)
>
> Cheers,
>

-- 
Pierre-Yves David