Patchwork [1,of,7] python3: wrap all uses of <exception>.strerror with unitolocal

login
register
mail settings
Submitter Augie Fackler
Date Aug. 29, 2017, 3:21 p.m.
Message ID <fd4664b32ec7cae6dbfd.1504020118@augie-macbookpro2.roam.corp.google.com>
Download mbox | patch
Permalink /patch/23471/
State Accepted
Headers show

Comments

Augie Fackler - Aug. 29, 2017, 3:21 p.m.
# HG changeset patch
# User Augie Fackler <raf@durin42.com>
# Date 1503446587 14400
#      Tue Aug 22 20:03:07 2017 -0400
# Node ID fd4664b32ec7cae6dbfdd6292c04e7f1a4888da2
# Parent  2ad028635ccd4d47e565c5d59af98ca32165eb3e
python3: wrap all uses of <exception>.strerror with unitolocal

Our string literals are bytes, and we mostly want to %-format a
strerror into a one of those literals, so this fixes a ton of issues.
Yuya Nishihara - Aug. 31, 2017, 1:56 p.m.
On Tue, 29 Aug 2017 11:21:58 -0400, Augie Fackler wrote:
> # HG changeset patch
> # User Augie Fackler <raf@durin42.com>
> # Date 1503446587 14400
> #      Tue Aug 22 20:03:07 2017 -0400
> # Node ID fd4664b32ec7cae6dbfdd6292c04e7f1a4888da2
> # Parent  2ad028635ccd4d47e565c5d59af98ca32165eb3e
> python3: wrap all uses of <exception>.strerror with unitolocal
> 
> Our string literals are bytes, and we mostly want to %-format a
> strerror into a one of those literals, so this fixes a ton of issues.

s/unitolocal/strtolocal/g and queued 1-4 and 6, thanks.

strerror is byte string on Python 2.x.

> --- a/mercurial/lock.py
> +++ b/mercurial/lock.py
> @@ -148,8 +148,9 @@ class lock(object):
>                                               self.vfs.join(self.f), self.desc,
>                                               locker)
>                  else:
> -                    raise error.LockUnavailable(why.errno, why.strerror,
> -                                                why.filename, self.desc)
> +                    raise error.LockUnavailable(
> +                        why.errno, encoding.unitolocal(why.strerror),
> +                        why.filename, self.desc)

LockUnavailable is a subclass of IOError, which should keep strerror as
a unicode string. Dropped this change.

> diff --git a/mercurial/vfs.py b/mercurial/vfs.py
> --- a/mercurial/vfs.py
> +++ b/mercurial/vfs.py
> @@ -16,6 +16,7 @@ import threading
>  
>  from .i18n import _
>  from . import (
> +    encoding,
>      error,
>      pathutil,
>      pycompat,
> @@ -434,7 +435,8 @@ class vfs(abstractvfs):
>                  os.symlink(src, linkname)
>              except OSError as err:
>                  raise OSError(err.errno, _('could not symlink to %r: %s') %
> -                              (src, err.strerror), linkname)
> +                              (src, encoding.unitolocal(err.strerror)),
> +                              linkname)

Perhaps we'll have to convert the message back to unicode by strfromlocal().

Patch

diff --git a/hgext/convert/common.py b/hgext/convert/common.py
--- a/hgext/convert/common.py
+++ b/hgext/convert/common.py
@@ -15,6 +15,7 @@  import subprocess
 
 from mercurial.i18n import _
 from mercurial import (
+    encoding,
     error,
     phases,
     util,
@@ -475,8 +476,9 @@  class mapfile(dict):
             try:
                 self.fp = open(self.path, 'a')
             except IOError as err:
-                raise error.Abort(_('could not open map file %r: %s') %
-                                 (self.path, err.strerror))
+                raise error.Abort(
+                    _('could not open map file %r: %s') %
+                    (self.path, encoding.unitolocal(err.strerror)))
         self.fp.write('%s %s\n' % (key, value))
         self.fp.flush()
         super(mapfile, self).__setitem__(key, value)
diff --git a/hgext/mq.py b/hgext/mq.py
--- a/hgext/mq.py
+++ b/hgext/mq.py
@@ -80,6 +80,7 @@  from mercurial import (
     cmdutil,
     commands,
     dirstateguard,
+    encoding,
     error,
     extensions,
     hg,
@@ -1206,7 +1207,7 @@  class queue(object):
                 p = self.opener(patchfn, "w")
             except IOError as e:
                 raise error.Abort(_('cannot write patch "%s": %s')
-                                 % (patchfn, e.strerror))
+                                 % (patchfn, encoding.unitolocal(e.strerror)))
             try:
                 defaultmsg = "[mq]: %s" % patchfn
                 editor = cmdutil.getcommiteditor(editform=editform)
diff --git a/mercurial/cmdutil.py b/mercurial/cmdutil.py
--- a/mercurial/cmdutil.py
+++ b/mercurial/cmdutil.py
@@ -775,7 +775,7 @@  def logmessage(ui, opts):
                 message = '\n'.join(util.readfile(logfile).splitlines())
         except IOError as inst:
             raise error.Abort(_("can't read commit message '%s': %s") %
-                             (logfile, inst.strerror))
+                             (logfile, encoding.unitolocal(inst.strerror)))
     return message
 
 def mergeeditform(ctxorbool, baseformname):
