[PATCH 1 of 7] tags: extract fnode retrieval in its own function

[Ryan McElroy]

On 3/28/17 7:16 AM, Pierre-Yves David wrote:
> # HG changeset patch
> # User Pierre-Yves David <pierre-yves.david at ens-lyon.org>
> # Date 1490673691 -7200
> #      Tue Mar 28 06:01:31 2017 +0200
> # Node ID 1e77e505e7bacf59d1200714dc92e827523d7301
> # Parent  e6fd7930cf0b37710e379de37b7d87d5c1ea5dc9
> # EXP-Topic tags
> tags: extract fnode retrieval in its own function
>
> My main goal here is to be able to reuse this logic easily. As a side effect
> this important logic is now insulated and the code is clearer.
>
> diff --git a/mercurial/tags.py b/mercurial/tags.py
> --- a/mercurial/tags.py
> +++ b/mercurial/tags.py
> @@ -341,15 +341,27 @@ def _readtagcache(ui, repo):
>           # potentially expensive search.
>           return ([], {}, valid, None, True)
>   
> -    starttime = util.timer()
>   
>       # Now we have to lookup the .hgtags filenode for every new head.
>       # This is the most expensive part of finding tags, so performance
>       # depends primarily on the size of newheads.  Worst case: no cache
>       # file, so newheads == repoheads.
> +    cachefnode = _getfnodes(ui, repo, repoheads)
> +
> +    # Caller has to iterate over all heads, but can use the filenodes in
> +    # cachefnode to get to each .hgtags revision quickly.
> +    return (repoheads, cachefnode, valid, None, True)
> +
> +def _getfnodes(ui, repo, nodes):
> +    """return fnode for .hgtags in a list of node

This comment doesn't quite parse for me. Maybe it should be "return list 
of .hgtags fnodes for tags computation"

> +
> +    return value is a {node: fnode} mapping. their will be no entry for node

s/their/there
s/node/nodes (at end of line)

Capitalization nitpicks: Return and There should be capitalized I think.

> +    without a '.hgtags' file.
> +    """
> +    starttime = util.timer()
>       fnodescache = hgtagsfnodescache(repo.unfiltered())
>       cachefnode = {}
> -    for head in reversed(repoheads):
> +    for head in reversed(nodes):
>           fnode = fnodescache.getfnode(head)
>           if fnode != nullid:
>               cachefnode[head] = fnode
> @@ -361,10 +373,7 @@ def _readtagcache(ui, repo):
>              '%d/%d cache hits/lookups in %0.4f '
>              'seconds\n',
>              fnodescache.hitcount, fnodescache.lookupcount, duration)
> -
> -    # Caller has to iterate over all heads, but can use the filenodes in
> -    # cachefnode to get to each .hgtags revision quickly.
> -    return (repoheads, cachefnode, valid, None, True)
> +    return cachefnode
>   
>   def _writetagcache(ui, repo, valid, cachetags):
>       filename = _filename(repo)
>