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.pipe function #47798

Open
tebeka mannequin opened this issue Aug 12, 2008 · 18 comments
Open

subprocess.pipe function #47798

tebeka mannequin opened this issue Aug 12, 2008 · 18 comments
Labels
stdlib Python modules in the Lib dir type-feature A feature request or enhancement

Comments

@tebeka
Copy link
Mannequin

tebeka mannequin commented Aug 12, 2008

BPO 3548
Nosy @amauryfa, @pitrou, @vapier, @merwok, @bitdancer, @desbma, @matheusportela
Files
  • pipe.patch: Patch file for "subprocess.pipe"
  • toto.py: alternate implementation (not a patch)
  • pipeline.py: clean, doc & unit test (still not a patch)
  • pipeline.py: new version (pylint, license, copyright)
  • 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 = None
    created_at = <Date 2008-08-12.17:23:58.445>
    labels = ['type-feature', 'library']
    title = 'subprocess.pipe function'
    updated_at = <Date 2020-03-05.02:01:55.533>
    user = 'https://github.com/tebeka'

    bugs.python.org fields:

    activity = <Date 2020-03-05.02:01:55.533>
    actor = 'vapier'
    assignee = 'none'
    closed = False
    closed_date = None
    closer = None
    components = ['Library (Lib)']
    creation = <Date 2008-08-12.17:23:58.445>
    creator = 'tebeka'
    dependencies = []
    files = ['11104', '11262', '11263', '11330']
    hgrepos = []
    issue_num = 3548
    keywords = ['patch']
    message_count = 18.0
    messages = ['71062', '71063', '71978', '71979', '72257', '72262', '72318', '114730', '122557', '125226', '249120', '249124', '249222', '249232', '249239', '249241', '249254', '363412']
    nosy_count = 11.0
    nosy_names = ['amaury.forgeotdarc', 'pitrou', 'vapier', 'vlegoll', 'eric.araujo', 'vincent.legoll', 'r.david.murray', 'aht', 'Netto', 'desbma', 'matheus.v.portela']
    pr_nums = []
    priority = 'normal'
    resolution = None
    stage = None
    status = 'open'
    superseder = None
    type = 'enhancement'
    url = 'https://bugs.python.org/issue3548'
    versions = ['Python 3.3']

    @tebeka
    Copy link
    Mannequin Author

    tebeka mannequin commented Aug 12, 2008

    Attached is a patch that add "pipe" command to the "subprocess" module.

    pipe(["ls"], ["grep", "test_"]) will return the output of
    "ls | grep test_".

    @tebeka tebeka mannequin added stdlib Python modules in the Lib dir type-feature A feature request or enhancement labels Aug 12, 2008
    @tebeka
    Copy link
    Mannequin Author

    tebeka mannequin commented Aug 12, 2008

    Not sure about the name, maybe "chain" will be better?

    @vincentlegoll
    Copy link
    Mannequin

    vincentlegoll mannequin commented Aug 26, 2008

    Hello,

    I was searching for a bug in subprocess
    module when I saw your patch.

    I was implementing the exact same functionality
    and mixed some of your ideas in what I use now,
    which is attached...

    Feel free to use it

    @vincentlegoll
    Copy link
    Mannequin

    vincentlegoll mannequin commented Aug 26, 2008

    Here's a clean version with doc & test

    enjoy !

    @vincentlegoll
    Copy link
    Mannequin

    vincentlegoll mannequin commented Sep 1, 2008

    • Added "shut pylint up" comment for ** keyword expansion
    • Added Copyright & license header

    @amauryfa
    Copy link
    Member

    amauryfa commented Sep 1, 2008

    Vincent,
    GPL licenced code is incompatible with the inclusion into python.
    And if I am correct, you should sign a contributor agreement. Then the
    licence text is not necessary.

    @vlegoll
    Copy link
    Mannequin

    vlegoll mannequin commented Sep 2, 2008

    On Mon, Sep 1, 2008 at 5:13 PM, Amaury Forgeot d'Arc
    <report@bugs.python.org> wrote:

    Amaury Forgeot d'Arc <amauryfa@gmail.com> added the comment:

    Vincent,
    GPL licenced code is incompatible with the inclusion into python.
    And if I am correct, you should sign a contributor agreement. Then the
    licence text is not necessary.

    This is not a patch against python, this is a standalone script, just
    implementing the same functionality as the original bug report in a
    slightly different way...

    But thanks for the input anyways.

    If this functionality is really interesting people and agreed to be integrated
    into upstream subprocess module, I can rework it the right way, or work
    from the original patch from the bug report.

    @aht
    Copy link
    Mannequin

    aht mannequin commented Aug 23, 2010

    I've written a package which can do this with arbitrary redirection in all subcommands (and some more).

    You can, for example, do this:

        >>> Pipe(Sh('echo -n foo >&2', {2: 1}), Sh('cat; echo ERROR >&2', {2: os.devnull})).capture(1).read()
        'foo'

    The package is at: http://github.com/aht/pc.py

    @merwok
    Copy link
    Member

    merwok commented Nov 27, 2010

    pipe.patch looks interesting to me. I would replace **kwargs with a keyword-only argument named stderr, since that’s the only key used. This requires more tests and docs.

    @pitrou
    Copy link
    Member

    pitrou commented Jan 3, 2011

    I think this would be more useful if you could pass an optional input string (as in communicate()) and if it returned a (stdout, stderr) tuple. Or perhaps even a (return code, stdout, stderr) tuple; alternately, non-zero return codes could raise an exception.

    @matheusportela
    Copy link
    Mannequin

    matheusportela mannequin commented Aug 25, 2015

    I just found this open issue and I can work on it. What is left to do before closing it?

    @bitdancer
    Copy link
    Member

    Thanks for being willing to work on it.

    If what is wanted is a way to pipeline shell commands, Python already has that functionality in the pipes module.

    So the interesting thing here would be pipelining *non* shell commands, to avoid the shell exploits that using a shell pipeline invites.

    The pipes module already has a worked out API, so perhaps it would be useful to see about re-implementing pipe's command execution using subprocess, and expand the API to allow for argv style command specification that would be fed to subprocess using the default shell=False. This would also presumably allow pipes to be used when there's no bash shell available.

    The downside is that we might break current uses of pipes if we replace the shell version of the pipelining with subprocess shell=True, while using subprocess only if the command specifications are argv lists would result in code with a split personality. But if I were working on it I'd experiment with that approach to see if it made sense.

    Other core devs may have other opinions on this :)

    @matheusportela
    Copy link
    Mannequin

    matheusportela mannequin commented Aug 27, 2015

    Let me check whether I understood you suggestion...

    What you are saying is that it is already possible to pipeline shell commands through subprocess, such as in the following line?

    subprocess.call('ls -l | grep Music', shell=True)

    However, this requires the command to be executed in a shell environment. Hence, it would be a good idea to extend it to non-shell command execution. Is this right?

    @bitdancer
    Copy link
    Member

    No, I'm talking about https://docs.python.org/3/library/pipes.html

    @matheusportela
    Copy link
    Mannequin

    matheusportela mannequin commented Aug 27, 2015

    Oh, I see... The pipes module uses os.system and os.popen to execute commands under a "/bin/sh" environment. So you are proposing to re-implement it with subprocess in order to execute commands without shell and, also, extend by accepting argv-style parameters. Is this right this time?

    How would an argv-style pipe look like?

    @bitdancer
    Copy link
    Member

    Exactly like the string pipes API, but cmd would be a list of arguments instead of a string. That would then become the argument list to the subprocess stage.

    It is an interesting question whether we'd want to allow mixing stage types. I'd say no (at least initially, if people complain we can add it as a feature later, but we can't subtract it if we implement it now and decide it is a bad idea after putting it in the field).

    Now, keep in mind that you've only got my opinion so far that this is even a good idea :)

    @matheusportela
    Copy link
    Mannequin

    matheusportela mannequin commented Aug 27, 2015

    Sure. I'll leave this issue for a while before others can emit their opinions.

    @vapier
    Copy link
    Mannequin

    vapier mannequin commented Mar 5, 2020

    this would have been nice to have more than once in my personal projects, and in build/infra tooling for Chromium OS. our alternatives so far have been the obvious ones: use subprocess.run to capture & return the output, then manually feed that as the input to the next command, and so on. we know it has obvious overhead so we've avoided with large output.

    we strongly discourage/ban attempts to write shell code, so the vast majority of our commands are argv style (list of strings), so the pipes module wouldn't help us.

    handling of SIGPIPE tends to be where things get tricky. that'll have to be handled by the API explicitly i think.

    @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
    stdlib Python modules in the Lib dir type-feature A feature request or enhancement
    Projects
    None yet
    Development

    No branches or pull requests

    4 participants