[PATCH] lfs: register the flag processors per repository

Matt Harbison mharbison72 at gmail.com
Fri Oct 5 00:48:25 EDT 2018


On Thu, 04 Oct 2018 14:35:49 -0400, Gregory Szorc  
<gregory.szorc at gmail.com> wrote:

> 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.

Should the revlog.addflagprocessor() method be copied to localrepo so that  
it still does the validation, and can also hide the fact that it's being  
tacked on to the store options?


More information about the Mercurial-devel mailing list