Patchwork [02,of,12] revlog: split constants into a new `revlogutils.constants` module

login
register
mail settings
Submitter Boris Feld
Date Aug. 18, 2018, 9:27 a.m.
Message ID <4a61a7a37a499387658b.1534584437@FB-lair>
Download mbox | patch
Permalink /patch/33869/
State Accepted
Headers show

Comments

Boris Feld - Aug. 18, 2018, 9:27 a.m.
# HG changeset patch
# User Boris Feld <boris.feld@octobus.net>
# Date 1534378093 -7200
#      Thu Aug 16 02:08:13 2018 +0200
# Node ID 4a61a7a37a499387658b6270ea0b2dadfef86156
# Parent  544933eed9653945bbeb2577fa1f6decf08dbb1f
# EXP-Topic sparse-snapshot
# Available At https://bitbucket.org/octobus/mercurial-devel/
#              hg pull https://bitbucket.org/octobus/mercurial-devel/ -r 4a61a7a37a49
revlog: split constants into a new `revlogutils.constants` module

We want to split some logic out of the main revlog file (the delta computing
logic).  However, this logic needs access to multiple constants related to the
revlog. So we move all revlog related constants into a new module that could
be imported from multiple places.

We don't copy the file (preserving blame history) because there are only a few
moving lines. Also, copying the file would result in annoying merge conflicts
with ongoing work from others contributors.
Gregory Szorc - Aug. 24, 2018, 5:37 p.m.
On Sat, Aug 18, 2018 at 2:27 AM Boris Feld <boris.feld@octobus.net> wrote:

> # HG changeset patch
> # User Boris Feld <boris.feld@octobus.net>
> # Date 1534378093 -7200
> #      Thu Aug 16 02:08:13 2018 +0200
> # Node ID 4a61a7a37a499387658b6270ea0b2dadfef86156
> # Parent  544933eed9653945bbeb2577fa1f6decf08dbb1f
> # EXP-Topic sparse-snapshot
> # Available At https://bitbucket.org/octobus/mercurial-devel/
> #              hg pull https://bitbucket.org/octobus/mercurial-devel/ -r
> 4a61a7a37a49
> revlog: split constants into a new `revlogutils.constants` module
>
> We want to split some logic out of the main revlog file (the delta
> computing
> logic).  However, this logic needs access to multiple constants related to
> the
> revlog. So we move all revlog related constants into a new module that
> could
> be imported from multiple places.
>
> We don't copy the file (preserving blame history) because there are only a
> few
> moving lines. Also, copying the file would result in annoying merge
> conflicts
> with ongoing work from others contributors.
>

I think extracting the constants outside of revlog.py makes perfect sense.
However, if I may paint a bike shed, tying them to the name "revlogutils"
isn't ideal because many of these constants are not revlog-specific. I
think a better place for the storage-agnostic constants is repository.py or
a to-be-invented storage module.

That being said, this change is strictly better and needs to be performed
to unblock future work in this series. So I think I'll queue it and other
patches later in the series. Just keep in mind that we want to support
alternate storage backends in the near future, so removing "revlog" from
naming for things that aren't revlog specific is justified and recommended.
This includes a lot of code related to deltas (because non-revlog stores
will likely still want to store deltas).


