Patchwork [2,of,7] py3: make sure osutil.listdir() returns what it gets

login
register
mail settings
Submitter Pulkit Goyal
Date Nov. 2, 2016, 10:23 p.m.
Message ID <6585e9c1d915818d5138.1478125387@pulkit-goyal>
Download mbox | patch
Permalink /patch/17299/
State Accepted
Headers show

Comments

Pulkit Goyal - Nov. 2, 2016, 10:23 p.m.
# HG changeset patch
# User Pulkit Goyal <7895pulkit@gmail.com>
# Date 1478118344 -19800
#      Thu Nov 03 01:55:44 2016 +0530
# Node ID 6585e9c1d915818d5138f6cb4134707002d5d749
# Parent  e541b0e5839988f63446c88509db68772a55775b
py3: make sure osutil.listdir() returns what it gets

osutil.listdir() on py3 was having problems with bytes. Since
os.sep is a str in Py3, we need to check if what passed is bytes and then
convert os.sep to bytes. On python 2, doing os.sep.encode() is okay.
After this patch, osutil.listdir() argument will return what is gets as an
argument on Python 3.5.
Yuya Nishihara - Nov. 4, 2016, 3:37 a.m.
On Thu, 03 Nov 2016 03:53:07 +0530, Pulkit Goyal wrote:
> # HG changeset patch
> # User Pulkit Goyal <7895pulkit@gmail.com>
> # Date 1478118344 -19800
> #      Thu Nov 03 01:55:44 2016 +0530
> # Node ID 6585e9c1d915818d5138f6cb4134707002d5d749
> # Parent  e541b0e5839988f63446c88509db68772a55775b
> py3: make sure osutil.listdir() returns what it gets
> 
> osutil.listdir() on py3 was having problems with bytes. Since
> os.sep is a str in Py3, we need to check if what passed is bytes and then
> convert os.sep to bytes. On python 2, doing os.sep.encode() is okay.
> After this patch, osutil.listdir() argument will return what is gets as an
> argument on Python 3.5.
> 
> diff -r e541b0e58399 -r 6585e9c1d915 mercurial/pure/osutil.py
> --- a/mercurial/pure/osutil.py	Thu Nov 03 00:28:33 2016 +0530
> +++ b/mercurial/pure/osutil.py	Thu Nov 03 01:55:44 2016 +0530
> @@ -51,8 +51,11 @@
>      '''
>      result = []
>      prefix = path
> -    if not prefix.endswith(os.sep):
> -        prefix += os.sep
> +    sep = os.sep
> +    if isinstance(path, bytes):
> +        sep = sep.encode('ascii')
> +    if not prefix.endswith(sep):
> +        prefix += sep

Suppose path must be bytes because osutil.listdir() is a Mercurial API, we
can always use bytes variant of os.sep everywhere, which could be provided
by pycompat.
Pulkit Goyal - Nov. 4, 2016, 11:36 p.m.
On Fri, Nov 4, 2016 at 9:07 AM, Yuya Nishihara <yuya@tcha.org> wrote:
> On Thu, 03 Nov 2016 03:53:07 +0530, Pulkit Goyal wrote:
>> # HG changeset patch
>> # User Pulkit Goyal <7895pulkit@gmail.com>
>> # Date 1478118344 -19800
>> #      Thu Nov 03 01:55:44 2016 +0530
>> # Node ID 6585e9c1d915818d5138f6cb4134707002d5d749
>> # Parent  e541b0e5839988f63446c88509db68772a55775b
>> py3: make sure osutil.listdir() returns what it gets
>>
>> osutil.listdir() on py3 was having problems with bytes. Since
>> os.sep is a str in Py3, we need to check if what passed is bytes and then
>> convert os.sep to bytes. On python 2, doing os.sep.encode() is okay.
>> After this patch, osutil.listdir() argument will return what is gets as an
>> argument on Python 3.5.
>>
>> diff -r e541b0e58399 -r 6585e9c1d915 mercurial/pure/osutil.py
>> --- a/mercurial/pure/osutil.py        Thu Nov 03 00:28:33 2016 +0530
>> +++ b/mercurial/pure/osutil.py        Thu Nov 03 01:55:44 2016 +0530
>> @@ -51,8 +51,11 @@
>>      '''
>>      result = []
>>      prefix = path
>> -    if not prefix.endswith(os.sep):
>> -        prefix += os.sep
>> +    sep = os.sep
>> +    if isinstance(path, bytes):
>> +        sep = sep.encode('ascii')
>> +    if not prefix.endswith(sep):
>> +        prefix += sep
>
> Suppose path must be bytes because osutil.listdir() is a Mercurial API, we
> can always use bytes variant of os.sep everywhere, which could be provided
> by pycompat.

osutil.listdir() is called from windows.py, scmwindows.py,
win32mbcs.py also and I am unsure about things being bytes there,
thats why I kept the function general. Other functions like scmposix.*
are meant for posix only. Will paths on windows will be bytes also?
Yuya Nishihara - Nov. 5, 2016, 1:27 a.m.
On Sat, 5 Nov 2016 05:06:39 +0530, Pulkit Goyal wrote:
> On Fri, Nov 4, 2016 at 9:07 AM, Yuya Nishihara <yuya@tcha.org> wrote:
> > On Thu, 03 Nov 2016 03:53:07 +0530, Pulkit Goyal wrote:
> >> # HG changeset patch
> >> # User Pulkit Goyal <7895pulkit@gmail.com>
> >> # Date 1478118344 -19800
> >> #      Thu Nov 03 01:55:44 2016 +0530
> >> # Node ID 6585e9c1d915818d5138f6cb4134707002d5d749
> >> # Parent  e541b0e5839988f63446c88509db68772a55775b
> >> py3: make sure osutil.listdir() returns what it gets
> >>
> >> osutil.listdir() on py3 was having problems with bytes. Since
> >> os.sep is a str in Py3, we need to check if what passed is bytes and then
> >> convert os.sep to bytes. On python 2, doing os.sep.encode() is okay.
> >> After this patch, osutil.listdir() argument will return what is gets as an
> >> argument on Python 3.5.
> >>
> >> diff -r e541b0e58399 -r 6585e9c1d915 mercurial/pure/osutil.py
> >> --- a/mercurial/pure/osutil.py        Thu Nov 03 00:28:33 2016 +0530
> >> +++ b/mercurial/pure/osutil.py        Thu Nov 03 01:55:44 2016 +0530
> >> @@ -51,8 +51,11 @@
> >>      '''
> >>      result = []
> >>      prefix = path
> >> -    if not prefix.endswith(os.sep):
> >> -        prefix += os.sep
> >> +    sep = os.sep
> >> +    if isinstance(path, bytes):
> >> +        sep = sep.encode('ascii')
> >> +    if not prefix.endswith(sep):
> >> +        prefix += sep
> >
> > Suppose path must be bytes because osutil.listdir() is a Mercurial API, we
> > can always use bytes variant of os.sep everywhere, which could be provided
> > by pycompat.
> 
> osutil.listdir() is called from windows.py, scmwindows.py,
> win32mbcs.py also and I am unsure about things being bytes there,
> thats why I kept the function general. Other functions like scmposix.*
> are meant for posix only. Will paths on windows will be bytes also?

They should be bytes API. windows.* and scmwindows.* functions are re-exported
as util.* and scmutil.* respectively.

Patch

diff -r e541b0e58399 -r 6585e9c1d915 mercurial/pure/osutil.py
--- a/mercurial/pure/osutil.py	Thu Nov 03 00:28:33 2016 +0530
+++ b/mercurial/pure/osutil.py	Thu Nov 03 01:55:44 2016 +0530
@@ -51,8 +51,11 @@ 
     '''
     result = []
     prefix = path
-    if not prefix.endswith(os.sep):
-        prefix += os.sep
+    sep = os.sep
+    if isinstance(path, bytes):
+        sep = sep.encode('ascii')
+    if not prefix.endswith(sep):
+        prefix += sep
     names = os.listdir(path)
     names.sort()
     for fn in names: