Patchwork [1,of,9] flagutil: create a `mercurial.revlogutils.flagutil` module

login
register
mail settings
Submitter Pierre-Yves David
Date Aug. 8, 2019, 1:47 a.m.
Message ID <536a1081efbe3cf9ae15.1565228837@nodosa.octopoid.net>
Download mbox | patch
Permalink /patch/41212/
State Accepted
Headers show

Comments

Pierre-Yves David - Aug. 8, 2019, 1:47 a.m.
# HG changeset patch
# User Pierre-Yves David <pierre-yves.david@octobus.net>
# Date 1565218981 -7200
#      Thu Aug 08 01:03:01 2019 +0200
# Node ID 536a1081efbe3cf9ae150af69ab10c6152e8197d
# Parent  11498aa91c036c6d70f7ac5ee5af2664a84a1130
# EXP-Topic flag-processors
# Available At https://bitbucket.org/octobus/mercurial-devel/
#              hg pull https://bitbucket.org/octobus/mercurial-devel/ -r 536a1081efbe
flagutil: create a `mercurial.revlogutils.flagutil` module

The flagprocessings logic is duplicated in 2 extra places, and usually in a less
robust flavor. This is a maintenance nightmare that I would like to see cleaned
up. To do so I am creating a `flagutil` module to move flag processings related
code and make it easily reusable by other code.
Gregory Szorc - Aug. 10, 2019, 11:34 p.m.
On Wed, Aug 7, 2019 at 6:53 PM Pierre-Yves David <
pierre-yves.david@ens-lyon.org> wrote:

> # HG changeset patch
> # User Pierre-Yves David <pierre-yves.david@octobus.net>
> # Date 1565218981 -7200
> #      Thu Aug 08 01:03:01 2019 +0200
> # Node ID 536a1081efbe3cf9ae150af69ab10c6152e8197d
> # Parent  11498aa91c036c6d70f7ac5ee5af2664a84a1130
> # EXP-Topic flag-processors
> # Available At https://bitbucket.org/octobus/mercurial-devel/
> #              hg pull https://bitbucket.org/octobus/mercurial-devel/ -r
> 536a1081efbe
> flagutil: create a `mercurial.revlogutils.flagutil` module
>

Queued parts 1-5.

I'm not super keen on keeping "revlog" in the name, since flag processors
are kinda/sorta storage agnostic (at least some of them). But naming is
hard. We can deal with it later if it becomes a problem.


