classification
Title: Add Python implementation of FileIO
Type: enhancement Stage: resolved
Components: IO, Library (Lib) Versions: Python 3.5
process
Status: closed Resolution: fixed
Dependencies: 23668 Superseder:
Assigned To: serhiy.storchaka Nosy List: alex, benjamin.peterson, haypo, martin.panter, piotr.dobrogost, pitrou, python-dev, serhiy.storchaka, stutzbach
Priority: normal Keywords: patch

Created on 2014-06-24 11:06 by serhiy.storchaka, last changed 2015-05-12 11:06 by serhiy.storchaka. This issue is now closed.

Files
File name Uploaded Description Edit
pyio_fileio.patch serhiy.storchaka, 2014-06-24 11:06 review
pyio_fileio_2.patch serhiy.storchaka, 2014-07-04 18:33 review
pyio_fileio_3.patch serhiy.storchaka, 2014-07-05 20:27 review
pyio_fileio_4.patch serhiy.storchaka, 2014-07-22 11:30 review
pyio_fileio_5.patch serhiy.storchaka, 2014-07-31 20:54 review
pyio_fileio_6.patch serhiy.storchaka, 2014-10-02 16:32 review
pyio_fileio_7.patch serhiy.storchaka, 2015-03-23 10:28 review
pyio_fileio_8.patch serhiy.storchaka, 2015-03-23 12:34 review
pyio_fileio_9.patch serhiy.storchaka, 2015-04-09 23:44 review
pyio_fileio_10.patch serhiy.storchaka, 2015-04-10 11:36 review
pyio_fileio_fix.patch serhiy.storchaka, 2015-04-10 16:01 review
Messages (21)
msg221449 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2014-06-24 11:06
Proposed patch adds Python implementation of FileIO in _pyio. This will help to make io and _pyio dependency on _io optional (issue17984).
msg222303 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2014-07-04 18:33
Updated patch has included recent changes from C implementation (issue21679 and issue21090).
msg222375 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2014-07-05 20:27
Many thanks Victor for your review. Updated patch addresses your comments. It also fixes debugging remnants in test_file_eintr.
msg223659 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2014-07-22 11:30
Next iteration of the patch addressed Victor's comments.
msg224446 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2014-07-31 20:53
Next iteration of the patch addressed Victor's and Akira's comments.
msg228240 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2014-10-02 16:32
Synchronized with the tip. _io.FileIO now behave same as _pyio,FileIO when there is a NUL in name.
msg239004 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2015-03-23 10:28
Updated to the tip (added the closefd attribute in repr). os.fstat() is now called only once in the constructor.
msg239012 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2015-03-23 12:34
Restored first os.fstat() (however it now is redundant) and addressed most other Victor's comments.

In general I prefer EAFP over BDFL, and often "except AttributeError" looks better to me than getattr().
msg239046 - (view) Author: STINNER Victor (haypo) * (Python committer) Date: 2015-03-23 16:51
I opened the issue #23752: "Cleanup io.FileIO".
msg239071 - (view) Author: STINNER Victor (haypo) * (Python committer) Date: 2015-03-23 21:19
New issue #23754: "Add a new os.read_into() function to avoid memory copies".
msg240390 - (view) Author: STINNER Victor (haypo) * (Python committer) Date: 2015-04-09 21:03
What's the status of the patch? Is it ready to be commited?
msg240402 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2015-04-09 23:44
Removed redundant fstat() call (as in issue23752). Slightly corrected docstrings (but for larger changes C and Python implementations should be changed consistently).

I wait only for your approval Victor.
msg240412 - (view) Author: STINNER Victor (haypo) * (Python committer) Date: 2015-04-10 07:52
> Slightly corrected docstrings (but for larger changes C and Python implementations should be changed consistently).

I reviewed pyio_fileio_9.patch. The main issue is that I don't understand how self._fd is set to None. In the C implemented, it's set to -1 even if closefd is False.

For comments on docstrings which also concern the C code, can you maybe open a new issue with a reference to this issue or maybe even a link to my review, if you don't want to modify them in this issue?
msg240415 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2015-04-10 11:36
Updated patch addresses Victor's and Berker's comments.
msg240416 - (view) Author: STINNER Victor (haypo) * (Python committer) Date: 2015-04-10 12:21
pyio_fileio_10.patch looks good to me. Great job.
msg240420 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2015-04-10 13:26
Thank you for your review Victor and others.
msg240421 - (view) Author: Roundup Robot (python-dev) Date: 2015-04-10 13:27
New changeset 0db36098b908 by Serhiy Storchaka in branch '2.7':
Issue #21859: Corrected FileIO docstrings.
https://hg.python.org/cpython/rev/0db36098b908