@@ -1097,7 +1097,7 @@  def copy(ui, repo, pats, opts, rename=Fa
                     srcexists = False
                 else:
                     ui.warn(_('%s: cannot copy - %s\n') %
-                            (relsrc, inst.strerror))
+                            (relsrc, encoding.unitolocal(inst.strerror)))
                     return True # report a failure
 
         if ui.verbose or not exact:
diff --git a/mercurial/dirstate.py b/mercurial/dirstate.py
--- a/mercurial/dirstate.py
+++ b/mercurial/dirstate.py
@@ -982,7 +982,7 @@  class dirstate(object):
                             matchedir(nf)
                         notfoundadd(nf)
                     else:
-                        badfn(ff, inst.strerror)
+                        badfn(ff, encoding.unitolocal(inst.strerror))
 
         # Case insensitive filesystems cannot rely on lstat() failing to detect
         # a case-only rename.  Prune the stat object for any file that does not
@@ -1088,7 +1088,8 @@  class dirstate(object):
                     entries = listdir(join(nd), stat=True, skip=skip)
                 except OSError as inst:
                     if inst.errno in (errno.EACCES, errno.ENOENT):
-                        match.bad(self.pathto(nd), inst.strerror)
+                        match.bad(self.pathto(nd),
+                                  encoding.unitolocal(inst.strerror))
                         continue
                     raise
                 for f, kind, st in entries:
diff --git a/mercurial/dispatch.py b/mercurial/dispatch.py
--- a/mercurial/dispatch.py
+++ b/mercurial/dispatch.py
@@ -88,7 +88,8 @@  def run():
             status = -1
     if util.safehasattr(req.ui, 'ferr'):
         if err is not None and err.errno != errno.EPIPE:
-            req.ui.ferr.write('abort: %s\n' % err.strerror)
+            req.ui.ferr.write('abort: %s\n' %
+                              encoding.unitolocal(err.strerror))
         req.ui.ferr.flush()
     sys.exit(status & 255)
 
@@ -676,7 +677,7 @@  def _getlocal(ui, rpath, wd=None):
             wd = pycompat.getcwd()
         except OSError as e:
             raise error.Abort(_("error getting current working directory: %s") %
-                              e.strerror)
+                              encoding.unitolocal(e.strerror))
     path = cmdutil.findrepo(wd) or ""
     if not path:
         lui = ui
diff --git a/mercurial/hgweb/common.py b/mercurial/hgweb/common.py
--- a/mercurial/hgweb/common.py
+++ b/mercurial/hgweb/common.py
@@ -178,7 +178,8 @@  def staticfile(directory, fname, req):
         if err.errno == errno.ENOENT:
             raise ErrorResponse(HTTP_NOT_FOUND)
         else:
-            raise ErrorResponse(HTTP_SERVER_ERROR, err.strerror)
+            raise ErrorResponse(HTTP_SERVER_ERROR,
+                                encoding.unitolocal(err.strerror))
 
 def paritygen(stripecount, offset=0):
     """count parity of horizontal stripes for easier reading"""
diff --git a/mercurial/hgweb/hgwebdir_mod.py b/mercurial/hgweb/hgwebdir_mod.py
--- a/mercurial/hgweb/hgwebdir_mod.py
+++ b/mercurial/hgweb/hgwebdir_mod.py
@@ -289,7 +289,7 @@  class hgwebdir(object):
                         repo = hg.repository(self.ui.copy(), real)
                         return hgweb_mod.hgweb(repo).run_wsgi(req)
                     except IOError as inst:
-                        msg = inst.strerror
+                        msg = encoding.unitolocal(inst.strerror)
                         raise ErrorResponse(HTTP_SERVER_ERROR, msg)
                     except error.RepoError as inst:
                         raise ErrorResponse(HTTP_SERVER_ERROR, str(inst))
diff --git a/mercurial/lock.py b/mercurial/lock.py
--- a/mercurial/lock.py
+++ b/mercurial/lock.py
@@ -148,8 +148,9 @@  class lock(object):
                                              self.vfs.join(self.f), self.desc,
                                              locker)
                 else:
-                    raise error.LockUnavailable(why.errno, why.strerror,
-                                                why.filename, self.desc)
+                    raise error.LockUnavailable(
+                        why.errno, encoding.unitolocal(why.strerror),
+                        why.filename, self.desc)
 
         if not self.held:
             # use empty locker to mean "busy for frequent lock/unlock