>
> diff --git a/contrib/import-checker.py b/contrib/import-checker.py
> --- a/contrib/import-checker.py
> +++ b/contrib/import-checker.py
> @@ -28,6 +28,8 @@ allowsymbolimports = (
>      'mercurial.hgweb.request',
>      'mercurial.i18n',
>      'mercurial.node',
> +    # for revlog to re-export constant to extensions
> +    'mercurial.revlogutils.constants',
>      # for cffi modules to re-export pure functions
>      'mercurial.pure.base85',
>      'mercurial.pure.bdiff',
> diff --git a/mercurial/revlog.py b/mercurial/revlog.py
> --- a/mercurial/revlog.py
> +++ b/mercurial/revlog.py
> @@ -35,6 +35,26 @@ from .node import (
>      wdirrev,
>  )
>  from .i18n import _
> +from .revlogutils.constants import (
> +    FLAG_GENERALDELTA,
> +    FLAG_INLINE_DATA,
> +    LIMIT_DELTA2TEXT,
> +    REVIDX_DEFAULT_FLAGS,
> +    REVIDX_ELLIPSIS,
> +    REVIDX_EXTSTORED,
> +    REVIDX_FLAGS_ORDER,
> +    REVIDX_ISCENSORED,
> +    REVIDX_KNOWN_FLAGS,
> +    REVIDX_RAWTEXT_CHANGING_FLAGS,
> +    REVLOGV0,
> +    REVLOGV1,
> +    REVLOGV1_FLAGS,
> +    REVLOGV2,
> +    REVLOGV2_FLAGS,
> +    REVLOG_DEFAULT_FLAGS,
> +    REVLOG_DEFAULT_FORMAT,
> +    REVLOG_DEFAULT_VERSION,
> +)
>  from .thirdparty import (
>      attr,
>  )
> @@ -51,40 +71,31 @@ from .utils import (
>      stringutil,
>  )
>
> +# blanked usage of all the name to prevent pyflakes constraints
> +# We need these name available in the module for extensions.
> +REVLOGV0
> +REVLOGV1
> +REVLOGV2
> +FLAG_INLINE_DATA
> +FLAG_GENERALDELTA
> +REVLOG_DEFAULT_FLAGS
> +REVLOG_DEFAULT_FORMAT
> +REVLOG_DEFAULT_VERSION
> +REVLOGV1_FLAGS
> +REVLOGV2_FLAGS
> +REVIDX_ISCENSORED
> +REVIDX_ELLIPSIS
> +REVIDX_EXTSTORED
> +REVIDX_DEFAULT_FLAGS
> +REVIDX_FLAGS_ORDER
> +REVIDX_KNOWN_FLAGS
> +REVIDX_RAWTEXT_CHANGING_FLAGS
> +
>  parsers = policy.importmod(r'parsers')
>
>  # Aliased for performance.
>  _zlibdecompress = zlib.decompress
>
> -# revlog header flags
> -REVLOGV0 = 0
> -REVLOGV1 = 1
> -# Dummy value until file format is finalized.
> -# Reminder: change the bounds check in revlog.__init__ when this is
> changed.
> -REVLOGV2 = 0xDEAD
> -FLAG_INLINE_DATA = (1 << 16)
> -FLAG_GENERALDELTA = (1 << 17)
> -REVLOG_DEFAULT_FLAGS = FLAG_INLINE_DATA
> -REVLOG_DEFAULT_FORMAT = REVLOGV1
> -REVLOG_DEFAULT_VERSION = REVLOG_DEFAULT_FORMAT | REVLOG_DEFAULT_FLAGS
> -REVLOGV1_FLAGS = FLAG_INLINE_DATA | FLAG_GENERALDELTA
> -REVLOGV2_FLAGS = REVLOGV1_FLAGS
> -
> -# revlog index flags
> -REVIDX_ISCENSORED = (1 << 15) # revision has censor metadata, must be
> verified
> -REVIDX_ELLIPSIS = (1 << 14) # revision hash does not match data (narrowhg)
> -REVIDX_EXTSTORED = (1 << 13) # revision data is stored externally
> -REVIDX_DEFAULT_FLAGS = 0
> -# stable order in which flags need to be processed and their processors
> applied
> -REVIDX_FLAGS_ORDER = [
> -    REVIDX_ISCENSORED,
> -    REVIDX_ELLIPSIS,
> -    REVIDX_EXTSTORED,
> -]
> -REVIDX_KNOWN_FLAGS = util.bitsfrom(REVIDX_FLAGS_ORDER)
> -# bitmark for flags that could cause rawdata content change
> -REVIDX_RAWTEXT_CHANGING_FLAGS = REVIDX_ISCENSORED | REVIDX_EXTSTORED
> -
>  # max size of revlog with inline data
>  _maxinline = 131072
>  _chunksize = 1048576
> @@ -838,9 +849,6 @@ class revlogoldindex(list):
>              return (0, 0, 0, -1, -1, -1, -1, nullid)
>          return list.__getitem__(self, i)
>
> -# maximum <delta-chain-data>/<revision-text-length> ratio
> -LIMIT_DELTA2TEXT = 2
> -
>  class revlogoldio(object):
>      def __init__(self):
>          self.size = indexformatv0.size
> diff --git a/mercurial/revlogutils/__init__.py
> b/mercurial/revlogutils/__init__.py
> new file mode 100644
> diff --git a/mercurial/revlogutils/constants.py
> b/mercurial/revlogutils/constants.py
> new file mode 100644
> --- /dev/null
> +++ b/mercurial/revlogutils/constants.py
> @@ -0,0 +1,46 @@
> +# revlogdeltas.py - constant used for revlog logic
> +#
> +# Copyright 2005-2007 Matt Mackall <mpm@selenic.com>
> +# Copyright 2018 Octobus <contact@octobus.net>
> +#
> +# This software may be used and distributed according to the terms of the
> +# GNU General Public License version 2 or any later version.
> +"""Helper class to compute deltas stored inside revlogs"""
> +
> +from __future__ import absolute_import
> +
> +from .. import (
> +    util,
> +)
> +
> +# revlog header flags
> +REVLOGV0 = 0
> +REVLOGV1 = 1
> +# Dummy value until file format is finalized.
> +# Reminder: change the bounds check in revlog.__init__ when this is
> changed.
> +REVLOGV2 = 0xDEAD
> +FLAG_INLINE_DATA = (1 << 16)
> +FLAG_GENERALDELTA = (1 << 17)
> +REVLOG_DEFAULT_FLAGS = FLAG_INLINE_DATA
> +REVLOG_DEFAULT_FORMAT = REVLOGV1
> +REVLOG_DEFAULT_VERSION = REVLOG_DEFAULT_FORMAT | REVLOG_DEFAULT_FLAGS
> +REVLOGV1_FLAGS = FLAG_INLINE_DATA | FLAG_GENERALDELTA
> +REVLOGV2_FLAGS = REVLOGV1_FLAGS
> +
> +# revlog index flags
> +REVIDX_ISCENSORED = (1 << 15) # revision has censor metadata, must be
> verified
> +REVIDX_ELLIPSIS = (1 << 14) # revision hash does not match data (narrowhg)
> +REVIDX_EXTSTORED = (1 << 13) # revision data is stored externally
> +REVIDX_DEFAULT_FLAGS = 0
> +# stable order in which flags need to be processed and their processors
> applied
> +REVIDX_FLAGS_ORDER = [
> +    REVIDX_ISCENSORED,
> +    REVIDX_ELLIPSIS,
> +    REVIDX_EXTSTORED,
> +]
> +REVIDX_KNOWN_FLAGS = util.bitsfrom(REVIDX_FLAGS_ORDER)
> +# bitmark for flags that could cause rawdata content change
> +REVIDX_RAWTEXT_CHANGING_FLAGS = REVIDX_ISCENSORED | REVIDX_EXTSTORED
> +
> +# maximum <delta-chain-data>/<revision-text-length> ratio
> +LIMIT_DELTA2TEXT = 2
> diff --git a/setup.py b/setup.py
> --- a/setup.py
> +++ b/setup.py
> @@ -817,6 +817,7 @@ packages = ['mercurial',
>              'mercurial.thirdparty.zope',
>              'mercurial.thirdparty.zope.interface',
>              'mercurial.utils',
> +            'mercurial.revlogutils',
>              'hgext', 'hgext.convert', 'hgext.fsmonitor',
>              'hgext.fsmonitor.pywatchman',
>              'hgext.infinitepush',
>

Patch

diff --git a/contrib/import-checker.py b/contrib/import-checker.py
--- a/contrib/import-checker.py
+++ b/contrib/import-checker.py
@@ -28,6 +28,8 @@  allowsymbolimports = (
     'mercurial.hgweb.request',
     'mercurial.i18n',
     'mercurial.node',
+    # for revlog to re-export constant to extensions
+    'mercurial.revlogutils.constants',
     # for cffi modules to re-export pure functions
     'mercurial.pure.base85',
     'mercurial.pure.bdiff',
diff --git a/mercurial/revlog.py b/mercurial/revlog.py
--- a/mercurial/revlog.py
+++ b/mercurial/revlog.py
@@ -35,6 +35,26 @@  from .node import (
     wdirrev,
 )
 from .i18n import _
+from .revlogutils.constants import (
+    FLAG_GENERALDELTA,
+    FLAG_INLINE_DATA,
+    LIMIT_DELTA2TEXT,
+    REVIDX_DEFAULT_FLAGS,
+    REVIDX_ELLIPSIS,
+    REVIDX_EXTSTORED,
+    REVIDX_FLAGS_ORDER,
+    REVIDX_ISCENSORED,
+    REVIDX_KNOWN_FLAGS,
+    REVIDX_RAWTEXT_CHANGING_FLAGS,
+    REVLOGV0,
+    REVLOGV1,
+    REVLOGV1_FLAGS,
+    REVLOGV2,
+    REVLOGV2_FLAGS,
+    REVLOG_DEFAULT_FLAGS,
+    REVLOG_DEFAULT_FORMAT,
+    REVLOG_DEFAULT_VERSION,
+)
 from .thirdparty import (
     attr,
 )
@@ -51,40 +71,31 @@  from .utils import (
     stringutil,
 )
 
+# blanked usage of all the name to prevent pyflakes constraints
+# We need these name available in the module for extensions.
+REVLOGV0
+REVLOGV1
+REVLOGV2
+FLAG_INLINE_DATA
+FLAG_GENERALDELTA
+REVLOG_DEFAULT_FLAGS
+REVLOG_DEFAULT_FORMAT
+REVLOG_DEFAULT_VERSION
+REVLOGV1_FLAGS
+REVLOGV2_FLAGS
+REVIDX_ISCENSORED
+REVIDX_ELLIPSIS
+REVIDX_EXTSTORED
+REVIDX_DEFAULT_FLAGS
+REVIDX_FLAGS_ORDER
+REVIDX_KNOWN_FLAGS
+REVIDX_RAWTEXT_CHANGING_FLAGS
+
 parsers = policy.importmod(r'parsers')
 
 # Aliased for performance.
 _zlibdecompress = zlib.decompress
 
-# revlog header flags
-REVLOGV0 = 0
-REVLOGV1 = 1
-# Dummy value until file format is finalized.
-# Reminder: change the bounds check in revlog.__init__ when this is changed.
-REVLOGV2 = 0xDEAD
-FLAG_INLINE_DATA = (1 << 16)
-FLAG_GENERALDELTA = (1 << 17)
-REVLOG_DEFAULT_FLAGS = FLAG_INLINE_DATA
-REVLOG_DEFAULT_FORMAT = REVLOGV1
-REVLOG_DEFAULT_VERSION = REVLOG_DEFAULT_FORMAT | REVLOG_DEFAULT_FLAGS
-REVLOGV1_FLAGS = FLAG_INLINE_DATA | FLAG_GENERALDELTA
-REVLOGV2_FLAGS = REVLOGV1_FLAGS
-
-# revlog index flags
-REVIDX_ISCENSORED = (1 << 15) # revision has censor metadata, must be verified
-REVIDX_ELLIPSIS = (1 << 14) # revision hash does not match data (narrowhg)
-REVIDX_EXTSTORED = (1 << 13) # revision data is stored externally
-REVIDX_DEFAULT_FLAGS = 0
-# stable order in which flags need to be processed and their processors applied
-REVIDX_FLAGS_ORDER = [
-    REVIDX_ISCENSORED,
-    REVIDX_ELLIPSIS,
-    REVIDX_EXTSTORED,
-]
-REVIDX_KNOWN_FLAGS = util.bitsfrom(REVIDX_FLAGS_ORDER)
-# bitmark for flags that could cause rawdata content change
-REVIDX_RAWTEXT_CHANGING_FLAGS = REVIDX_ISCENSORED | REVIDX_EXTSTORED
-
 # max size of revlog with inline data
 _maxinline = 131072
 _chunksize = 1048576
@@ -838,9 +849,6 @@  class revlogoldindex(list):
             return (0, 0, 0, -1, -1, -1, -1, nullid)
         return list.__getitem__(self, i)
 
-# maximum <delta-chain-data>/<revision-text-length> ratio
-LIMIT_DELTA2TEXT = 2
-
 class revlogoldio(object):
     def __init__(self):
         self.size = indexformatv0.size
diff --git a/mercurial/revlogutils/__init__.py b/mercurial/revlogutils/__init__.py
new file mode 100644
diff --git a/mercurial/revlogutils/constants.py b/mercurial/revlogutils/constants.py
new file mode 100644
--- /dev/null
+++ b/mercurial/revlogutils/constants.py
@@ -0,0 +1,46 @@ 
+# revlogdeltas.py - constant used for revlog logic
+#
+# Copyright 2005-2007 Matt Mackall <mpm@selenic.com>
+# Copyright 2018 Octobus <contact@octobus.net>
+#
+# This software may be used and distributed according to the terms of the
+# GNU General Public License version 2 or any later version.
+"""Helper class to compute deltas stored inside revlogs"""
+
+from __future__ import absolute_import
+
+from .. import (
+    util,
+)
+
+# revlog header flags
+REVLOGV0 = 0
+REVLOGV1 = 1
+# Dummy value until file format is finalized.
+# Reminder: change the bounds check in revlog.__init__ when this is changed.
+REVLOGV2 = 0xDEAD
+FLAG_INLINE_DATA = (1 << 16)
+FLAG_GENERALDELTA = (1 << 17)
+REVLOG_DEFAULT_FLAGS = FLAG_INLINE_DATA
+REVLOG_DEFAULT_FORMAT = REVLOGV1
+REVLOG_DEFAULT_VERSION = REVLOG_DEFAULT_FORMAT | REVLOG_DEFAULT_FLAGS
+REVLOGV1_FLAGS = FLAG_INLINE_DATA | FLAG_GENERALDELTA
+REVLOGV2_FLAGS = REVLOGV1_FLAGS
+
+# revlog index flags
+REVIDX_ISCENSORED = (1 << 15) # revision has censor metadata, must be verified
+REVIDX_ELLIPSIS = (1 << 14) # revision hash does not match data (narrowhg)
+REVIDX_EXTSTORED = (1 << 13) # revision data is stored externally
+REVIDX_DEFAULT_FLAGS = 0
+# stable order in which flags need to be processed and their processors applied
+REVIDX_FLAGS_ORDER = [
+    REVIDX_ISCENSORED,
+    REVIDX_ELLIPSIS,
+    REVIDX_EXTSTORED,
+]
+REVIDX_KNOWN_FLAGS = util.bitsfrom(REVIDX_FLAGS_ORDER)
+# bitmark for flags that could cause rawdata content change
+REVIDX_RAWTEXT_CHANGING_FLAGS = REVIDX_ISCENSORED | REVIDX_EXTSTORED
+
+# maximum <delta-chain-data>/<revision-text-length> ratio
+LIMIT_DELTA2TEXT = 2
diff --git a/setup.py b/setup.py
--- a/setup.py
+++ b/setup.py
@@ -817,6 +817,7 @@  packages = ['mercurial',
             'mercurial.thirdparty.zope',
             'mercurial.thirdparty.zope.interface',
             'mercurial.utils',
+            'mercurial.revlogutils',
             'hgext', 'hgext.convert', 'hgext.fsmonitor',
             'hgext.fsmonitor.pywatchman',
             'hgext.infinitepush',