[Python-Dev] r87010 - in python/branches/py3k: Doc/library/subprocess.rst Lib/subprocess.py Lib/test/test_subprocess.py

[Gregory P. Smith]

On Sat, Dec 4, 2010 at 11:28 AM, Paul Moore <p.f.moore at gmail.com> wrote:

> On 4 December 2010 18:14, Gregory P. Smith <greg at krypto.org> wrote:
> >
> >
> > On Sat, Dec 4, 2010 at 3:45 AM, Antoine Pitrou <solipsis at pitrou.net>
> wrote:
> >>
> >> On Sat,  4 Dec 2010 10:10:44 +0100 (CET)
> >> gregory.p.smith <python-checkins at python.org> wrote:
> >> > Author: gregory.p.smith
> >> > Date: Sat Dec  4 10:10:44 2010
> >> > New Revision: 87010
> >> >
> >> > Log:
> >> > issue7213 + issue2320: Cause a DeprecationWarning if the close_fds
> >> > argument is
> >> > not passed to subprocess.Popen as the default value will be changing
> in
> >> > a
> >> > future Python to the safer and more often desired value of True.
> >>
> >> That doesn't seem to be a good idea under Windows, is it?
> >>
> >> (“Note that on Windows, you cannot set *close_fds* to true and
> >> also redirect the standard handles by setting *stdin*, *stdout* or
> >> *stderr*.”)
> >
> > Regardless of what platform you are on I think the warning makes sense,
> it
> > was just worded too strongly.  It is asking people to be explicit with
> > close_fds.  Explicitly setting close_fds=False when that is desired is
> good.
> > I modified the docs and warning message to just say that the default
> > close_fds behavior will change in the future without specifying exactly
> what
> > it will be.  It could make sense for the default to be a soft-True on
> > windows that changes behavior if any of the std handles are set if that
> > matches what users expect and find easiest.  We may want to reconsider
> its
> > entire future in the face of the new pass_fds parameter, etc.  Those are
> 3.3
> > decisions.
>
> This sounds like omitting the close_fds parameter is now considered
> ill-advised, if not outright wrong. That's silly - I certainly never
> specify close_fds, expecting the module to do the correct thing if I
> don't know/care enough to say. I use Popen as a convenience function,
> so ignoring low-level details is the whole point in my opinion (and
> close_fds *is* a low-level detail for what I do, on Windows).
>
> At the very least, the whole of the section "Replacing Older Functions
> with the subprocess Module" (with a couple of small exceptions) is in
> need of updating, as it is recommending bad practice (not specifying
> close_fds) at the moment...
>
> OK, I'm exaggerating a touch here. But can someone point me at the
> discussion of this change? Or if there hasn't been one, can we have
> one (i.e., to start the ball rolling, can someone justify the change,
> please).
>
> Paul.
>

Making the change was intended to force the discussion.  I'm glad that
worked. :)

I don't like the thought of requiring people to specify close_fds either but
the default is a bad one (see http://bugs.python.org/issue7213 and
http://bugs.python.org/issue2320) so we should change it.

The real question is how should we go about doing the change?  This warning
asking people to always specify close_fds=True is heavy handed.  Other
places in the stdlib and docs no doubt still need to be updated to reflect
it, etc.


Options that seem worthy of debate:

A) The DeprecationWarning that exists in py3k as of today.

B) Remove the DeprecationWarning, simply document that people should be
explicit about it if they care at all and that the default will change in
3.3.

C) Never change close_fds behavior.  we're stuck with it for life.

D) Deprecate close_fds so that it becomes a legacy no-op.  the new pass_fds
flag could evolve into this.  this will break existing code that depends on
close_fds one way or another.


I'm in 100% agreement that forcing people to pass close_fds in makes the API
less friendly (granted, people already find it unfriendly but why make it
worse?).

Option B seems the most friendly to me.

Option D is always on the table but I haven't planned out what a future
without it should look like.  I prefer requiring people who need open file
descriptors to pass them in; a semaphore for "all fds" could be created and
pass_fds=ALL becomes the new "close_fds=False" (I think one of the bugs
suggests this).

-gps
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.python.org/pipermail/python-dev/attachments/20101204/711d4feb/attachment.html>