classification
Title: Support os.ftruncate on Windows
Type: enhancement Stage: resolved
Components: Windows Versions: Python 3.5
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: steve.dower Nosy List: haypo, python-dev, serhiy.storchaka, steve.dower, tim.golden, zach.ware
Priority: normal Keywords: patch

Created on 2015-03-15 01:41 by steve.dower, last changed 2015-04-27 05:32 by steve.dower. This issue is now closed.

Files
File name Uploaded Description Edit
23668_1.diff steve.dower, 2015-03-15 01:55
23668_2.diff steve.dower, 2015-03-15 15:18
23668_2.patch steve.dower, 2015-03-15 15:51
23668_2.patch serhiy.storchaka, 2015-03-16 08:10 Regenerated for Rietveld review
23668_3.patch steve.dower, 2015-03-21 03:26 review
Messages (18)
msg238113 - (view) Author: Steve Dower (steve.dower) * (Python committer) Date: 2015-03-15 01:41
With _chsize_s (which supports 64-bit sizes, unlike _chsize), this seems fairly trivial to do, but I'll put a patch up for it in case there's something I've missed.
msg238135 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2015-03-15 08:40
Rietveld doesn't accept patches in git format.
msg238144 - (view) Author: Steve Dower (steve.dower) * (Python committer) Date: 2015-03-15 13:40
It normally does... I'll regenerate the patch when I get a chance later today.
msg238149 - (view) Author: Steve Dower (steve.dower) * (Python committer) Date: 2015-03-15 15:18
Regenerated the patch file. Rietveld may not have liked that the parent changeset in the previous patch doesn't exist (I pulled this out of my patch queue).
msg238152 - (view) Author: Steve Dower (steve.dower) * (Python committer) Date: 2015-03-15 15:51
Hmm... doesn't even know that the issue has been changed. Reuploading with a different extension.
msg238153 - (view) Author: Steve Dower (steve.dower) * (Python committer) Date: 2015-03-15 16:02
I get a 405 error if I try and upload the patch on http://bugs.python.org/review/23668/

My other patches from last night worked fine, so maybe Rietveld doesn't like this issue for some reason?
msg238184 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2015-03-16 08:45
At first glance the patch looks good, but I did not test it. Are there tests for truncate() with large files (> 4 GiB)?

ftruncate() is used also im mmap.resize(). Do you want to provide a patch also for mmap?
msg238185 - (view) Author: STINNER Victor (haypo) * (Python committer) Date: 2015-03-16 08:48
I reviewed 23668_2.patch.
msg238214 - (view) Author: Steve Dower (steve.dower) * (Python committer) Date: 2015-03-16 15:49
It looks like mmap uses pure Win32 APIs for the Windows implementation, and so _chsize_s isn't necessary. There is a comment "todo: need ... a 'chsize' analog" in the file, but I'm not actually sure what that means.

I've made some minor changes from the review, but I had a question about possibly defining HAVE_[F]TRUNCATE in *.c rather than *.h. See the review, and I'll update the patch once we've figured that out.
msg238752 - (view) Author: Steve Dower (steve.dower) * (Python committer) Date: 2015-03-21 03:26
Updated the patch, since there's been a lot of checkins.

I also removed the pyconfig.h changes and updated the #ifdef in posixmodule.c to enable truncate/ftruncate and define PATH_HAVE_FTRUNCATE.

And I know in the last review I said I'd switch to _Py_wopen(), but if I do that there's no way to avoid passing _O_CREAT, so I opted to stick with _wopen() and pass _O_NOINHERIT instead.

Hopefully Reitveld handles this patch file. I'm sure I'm not doing anything differently from normal...
msg238841 - (view) Author: STINNER Victor (haypo) * (Python committer) Date: 2015-03-21 22:48
Is _chsize_s() available on all Windows versions and all Visual Studio
(msvcrt) versions?
Le 21 mars 2015 04:26, "Steve Dower" <report@bugs.python.org> a écrit :

>
> Steve Dower added the comment:
>
> Updated the patch, since there's been a lot of checkins.
>
> I also removed the pyconfig.h changes and updated the #ifdef in
> posixmodule.c to enable truncate/ftruncate and define PATH_HAVE_FTRUNCATE.
>
> And I know in the last review I said I'd switch to _Py_wopen(), but if I
> do that there's no way to avoid passing _O_CREAT, so I opted to stick with
> _wopen() and pass _O_NOINHERIT instead.
>
> Hopefully Reitveld handles this patch file. I'm sure I'm not doing
> anything differently from normal...
>
> ----------
> Added file: http://bugs.python.org/file38614/23668_3.patch
>
> _______________________________________
> Python tracker <report@bugs.python.org>
> <http://bugs.python.org/issue23668>
> _______________________________________
>
msg238843 - (view) Author: Steve Dower (steve.dower) * (Python committer) Date: 2015-03-21 22:53
Yep, all the way back to VS 2005 and Windows 95. Not sure why it wasn't used previously (_chsize doesn't support 64-bit values, which is a reasonable reason to not use it).
msg240003 - (view) Author: Steve Dower (steve.dower) * (Python committer) Date: 2015-04-03 15:50
I responded to Victor's suggestion about _Py_open instead of _open, but on rereading I see that it also handles EINTR. 

