[PATCH 3 of 5] phases: move root phase assignment to it's own function

Durham Goode durham at fb.com
Mon Oct 13 12:59:00 CDT 2014


On 10/12/14 11:33 PM, Pierre-Yves David wrote:
>
>
> On 10/09/2014 12:22 PM, Durham Goode wrote:
>> # HG changeset patch
>> # User Durham Goode <durham at fb.com>
>> # Date 1412707357 25200
>> #      Tue Oct 07 11:42:37 2014 -0700
>> # Node ID 9291d6a89c9b9bf93cbb920c98795fa13fbd6fa0
>> # Parent  34e934d15e8c9a8c5809dc1909cbdfcba1017603
>> phases: move root phase assignment to it's own function
>>
>> This moves the initial root phase assignment to it's own function. 
>> Future
>> patches which make phase calculations lazy will use this function to 
>> pre-fill
>> certain phases which can be deduced from the roots.
>>
>> diff --git a/mercurial/phases.py b/mercurial/phases.py
>> --- a/mercurial/phases.py
>> +++ b/mercurial/phases.py
>> @@ -167,6 +167,8 @@ class phasecache(object):
>>           if self._phaserevs is None:
>>               repo = repo.unfiltered()
>>               revs = [public] * len(repo.changelog)
>> +            self._phaserevs = revs
>> +            self._populatephaseroots(repo)
>>               for phase in trackedphases:
>>                   roots = map(repo.changelog.rev, 
>> self.phaseroots[phase])
>>                   if roots:
>> @@ -174,11 +176,27 @@ class phasecache(object):
>>                           revs[rev] = phase
>>                       for rev in repo.changelog.descendants(roots):
>>                           revs[rev] = phase
>> -            self._phaserevs = revs
>>           return self._phaserevs
>> +
>>       def invalidate(self):
>>           self._phaserevs = None
>>
>> +    def _populatephaseroots(self, repo):
>> +        """Fills the _phaserevs cache with phases for the roots.
>> +        """
>> +        cl = repo.changelog
>> +        phaserevs = self._phaserevs
>> +        for phase in trackedphases:
>> +            roots = map(cl.rev, self.phaseroots[phase])
>> +            for root in roots:
>> +                phaserevs[root] = phase
>> +                # We know that the parent of a root has a phase less 
>> than
>> +                # the root, so if phase == 1, the parent must be 
>> phase 0.
>> +                if phase == 1:
>> +                    for parent in cl.parentrevs(root):
>> +                        if parent != -1:
>> +                            phaserevs[parent] = 0
>> +
>
> The second part look like an addition and does not belong into this 
> code movement patch. Should be in an independant patches and will fit 
> well in your next series that makes the computation lazy.
>
> I've dropped that part and queued the first three.
I think the phase == 1 logic fits well enough into this patch, without 
increasing it's complexity or reviewability, that requiring it be split 
out is a waste of both of our times.


More information about the Mercurial-devel mailing list