-
-
Notifications
You must be signed in to change notification settings - Fork 31.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
bpo-28647: Update -u documentation after bpo-30404 #3961
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you. Please update also the man page.
@serhiy-storchaka should I add the "stdin is always buffered" part to 77732be#diff-1dc031ac9a0d3915ff7805109a005799R306 too? |
I don't know. This option doesn't affect stdin. Does it add clarity or noise if document this explicitly? Maybe add this to the RST documentation, but remove from the help and manpage? What other core developers think? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM.
I like the idea of mentionning that stdin is always buffered in documentation of the "-u" command line option. You may also update the man page in this PR, or it can be done in a different PR. |
Strange. The docs job of Travis CI failed on "pip --version":
First time that I see this error. |
I've addressed review comments. |
Modules/main.c
Outdated
-u : unbuffered binary stdout and stderr, stdin always buffered;\n\ | ||
also PYTHONUNBUFFERED=x\n\ | ||
-u : force the stdout and stderr streams to be unbuffered;\n\ | ||
stdin is always buffered; also PYTHONUNBUFFERED=x\n\ | ||
see man page for details on internal buffering relating to '-u'\n\ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The man page doesn't contain additional information in Python 3.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch, removed outdated sentence.
@serhiy-storchaka documented stdin as unbuffered. |
LGTM, but wait if there are other comments. |
Wait, what? It's always buffered, no? Reading one byte in Python reads 1024 bytes at the C level:
If stdin is a not a TTY, we read even more at the C level: 4096 bytes.
The -u option has an effect on sys.stdin: line_buffering. I don't think that it matters in practice, since line buffering only impacts write() whereas sys.stdin is open for read-only.
|
I checked with gdb just to be sure: I confirm that sys.stdin.buffer.read(1) calls read(1024), so reads more than 1 byte, store the following bytes for the following buffer.read() and so is buffered. |
Doc/using/cmdline.rst
Outdated
@@ -303,7 +303,8 @@ Miscellaneous options | |||
|
|||
.. cmdoption:: -u | |||
|
|||
Force the stdout and stderr streams to be unbuffered. | |||
Force the stdout and stderr streams to be unbuffered. The stdin stream is | |||
always unbuffered. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
always buffered
A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated. Once you have made the requested changes, please leave a comment on this pull request containing the phrase |
It is buffered in other meaning than in Python 2. This buffering doesn't affect behavior (unlike to Python 2). The note about buffering was related to Python 2 buffering. Try the following code in Python 2 and Python 3:
|
In that case, we should elaborate that in the documentation. I disagree to simply say "unbuffered". I suggest to not compare Python 3 to Python 2. Python 2 behaviour is weird and cannot be defined :-D |
We are fixing the outdated documentation inherited from Python 2. First than keep some statement we should consider what it means in the context of Python 2 and what it means in the context of Python 3. I suggest to continue the discussion on the tracker. |
For comparison, this is fully unbuffered read:
Reading a single byte does... read exactly one byte, and not more. |
I've now updated the PR. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM.
What do you think Serhiy?
Doc/library/sys.rst
Outdated
@@ -1276,8 +1276,10 @@ always available. | |||
:envvar:`PYTHONIOENCODING` environment variable before starting Python. | |||
|
|||
* When interactive, standard streams are line-buffered. Otherwise, they |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think all this paragraph is related to stdout and stderr. Just replace "standard streams" with "stdout and stdin". No other changes are needed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just replace "standard streams" with "stdout and stdin".
Did you mean "stdout and stderr" instead of "stdout and stdin"?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes. Sorry.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure, I can change that. Another question is that shouldn't we still document stdin buffering?
https://bugs.python.org/issue28647