D588: win32: use fewer system calls for unlink()

Adrian Buehlmann adrian at cadifra.com
Fri Sep 1 14:15:10 EDT 2017


On 2017-09-01 08:16, Adrian Buehlmann wrote:
> Ugh. Can I reply to a phabricator notification by email?
> 
> Adding gregory.szorc at gmail.com manually, as I'm not sure replaying to
> those nasty phabricator emails is going to work...
> 
> On 2017-09-01 00:32, indygreg (Gregory Szorc) wrote:
>> diff --git a/mercurial/win32.py b/mercurial/win32.py
>> --- a/mercurial/win32.py
>> +++ b/mercurial/win32.py
>> @@ -12,6 +12,7 @@
>>  import msvcrt
>>  import os
>>  import random
>> +import stat
>>  import subprocess
>>  
>>  from . import (
>> @@ -522,11 +523,26 @@
>>  def unlink(f):
>>      '''try to implement POSIX' unlink semantics on Windows'''
>>  
>> -    if os.path.isdir(f):
>> -        # use EPERM because it is POSIX prescribed value, even though
>> -        # unlink(2) on directories returns EISDIR on Linux
>> -        raise IOError(errno.EPERM,
>> -                      "Unlinking directory not permitted: '%s'" % f)
>> +    # If the path doesn't exist, raise that exception.
>> +    # If it is a directory, emulate POSIX behavior.
>> +    try:
>> +        st = os.stat(f)
>> +        if stat.S_ISDIR(st.st_mode):
>> +            # use EPERM because it is POSIX prescribed value, even though
>> +            # unlink(2) on directories returns EISDIR on Linux
>> +            raise IOError(errno.EPERM,
>> +                          "Unlinking directory not permitted: '%s'" % f)
>> +    except OSError as e:
>> +        if e.errno == errno.ENOENT:
>> +            raise
>> +
>> +    # In the common case, a normal unlink will work. Try that first and fall
>> +    # back to more complexity if and only if we need to.
>> +    try:
>> +        os.unlink(f)
>> +        return
>> +    except (IOError, OSError) as e:
>> +        pass
>>  
>>      # POSIX allows to unlink and rename open files. Windows has serious
>>      # problems with doing that:
> 
> Do you get an error at all, if a file, which is in open state, is unlinked?
> 
> My fear is: You won't get an error, but instead the filename is blocked
> by the file being held in place by the other process, until the other
> process closes it. Which means: You already lost the game.
> 
> Which would explain why we didn't do things like you propose here.
> 
> See also
> 
>    https://www.mercurial-scm.org/wiki/UnlinkingFilesOnWindows
> 
> (specifically, heading 2)
> 

In other words:

The "zombie state" - which the rename dance afterwards tries to avoid -
has already been initiated (by calling unlink) when the rename dance is
started. Which is pointless. You then might as well remove the rename
step: It can't help any more. It will even fail, as you can't rename a
file in zombie state.


More information about the Mercurial-devel mailing list