New changeset d080f5ecdcd3 by Serhiy Storchaka in branch '3.4':
Issue #21859: Corrected FileIO docstrings.
https://hg.python.org/cpython/rev/d080f5ecdcd3

New changeset 330910697e23 by Serhiy Storchaka in branch 'default':
Issue #21859: Corrected FileIO docstrings.
https://hg.python.org/cpython/rev/330910697e23

New changeset 9ef5765d56b7 by Serhiy Storchaka in branch 'default':
Issue #21859: Added Python implementation of io.FileIO.
https://hg.python.org/cpython/rev/9ef5765d56b7
msg240429 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2015-04-10 16:01
There are test failures on Windows: http://buildbot.python.org/all/builders/AMD64%20Windows7%20SP1%203.x/builds/6073/steps/test/logs/stdio

1. ValueError is not raised if file name contains a null character.

2. os.ftruncate doesn't exist on Windows.

Here is a patch that adds explicit check for null character and skips tests with not implemented truncate.
msg240430 - (view) Author: STINNER Victor (haypo) * (Python committer) Date: 2015-04-10 16:14
> 2. os.ftruncate doesn't exist on Windows.

Steve Dower wrote a patch and the latest version looks good to me:
https://bugs.python.org/issue23668#msg240385

+            if 0 in os.fsencode(file):
+                raise ValueError('embedded null character')

IMO the check must be implemented in os.open() (in path_converter). It doesn't look right to have a differen behaviour on UNIX and Windows. path_converter() calls PyUnicode_FSConverter() on UNIX and this function checks for embedded null bytes.

So I would prefer to see the issue #23668 fixed and path_converter() fixed, instead of pushing pyio_fileio_fix.patch.
msg240437 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2015-04-10 16:46
Opened issue23908 for null characters in paths.
msg242961 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2015-05-12 11:06
Both issues are fixed.
History
Date User Action Args
2015-05-12 11:06:09serhiy.storchakasetstatus: open -> closed
messages: + msg242961

assignee: serhiy.storchaka
dependencies: - Check path arguments of os functions for null character
resolution: fixed
stage: patch review -> resolved
2015-04-10 22:19:29martin.pantersetnosy: + martin.panter
2015-04-10 16:46:33serhiy.storchakasetmessages: + msg240437
2015-04-10 16:44:48serhiy.storchakasetdependencies: + Support os.ftruncate on Windows, Check path arguments of os functions for null character
2015-04-10 16:14:25hayposetmessages: + msg240430
2015-04-10 16:01:24serhiy.storchakasetstatus: closed -> open
files: + pyio_fileio_fix.patch
messages: + msg240429

resolution: fixed -> (no value)
stage: resolved -> patch review
2015-04-10 13:27:25python-devsetnosy: + python-dev
messages: + msg240421
2015-04-10 13:26:50serhiy.storchakasetstatus: open -> closed
resolution: fixed
messages: + msg240420

stage: patch review -> resolved
2015-04-10 12:21:25hayposetmessages: + msg240416
2015-04-10 11:36:41serhiy.storchakasetfiles: + pyio_fileio_10.patch

messages: + msg240415
2015-04-10 07:52:08hayposetmessages: + msg240412
2015-04-09 23:44:31serhiy.storchakasetfiles: + pyio_fileio_9.patch

messages: + msg240402
2015-04-09 21:03:09hayposetmessages: + msg240390
2015-03-23 21:19:17hayposetmessages: + msg239071
2015-03-23 16:51:21hayposetmessages: + msg239046
2015-03-23 14:57:37hyneksetnosy: - hynek
2015-03-23 12:34:41serhiy.storchakasetfiles: + pyio_fileio_8.patch

messages: + msg239012
2015-03-23 10:28:28serhiy.storchakasetfiles: + pyio_fileio_7.patch

messages: + msg239004
2014-10-02 16:32:41serhiy.storchakasetfiles: + pyio_fileio_6.patch

messages: + msg228240
2014-07-31 20:54:33serhiy.storchakasetfiles: + pyio_fileio_5.patch
2014-07-31 20:53:48serhiy.storchakasetmessages: + msg224446
2014-07-22 11:30:54serhiy.storchakasetfiles: + pyio_fileio_4.patch

messages: + msg223659
2014-07-10 12:53:05piotr.dobrogostsetnosy: + piotr.dobrogost
2014-07-10 12:50:20hayposetnosy: + haypo
2014-07-05 20:27:34serhiy.storchakasetfiles: + pyio_fileio_3.patch

messages: + msg222375
2014-07-04 18:33:47serhiy.storchakasetfiles: + pyio_fileio_2.patch

messages: + msg222303
2014-06-24 11:06:53serhiy.storchakacreate