Patchwork [V2] py3: add "b" prefix to string literals related to module policy

login
register
mail settings
Submitter Katsunori FUJIWARA
Date March 12, 2017, 7:13 p.m.
Message ID <2526bffdc7f01acb4e0f.1489346035@speaknoevil>
Download mbox | patch
Permalink /patch/19212/
State Accepted
Headers show

Comments

Katsunori FUJIWARA - March 12, 2017, 7:13 p.m.
# HG changeset patch
# User FUJIWARA Katsunori <foozy@lares.dti.ne.jp>
# Date 1489345596 -32400
#      Mon Mar 13 04:06:36 2017 +0900
# Node ID 2526bffdc7f01acb4e0f77865fce0ffab675250f
# Parent  62939e0148f170b67ca8c7374f36c413b67fd387
py3: add "b" prefix to string literals related to module policy

String literals without explicit prefix in __init__.py and policy.py
are treated as unicode object on Python3, because these modules are
loaded before setup of our specific code transformation (the later
module is imported at the beginning of __init__.py).

BTW, "modulepolicy" in __init__.py is initialized by "policy.policy".

This causes issues below;

  - checking "policy" value in other modules causes unintentional result

    For example, "b'py' not in (u'c', u'py')" returns True
    unintentionally on Python3.

  - writing "policy" out fails at conversion from unicode to bytes

    62939e0148f1 fixed this issue for default code path, but "policy"
    can be overridden by HGMODULEPOLICY environment variable (it should
    be rare case for developer using Python3, though).

This patch does:

  - add "b" prefix to all string literals, which are related to module
    policy, in modules above.

  - check existence of HGMODULEPOLICY, and overwrite "policy" only if
    it exists

    For simplicity, this patch omits checking "supports_bytes_environ",
    switching os.environ/os.environb, and so on (Yuya agreed this in
    personal talking)
Augie Fackler - March 12, 2017, 7:40 p.m.
On Mon, Mar 13, 2017 at 04:13:55AM +0900, FUJIWARA Katsunori wrote:
> # HG changeset patch
> # User FUJIWARA Katsunori <foozy@lares.dti.ne.jp>
> # Date 1489345596 -32400
> #      Mon Mar 13 04:06:36 2017 +0900
> # Node ID 2526bffdc7f01acb4e0f77865fce0ffab675250f
> # Parent  62939e0148f170b67ca8c7374f36c413b67fd387
> py3: add "b" prefix to string literals related to module policy

Queued, thanks.

>
> String literals without explicit prefix in __init__.py and policy.py
> are treated as unicode object on Python3, because these modules are
> loaded before setup of our specific code transformation (the later
> module is imported at the beginning of __init__.py).
>
> BTW, "modulepolicy" in __init__.py is initialized by "policy.policy".
>
> This causes issues below;
>
>   - checking "policy" value in other modules causes unintentional result
>
>     For example, "b'py' not in (u'c', u'py')" returns True
>     unintentionally on Python3.
>
>   - writing "policy" out fails at conversion from unicode to bytes
>
>     62939e0148f1 fixed this issue for default code path, but "policy"
>     can be overridden by HGMODULEPOLICY environment variable (it should
>     be rare case for developer using Python3, though).
>
> This patch does:
>
>   - add "b" prefix to all string literals, which are related to module
>     policy, in modules above.
>
>   - check existence of HGMODULEPOLICY, and overwrite "policy" only if
>     it exists
>
>     For simplicity, this patch omits checking "supports_bytes_environ",
>     switching os.environ/os.environb, and so on (Yuya agreed this in
>     personal talking)
>
> diff --git a/mercurial/__init__.py b/mercurial/__init__.py
> --- a/mercurial/__init__.py
> +++ b/mercurial/__init__.py
> @@ -68,7 +68,7 @@ class hgimporter(object):
>                  # indicates the type of module. So just assume what we found
>                  # is OK (even though it could be a pure Python module).
>              except ImportError:
> -                if modulepolicy == 'c':
> +                if modulepolicy == b'c':
>                      raise
>                  zl = ziploader('mercurial', 'pure')
>                  mod = zl.load_module(name)
> @@ -106,7 +106,7 @@ class hgimporter(object):
>                                    'version should exist' % name)
>
>          except ImportError:
> -            if modulepolicy == 'c':
> +            if modulepolicy == b'c':
>                  raise
>
>              # Could not load the C extension and pure Python is allowed. So
> diff --git a/mercurial/policy.py b/mercurial/policy.py
> --- a/mercurial/policy.py
> +++ b/mercurial/policy.py
> @@ -19,9 +19,9 @@ import sys
>  #    py - only load pure Python modules
>  #
>  # By default, require the C extensions for performance reasons.
> -policy = 'c'
> -policynoc = ('cffi', 'cffi-allow', 'py')
> -policynocffi = ('c', 'py')
> +policy = b'c'
> +policynoc = (b'cffi', b'cffi-allow', b'py')
> +policynocffi = (b'c', b'py')
>
>  try:
>      from . import __modulepolicy__
> @@ -42,4 +42,8 @@ if sys.version_info[0] >= 3:
>      policy = b'py'
>
>  # Environment variable can always force settings.
> -policy = os.environ.get('HGMODULEPOLICY', policy)
> +if sys.version_info[0] >= 3:
> +    if 'HGMODULEPOLICY' in os.environ:
> +        policy = os.environ['HGMODULEPOLICY'].encode('utf-8')
> +else:
> +    policy = os.environ.get('HGMODULEPOLICY', policy)
> diff --git a/tests/test-check-code.t b/tests/test-check-code.t
> --- a/tests/test-check-code.t
> +++ b/tests/test-check-code.t
> @@ -27,8 +27,14 @@ New errors are not allowed. Warnings are
>     use encoding.environ instead (py3)
>    Skipping mercurial/httpclient/__init__.py it has no-che?k-code (glob)
>    Skipping mercurial/httpclient/_readers.py it has no-che?k-code (glob)
> -  mercurial/policy.py:45:
> -   > policy = os.environ.get('HGMODULEPOLICY', policy)
> +  mercurial/policy.py:46:
> +   >     if 'HGMODULEPOLICY' in os.environ:
> +   use encoding.environ instead (py3)
> +  mercurial/policy.py:47:
> +   >         policy = os.environ['HGMODULEPOLICY'].encode('utf-8')
> +   use encoding.environ instead (py3)
> +  mercurial/policy.py:49:
> +   >     policy = os.environ.get('HGMODULEPOLICY', policy)
>     use encoding.environ instead (py3)
>    Skipping mercurial/statprof.py it has no-che?k-code (glob)
>    [1]
> diff --git a/tests/test-check-py3-commands.t b/tests/test-check-py3-commands.t
> --- a/tests/test-check-py3-commands.t
> +++ b/tests/test-check-py3-commands.t
> @@ -23,3 +23,10 @@ The full traceback is hidden to have a s
>    $ $PYTHON3 `which hg` version | tail -1
>    *** failed to import extension babar from imaginary_elephant: *: 'imaginary_elephant' (glob)
>    warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
> +
> +Test bytes-ness of policy.policy with HGMODULEPOLICY
> +
> +  $ HGMODULEPOLICY=py
> +  $ export HGMODULEPOLICY
> +  $ $PYTHON3 `which hg` debuginstall 2>&1 2>&1 | tail -1
> +  no problems detected
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

