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
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.
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?
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: