[PATCH 1 of 2] pycompat: stop setting LC_CTYPE unconditionally

classic Classic list List threaded Threaded
3 messages Options
Reply | Threaded
Open this post in threaded view
|

[PATCH 1 of 2] pycompat: stop setting LC_CTYPE unconditionally

Manuel Jacob
# HG changeset patch
# User Manuel Jacob <[hidden email]>
# Date 1593284746 -7200
#      Sat Jun 27 21:05:46 2020 +0200
# Node ID 52c536124224ca4e87aee09cdff7ded520d6d40d
# Parent  94987aadb22cee1d7da52f365b3f532a94bbd4a6
# EXP-Topic with_lc_type
pycompat: stop setting LC_CTYPE unconditionally

Changeset a25343d16ebe aimed to align how LC_CTYPE is initialized across Python
versions. However, as Yuya Nishihara pointed out, it changes the behavior of
some str methods on Python 2.

Curses requires that LC_CTYPE is initialized correctly. Therefore LC_CTYPE is
set while curses is used and reset afterwards. It shouldn’t be a problem that
some str methods behave differently on Python 2 while curses is used. At least
it’s not a regression compared to what was done before d2227d4c9e6b.

This again breaks non-ASCII filenames passed to the Subversion bindings on
Python 2. Since it didn’t work before a25343d16ebe, it’s however not really a
regression. A separate patch will be sent.

diff --git a/hgext/histedit.py b/hgext/histedit.py
--- a/hgext/histedit.py
+++ b/hgext/histedit.py
@@ -1710,7 +1710,8 @@
         ctxs = []
         for i, r in enumerate(revs):
             ctxs.append(histeditrule(ui, repo[r], i))
-        rc = curses.wrapper(functools.partial(_chisteditmain, repo, ctxs))
+        with pycompat.with_lc_ctype():
+            rc = curses.wrapper(functools.partial(_chisteditmain, repo, ctxs))
         curses.echo()
         curses.endwin()
         if rc is False:
diff --git a/mercurial/crecord.py b/mercurial/crecord.py
--- a/mercurial/crecord.py
+++ b/mercurial/crecord.py
@@ -569,7 +569,8 @@
     if util.safehasattr(signal, b'SIGTSTP'):
         origsigtstp = signal.getsignal(signal.SIGTSTP)
     try:
-        curses.wrapper(chunkselector.main)
+        with pycompat.with_lc_ctype():
+            curses.wrapper(chunkselector.main)
         if chunkselector.initexc is not None:
             raise chunkselector.initexc
         # ncurses does not restore signal handler for SIGTSTP
diff --git a/mercurial/pycompat.py b/mercurial/pycompat.py
--- a/mercurial/pycompat.py
+++ b/mercurial/pycompat.py
@@ -10,6 +10,7 @@
 
 from __future__ import absolute_import
 
+import contextlib
 import getopt
 import inspect
 import json
@@ -105,13 +106,22 @@
 # initialized. Since CPython commit 177d921c8c03d30daa32994362023f777624b10d,
 # LC_CTYPE is always initialized. If we require Python 3.8+, we should re-check
 # if we can remove this code.
-if locale.setlocale(locale.LC_CTYPE, None) == 'C':
-    try:
-        locale.setlocale(locale.LC_CTYPE, '')
-    except locale.Error:
-        # The likely case is that the locale from the environment variables is
-        # unknown.
-        pass
+@contextlib.contextmanager
+def with_lc_ctype():
+    oldloc = locale.setlocale(locale.LC_CTYPE, None)
+    if oldloc == 'C':
+        try:
+            try:
+                locale.setlocale(locale.LC_CTYPE, '')
+            except locale.Error:
+                # The likely case is that the locale from the environment
+                # variables is unknown.
+                pass
+            yield
+        finally:
+            locale.setlocale(locale.LC_CTYPE, oldloc)
+    else:
+        yield
 
 
 if ispy3:
_______________________________________________
Mercurial-devel mailing list
[hidden email]
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Reply | Threaded
Open this post in threaded view
|

[PATCH 2 of 2] convert: set LC_CTYPE around calls to Subversion bindings

Manuel Jacob
# HG changeset patch
# User Manuel Jacob <[hidden email]>
# Date 1593287154 -7200
#      Sat Jun 27 21:45:54 2020 +0200
# Node ID 67e293df30a0bbdfc4b035400efe8b5a3c0f6336
# Parent  52c536124224ca4e87aee09cdff7ded520d6d40d
# EXP-Topic with_lc_type
convert: set LC_CTYPE around calls to Subversion bindings

The Subversion bindings require that LC_CTYPE is set. However, we don’t want to
set it all the time, as it changes the behavior of str methods on Python 2. The
taken approach is hopefully fine-grained enough to not trigger any
locale-specfic behavior of the str methods and coarse-grained enough to not
clutter the code.

Emulating the with-statement behavior in before() and after() should be safe, as
after() is always called when before() is called. hgext.convert.hg takes a
similar approach.

diff --git a/hgext/convert/subversion.py b/hgext/convert/subversion.py
--- a/hgext/convert/subversion.py
+++ b/hgext/convert/subversion.py
@@ -187,13 +187,14 @@
     """Fetch SVN log in a subprocess and channel them back to parent to
     avoid memory collection issues.
     """
-    if svn is None:
-        raise error.Abort(
-            _(b'debugsvnlog could not load Subversion python bindings')
-        )
+    with pycompat.with_lc_ctype():
+        if svn is None:
+            raise error.Abort(
+                _(b'debugsvnlog could not load Subversion python bindings')
+            )
 
-    args = decodeargs(ui.fin.read())
-    get_log_child(ui.fout, *args)
+        args = decodeargs(ui.fin.read())
+        get_log_child(ui.fout, *args)
 
 
 class logstream(object):
@@ -420,18 +421,19 @@
         self.url = geturl(url)
         self.encoding = b'UTF-8'  # Subversion is always nominal UTF-8
         try:
-            self.transport = transport.SvnRaTransport(url=self.url)
-            self.ra = self.transport.ra
-            self.ctx = self.transport.client
-            self.baseurl = svn.ra.get_repos_root(self.ra)
-            # Module is either empty or a repository path starting with
-            # a slash and not ending with a slash.
-            self.module = urlreq.unquote(self.url[len(self.baseurl) :])
-            self.prevmodule = None
-            self.rootmodule = self.module
-            self.commits = {}
-            self.paths = {}
-            self.uuid = svn.ra.get_uuid(self.ra)
+            with pycompat.with_lc_ctype():
+                self.transport = transport.SvnRaTransport(url=self.url)
+                self.ra = self.transport.ra
+                self.ctx = self.transport.client
+                self.baseurl = svn.ra.get_repos_root(self.ra)
+                # Module is either empty or a repository path starting with
+                # a slash and not ending with a slash.
+                self.module = urlreq.unquote(self.url[len(self.baseurl) :])
+                self.prevmodule = None
+                self.rootmodule = self.module
+                self.commits = {}
+                self.paths = {}
+                self.uuid = svn.ra.get_uuid(self.ra)
         except svn.core.SubversionException:
             ui.traceback()
             svnversion = b'%d.%d.%d' % (
@@ -477,7 +479,8 @@
             )
 
         try:
-            self.head = self.latest(self.module, latest)
+            with pycompat.with_lc_ctype():
+                self.head = self.latest(self.module, latest)
         except SvnPathNotFound:
             self.head = None
         if not self.head:
@@ -494,6 +497,13 @@
             self.wc = None
         self.convertfp = None
 
+    def before(self):
+        self.with_lc_ctype = pycompat.with_lc_ctype()
+        self.with_lc_ctype.__enter__()
+
+    def after(self):
+        self.with_lc_ctype.__exit__(None, None, None)
+
     def setrevmap(self, revmap):
         lastrevs = {}
         for revid in revmap:
_______________________________________________
Mercurial-devel mailing list
[hidden email]
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 1 of 2] pycompat: stop setting LC_CTYPE unconditionally

Yuya Nishihara
In reply to this post by Manuel Jacob
On Sat, 27 Jun 2020 22:58:13 +0200, Manuel Jacob wrote:
> # HG changeset patch
> # User Manuel Jacob <[hidden email]>
> # Date 1593284746 -7200
> #      Sat Jun 27 21:05:46 2020 +0200
> # Node ID 52c536124224ca4e87aee09cdff7ded520d6d40d
> # Parent  94987aadb22cee1d7da52f365b3f532a94bbd4a6
> # EXP-Topic with_lc_type
> pycompat: stop setting LC_CTYPE unconditionally

> --- a/mercurial/pycompat.py
> +++ b/mercurial/pycompat.py
> @@ -10,6 +10,7 @@
>  
>  from __future__ import absolute_import
>  
> +import contextlib
>  import getopt
>  import inspect
>  import json
> @@ -105,13 +106,22 @@
>  # initialized. Since CPython commit 177d921c8c03d30daa32994362023f777624b10d,
>  # LC_CTYPE is always initialized. If we require Python 3.8+, we should re-check
>  # if we can remove this code.
> -if locale.setlocale(locale.LC_CTYPE, None) == 'C':
> -    try:
> -        locale.setlocale(locale.LC_CTYPE, '')
> -    except locale.Error:
> -        # The likely case is that the locale from the environment variables is
> -        # unknown.
> -        pass
> +@contextlib.contextmanager
> +def with_lc_ctype():
> +    oldloc = locale.setlocale(locale.LC_CTYPE, None)
> +    if oldloc == 'C':
> +        try:
> +            try:
> +                locale.setlocale(locale.LC_CTYPE, '')
> +            except locale.Error:
> +                # The likely case is that the locale from the environment
> +                # variables is unknown.
> +                pass
> +            yield
> +        finally:
> +            locale.setlocale(locale.LC_CTYPE, oldloc)
> +    else:
> +        yield

Let's move this function to util.py. Other than that, the patch looks
good to me.
_______________________________________________
Mercurial-devel mailing list
[hidden email]
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel