[PATCH V2] py3: convert os.readlink() path to native strings

Yuya Nishihara yuya at tcha.org
Fri Sep 28 08:14:19 EDT 2018


On Fri, 28 Sep 2018 07:59:17 -0400, Matt Harbison wrote:
> 
> > On Sep 28, 2018, at 7:32 AM, Yuya Nishihara <yuya at tcha.org> wrote:
> > 
> >> On Thu, 27 Sep 2018 22:51:27 -0400, Matt Harbison wrote:
> >> # HG changeset patch
> >> # User Matt Harbison <matt_harbison at yahoo.com>
> >> # 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.


More information about the Mercurial-devel mailing list