>
> The flagprocessings logic is duplicated in 2 extra places, and usually in
> a less
> robust flavor. This is a maintenance nightmare that I would like to see
> cleaned
> up. To do so I am creating a `flagutil` module to move flag processings
> related
> code and make it easily reusable by other code.
>
> diff --git a/contrib/import-checker.py b/contrib/import-checker.py
> --- a/contrib/import-checker.py
> +++ b/contrib/import-checker.py
> @@ -31,6 +31,7 @@ allowsymbolimports = (
>      'mercurial.node',
>      # for revlog to re-export constant to extensions
>      'mercurial.revlogutils.constants',
> +    'mercurial.revlogutils.flagutil',
>      # 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
> @@ -38,13 +38,6 @@ from .i18n import _
>  from .revlogutils.constants import (
>      FLAG_GENERALDELTA,
>      FLAG_INLINE_DATA,
> -    REVIDX_DEFAULT_FLAGS,
> -    REVIDX_ELLIPSIS,
> -    REVIDX_EXTSTORED,
> -    REVIDX_FLAGS_ORDER,
> -    REVIDX_ISCENSORED,
> -    REVIDX_KNOWN_FLAGS,
> -    REVIDX_RAWTEXT_CHANGING_FLAGS,
>      REVLOGV0,
>      REVLOGV1,
>      REVLOGV1_FLAGS,
> @@ -54,6 +47,15 @@ from .revlogutils.constants import (
>      REVLOG_DEFAULT_FORMAT,
>      REVLOG_DEFAULT_VERSION,
>  )
> +from .revlogutils.flagutil import (
> +    REVIDX_DEFAULT_FLAGS,
> +    REVIDX_ELLIPSIS,
> +    REVIDX_EXTSTORED,
> +    REVIDX_FLAGS_ORDER,
> +    REVIDX_ISCENSORED,
> +    REVIDX_KNOWN_FLAGS,
> +    REVIDX_RAWTEXT_CHANGING_FLAGS,
> +)
>  from .thirdparty import (
>      attr,
>  )
> diff --git a/mercurial/revlogutils/flagutil.py
> b/mercurial/revlogutils/flagutil.py
> new file mode 100644
> --- /dev/null
> +++ b/mercurial/revlogutils/flagutil.py
> @@ -0,0 +1,31 @@
> +# flagutils.py - code to deal with revlog flags and their processors
> +#
> +# Copyright 2016 Remi Chaintron <remi@fb.com>
> +# Copyright 2016-2019 Pierre-Yves David <pierre-yves.david@ens-lyon.org>
> +#
> +# This software may be used and distributed according to the terms of the
> +# GNU General Public License version 2 or any later version.
> +
> +from __future__ import absolute_import
> +
> +from .constants import (
> +    REVIDX_DEFAULT_FLAGS,
> +    REVIDX_ELLIPSIS,
> +    REVIDX_EXTSTORED,
> +    REVIDX_FLAGS_ORDER,
> +    REVIDX_ISCENSORED,
> +    REVIDX_KNOWN_FLAGS,
> +    REVIDX_RAWTEXT_CHANGING_FLAGS,
> +)
> +
> +# blanked usage of all the name to prevent pyflakes constraints
> +# We need these name available in the module for extensions.
> +REVIDX_ISCENSORED
> +REVIDX_ELLIPSIS
> +REVIDX_EXTSTORED
> +REVIDX_DEFAULT_FLAGS
> +REVIDX_FLAGS_ORDER
> +REVIDX_KNOWN_FLAGS
> +REVIDX_RAWTEXT_CHANGING_FLAGS
> +
> +
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
>
Pierre-Yves David - Aug. 11, 2019, 9:57 a.m.
On 8/11/19 1:34 AM, Gregory Szorc wrote:
> On Wed, Aug 7, 2019 at 6:53 PM Pierre-Yves David 
> <pierre-yves.david@ens-lyon.org <mailto:pierre-yves.david@ens-lyon.org>> 
> wrote:
> 
>     # HG changeset patch
>     # User Pierre-Yves David <pierre-yves.david@octobus.net
>     <mailto:pierre-yves.david@octobus.net>>
>     # Date 1565218981 -7200
>     #      Thu Aug 08 01:03:01 2019 +0200
>     # Node ID 536a1081efbe3cf9ae150af69ab10c6152e8197d
>     # Parent  11498aa91c036c6d70f7ac5ee5af2664a84a1130
>     # EXP-Topic flag-processors
>     # Available At https://bitbucket.org/octobus/mercurial-devel/
>     #              hg pull
>     https://bitbucket.org/octobus/mercurial-devel/ -r 536a1081efbe
>     flagutil: create a `mercurial.revlogutils.flagutil` module
> 
> 
> Queued parts 1-5.
> 
> I'm not super keen on keeping "revlog" in the name, since flag 
> processors are kinda/sorta storage agnostic (at least some of them). But 
> naming is hard. We can deal with it later if it becomes a problem.

The current code focus on revlog, even if the concept could be extended 
to more storage type. More work would be needed to split the agnostic 
part and the storage specific part.

(currently not on my list, but +1 for anyone willing to look into it.)

Patch

diff --git a/contrib/import-checker.py b/contrib/import-checker.py
--- a/contrib/import-checker.py
+++ b/contrib/import-checker.py
@@ -31,6 +31,7 @@  allowsymbolimports = (
     'mercurial.node',
     # for revlog to re-export constant to extensions
     'mercurial.revlogutils.constants',
+    'mercurial.revlogutils.flagutil',
     # 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
@@ -38,13 +38,6 @@  from .i18n import _
 from .revlogutils.constants import (
     FLAG_GENERALDELTA,
     FLAG_INLINE_DATA,
-    REVIDX_DEFAULT_FLAGS,
-    REVIDX_ELLIPSIS,
-    REVIDX_EXTSTORED,
-    REVIDX_FLAGS_ORDER,
-    REVIDX_ISCENSORED,
-    REVIDX_KNOWN_FLAGS,
-    REVIDX_RAWTEXT_CHANGING_FLAGS,
     REVLOGV0,
     REVLOGV1,
     REVLOGV1_FLAGS,
@@ -54,6 +47,15 @@  from .revlogutils.constants import (
     REVLOG_DEFAULT_FORMAT,
     REVLOG_DEFAULT_VERSION,
 )
+from .revlogutils.flagutil import (
+    REVIDX_DEFAULT_FLAGS,
+    REVIDX_ELLIPSIS,
+    REVIDX_EXTSTORED,
+    REVIDX_FLAGS_ORDER,
+    REVIDX_ISCENSORED,
+    REVIDX_KNOWN_FLAGS,
+    REVIDX_RAWTEXT_CHANGING_FLAGS,
+)
 from .thirdparty import (
     attr,
 )
diff --git a/mercurial/revlogutils/flagutil.py b/mercurial/revlogutils/flagutil.py
new file mode 100644
--- /dev/null
+++ b/mercurial/revlogutils/flagutil.py
@@ -0,0 +1,31 @@ 
+# flagutils.py - code to deal with revlog flags and their processors
+#
+# Copyright 2016 Remi Chaintron <remi@fb.com>
+# Copyright 2016-2019 Pierre-Yves David <pierre-yves.david@ens-lyon.org>
+#
+# This software may be used and distributed according to the terms of the
+# GNU General Public License version 2 or any later version.
+
+from __future__ import absolute_import
+
+from .constants import (
+    REVIDX_DEFAULT_FLAGS,
+    REVIDX_ELLIPSIS,
+    REVIDX_EXTSTORED,
+    REVIDX_FLAGS_ORDER,
+    REVIDX_ISCENSORED,
+    REVIDX_KNOWN_FLAGS,
+    REVIDX_RAWTEXT_CHANGING_FLAGS,
+)
+
+# blanked usage of all the name to prevent pyflakes constraints
+# We need these name available in the module for extensions.
+REVIDX_ISCENSORED
+REVIDX_ELLIPSIS
+REVIDX_EXTSTORED
+REVIDX_DEFAULT_FLAGS
+REVIDX_FLAGS_ORDER
+REVIDX_KNOWN_FLAGS
+REVIDX_RAWTEXT_CHANGING_FLAGS
+
+