[PATCH] lfs: register the flag processors per repository

[Gregory Szorc]

On Thu, Oct 4, 2018 at 6:09 AM Yuya Nishihara <yuya at tcha.org> wrote:

> On Thu, 04 Oct 2018 00:39:12 -0400, Matt Harbison wrote:
> > # HG changeset patch
> > # User Matt Harbison <matt_harbison at yahoo.com>
> > # Date 1538626646 14400
> > #      Thu Oct 04 00:17:26 2018 -0400
> > # Node ID e010a7be6dc96ea7d48be81a7c5e8a8ed8bf6c31
> > # Parent  7a347d362a455d84bccf34347171d89724b9c9df
> > lfs: register the flag processors per repository
>
> > +    if b'lfs' in requirements:
> > +        options[b'enableextstored'] = True
> > +
> >      if repository.NARROW_REQUIREMENT in requirements:
> >          options[b'enableellipsis'] = True
> >
> > diff --git a/mercurial/revlog.py b/mercurial/revlog.py
> > --- a/mercurial/revlog.py
> > +++ b/mercurial/revlog.py
> > @@ -110,6 +110,26 @@ parsers = policy.importmod(r'parsers')
> >      REVIDX_ISCENSORED: None,
> >  }
> >
> > +# Flag processors for REVIDX_EXTSTORED.
> > +def extstoredreadprocessor(rl, text):
> > +    raise error.ProgrammingError(b'extstored flags processor enabled
> without'
> > +                                 b' wrapping processor function')
> > +
> > +def extstoredwriteprocessor(rl, text):
> > +    raise error.ProgrammingError(b'extstored flags processor enabled
> without'
> > +                                 b' wrapping processor function')
> > +
> > +def extstoredrawprocessor(rl, text):
> > +    raise error.ProgrammingError(b'extstored flags processor enabled
> without'
> > +                                 b' wrapping processor function')
> > +
> > +# Lambdas are needed here so that these methods can be wrapped by lfs.
> > +extstoredprocessor = (
> > +    lambda rl, text: extstoredreadprocessor(rl, text),
> > +    lambda rl, text: extstoredwriteprocessor(rl, text),
> > +    lambda rl, text: extstoredrawprocessor(rl, text),
> > +)
> > +
> >  # Flag processors for REVIDX_ELLIPSIS.
> >  def ellipsisreadprocessor(rl, text):
> >      return text, False
> > @@ -405,6 +425,8 @@ class revlog(object):
> >                  self._srdensitythreshold =
> opts['sparse-read-density-threshold']
> >              if 'sparse-read-min-gap-size' in opts:
> >                  self._srmingapsize = opts['sparse-read-min-gap-size']
> > +            if opts.get('enableextstored'):
> > +                self._flagprocessors[REVIDX_EXTSTORED] =
> extstoredprocessor
>
> I feel it's awkward that we have to wrap the stub extstoredprocessor and
> pass in enableextstored=True.
>
> I don't have a concrete idea, but maybe we can provide an API to register
> a stock flagprocessor which won't be copied to the revlog instance by
> default.
>

I also found this a bit awkward.

I wrote up a patch a few weeks ago that added an "extraflagprocessors"
argument or some such to revlog.__init__ that would allow extra flag
processors to be passed in. I never submitted it because I had originally
wrote it for censoring and I solved that problem by moving the censor flag
processor into revlog.py and introduced the "censorable=False" argument
instead.

But the flag processor for LFS is in the same boat and we may want to do
something similar. But since LFS's flag processors require code in the LFS
extension, we can't simply move the flag processors code to revlog.py.

I think our best bet is to pass a flag processor into revlog.__init__ -
either via an argument or as a key in the store/opener options. Store
options is simpler, as we'd only need to teach revlog.py to add extra flag
processors defined in a store options key and the LFS extension would only
need to monkeypatch localrepo.resolverevlogstorevfsoptions(). Adding an
argument to revlog.__init__ requires a bit more work to thread through
everything. If the LFS flag processor code were in core, I'd be fine with
introducing that argument. But since it isn't, I think going through the
store options is the way to go.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://www.mercurial-scm.org/pipermail/mercurial-devel/attachments/20181004/89dfde40/attachment-0001.html>