Patchwork [6,of,8,py3,v3] config: guard against setconfig specifying unicode values on py3

login
register
mail settings
Submitter Augie Fackler
Date March 8, 2017, 11:22 p.m.
Message ID <9788576dc5009344c264.1489015365@augie-macbookair2.roam.corp.google.com>
Download mbox | patch
Permalink /patch/19037/
State Accepted
Headers show

Comments

Augie Fackler - March 8, 2017, 11:22 p.m.
# HG changeset patch
# User Augie Fackler <raf@durin42.com>
# Date 1488570176 18000
#      Fri Mar 03 14:42:56 2017 -0500
# Node ID 9788576dc5009344c2649a7a56565cdf20713112
# Parent  9269a48bd35f1eaafdb00c58c6056382c6a0ba88
config: guard against setconfig specifying unicode values on py3

This was leading to some difficult to trace problems because the
values were set in one place, but then blew up much later in the
program. Exploding violently with an assertion seems reasonable here.
Yuya Nishihara - March 9, 2017, 8:04 a.m.
On Wed, 08 Mar 2017 18:22:45 -0500, Augie Fackler wrote:
> # HG changeset patch
> # User Augie Fackler <raf@durin42.com>
> # Date 1488570176 18000
> #      Fri Mar 03 14:42:56 2017 -0500
> # Node ID 9788576dc5009344c2649a7a56565cdf20713112
> # Parent  9269a48bd35f1eaafdb00c58c6056382c6a0ba88
> config: guard against setconfig specifying unicode values on py3
> 
> This was leading to some difficult to trace problems because the
> values were set in one place, but then blew up much later in the
> program. Exploding violently with an assertion seems reasonable here.
> 
> diff --git a/mercurial/config.py b/mercurial/config.py
> --- a/mercurial/config.py
> +++ b/mercurial/config.py
> @@ -13,6 +13,7 @@ import os
>  from .i18n import _
>  from . import (
>      error,
> +    pycompat,
>      util,
>  )
>  
> @@ -69,6 +70,9 @@ class config(object):
>      def items(self, section):
>          return self._data.get(section, {}).items()
>      def set(self, section, item, value, source=""):
> +        if pycompat.ispy3:
> +            assert not isinstance(value, str), (
> +                'config values may not be unicode strings on Python 3')

IIRC, internal API may set non-str such as booleans. And str on Python 3 is
unicode.
Yuya Nishihara - March 9, 2017, 6:26 p.m.
On Mar 9, 2017 5:05 PM, "Yuya Nishihara" <yuya@tcha.org> wrote:

On Wed, 08 Mar 2017 18:22:45 -0500, Augie Fackler wrote:
> # HG changeset patch
> # User Augie Fackler <raf@durin42.com>
> # Date 1488570176 18000
> #      Fri Mar 03 14:42:56 2017 -0500
> # Node ID 9788576dc5009344c2649a7a56565cdf20713112
> # Parent  9269a48bd35f1eaafdb00c58c6056382c6a0ba88
> config: guard against setconfig specifying unicode values on py3
>
> This was leading to some difficult to trace problems because the
> values were set in one place, but then blew up much later in the
> program. Exploding violently with an assertion seems reasonable here.
>
> diff --git a/mercurial/config.py b/mercurial/config.py
> --- a/mercurial/config.py
> +++ b/mercurial/config.py
> @@ -13,6 +13,7 @@ import os
>  from .i18n import _
>  from . import (
>      error,
> +    pycompat,
>      util,
>  )
>
> @@ -69,6 +70,9 @@ class config(object):
>      def items(self, section):
>          return self._data.get(section, {}).items()
>      def set(self, section, item, value, source=""):
> +        if pycompat.ispy3:
> +            assert not isinstance(value, str), (
> +                'config values may not be unicode strings on Python 3')

IIRC, internal API may set non-str such as booleans. And str on Python 3 is
unicode.


Oops, I was tricked by the type name. This looks good.

Patch

diff --git a/mercurial/config.py b/mercurial/config.py
--- a/mercurial/config.py
+++ b/mercurial/config.py
@@ -13,6 +13,7 @@  import os
 from .i18n import _
 from . import (
     error,
+    pycompat,
     util,
 )
 
@@ -69,6 +70,9 @@  class config(object):
     def items(self, section):
         return self._data.get(section, {}).items()
     def set(self, section, item, value, source=""):
+        if pycompat.ispy3:
+            assert not isinstance(value, str), (
+                'config values may not be unicode strings on Python 3')
         if section not in self:
             self._data[section] = util.sortdict()
         self._data[section][item] = value