[PATCH 3 of 5 flagprocessor v6] revlog: pass revlog flags to addrevision

Pierre-Yves David pierre-yves.david at ens-lyon.org
Fri Dec 30 05:24:58 EST 2016



On 12/24/2016 05:36 PM, Remi Chaintron wrote:
> # HG changeset patch
> # User Remi Chaintron <remi at fb.com>
> # Date 1482451731 18000
> #      Thu Dec 22 19:08:51 2016 -0500
> # Node ID b1d220e584e6fc861a40a5aeefa0c3b19ae09126
> # Parent  d0476160913323da1dada49ae46e72d6a7c17d78
> revlog: pass revlog flags to addrevision
>
> Adding the ability to passing flags to addrevision instead of simply passing
> default flags to _addrevision will allow extensions relying on flag transforms
> to wrap around addrevision() in order to update revlog flags.
>
> The first use case of this patch will be the lfs extension marking nodes as
> stored externally when the contents are larger than the defined threshold.
>
> One of the reasons leading to setting flags in addrevision() wrappers in the
> flag processor design is that it allows to detect files larger than the 2GB
> limit before the check is performed, which allows lfs to transform the contents
> into metadata.

As we discussed over voice, that approach seems fine. However, there is 
a strange, apparently unrelated code-drop in this patch. See below.

> diff --git a/mercurial/revlog.py b/mercurial/revlog.py
> --- a/mercurial/revlog.py
> +++ b/mercurial/revlog.py
> @@ -1235,11 +1235,6 @@
>          if rev is None:
>              rev = self.rev(node)
>
> -        # check rev flags
> -        if self.flags(rev) & ~REVIDX_KNOWN_FLAGS:
> -            raise RevlogError(_('incompatible revision flag %x') %
> -                              (self.flags(rev) & ~REVIDX_KNOWN_FLAGS))
> -

That seems like an related code-drop in 'revision(…)'. I don't think you 
intended to do that. Can you drop that from this patch (and, if needed, 
reintroduce it on an appropriate patches documenting why it is something 
we need to do)

>          chain, stopped = self._deltachain(rev, stoprev=cachedrev)
>          if stopped:
>              text = self._cache[2]
> @@ -1332,7 +1327,7 @@
>          self._chunkclear()
>
>      def addrevision(self, text, transaction, link, p1, p2, cachedelta=None,
> -                    node=None):
> +                    node=None, flags=REVIDX_DEFAULT_FLAGS):
>          """add a revision to the log
>
>          text - the revision data to add
> @@ -1343,6 +1338,7 @@
>          node - nodeid of revision; typically node is not specified, and it is
>              computed by default as hash(text, p1, p2), however subclasses might
>              use different hashing method (and override checkhash() in such case)
> +        flags - the known flags to set on the revision
>          """
>          if link == nullrev:
>              raise RevlogError(_("attempted to add linkrev -1 to %s")
> @@ -1363,8 +1359,7 @@
>          ifh = self.opener(self.indexfile, "a+", checkambig=self._checkambig)
>          try:
>              return self._addrevision(node, text, transaction, link, p1, p2,
> -                                     REVIDX_DEFAULT_FLAGS, cachedelta, ifh, dfh,
> -                                     raw=False)
> +                                     flags, cachedelta, ifh, dfh, raw=False)
>          finally:
>              if dfh:
>                  dfh.close()
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel at mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
>

-- 
Pierre-Yves David


More information about the Mercurial-devel mailing list