diff --git a/mercurial/scmutil.py b/mercurial/scmutil.py
--- a/mercurial/scmutil.py
+++ b/mercurial/scmutil.py
@@ -163,7 +163,8 @@  def callcatch(ui, func):
             ui.warn(_("(lock might be very busy)\n"))
     except error.LockUnavailable as inst:
         ui.warn(_("abort: could not lock %s: %s\n") %
-               (inst.desc or inst.filename, inst.strerror))
+               (inst.desc or inst.filename,
+                encoding.unitolocal(inst.strerror)))
     except error.OutOfBandError as inst:
         if inst.args:
             msg = _("abort: remote error:\n")
@@ -226,16 +227,18 @@  def callcatch(ui, func):
             pass
         elif getattr(inst, "strerror", None):
             if getattr(inst, "filename", None):
-                ui.warn(_("abort: %s: %s\n") % (inst.strerror, inst.filename))
+                ui.warn(_("abort: %s: %s\n") % (
+                    encoding.unitolocal(inst.strerror), inst.filename))
             else:
-                ui.warn(_("abort: %s\n") % inst.strerror)
+                ui.warn(_("abort: %s\n") % encoding.unitolocal(inst.strerror))
         else:
             raise
     except OSError as inst:
         if getattr(inst, "filename", None) is not None:
-            ui.warn(_("abort: %s: '%s'\n") % (inst.strerror, inst.filename))
+            ui.warn(_("abort: %s: '%s'\n") % (
+                encoding.unitolocal(inst.strerror), inst.filename))
         else:
-            ui.warn(_("abort: %s\n") % inst.strerror)
+            ui.warn(_("abort: %s\n") % encoding.unitolocal(inst.strerror))
     except MemoryError:
         ui.warn(_("abort: out of memory\n"))
     except SystemExit as inst:
diff --git a/mercurial/subrepo.py b/mercurial/subrepo.py
--- a/mercurial/subrepo.py
+++ b/mercurial/subrepo.py
@@ -1346,7 +1346,8 @@  class gitsubrepo(abstractsubrepo):
             genericerror = _("error executing git for subrepo '%s': %s")
             notfoundhint = _("check git is installed and in your PATH")
             if e.errno != errno.ENOENT:
-                raise error.Abort(genericerror % (self._path, e.strerror))
+                raise error.Abort(genericerror % (
+                    self._path, encoding.unitolocal(e.strerror)))
             elif pycompat.osname == 'nt':
                 try:
                     self._gitexecutable = 'git.cmd'
@@ -1358,7 +1359,7 @@  class gitsubrepo(abstractsubrepo):
                             hint=notfoundhint)
                     else:
                         raise error.Abort(genericerror % (self._path,
-                            e2.strerror))
+                            encoding.unitolocal(e2.strerror)))
             else:
                 raise error.Abort(_("couldn't find git for subrepo '%s'")
                     % self._path, hint=notfoundhint)
diff --git a/mercurial/vfs.py b/mercurial/vfs.py
--- a/mercurial/vfs.py
+++ b/mercurial/vfs.py
@@ -16,6 +16,7 @@  import threading
 
 from .i18n import _
 from . import (
+    encoding,
     error,
     pathutil,
     pycompat,
@@ -434,7 +435,8 @@  class vfs(abstractvfs):
                 os.symlink(src, linkname)
             except OSError as err:
                 raise OSError(err.errno, _('could not symlink to %r: %s') %
-                              (src, err.strerror), linkname)
+                              (src, encoding.unitolocal(err.strerror)),
+                              linkname)
         else:
             self.write(dst, src)
 
diff --git a/mercurial/win32.py b/mercurial/win32.py
--- a/mercurial/win32.py
+++ b/mercurial/win32.py
@@ -286,7 +286,8 @@  def _raiseoserror(name):
     if code > 0x7fffffff:
         code -= 2**32
     err = ctypes.WinError(code=code)
-    raise OSError(err.errno, '%s: %s' % (name, err.strerror))
+    raise OSError(err.errno, '%s: %s' % (name,
+                                         encoding.unitolocal(err.strerror)))
 
 def _getfileinfo(name):
     fh = _kernel32.CreateFileA(name, 0,
diff --git a/mercurial/windows.py b/mercurial/windows.py
--- a/mercurial/windows.py
+++ b/mercurial/windows.py
@@ -137,7 +137,8 @@  def posixfile(name, mode='r', buffering=
         return fp
     except WindowsError as err:
         # convert to a friendlier exception
-        raise IOError(err.errno, '%s: %s' % (name, err.strerror))
+        raise IOError(err.errno, '%s: %s' % (
+            name, encoding.unitolocal(err.strerror)))
 
 # may be wrapped by win32mbcs extension
 listdir = osutil.listdir