Re: [PATCH V2] py3: convert os.readlink() path to native strings
On Fri, 28 Sep 2018 07:59:17 -0400, Matt Harbison wrote: > > > On Sep 28, 2018, at 7:32 AM, Yuya Nishihara wrote: > > > >> On Thu, 27 Sep 2018 22:51:27 -0400, Matt Harbison wrote: > >> # HG changeset patch > >> # User Matt Harbison > >> # Date 1537924572 14400 > >> # Tue Sep 25 21:16:12 2018 -0400 > >> # Node ID 216114ff8d2bc57d9aa8913cf75f14267a8332f6 > >> # Parent df02cb5b9b3496aa95cbe754a92d714f4c68262b > >> py3: convert os.readlink() path to native strings > >> > >> Windows insisted that it needs to be str. I skipped the stuff that's > >> obviously > >> posix only, and left `tests/f` and `run-tests.py` alone for now. > >> > >> diff --git a/mercurial/util.py b/mercurial/util.py > >> --- a/mercurial/util.py > >> +++ b/mercurial/util.py > >> @@ -1841,7 +1841,7 @@ def makelock(info, pathname): > >> > >> def readlock(pathname): > >> try: > >> -return os.readlink(pathname) > >> +return pycompat.fsencode(os.readlink(pycompat.fsdecode(pathname))) > > > > Well, this is still bad as it goes non-native route on Unix. If we really > > need to support symlinks on Windows, we'll have to move it to platform > > module. > > Even though I turned off symlinks in tests for now because you need admin > rights to create them, I read recently that some new-ish builds of Windows 10 > allow them to be created by a normal user with developer mode enabled. So we > probably shouldn’t cut too many corners here. > > I originally coded it using procutil.tonativestr(), but figured an os > specific module with a filesystem function would be the right thing. Perhaps. I think that's what the vfs object is meant to be. > The module seemed out of place for this usage too. I didn’t look to see how > much would need to be moved to move that to encoding. I think this can be just posix/windows.readlink() for now. At some point, we might want to make windows vfs directly call windows.readlink"u"() to support unicode filenames. ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: [PATCH V2] py3: convert os.readlink() path to native strings
> On Sep 28, 2018, at 7:32 AM, Yuya Nishihara wrote: > >> On Thu, 27 Sep 2018 22:51:27 -0400, Matt Harbison wrote: >> # HG changeset patch >> # User Matt Harbison >> # Date 1537924572 14400 >> # Tue Sep 25 21:16:12 2018 -0400 >> # Node ID 216114ff8d2bc57d9aa8913cf75f14267a8332f6 >> # Parent df02cb5b9b3496aa95cbe754a92d714f4c68262b >> py3: convert os.readlink() path to native strings >> >> Windows insisted that it needs to be str. I skipped the stuff that's >> obviously >> posix only, and left `tests/f` and `run-tests.py` alone for now. >> >> diff --git a/mercurial/util.py b/mercurial/util.py >> --- a/mercurial/util.py >> +++ b/mercurial/util.py >> @@ -1841,7 +1841,7 @@ def makelock(info, pathname): >> >> def readlock(pathname): >> try: >> -return os.readlink(pathname) >> +return pycompat.fsencode(os.readlink(pycompat.fsdecode(pathname))) > > Well, this is still bad as it goes non-native route on Unix. If we really > need to support symlinks on Windows, we'll have to move it to platform module. Even though I turned off symlinks in tests for now because you need admin rights to create them, I read recently that some new-ish builds of Windows 10 allow them to be created by a normal user with developer mode enabled. So we probably shouldn’t cut too many corners here. I originally coded it using procutil.tonativestr(), but figured an os specific module with a filesystem function would be the right thing. The module seemed out of place for this usage too. I didn’t look to see how much would need to be moved to move that to encoding. ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: [PATCH V2] py3: convert os.readlink() path to native strings
On Thu, 27 Sep 2018 22:51:27 -0400, Matt Harbison wrote: > # HG changeset patch > # User Matt Harbison > # Date 1537924572 14400 > # Tue Sep 25 21:16:12 2018 -0400 > # Node ID 216114ff8d2bc57d9aa8913cf75f14267a8332f6 > # Parent df02cb5b9b3496aa95cbe754a92d714f4c68262b > py3: convert os.readlink() path to native strings > > Windows insisted that it needs to be str. I skipped the stuff that's > obviously > posix only, and left `tests/f` and `run-tests.py` alone for now. > > diff --git a/mercurial/util.py b/mercurial/util.py > --- a/mercurial/util.py > +++ b/mercurial/util.py > @@ -1841,7 +1841,7 @@ def makelock(info, pathname): > > def readlock(pathname): > try: > -return os.readlink(pathname) > +return pycompat.fsencode(os.readlink(pycompat.fsdecode(pathname))) Well, this is still bad as it goes non-native route on Unix. If we really need to support symlinks on Windows, we'll have to move it to platform module. ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
[PATCH V2] py3: convert os.readlink() path to native strings
# HG changeset patch # User Matt Harbison # Date 1537924572 14400 # Tue Sep 25 21:16:12 2018 -0400 # Node ID 216114ff8d2bc57d9aa8913cf75f14267a8332f6 # Parent df02cb5b9b3496aa95cbe754a92d714f4c68262b py3: convert os.readlink() path to native strings Windows insisted that it needs to be str. I skipped the stuff that's obviously posix only, and left `tests/f` and `run-tests.py` alone for now. diff --git a/mercurial/util.py b/mercurial/util.py --- a/mercurial/util.py +++ b/mercurial/util.py @@ -1841,7 +1841,7 @@ def makelock(info, pathname): def readlock(pathname): try: -return os.readlink(pathname) +return pycompat.fsencode(os.readlink(pycompat.fsdecode(pathname))) except OSError as why: if why.errno not in (errno.EINVAL, errno.ENOSYS): raise diff --git a/mercurial/vfs.py b/mercurial/vfs.py --- a/mercurial/vfs.py +++ b/mercurial/vfs.py @@ -206,7 +206,8 @@ class abstractvfs(object): return util.rename(srcpath, dstpath) def readlink(self, path): -return os.readlink(self.join(path)) +abspath = self.join(path) +return pycompat.fsencode(os.readlink(pycompat.fsdecode(abspath))) def removedirs(self, path=None): """Remove a leaf directory and all empty intermediate ones ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
