[PATCH 2 of 6] revlog: add flagprocessor

RĂ©mi Chaintron remi at fb.com
Wed Nov 16 08:59:21 EST 2016



On 11/9/16, 5:17 PM, "Augie Fackler" <raf at durin42.com> wrote:



    On Thu, Oct 27, 2016 at 04:04:00PM +0100, Remi Chaintron wrote:

    > # HG changeset patch

    > # User Remi Chaintron <remi at fb.com>

    > # Date 1477579974 -3600

    > #      Thu Oct 27 15:52:54 2016 +0100

    > # Branch stable

    > # Node ID c16fc21427f7d3db2adce1d34b6a8f7fb04f9ca7

    > # Parent  7423a5fcbc05f69c84a9e0eb4d4b4a94444d83f0

    > revlog: add flagprocessor

    >

    > This adds a mechanism for extensions/subclasses of revlog to register transforms

    > on revlog operations and manage flags priority to handle non-reflexive

    > operations and lays the groundwork for the lfs extension's implementation.



    [...]



    >

    > diff --git a/mercurial/revlog.py b/mercurial/revlog.py

    > --- a/mercurial/revlog.py

    > +++ b/mercurial/revlog.py

    > @@ -76,6 +76,76 @@

    >

    >  _nullhash = hashlib.sha1(nullid)

    >

    > +class flagprocessor(object):

    > +    """process revlog objects contents based on flags

    > +

    > +    flagprocessor objects are the interface between revlog objects and

    > +    subclasses/extensions needing to change or update the contents of revlog

    > +    objects, based on flags.

    > +

    > +    The role of the flagprocessor is to allow defining priorities between known

    > +    extensions and register transforms to be applied on detecting such flags on

    > +    revlog objects.

    > +

    > +    Defining priorities allows for non-reflexive operations to be composed and

    > +    gracefully handled in reverse order depending on the operation on the revlog

    > +    object (read or write).

    > +    """

    > +

    > +    def __init__(self, revlogobject):

    > +        self.flagsbypriority = [

    > +            REVIDX_ISCENSORED,

    > +        ]

    > +        self.transformmap = {}

    > +        self.revlogobject = revlogobject

    > +

    > +    def register(self, transformmap):

   > +        """Register transforms to be applied for flags.

    > +

    > +        ``transformmap`` - a map of flag to transform

    > +        """

    > +        for flag in transformmap:

    > +            if flag in self.flagsbypriority:

    > +                self.transformmap[flag] = transformmap[flag]



    It weirds me out a little to have a registrar function, but then to

    have a flagsbypriority attribute that I guess clients just twiddle

    manually. Could we come up with a better registrar mechanism here? I

    guess we're trying to allow for a stable total ordering of the flag

    processors? Can you elaborate why we'd need that?



The need for keeping the ordering of the flags when applying transforms on the contents of the filelog objects comes from the need to handle non-commutative operations (contrary to the commented erroneous non-reflexive).

In such cases, the operations need to be applied in the reverse order when read, compared to when written: A(B(text) -> newtext vs. B(A(newtext) -> text.

It is therefore important the ordering is kept, which indeed might create problems when opening the flagsbypriority attribute to modification by clients. Would you feel better about moving the ordering of such flags out of the flag processor in this case?




-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://www.mercurial-scm.org/pipermail/mercurial-devel/attachments/20161116/4fa4872e/attachment.html>


More information about the Mercurial-devel mailing list