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

subprocess.DEVNULL #50120

Closed
MrJean1 mannequin opened this issue Apr 28, 2009 · 9 comments
Closed

subprocess.DEVNULL #50120

MrJean1 mannequin opened this issue Apr 28, 2009 · 9 comments
Labels
extension-modules C modules in the Modules dir type-feature A feature request or enhancement

Comments

@MrJean1
Copy link
Mannequin

MrJean1 mannequin commented Apr 28, 2009

BPO 5870
Nosy @birkenfeld, @gpshead, @pitrou, @giampaolo, @socketpair
Files
  • 5870_v1.patch: Patch + test for issue
  • 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 2011-03-16.17:38:53.804>
    created_at = <Date 2009-04-28.19:24:02.743>
    labels = ['extension-modules', 'type-feature']
    title = 'subprocess.DEVNULL'
    updated_at = <Date 2011-03-16.17:38:53.803>
    user = 'https://bugs.python.org/MrJean1'

    bugs.python.org fields:

    activity = <Date 2011-03-16.17:38:53.803>
    actor = 'rosslagerwall'
    assignee = 'rosslagerwall'
    closed = True
    closed_date = <Date 2011-03-16.17:38:53.804>
    closer = 'rosslagerwall'
    components = ['Extension Modules']
    creation = <Date 2009-04-28.19:24:02.743>
    creator = 'MrJean1'
    dependencies = []
    files = ['20215']
    hgrepos = []
    issue_num = 5870
    keywords = ['patch']
    message_count = 9.0
    messages = ['86761', '86771', '90444', '90446', '125030', '125057', '125058', '125063', '131146']
    nosy_count = 11.0
    nosy_names = ['georg.brandl', 'gregory.p.smith', 'pitrou', 'giampaolo.rodola', 'OG7', 'MrJean1', 'lucaspmelo', 'santoso.wijaya', 'rosslagerwall', 'socketpair', 'python-dev']
    pr_nums = []
    priority = 'normal'
    resolution = 'accepted'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'enhancement'
    url = 'https://bugs.python.org/issue5870'
    versions = ['Python 3.3']

    @MrJean1
    Copy link
    Mannequin Author

    MrJean1 mannequin commented Apr 28, 2009

    The subprocess module exposes the PIPE and STDOUT constants to be used
    for the stdin, stdout or stderr keyword arguments.

    Often, stderr needs to be redirected to /dev/null (on posix). It would
    nice if another constant was available for that purpose, e.g.
    subprocess.DEVNULL.

    Perhaps, that might be as simple as

    DEVNULL = os.open(os.devnull, os.OS_RDWR)

    and re-use the same, single file descriptor.

    @MrJean1 MrJean1 mannequin added extension-modules C modules in the Modules dir type-feature A feature request or enhancement labels Apr 28, 2009
    @MrJean1
    Copy link
    Mannequin Author

    MrJean1 mannequin commented Apr 28, 2009

    Typo. That should be ..., os.O_RDWR)

    @lucaspmelo
    Copy link
    Mannequin

    lucaspmelo mannequin commented Jul 12, 2009

    -1 on this one.
    It is not a portable decision (only *nix OSes do have /dev/null).
    Also, why would we want it as a default constant? The subprocess module
    would need to open /dev/null every time. Despite that, I can't see how
    would someone use the redirection of errors to /dev/null through a
    python script and, at the same time, make it seem not a bad practice at all.

    @pitrou
    Copy link
    Member

    pitrou commented Jul 12, 2009

    Lucas, Windows has a /dev/null-like device. I think Jean's suggestion is
    reasonable, but it should be a special constant instead of creating a
    file descriptor unconditionnally (that is, the file should only be open
    if needed). Also, a patch should be provided (with tests :-)).

    @rosslagerwall
    Copy link
    Mannequin

    rosslagerwall mannequin commented Jan 2, 2011

    Here is a fairly simple patch that adds the subprocess.DEVNULL constant.

    @birkenfeld
    Copy link
    Member

    birkenfeld commented Jan 2, 2011

    Hmm, we don't like these open-for-eternity file descriptors; we had such a thing for os.urandom() but removed it (see bpo-1177468).

    I'm okay with DEVNULL (or even just NULL) as a shorthand, but it should open (and close) the devnull device each time just as if a normal fd was given.

    @rosslagerwall
    Copy link
    Mannequin

    rosslagerwall mannequin commented Jan 2, 2011

    I think if you look closely at the patch, the fd does not stay open the whole time. It is opened if necessary in _get_handles() with e.g.:

    elif stdin == DEVNULL:
    p2cread = self._get_devnull()

    and then closed in _execute_child() with:

    if hasattr(self, '_devnull'):
        os.close(self._devnull)

    which is executed from __init__(). So I don't think it stays open for eternity :-)

    @birkenfeld
    Copy link
    Member

    birkenfeld commented Jan 2, 2011

    Right, sorry then :)

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Mar 16, 2011

    New changeset eaf93e156dff by Ross Lagerwall in branch 'default':
    Issue bpo-5870: Add subprocess.DEVNULL constant.
    http://hg.python.org/cpython/rev/eaf93e156dff

    @rosslagerwall rosslagerwall mannequin closed this as completed Mar 16, 2011
    @rosslagerwall rosslagerwall mannequin assigned rosslagerwall and unassigned gpshead Mar 16, 2011
    @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
    extension-modules C modules in the Modules dir type-feature A feature request or enhancement
    Projects
    None yet
    Development

    No branches or pull requests

    3 participants