Skip to content
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

Document how to make pty.spawn not copy data #67054

Closed
RadicalZephyr mannequin opened this issue Nov 13, 2014 · 15 comments
Closed

Document how to make pty.spawn not copy data #67054

RadicalZephyr mannequin opened this issue Nov 13, 2014 · 15 comments
Labels
3.7 (EOL) end of life 3.8 only security fixes docs Documentation in the Doc dir type-feature A feature request or enhancement

Comments

@RadicalZephyr
Copy link
Mannequin

RadicalZephyr mannequin commented Nov 13, 2014

BPO 22865
Nosy @Yhg1s, @vstinner, @vadmium, @RadicalZephyr, @csabella
PRs
  • bpo-22865: Expand on documentation for the pty.spawn function #11980
  • [3.7] bpo-22865: Expand on documentation for the pty.spawn function (GH-11980) #13455
  • Files
  • pty.patch
  • pty.patch: Updated patch
  • pty.patch: Documentation update
  • Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

    Show more details

    GitHub fields:

    assignee = None
    closed_at = <Date 2019-05-21.09:38:34.948>
    created_at = <Date 2014-11-13.18:01:26.047>
    labels = ['3.8', 'type-feature', '3.7', 'docs']
    title = 'Document how to make pty.spawn not copy data'
    updated_at = <Date 2019-05-21.09:38:34.947>
    user = 'https://github.com/RadicalZephyr'

    bugs.python.org fields:

    activity = <Date 2019-05-21.09:38:34.947>
    actor = 'vstinner'
    assignee = 'docs@python'
    closed = True
    closed_date = <Date 2019-05-21.09:38:34.948>
    closer = 'vstinner'
    components = ['Documentation']
    creation = <Date 2014-11-13.18:01:26.047>
    creator = 'RadicalZephyr'
    dependencies = []
    files = ['37189', '37620', '39313']
    hgrepos = []
    issue_num = 22865
    keywords = ['patch']
    message_count = 15.0
    messages = ['231126', '233561', '242730', '244956', '252444', '252446', '252453', '336171', '336246', '336257', '336268', '342871', '342910', '342993', '342994']
    nosy_count = 6.0
    nosy_names = ['twouters', 'vstinner', 'docs@python', 'martin.panter', 'RadicalZephyr', 'cheryl.sabella']
    pr_nums = ['11980', '13455']
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'enhancement'
    url = 'https://bugs.python.org/issue22865'
    versions = ['Python 3.7', 'Python 3.8']

    @RadicalZephyr
    Copy link
    Mannequin Author

    RadicalZephyr mannequin commented Nov 13, 2014

    While using the pty.py module I had cause to want to be able to silently eat characters from the input stream. I discovered that because of the check for end of file being "if not data" that there was no way to do this.

    Since the default function used by spawn and _copy is a thin wrapper on os.read, I figured that it would be useful to interpret None as a sentinel value meaning "this is not EOF, but there's nothing I actually want to copy over."

    @RadicalZephyr RadicalZephyr mannequin added the stdlib Python modules in the Lib dir label Nov 13, 2014
    @RadicalZephyr RadicalZephyr mannequin added docs Documentation in the Doc dir type-feature A feature request or enhancement labels Nov 13, 2014
    @RadicalZephyr
    Copy link
    Mannequin Author

    RadicalZephyr mannequin commented Jan 7, 2015

    I tweaked the patch a bit to not include the parentheses since that seems to be the style here.

    @RadicalZephyr
    Copy link
    Mannequin Author

    RadicalZephyr mannequin commented May 7, 2015

    Okay, I just found another way to achieve the same effect of letting the _read function ignore data but not inadvertantly close the stream. It relies on the fact that terminals will ignore null bytes fed to them.

    Now there are no code changes required, just an addition to the documentation.

    @RadicalZephyr RadicalZephyr mannequin removed the stdlib Python modules in the Lib dir label May 7, 2015
    @RadicalZephyr RadicalZephyr mannequin changed the title Allow pty.spawn to ignore data to copy Document how to make pty.spawn not copy data May 7, 2015
    @RadicalZephyr
    Copy link
    Mannequin Author

    RadicalZephyr mannequin commented Jun 7, 2015

    Hey, pinging this issue. Hoping someone has time to take a look at it.

    @vadmium
    Copy link
    Member

    vadmium commented Oct 7, 2015

    If you return null bytes, they will be written to the parent process’s stdout file descriptor, rather than being ignored. I do not think it is possible to ignore output with the spawn() API, unless perhaps you previously set up stdout write to /dev/null or similar.

    Do you have a reference for your fact about terminals ignoring null input bytes? I’m skeptical because a basic serial terminal can send arbitrary data including nulls, but I am not so familiar with pseudo-terminals.

    However the bit about interpreting an empty string could be useful. I would drop the “or any falsey value”; is that a typo? I suspect that the functions have to return byte strings (not text). It would be nice to say what the consequences of the EOF condition is, e.g. I can see for the child’s output it would stop copying, but not actually close the parent’s stdout to signal EOF. What about in the other direction? Can you signal multiple EOFs with data in between?

    @RadicalZephyr
    Copy link
    Mannequin Author

    RadicalZephyr mannequin commented Oct 7, 2015

    Hmm, I spoke improperly. I think you are entirely correct in your statements. What I meant by "terminals ignore null bytes" is that returning a string consisting of only a null byte doesn't cause anything observable to happen, including anything to be written to the screen. But I have no idea why this is.

    My basis for this is limited to empirical observation of how my terminal behaves (I use the iTerm2 terminal emulator on Mac OS X), and I don't recall whether I checked it with other terminals like the regular terminal on Mac and terminal emulators on Linux.

    Re: "or any falsey value". In general, the read functions should return byte strings yes, but since the check for EOF in python is whether a stream returned an empty string, and the way that pty.spawn() currently checks for that is if not data then any falsey value will cause pty.spawn to think that the underlying fd was closed.

    I agree that the documentation on this could use more fleshing out in general, especially regarding what happens when certain values are returned. I'm not very familiar with this code any more, but I might have time to play with it and try to add some more information in the coming weeks.

    @vadmium
    Copy link
    Member

    vadmium commented Oct 7, 2015

    I would avoid suggesting one can return non-byte-strings; treat that as an implementation detail. Maybe something like:

    The functions *master_read* and *stdin_read* are passed a file descriptor which they should read from, and should return a byte string. The defaults try to read 1024 bytes each time they are called. Returning an empty string is be interpreted as an EOF condition, and the *_read* function will no longer be called.

    @csabella
    Copy link
    Contributor

    @RadicalZephyr, are you interested in making a GitHub pull request with the recommended the documentation change?

    @csabella csabella added 3.7 (EOL) end of life 3.8 only security fixes labels Feb 21, 2019
    @RadicalZephyr
    Copy link
    Mannequin Author

    RadicalZephyr mannequin commented Feb 21, 2019

    It is submitted @cheryl.sabella. Thanks for reviving this, I had totally lost track of it.

    @csabella
    Copy link
    Contributor

    Thanks @RadicalZephyr!

    @martin.panter, please review the PR when you get a chance. Thank you!

    @vadmium
    Copy link
    Member

    vadmium commented Feb 21, 2019

    I'm not sure it is wise for the Python documentation to suggest inserting null bytes in general. This seems more like an application-specific hack. There is nothing in Python that handles these null bytes specially, and I expect they will be seen if the child reads the terminal in raw mode, or if the parent's output is redirected to a file, sent over the network or a real serial link.

    @RadicalZephyr
    Copy link
    Mannequin Author

    RadicalZephyr mannequin commented May 19, 2019

    @martin.panter I removed the mention of inserting null bytes and restricted the documentation updates to more fully documenting the current behaviour.

    @vstinner
    Copy link
    Member

    New changeset 522ccef by Victor Stinner (Geoff Shannon) in branch 'master':
    bpo-22865: Expand on documentation for the pty.spawn function (GH-11980)
    522ccef

    @vstinner
    Copy link
    Member

    New changeset cdb2dbf by Victor Stinner (Geoff Shannon) in branch '3.7':
    [3.7] bpo-22865: Expand on documentation for the pty.spawn function (GH-11980) (GH-13455)
    cdb2dbf

    @vstinner
    Copy link
    Member

    Thanks Geoff Shannon for the doc enhancement!

    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    3.7 (EOL) end of life 3.8 only security fixes docs Documentation in the Doc dir type-feature A feature request or enhancement
    Projects
    None yet
    Development

    No branches or pull requests

    3 participants