[PATCH STABLE] repoview: fix corrupted hiddencache crash Mercurial (issue5042)

[Matt Mackall]

On Wed, 2016-01-20 at 20:46 +0000, Laurent Charignon wrote:
> > On Jan 20, 2016, at 10:18 AM, Gregory Szorc <gregory.szorc at gmail.com> wrote:
> > 
> > 
> > 
> > > On Jan 20, 2016, at 10:09, Laurent Charignon <lcharignon at fb.com> wrote:
> > > 
> > > # HG changeset patch
> > > # User Laurent Charignon <lcharignon at fb.com>
> > > # Date 1453313317 28800
> > > #      Wed Jan 20 10:08:37 2016 -0800
> > > # Branch stable
> > > # Node ID 688f61fa0e0a8cc9139a533cb3fe6cb411b1dc8f
> > > # Parent  3203dfe341f962e33256d6475fc3585563db78ad
> > > repoview: fix corrupted hiddencache crash Mercurial (issue5042)
> > > 
> > > Before this patch if the hiddencache existed but was empty, it would crash
> > > mercurial. This patch adds exception handling when reading the hiddencache
> > > to
> > > avoid the issue.
> > > When encountering a corrupted cache file we print a devel warning. There
> > > would
> > > be no point in issuing a normal warning as the user wouldn't be able to do
> > > anything about the situation.
> > > 
> > > The warning looks like:
> > > 
> > > devel-warn: corrupted hidden cache, removing it at: /path/to/repoview.py
> > > 
> > > diff --git a/mercurial/repoview.py b/mercurial/repoview.py
> > > --- a/mercurial/repoview.py
> > > +++ b/mercurial/repoview.py
> > > @@ -150,6 +150,12 @@
> > >                count = len(data) / 4
> > >                hidden = frozenset(struct.unpack('>%ii' % count, data))
> > >        return hidden
> > > +    except struct.error:
> > > +        repo.ui.develwarn('corrupted hidden cache, removing it')
> > > +        if fh:
> > > +            fh.close()
> > > +        repo.vfs.unlink(cachefile)
> > > +        return hidden
> > 
> > I don't have the full source in front of me. Do we handle read errors the
> > same way? Do we have test coverage for e.g. permissions failure on file
> > reading? If not, we should add it.

I think deleting the file is not the right policy. We should overwrite it (if we
have permission) when refreshing the cache, but otherwise silently ignore it.

> What you mention is already handled at a lower level and we should probably
> add test for it in another patch.
> 
> $ chmod 000 .hg/cache/hidden
> $ hg st
> abort: Permission denied: /home/lcharignon/hg/.hg/cache/hidden

This should also be silently ignored.

-- 
Mathematics is the supreme nostalgia of our time.