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

Matt Harbison mharbison72 at gmail.com
Fri Sep 28 07:59:17 EDT 2018


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



More information about the Mercurial-devel mailing list