Patch

diff --git a/mercurial/__init__.py b/mercurial/__init__.py
--- a/mercurial/__init__.py
+++ b/mercurial/__init__.py
@@ -68,7 +68,7 @@  class hgimporter(object):
                 # indicates the type of module. So just assume what we found
                 # is OK (even though it could be a pure Python module).
             except ImportError:
-                if modulepolicy == 'c':
+                if modulepolicy == b'c':
                     raise
                 zl = ziploader('mercurial', 'pure')
                 mod = zl.load_module(name)
@@ -106,7 +106,7 @@  class hgimporter(object):
                                   'version should exist' % name)
 
         except ImportError:
-            if modulepolicy == 'c':
+            if modulepolicy == b'c':
                 raise
 
             # Could not load the C extension and pure Python is allowed. So
diff --git a/mercurial/policy.py b/mercurial/policy.py
--- a/mercurial/policy.py
+++ b/mercurial/policy.py
@@ -19,9 +19,9 @@  import sys
 #    py - only load pure Python modules
 #
 # By default, require the C extensions for performance reasons.
-policy = 'c'
-policynoc = ('cffi', 'cffi-allow', 'py')
-policynocffi = ('c', 'py')
+policy = b'c'
+policynoc = (b'cffi', b'cffi-allow', b'py')
+policynocffi = (b'c', b'py')
 
 try:
     from . import __modulepolicy__
@@ -42,4 +42,8 @@  if sys.version_info[0] >= 3:
     policy = b'py'
 
 # Environment variable can always force settings.
-policy = os.environ.get('HGMODULEPOLICY', policy)
+if sys.version_info[0] >= 3:
+    if 'HGMODULEPOLICY' in os.environ:
+        policy = os.environ['HGMODULEPOLICY'].encode('utf-8')
+else:
+    policy = os.environ.get('HGMODULEPOLICY', policy)
diff --git a/tests/test-check-code.t b/tests/test-check-code.t
--- a/tests/test-check-code.t
+++ b/tests/test-check-code.t
@@ -27,8 +27,14 @@  New errors are not allowed. Warnings are
    use encoding.environ instead (py3)
   Skipping mercurial/httpclient/__init__.py it has no-che?k-code (glob)
   Skipping mercurial/httpclient/_readers.py it has no-che?k-code (glob)
-  mercurial/policy.py:45:
-   > policy = os.environ.get('HGMODULEPOLICY', policy)
+  mercurial/policy.py:46:
+   >     if 'HGMODULEPOLICY' in os.environ:
+   use encoding.environ instead (py3)
+  mercurial/policy.py:47:
+   >         policy = os.environ['HGMODULEPOLICY'].encode('utf-8')
+   use encoding.environ instead (py3)
+  mercurial/policy.py:49:
+   >     policy = os.environ.get('HGMODULEPOLICY', policy)
    use encoding.environ instead (py3)
   Skipping mercurial/statprof.py it has no-che?k-code (glob)
   [1]
diff --git a/tests/test-check-py3-commands.t b/tests/test-check-py3-commands.t
--- a/tests/test-check-py3-commands.t
+++ b/tests/test-check-py3-commands.t
@@ -23,3 +23,10 @@  The full traceback is hidden to have a s
   $ $PYTHON3 `which hg` version | tail -1
   *** failed to import extension babar from imaginary_elephant: *: 'imaginary_elephant' (glob)
   warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
+
+Test bytes-ness of policy.policy with HGMODULEPOLICY
+
+  $ HGMODULEPOLICY=py
+  $ export HGMODULEPOLICY
+  $ $PYTHON3 `which hg` debuginstall 2>&1 2>&1 | tail -1
+  no problems detected