Patchwork [2,of,6,py3] config: add sanity assert that files are opened as binary

login
register
mail settings
Submitter Augie Fackler
Date March 3, 2017, 7:30 p.m.
Message ID <0034a661fbdc1e29e51f.1488569405@imladris.local>
Download mbox | patch
Permalink /patch/18895/
State Accepted
Headers show

Comments

Augie Fackler - March 3, 2017, 7:30 p.m.
# HG changeset patch
# User Augie Fackler <raf@durin42.com>
# Date 1488563711 18000
#      Fri Mar 03 12:55:11 2017 -0500
# Node ID 0034a661fbdc1e29e51fe8e2877467204282f669
# Parent  2a02a4ec5147f8bd6685a609ce9499b1266cc6db
config: add sanity assert that files are opened as binary

This helps with some debugging in Python 3, and shouldn't hurt
anything in Python 2.
Yuya Nishihara - March 5, 2017, 6:24 a.m.
On Fri, 03 Mar 2017 14:30:05 -0500, Augie Fackler wrote:
> # HG changeset patch
> # User Augie Fackler <raf@durin42.com>
> # Date 1488563711 18000
> #      Fri Mar 03 12:55:11 2017 -0500
> # Node ID 0034a661fbdc1e29e51fe8e2877467204282f669
> # Parent  2a02a4ec5147f8bd6685a609ce9499b1266cc6db
> config: add sanity assert that files are opened as binary
> 
> This helps with some debugging in Python 3, and shouldn't hurt
> anything in Python 2.
> 
> diff --git a/mercurial/config.py b/mercurial/config.py
> --- a/mercurial/config.py
> +++ b/mercurial/config.py
> @@ -170,4 +170,7 @@ class config(object):
>      def read(self, path, fp=None, sections=None, remap=None):
>          if not fp:
>              fp = util.posixfile(path, r'rb')
> +        assert fp.mode == r'rb', ('config files must be opened'
> +                                  ' in binary mode, got fp=%r mode=%r' % (
> +                                      fp, fp.mode))

Seems okay, but I don't think it's good idea to ban the use of pseudo files
such as BytesIO. Also, as config parser will skip CR chars, a text-mode file
should have no problem as long as it returns bytes objects.

Patch

diff --git a/mercurial/config.py b/mercurial/config.py
--- a/mercurial/config.py
+++ b/mercurial/config.py
@@ -170,4 +170,7 @@  class config(object):
     def read(self, path, fp=None, sections=None, remap=None):
         if not fp:
             fp = util.posixfile(path, r'rb')
+        assert fp.mode == r'rb', ('config files must be opened'
+                                  ' in binary mode, got fp=%r mode=%r' % (
+                                      fp, fp.mode))
         self.parse(path, fp.read(), sections, remap, self.read)