[PATCH] filedata: use mercurial.util.posixfile
Adrian Buehlmann
# User Adrian Buehlmann <adr...@cadifra.com>
# Date 1302335989 -7200
# Node ID a1f52609de836c2742eb4444e883dc47cdb14ee0
# Parent 6105aded66069c3e6405dd89127c56f3e75f8548
filedata: use mercurial.util.posixfile
instead of Python's open().
Python's silly open() does not set FILE_SHARE_DELETE on the dwShareMode for the
CreateFile API function of Windows. Deleting files will fail in that state on
Windows.
As usual with this stuff, I cannot prove anything conclusive that this exact
open() call was truly bad here and the file is closed quickly anyway, but *not*
setting FILE_SHARE_DELETE when reading a file on Windows is a stupid thing.
One day, I might finally convince myself to try having a chat with the
Python folks about fixing their open(). Until that happens, I'm using
Mercurial's posixfile.
See also http://mercurial.selenic.com/wiki/UnlinkingFilesOnWindows
diff --git a/tortoisehg/hgqt/filedata.py b/tortoisehg/hgqt/filedata.py
--- a/tortoisehg/hgqt/filedata.py
+++ b/tortoisehg/hgqt/filedata.py
@@ -255,7 +255,7 @@
if os.path.getsize(repo.wjoin(wfile)) > ctx._repo.maxdiff:
self.error = mde
else:
- data = open(repo.wjoin(wfile), 'r').read()
+ data = util.posixfile(repo.wjoin(wfile), 'r').read()
if '\0' in data:
self.error = 'binary file'
else:
Adrian Buehlmann
> # HG changeset patch
> # User Adrian Buehlmann <adr...@cadifra.com>
> # Date 1302335989 -7200
> # Node ID a1f52609de836c2742eb4444e883dc47cdb14ee0
> # Parent 6105aded66069c3e6405dd89127c56f3e75f8548
> filedata: use mercurial.util.posixfile
I've just pushed this.
Adrian Buehlmann
> Python folks about fixing their open(). Until that happens, I'm using
> Mercurial's posixfile.
I know this is totally off topic here, but I started hacking on python (yikes!).
Luckily, they are on Mercurial now :-)
TODO: Test and figure out how to propose that to the Python hackers. Sigh.
Patch against their default branch (which is 3.x-ish :-/):
# HG changeset patch
# User Adrian Buehlmann <adr...@cadifra.com>
# Date 1302371108 -7200
# Node ID 06cb83e97fa4678670e23bb521d934753481a79b
# Parent 2d0a855ce30adc10b4796502742b735b7722a6f3
set FILE_SHARE_DELETE on Windows for open()
allows to delete open files
diff --git a/Modules/_io/fileio.c b/Modules/_io/fileio.c
--- a/Modules/_io/fileio.c
+++ b/Modules/_io/fileio.c
@@ -224,6 +224,9 @@
char *s;
#ifdef MS_WINDOWS
Py_UNICODE *widename = NULL;
+ DWORD access = 0;
+ DWORD creation;
+ HANDLE handle;
#endif
int ret = 0;
int rwa = 0, plus = 0, append = 0;
@@ -302,6 +305,10 @@
}
rwa = 1;
self->readable = 1;
+#ifdef MS_WINDOWS
+ creation = OPEN_EXISTING;
+ access = GENERIC_READ;
+#endif
break;
case 'w':
if (rwa)
@@ -309,6 +316,10 @@
rwa = 1;
self->writable = 1;
flags |= O_CREAT | O_TRUNC;
+#ifdef MS_WINDOWS
+ creation = CREATE_ALWAYS;
+ access = GENERIC_READ | GENERIC_WRITE;
+#endif
break;
case 'a':
if (rwa)
@@ -317,6 +328,10 @@
self->writable = 1;
flags |= O_CREAT;
append = 1;
+#ifdef MS_WINDOWS
+ creation = OPEN_ALWAYS;
+ access = GENERIC_READ | GENERIC_WRITE;
+#endif
break;
case 'b':
break;
@@ -325,6 +340,9 @@
goto bad_mode;
self->readable = self->writable = 1;
plus = 1;
+#ifdef MS_WINDOWS
+ access = GENERIC_READ | GENERIC_WRITE;
+#endif
break;
default:
PyErr_Format(PyExc_ValueError,
@@ -370,13 +388,26 @@
errno = 0;
#ifdef MS_WINDOWS
if (widename != NULL)
- self->fd = _wopen(widename, flags, 0666);
+ handle = CreateFileW(widename, access,
+ FILE_SHARE_READ | FILE_SHARE_WRITE |
+ FILE_SHARE_DELETE,
+ NULL, creation, FILE_ATTRIBUTE_NORMAL, 0);
else
+ handle = CreateFileA(name, access,
+ FILE_SHARE_READ | FILE_SHARE_WRITE |
+ FILE_SHARE_DELETE,
+ NULL, creation, FILE_ATTRIBUTE_NORMAL, 0);
+ if (handle == INVALID_HANDLE_VALUE)
+ self->fd = -1
+ else
+ self->fd = _open_osfhandle((intptr_t)handle, flags);
+#else
+ self->fd = open(name, flags, 0666);
#endif
- self->fd = open(name, flags, 0666);
Py_END_ALLOW_THREADS
if (self->fd < 0) {
#ifdef MS_WINDOWS
+ CloseHandle(handle);
if (widename != NULL)
PyErr_SetFromErrnoWithUnicodeFilename(PyExc_IOError, widename);
else
Steve Borho
> On 2011-04-09 10:24, Adrian Buehlmann wrote:
>> One day, I might finally convince myself to try having a chat with the
>> Python folks about fixing their open(). Until that happens, I'm using
>> Mercurial's posixfile.
>
> I know this is totally off topic here, but I started hacking on python (yikes!).
> Luckily, they are on Mercurial now :-)
>
> TODO: Test and figure out how to propose that to the Python hackers. Sigh.
>
> Patch against their default branch (which is 3.x-ish :-/):
Yikes. If only they could be convinced to back-port to 2.7.x, we
might actually benefit from it.
--
Steve Borho
Adrian Buehlmann
> On Sat, Apr 9, 2011 at 1:13 PM, Adrian Buehlmann <adr...@cadifra.com> wrote:
>> On 2011-04-09 10:24, Adrian Buehlmann wrote:
>>> One day, I might finally convince myself to try having a chat with the
>>> Python folks about fixing their open(). Until that happens, I'm using
>>> Mercurial's posixfile.
>>
>> I know this is totally off topic here, but I started hacking on python (yikes!).
>> Luckily, they are on Mercurial now :-)
>>
>> TODO: Test and figure out how to propose that to the Python hackers. Sigh.
>>
>> Patch against their default branch (which is 3.x-ish :-/):
>
> Yikes. If only they could be convinced to back-port to 2.7.x, we
> might actually benefit from it.
Having that in 2.7.x would be nicer indeed. No idea of they still take
changes for 2.7.
But things probably won't get better if I do nothing :/
Adrian Buehlmann
> On 2011-04-09 10:24, Adrian Buehlmann wrote:
>> One day, I might finally convince myself to try having a chat with the
>> Python folks about fixing their open(). Until that happens, I'm using
>> Mercurial's posixfile.
>
> I know this is totally off topic here, but I started hacking on python (yikes!).
> Luckily, they are on Mercurial now :-)
>
> TODO: Test and figure out how to propose that to the Python hackers. Sigh.
>
> Patch against their default branch (which is 3.x-ish :-/):
>
> # HG changeset patch
> # User Adrian Buehlmann <adr...@cadifra.com>
> # Date 1302371108 -7200
> # Node ID 06cb83e97fa4678670e23bb521d934753481a79b
> # Parent 2d0a855ce30adc10b4796502742b735b7722a6f3
> set FILE_SHARE_DELETE on Windows for open()
>
> allows to delete open files
>
Cool. And my patch even seems to work for a simple test case
(debug session run from Visual C++ Express 2008):
Python 3.3a0 (default, Apr 9 2011, 19:55:59) [MSC v.1500 32 bit (Intel)] on win
32
Type "help", "copyright", "credits" or "license" for more information.
>>> f = open('C:\\Users/adi/hgrepos/tests/bla')
[55064 refs]
>>> import os
[55066 refs]
>>> os.unlink('C:\\Users/adi/hgrepos/tests/bla')
[55067 refs]
>>> f.close()
[55073 refs]
>>>
The file is gone after the close - as expected. And no error!
Compare:
$ python
Python 2.6.6 (r266:84297, Aug 24 2010, 18:13:38) [MSC v.1500 64 bit (AMD64)] on win32
Type "help", "copyright", "credits" or "license" for more information.
>>> f=open('bla')
>>> import os
>>> os.unlink('bla')
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
WindowsError: [Error 32] The process cannot access the file because it is being used by another process: 'bla'
>>>
(the error message is even misleading, since it's the very
same process which is using the file :-)
Now, I think I really need to talk with the Python folks...
(sorry for the noise)
Adrian Buehlmann
Sorry for adding to such an old obscure thread.
FWIW
It just occurred to me that the real root cause of the problem is
Microsoft's implementation of the C runtime (aka the CRT).
(not really a surprise :-)
Python's 'file' implementation just calls the C runtime.
I remembered that the full version of visual studio does install the
source for the CRT and the license allows to modify it for own projects,
but does not allow to publish the modified code (what a pity).
It only allows to distribute the compiled and linked program.
The relevant part of the license is:
<quote>
� MFCs, ATLs and CRTs. You may modify the source code form of Microsoft
Foundation Classes (MFCs), Active Template Libraries (ATLs), and C
runtimes (CRTs) to design, develop and test your programs, and copy and
distribute the object code form of your modified files under a new name.
</quote>
I've looked at the implementation of the CRT's _open: they don't set
FILE_SHARE_DELETE when calling the win32 API CreateFile (except when
_O_TEMPORARY is set in the flags - sigh).
Technically, patching the source of _open to set FILE_SHARE_DELETE when
calling CreateFile is trivial (man, this would have saved us a lot of
troubles... In theory).
What I have no idea is how to share such a modified CRT source in an
open source project and distribute programs using it.
I think in theory, it should be possible to build a modified python
which is "linked" against a modified CRT.
What a wild idea. Sigh.
BTW, I found someone else pondering modifying the MS CRT:
http://benjamin.smedbergs.us/blog/2008-01-10/patching-the-windows-crt/
(no solution there, just listing the same problem).
Steve Borho
> Foundation Classes (MFCs), Active Template Libraries (ATLs), and C
> runtimes (CRTs) to design, develop and test your programs, and copy and
> distribute the object code form of your modified files under a new name.
> </quote>
>
> I've looked at the implementation of the CRT's _open: they don't set
> FILE_SHARE_DELETE when calling the win32 API CreateFile (except when
> _O_TEMPORARY is set in the flags - sigh).
>
> Technically, patching the source of _open to set FILE_SHARE_DELETE when
> calling CreateFile is trivial (man, this would have saved us a lot of
> troubles... In theory).
>
> What I have no idea is how to share such a modified CRT source in an
> open source project and distribute programs using it.
>
> I think in theory, it should be possible to build a modified python
> which is "linked" against a modified CRT.
>
> What a wild idea. Sigh.
>
> BTW, I found someone else pondering modifying the MS CRT:
> http://benjamin.smedbergs.us/blog/2008-01-10/patching-the-windows-crt/
> (no solution there, just listing the same problem).
>
Do we get updated source to the CRT when they release security
patches? If we shipped our on CRT libs, we could quit including their
merge modules. But what a gawd awful pain it would be.
--
Steve Borho
Adrian Buehlmann
>> Foundation Classes (MFCs), Active Template Libraries (ATLs), and C
>> runtimes (CRTs) to design, develop and test your programs, and copy and
>> distribute the object code form of your modified files under a new name.
>> </quote>
>>
>> I've looked at the implementation of the CRT's _open: they don't set
>> FILE_SHARE_DELETE when calling the win32 API CreateFile (except when
>> _O_TEMPORARY is set in the flags - sigh).
>>
>> Technically, patching the source of _open to set FILE_SHARE_DELETE when
>> calling CreateFile is trivial (man, this would have saved us a lot of
>> troubles... In theory).
>>
>> What I have no idea is how to share such a modified CRT source in an
>> open source project and distribute programs using it.
>>
>> I think in theory, it should be possible to build a modified python
>> which is "linked" against a modified CRT.
>>
>> What a wild idea. Sigh.
>>
>> BTW, I found someone else pondering modifying the MS CRT:
>> http://benjamin.smedbergs.us/blog/2008-01-10/patching-the-windows-crt/
>> (no solution there, just listing the same problem).
>>
>
> Do we get updated source to the CRT when they release security
> patches? If we shipped our on CRT libs, we could quit including their
> merge modules. But what a gawd awful pain it would be.
No idea if security fixes are distributed on source level in any way at
all. In theory MS could do so with visual studio service packs. No idea
if they do.
All sorts of pains indeed.
I think the only paths where that could make sense are:
A) ask MS to change their CRT (or, failing that)
B) talk with the Python folks and see if they would be willing
to ship python for Windows with a patch own CRT.
I guess the chances that any of the two would happen is very low.
It's just amazing how trivial it is to fix the CRT source (on a
technical level). So I decided to post the idea.