[PATCH 02 of 10 sparse V2] sparse: clean up config signature code

[Gregory Szorc]

On Thu, Jul 6, 2017 at 4:57 PM, Martin von Zweigbergk <martinvonz at google.com
> wrote:

> On Thu, Jul 6, 2017 at 4:36 PM, Gregory Szorc <gregory.szorc at gmail.com>
> wrote:
> > # HG changeset patch
> > # User Gregory Szorc <gregory.szorc at gmail.com>
> > # Date 1499382096 25200
> > #      Thu Jul 06 16:01:36 2017 -0700
> > # Node ID 766f7cb3a49bde4dc740870d6e34848eb64743a6
> > # Parent  fea9f0ce2a1c18647087589d5bb23115cdf81419
> > sparse: clean up config signature code
> >
> > Before, 0 was being used as the default signature value and we cast
> > the int to a string. We also handled I/O exceptions manually.
> >
> > The new code uses cfs.tryread() so we always feed data into the
> > hasher. The empty string does hash and and should be suitable
> > for input into a cache key.
> >
> > The changes made the code simple enough that the separate checksum
> > function could be inlined.
> >
> > diff --git a/mercurial/sparse.py b/mercurial/sparse.py
> > --- a/mercurial/sparse.py
> > +++ b/mercurial/sparse.py
> > @@ -132,10 +132,6 @@ def activeprofiles(repo):
> >  def invalidatesignaturecache(repo):
> >      repo._sparsesignaturecache.clear()
> >
> > -def _checksum(self, path):
> > -    data = self.vfs.read(path)
> > -    return hashlib.sha1(data).hexdigest()
> > -
> >  def configsignature(repo, includetemp=True):
> >      """Obtain the signature string for the current sparse configuration.
> >
> > @@ -148,25 +144,18 @@ def configsignature(repo, includetemp=Tr
> >      if includetemp:
> >          tempsignature = cache.get('tempsignature')
> >      else:
> > -        tempsignature = 0
> > +        tempsignature = '0'
>
> '0' is unusual for a placeholder string (it's usually an empty
> string), but doesn't matter :-)
>
> >
> >      if signature is None or (includetemp and tempsignature is None):
> > -        signature = 0
> > -        try:
> > -            signature = _checksum('sparse')
> > -        except (OSError, IOError):
> > -            pass
> > +        signature = hashlib.sha1(repo.vfs.tryread(
> 'sparse')).hexdigest()
>
> tryread does not catch other OSError and non-ENOENT IOError. Does that
> mean that e.g. a present-but-unreadable .hg/sparse used to be okay,
> but will now raise an exception? Desirable or not?
>

Correct. I think it is desirable: if .hg/sparse is there, I think you
should require reading it to open the repo. Otherwise, you may have a
sparse working directory and dirstate will complain about a bunch of
missing files due to inability to filter via the unloadable sparse config.


>
> >          cache['signature'] = signature
> >
> > -        tempsignature = 0
> >          if includetemp:
> > -            try:
> > -                tempsignature = _checksum('tempsparse')
> > -            except (OSError, IOError):
> > -                pass
> > +            raw = repo.vfs.tryread('tempsparse')
> > +            tempsignature = hashlib.sha1(raw).hexdigest()
> >              cache['tempsignature'] = tempsignature
> >
> > -    return '%s %s' % (str(signature), str(tempsignature))
> > +    return '%s %s' % (signature, tempsignature)
> >
> >  def writeconfig(repo, includes, excludes, profiles):
> >      """Write the sparse config file given a sparse configuration."""
> > _______________________________________________
> > Mercurial-devel mailing list
> > Mercurial-devel at mercurial-scm.org
> > https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://www.mercurial-scm.org/pipermail/mercurial-devel/attachments/20170706/0dece311/attachment.html>