Patchwork [2,of,2] py3: convert unicode paths given for hgweb config

login
register
mail settings
Submitter Ludovic Chabant
Date April 19, 2019, 2:34 p.m.
Message ID <b1baf28288faf3fd628d.1555684454@devahi>
Download mbox | patch
Permalink /patch/39787/
State New
Headers show

Comments

Ludovic Chabant - April 19, 2019, 2:34 p.m.
# HG changeset patch
# User Ludovic Chabant <ludovic@chabant.com>
# Date 1555683992 0
#      Fri Apr 19 14:26:32 2019 +0000
# Branch stable
# Node ID b1baf28288faf3fd628d5104211e1482e77bdf39
# Parent  96a51193678400abf9d04af65e60a8dccf540cd7
py3: convert unicode paths given for hgweb config
Yuya Nishihara - April 22, 2019, 11:22 p.m.
On Fri, 19 Apr 2019 14:34:14 +0000, Ludovic Chabant wrote:
> # HG changeset patch
> # User Ludovic Chabant <ludovic@chabant.com>
> # Date 1555683992 0
> #      Fri Apr 19 14:26:32 2019 +0000
> # Branch stable
> # Node ID b1baf28288faf3fd628d5104211e1482e77bdf39
> # Parent  96a51193678400abf9d04af65e60a8dccf540cd7
> py3: convert unicode paths given for hgweb config
> 
> diff --git a/mercurial/hgweb/__init__.py b/mercurial/hgweb/__init__.py
> --- a/mercurial/hgweb/__init__.py
> +++ b/mercurial/hgweb/__init__.py
> @@ -38,6 +38,8 @@
>      - list of virtual:real tuples (multi-repo view)
>      '''

We'll probably need to fix the caller to not pass in a unicode.
Can you copy-paste the traceback?

> +    if isinstance(config, str):

Should be pycompat.unicode for py2 compatibility.

> +        config = config.encode()

Maybe encoding.unitolocal() as the best guess. The encoding isn't known.
Ludovic Chabant - April 23, 2019, 1:40 a.m.
> We'll probably need to fix the caller to not pass in a unicode. Can
> you copy-paste the traceback?

The caller is my own code. It's basically a custom version of the
hgweb.wsgi file (see
https://www.mercurial-scm.org/repo/hg/file/tip/contrib/hgweb.wsgi).

If we want to be consistent with other things like the hgclient API,
maybe we shouldn't be nice and encode the string for the caller, and
instead reject anything that's not bytes?
Yuya Nishihara - April 23, 2019, 12:39 p.m.
On Mon, 22 Apr 2019 21:40:29 -0400, Ludovic Chabant wrote:
> 
> > We'll probably need to fix the caller to not pass in a unicode. Can
> > you copy-paste the traceback?
> 
> The caller is my own code. It's basically a custom version of the
> hgweb.wsgi file (see
> https://www.mercurial-scm.org/repo/hg/file/tip/contrib/hgweb.wsgi).
> 
> If we want to be consistent with other things like the hgclient API,
> maybe we shouldn't be nice and encode the string for the caller, and
> instead reject anything that's not bytes?

Sounds good to reject unicodes explicitly.
Ludovic Chabant - April 23, 2019, 3:20 p.m.
> > If we want to be consistent with other things like the hgclient API,
> > maybe we shouldn't be nice and encode the string for the caller, and
> > instead reject anything that's not bytes?
> 
> Sounds good to reject unicodes explicitly.

I had sent a v2 of my patch with a better way to encode the string, but now that I'm checking the hgclient code I don't think it explicitly rejects unicode strings? I think it just dies very quickly as soon as it tries to concatenate a string and a bytes.
Is there some other precedent for rejecting arguments explicitly? Is it just about raising some ValueError exception?
Yuya Nishihara - April 23, 2019, 11:22 p.m.
On Tue, 23 Apr 2019 11:20:05 -0400, Ludovic Chabant wrote:
> 
> > > If we want to be consistent with other things like the hgclient API,
> > > maybe we shouldn't be nice and encode the string for the caller, and
> > > instead reject anything that's not bytes?
> > 
> > Sounds good to reject unicodes explicitly.
> 
> I had sent a v2 of my patch with a better way to encode the string, but now that I'm checking the hgclient code I don't think it explicitly rejects unicode strings? I think it just dies very quickly as soon as it tries to concatenate a string and a bytes.

True, but I think it's good idea to provide a better error indication at
commonly-used API boundary so long as the cost doesn't matter.

> Is there some other precedent for rejecting arguments explicitly? Is it just about raising some ValueError exception?

You can raise error.ProgrammingError. templatefilters.json() has an example.

Patch

diff --git a/mercurial/hgweb/__init__.py b/mercurial/hgweb/__init__.py
--- a/mercurial/hgweb/__init__.py
+++ b/mercurial/hgweb/__init__.py
@@ -38,6 +38,8 @@ 
     - list of virtual:real tuples (multi-repo view)
     '''
 
+    if isinstance(config, str):
+        config = config.encode()
     if ((isinstance(config, bytes) and not os.path.isdir(config)) or
         isinstance(config, dict) or isinstance(config, list)):
         # create a multi-dir interface