[PATCH STABLE] revlog: make sure we never use sparserevlog without general delta (issue6056)

[Gregory Szorc]

On Tue, Jan 29, 2019 at 3:44 AM Yuya Nishihara <yuya at tcha.org> wrote:

> On Mon, 28 Jan 2019 21:20:07 +0900, Yuya Nishihara wrote:
> > On Fri, 25 Jan 2019 15:00:52 -0500, Boris Feld wrote:
> > > # HG changeset patch
> > > # User Boris Feld <boris.feld at octobus.net>
> > > # Date 1548372167 18000
> > > #      Thu Jan 24 18:22:47 2019 -0500
> > > # Branch stable
> > > # Node ID c7661fdcaeac457aa5b8ae6b6a10b259713934b9
> > > # Parent  26ee61c33dee366576a0f7266fea9834985e78ec
> > > # EXP-Topic issue6056
> > > # Available At https://bitbucket.org/octobus/mercurial-devel/
> > > #              hg pull https://bitbucket.org/octobus/mercurial-devel/
> -r c7661fdcaeac
> > > revlog: make sure we never use sparserevlog without general delta
> (issue6056)
> > >
> > > We are getting user report where the delta code tries to use
> `sparse-revlog`
> > > logic on repository where `generaldelta` is disabled. This can't work
> so we
> > > ensure the two booleans have a consistent value.
> > >
> > > Creating this kind of repository is not expected to be possible and we
> weren't
> > > able to create one for testing purpose. But if users run into this, we
> better
> > > put stronger protection.
>
> > > --- a/mercurial/revlog.py
> > > +++ b/mercurial/revlog.py
> > > @@ -497,6 +497,9 @@ class revlog(object):
> > >          else:
> > >              raise error.RevlogError(_('unknown version (%d) in revlog
> %s') %
> > >                                      (fmt, self.indexfile))
> > > +        # sparse-revlog can't be on without general-delta (issue6056)
> > > +        if not self._generaldelta:
> > > +            self._sparserevlog = False
> >
> > Maybe this can be handled at localrepo.ensurerequirements*() or
> > esolverevlogstorevfsoptions(), where ui is available so we can add
> > some debug/warning messages.
>
> Okay, it doesn't work since generaldelta is a revlog flag, whereas
> sparserevlog
> is a global requirement. I don't know if sparserevlog should be a revlog
> flag
> or not, but anyway only thing we can do here is to turn off the sparse, as
> your
> patch does.
>

Sparse should have a revlog flag in addition to a repo requirement. My
understanding is we already shipped sparse support in 4.8. This means we
can't introduce a revlog flag (and corresponding repo requirement) without
a BC change. I started a series to introduce revlog version 2 (mainly to
support zstd compression in revlogs). We can do things more properly in
revlog version 2. But I fear the lack of a sparse revlog flag will just
have to be an historical wart since we've already shipped it.


>
> Can you add some tests with crafted repo?
>
> Also, I noticed format.sparse-revlog isn't documented. Since it's
> incompatible
> with older versions, it needs some doc.
> _______________________________________________
> 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/20190129/a903c2db/attachment.html>