Patchwork [1,of,7,STABLE] windows: check target type before actual unlinking to follow POSIX semantics

login
register
mail settings
Submitter Katsunori FUJIWARA
Date May 6, 2013, 8:35 p.m.
Message ID <127d7233b406699617f6.1367872501@juju>
Download mbox | patch
Permalink /patch/1565/
State Accepted, archived
Commit 1b329f8c7b2479425150f79c2d4a9bc11de9ea74
Headers show

Comments

Katsunori FUJIWARA - May 6, 2013, 8:35 p.m.
# HG changeset patch
# User FUJIWARA Katsunori <foozy@lares.dti.ne.jp>
# Date 1367870651 -32400
#      Tue May 07 05:04:11 2013 +0900
# Branch stable
# Node ID 127d7233b406699617f6dbcbc34e5ea00c2aa223
# Parent  0a12e5f3a979ee302dc10647483200df00a105ab
windows: check target type before actual unlinking to follow POSIX semantics

Creation and writing into target file via vfs (a.k.a opener) is done
after "unlink()" target file, if it exists.

For example, it is assumed that the revision X consists of file 'A',
and the revision Y consists of file 'A/B'. Merging revision X into Y
tries to "unlink()" on directory 'A' of 'A/B', before creation of file
'A'.

On POSIX environment, directories should be removed by "rmdir(2)", and
"unlink(2)" on directories fails. "unlink()" of Mercurial (and Python)
uses "unlink(2)" directly, so unlinking in the merge case above would
fail.

In the other hand, on Windows environment, "unlink()" of Mercurial
tries to rename before actual unlinking, to follow POSIX semantics:
already opened file can be unlinked safely.

This causes unexpected success in unlinking in the merge case above,
even though directory 'A' is renamed to another. This confuses users.

This patch checks whether target is directory or not before renaming,
and raises IOError(errno.EPERM) if so, to follow POSIX semantics.
Matt Mackall - May 7, 2013, 9:08 p.m.
On Tue, 2013-05-07 at 05:35 +0900, FUJIWARA Katsunori wrote:
> # HG changeset patch
> # User FUJIWARA Katsunori <foozy@lares.dti.ne.jp>
> # Date 1367870651 -32400
> #      Tue May 07 05:04:11 2013 +0900
> # Branch stable
> # Node ID 127d7233b406699617f6dbcbc34e5ea00c2aa223
> # Parent  0a12e5f3a979ee302dc10647483200df00a105ab
> windows: check target type before actual unlinking to follow POSIX semantics

I've queued this first patch for stable, thanks.

Patch

diff --git a/mercurial/win32.py b/mercurial/win32.py
--- a/mercurial/win32.py
+++ b/mercurial/win32.py
@@ -344,6 +344,12 @@ 
 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)
+
     # POSIX allows to unlink and rename open files. Windows has serious
     # problems with doing that:
     # - Calling os.unlink (or os.rename) on a file f fails if f or any