Patchwork [2,of,2] py3: conditionalize _winreg import

login
register
mail settings
Submitter Pulkit Goyal
Date Aug. 8, 2016, 7:05 p.m.
Message ID <be03c41372451c7e867d.1470683138@waste.org>
Download mbox | patch
Permalink /patch/16214/
State Rejected
Headers show

Comments

Pulkit Goyal - Aug. 8, 2016, 7:05 p.m.
# HG changeset patch
# User Pulkit Goyal <7895pulkit@gmail.com>
# Date 1470682969 -19800
#      Tue Aug 09 00:32:49 2016 +0530
# Node ID be03c41372451c7e867d1bff0c5d1500cf082754
# Parent  9a344ce563ce2221bdfc9031b8f249d9da2f08b0
py3: conditionalize _winreg import

_winreg module is renamed to winreg in python 3. Added the conditionalize
statements in the respective file because adding this in pycompat will result
in pycompat throwing error as this is a windows registry module and we have
buildbots and most of the contributors on linux.
Yuya Nishihara - Aug. 9, 2016, 3:55 p.m.
On Mon, 08 Aug 2016 14:05:38 -0500, Pulkit Goyal wrote:
> # HG changeset patch
> # User Pulkit Goyal <7895pulkit@gmail.com>
> # Date 1470682969 -19800
> #      Tue Aug 09 00:32:49 2016 +0530
> # Node ID be03c41372451c7e867d1bff0c5d1500cf082754
> # Parent  9a344ce563ce2221bdfc9031b8f249d9da2f08b0
> py3: conditionalize _winreg import
> 
> _winreg module is renamed to winreg in python 3. Added the conditionalize
> statements in the respective file because adding this in pycompat will result
> in pycompat throwing error as this is a windows registry module and we have
> buildbots and most of the contributors on linux.
> 
> diff --git a/mercurial/scmwindows.py b/mercurial/scmwindows.py
> --- a/mercurial/scmwindows.py
> +++ b/mercurial/scmwindows.py
> @@ -1,6 +1,5 @@
>  from __future__ import absolute_import
>  
> -import _winreg
>  import os
>  
>  from . import (
> @@ -8,6 +7,11 @@
>      util,
>  )
>  
> +try:
> +    import _winreg as winreg
> +except ImportError:
> +    import winreg

It appears _winreg is excluded from demandimport for unrelated reason. I think
it's better to make this work even if _winreg were demand-loaded.
Pulkit Goyal - Aug. 9, 2016, 4:35 p.m.
> It appears _winreg is excluded from demandimport for unrelated reason. I think
> it's better to make this work even if _winreg were demand-loaded.

For sure, v2 on its way :)

Patch

diff --git a/mercurial/scmwindows.py b/mercurial/scmwindows.py
--- a/mercurial/scmwindows.py
+++ b/mercurial/scmwindows.py
@@ -1,6 +1,5 @@ 
 from __future__ import absolute_import
 
-import _winreg
 import os
 
 from . import (
@@ -8,6 +7,11 @@ 
     util,
 )
 
+try:
+    import _winreg as winreg
+except ImportError:
+    import winreg
+
 def systemrcpath():
     '''return default os-specific hgrc search path'''
     rcpath = []
@@ -23,7 +27,7 @@ 
                 rcpath.append(os.path.join(progrcd, f))
     # else look for a system rcpath in the registry
     value = util.lookupreg('SOFTWARE\\Mercurial', None,
-                           _winreg.HKEY_LOCAL_MACHINE)
+                           winreg.HKEY_LOCAL_MACHINE)
     if not isinstance(value, str) or not value:
         return rcpath
     value = util.localpath(value)
diff --git a/mercurial/windows.py b/mercurial/windows.py
--- a/mercurial/windows.py
+++ b/mercurial/windows.py
@@ -7,7 +7,6 @@ 
 
 from __future__ import absolute_import
 
-import _winreg
 import errno
 import msvcrt
 import os
@@ -22,6 +21,11 @@ 
     win32,
 )
 
+try:
+    import _winreg as winreg
+except ImportError:
+    import winreg
+
 executablepath = win32.executablepath
 getuser = win32.getuser
 hidewindow = win32.hidewindow
@@ -432,12 +436,12 @@ 
     LOCAL_MACHINE).
     '''
     if scope is None:
-        scope = (_winreg.HKEY_CURRENT_USER, _winreg.HKEY_LOCAL_MACHINE)
+        scope = (winreg.HKEY_CURRENT_USER, winreg.HKEY_LOCAL_MACHINE)
     elif not isinstance(scope, (list, tuple)):
         scope = (scope,)
     for s in scope:
         try:
-            val = _winreg.QueryValueEx(_winreg.OpenKey(s, key), valname)[0]
+            val = winreg.QueryValueEx(winreg.OpenKey(s, key), valname)[0]
             # never let a Unicode string escape into the wild
             return encoding.tolocal(val.encode('UTF-8'))
         except EnvironmentError:
diff --git a/tests/test-check-py3-compat.t b/tests/test-check-py3-compat.t
--- a/tests/test-check-py3-compat.t
+++ b/tests/test-check-py3-compat.t
@@ -147,7 +147,7 @@ 
   mercurial/revlog.py: error importing: <TypeError> getattr(): attribute name must be string (error at util.py:*) (glob)
   mercurial/revset.py: error importing: <TypeError> getattr(): attribute name must be string (error at util.py:*) (glob)
   mercurial/scmutil.py: error importing: <TypeError> getattr(): attribute name must be string (error at util.py:*) (glob)
-  mercurial/scmwindows.py: error importing module: <ImportError> No module named '_winreg' (line *) (glob)
+  mercurial/scmwindows.py: error importing: <TypeError> getattr(): attribute name must be string (error at util.py:*) (glob)
   mercurial/similar.py: error importing: <TypeError> getattr(): attribute name must be string (error at util.py:*) (glob)
   mercurial/simplemerge.py: error importing: <TypeError> getattr(): attribute name must be string (error at util.py:*) (glob)
   mercurial/sshpeer.py: error importing: <TypeError> getattr(): attribute name must be string (error at util.py:*) (glob)
@@ -168,7 +168,7 @@ 
   mercurial/url.py: error importing: <TypeError> getattr(): attribute name must be string (error at util.py:*) (glob)
   mercurial/verify.py: error importing: <TypeError> attribute name must be string, not 'bytes' (error at mdiff.py:*) (glob)
   mercurial/win32.py: error importing module: <ImportError> No module named 'msvcrt' (line *) (glob)
-  mercurial/windows.py: error importing module: <ImportError> No module named '_winreg' (line *) (glob)
+  mercurial/windows.py: error importing module: <ImportError> No module named 'msvcrt' (line *) (glob)
   mercurial/wireproto.py: error importing module: <TypeError> a bytes-like object is required, not 'str' (line *) (glob)
 
 #endif