[PATCH 10 of 10] branchmap: move validity logic in the object itself

[Augie Fackler]

Other than my question on patch 4 and Dave S's English tweaks, I think this series is probably a good start, but I wonder why read isn't moved into the branchmap class as well? Or maybe I overlooked that.

On Dec 21, 2012, at 8:48 PM, Pierre-Yves David <pierre-yves.david at ens-lyon.org> wrote:

> # HG changeset patch
> # User Pierre-Yves David <pierre-yves.david at ens-lyon.org>
> # Date 1356140640 -3600
> # Node ID 08bed0e9a2d5aa130dc84bac6ec5da8e3be7b832
> # Parent  5405ded918f63d80b075cd85edf635bbd87eec7b
> branchmap: move validity logic in the object itself
> 
> In several place, We check if a branchcache is still valid regarding the current
> state of the repository. This changeset puts this logic in a method of the object
> that can be reused when necessary.
> 
> A branch map is considered valid whenever it is up to date or a strict subset of
> the repository state.
> 
> The change will help making branchcache aware of filtered revision.
> 
> diff --git a/mercurial/branchmap.py b/mercurial/branchmap.py
> --- a/mercurial/branchmap.py
> +++ b/mercurial/branchmap.py
> @@ -7,22 +7,22 @@
> 
> from node import bin, hex, nullid, nullrev
> import encoding
> 
> def read(repo):
> -    partial = branchcache()
>     try:
>         f = repo.opener("cache/branchheads")
>         lines = f.read().split('\n')
>         f.close()
>     except (IOError, OSError):
>         return branchcache()
> 
>     try:
>         last, lrev = lines.pop(0).split(" ", 1)
>         last, lrev = bin(last), int(lrev)
> -        if lrev >= len(repo) or repo[lrev].node() != last:
> +        partial = branchcache(tipnode=last, tiprev=lrev)
> +        if not partial.validfor(repo):
>             # invalidate the cache
>             raise ValueError('invalidating branch cache (tip differs)')
>         for l in lines:
>             if not l:
>                 continue
> @@ -30,12 +30,10 @@ def read(repo):
>             label = encoding.tolocal(label.strip())
>             if not node in repo:
>                 raise ValueError('invalidating branch cache because node '+
>                                  '%s does not exist' % node)
>             partial.setdefault(label, []).append(bin(node))
> -        partial.tipnode = last
> -        partial.tiprev = lrev
>     except KeyboardInterrupt:
>         raise
>     except Exception, inst:
>         if repo.ui.debugflag:
>             repo.ui.warn(str(inst), '\n')
> @@ -45,16 +43,13 @@ def read(repo):
> 
> 
> def updatecache(repo):
>     repo = repo.unfiltered()  # Until we get a smarter cache management
>     cl = repo.changelog
> -    tip = cl.tip()
>     partial = repo._branchcache
> -    if partial is not None and partial.tipnode == tip:
> -        return
> 
> -    if partial is None or partial.tipnode not in cl.nodemap:
> +    if partial is None or partial.validfor(repo):
>         partial = read(repo)
> 
>     catip = repo._cacheabletip()
>     # if partial.tiprev == catip: cache is already up to date
>     # if partial.tiprev >  catip: we have uncachable element in `partial` can't
> @@ -78,10 +73,21 @@ class branchcache(dict):
>     def __init__(self, entries=(), tipnode=nullid, tiprev=nullrev):
>         super(branchcache, self).__init__(entries)
>         self.tipnode = tipnode
>         self.tiprev = tiprev
> 
> +    def validfor(self, repo):
> +        """Is the cache content valide regarding a repo
> +
> +        - False when cached tipnode are unknown or if we detect a strip.
> +        - True when cache is up to date or a subset of current repo."""
> +        try:
> +            return self.tipnode == repo.changelog.node(self.tiprev)
> +        except IndexError:
> +            return False
> +
> +
>     def write(self, repo):
>         try:
>             f = repo.opener("cache/branchheads", "w", atomictemp=True)
>             f.write("%s %s\n" % (hex(self.tipnode), self.tiprev))
>             for label, nodes in self.iteritems():
> @@ -160,12 +166,12 @@ class branchcache(dict):
>                 del self[branch]
>         try:
>             node = cl.node(self.tiprev)
>         except IndexError:
>             node = None
> -        if ((self.tipnode != node)
> -            or (self.tipnode in droppednodes)):
> +        if ((not self.validfor(repo)) or (self.tipnode in droppednodes)):
> +
>             # cache key are not valid anymore
>             self.tipnode = nullid
>             self.tiprev = nullrev
>             for heads in self.values():
>                 tiprev = max(cl.rev(node) for node in heads)