AFAICT (from https://msdn.microsoft.com/en-us/library/5814770t.aspx, mainly), Windows isn't ever going to return EINTR from the CRT. I don't especially want to have to modify _Py_open to take another parameter to suppress creating a new file, but if someone else does then I'm happy to use it.
msg240007 - (view) Author: STINNER Victor (haypo) * (Python committer) Date: 2015-04-03 16:09
Sorry, I still fail to see how _Py_open() or _Py_open_noraise() add the O_CREAT flag: they are thin wrapper to the underyling open() function. I cannot see O_CREAT in Python/fileutils.c. Could you please show me that the line number adding O_CREAT implictly?

I don't understand.
msg240014 - (view) Author: Steve Dower (steve.dower) * (Python committer) Date: 2015-04-03 17:38
Sorry, _Py_open requires a double encoding (wchar->char->wchar), which is why I'm not going to use it.

_Py_wfopen takes a mode string rather than _O_* flags, and so implicitly includes _O_CREAT.

Guessing we should add _Py_wopen?
msg240385 - (view) Author: STINNER Victor (haypo) * (Python committer) Date: 2015-04-09 20:43
23668_3.patch looks good to me.

I agree that handling EINTR is not needed on Windows, and so there is no need for an helper function like _Py_open_noraise().
msg240537 - (view) Author: Roundup Robot (python-dev) Date: 2015-04-12 04:32
New changeset 505bf6086ec5 by Steve Dower in branch 'default':
Issue #23668: Adds support for os.truncate and os.ftruncate on Windows
https://hg.python.org/cpython/rev/505bf6086ec5

New changeset eba85ae7d128 by Steve Dower in branch 'default':
Issue #23668: Suppresses invalid parameter handler around chsize calls.
https://hg.python.org/cpython/rev/eba85ae7d128
msg240566 - (view) Author: Roundup Robot (python-dev) Date: 2015-04-12 19:45
New changeset cb7ca578a0c3 by Steve Dower in branch 'default':
Issue #23668: Regenerates posixmodule.c.h for new ifdefs
https://hg.python.org/cpython/rev/cb7ca578a0c3
History
Date User Action Args
2015-04-27 05:32:23steve.dowersetstatus: open -> closed
resolution: fixed
stage: commit review -> resolved
2015-04-12 19:45:38python-devsetmessages: + msg240566
2015-04-12 04:33:04steve.dowersetstage: commit review
2015-04-12 04:32:35python-devsetnosy: + python-dev
messages: + msg240537
2015-04-10 16:44:48serhiy.storchakalinkissue21859 dependencies
2015-04-09 20:43:46hayposetmessages: + msg240385
2015-04-03 17:38:56steve.dowersetmessages: + msg240014
2015-04-03 16:09:45hayposetmessages: + msg240007
2015-04-03 15:50:09steve.dowersetmessages: + msg240003
2015-03-21 22:53:12steve.dowersetmessages: + msg238843
title: Support os. -> Support os.ftruncate on Windows
2015-03-21 22:48:19hayposetmessages: + msg238841
title: Support os.[f]truncate on Windows -> Support os.
2015-03-21 03:26:48steve.dowersetfiles: + 23668_3.patch

messages: + msg238752
2015-03-16 15:49:24steve.dowersetmessages: + msg238214
2015-03-16 08:48:35hayposetmessages: + msg238185
2015-03-16 08:45:39serhiy.storchakasetmessages: + msg238184
2015-03-16 08:10:39serhiy.storchakasetfiles: + 23668_2.patch
2015-03-15 16:02:56hayposetnosy: + haypo
2015-03-15 16:02:32steve.dowersetmessages: + msg238153
2015-03-15 15:51:59steve.dowersetfiles: + 23668_2.patch

messages: + msg238152
2015-03-15 15:18:16steve.dowersetfiles: + 23668_2.diff

messages: + msg238149
2015-03-15 13:40:12steve.dowersetmessages: + msg238144
2015-03-15 08:40:36serhiy.storchakasetnosy: + serhiy.storchaka
messages: + msg238135
2015-03-15 01:55:53steve.dowersetfiles: + 23668_1.diff
keywords: + patch
2015-03-15 01:41:38steve.dowercreate