webkitpy: add a popen() call to executive

• Component: New Bugs
• Status: RESOLVED FIXED

[Dirk Pranke]

webkitpy: add a popen() call to executive

[Dirk Pranke]

Created attachment 96182 [details]
Patch

[Dirk Pranke]

Created attachment 96183 [details]
Patch

[Eric Seidel (no email)]

Comment on attachment 96183 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=96183&action=review

I'm OK with this change.  It's more interesting when we see it used.

> Tools/Scripts/webkitpy/common/system/executive.py:107
> +    PIPE = subprocess.PIPE
> +    STDOUT = subprocess.STDOUT

I'm not sure this bit is super useful.  Especially since we don't ahve STDERR and STDIN there.

[Eric Seidel (no email)]

Another quesiton to ask... why do some parts of the code need popen?  Should we be providing more limited APIs with Executive instead of adding popen?

[Dirk Pranke]

(In reply to comment #3)
> (From update of attachment 96183 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=96183&action=review
> 
> I'm OK with this change.  It's more interesting when we see it used.
> 

Sure, that'll be showing up shortly in a different patch.

> > Tools/Scripts/webkitpy/common/system/executive.py:107
> > +    PIPE = subprocess.PIPE
> > +    STDOUT = subprocess.STDOUT
> 
> I'm not sure this bit is super useful.  Especially since we don't ahve STDERR and STDIN there.

These are useful insofar as they allow me to not have to import subprocess directly. STDERR and STDIN aren't there because my code didn't need them. We could certainly add them for completeness.

Note that I'm not usually a fan of shadowing variables like this, but avoiding the import seemed like a win.

(In reply to comment #4)
> Another quesiton to ask... why do some parts of the code need popen?  Should we be providing more limited APIs with Executive instead of adding popen?

The callers are using popen() to start (and subsequently stop) processes (apache, lighttpd, the python websocket server). We probably don't need the full generality of popen(), but, annoyingly, all three processes need different sets of arguments, so we end up needing at least some of popen.

[Dirk Pranke]

(In reply to comment #5)
> > > Tools/Scripts/webkitpy/common/system/executive.py:107
> > > +    PIPE = subprocess.PIPE
> > > +    STDOUT = subprocess.STDOUT
> > 
> > I'm not sure this bit is super useful.  Especially since we don't ahve STDERR and STDIN there.
> 
> These are useful insofar as they allow me to not have to import subprocess directly. STDERR and STDIN aren't there because my code didn't need them. We could certainly add them for completeness.
> 

Oh, er, also because they don't exist :) PIPE and STDOUT are the only two constants in the subprocess interface.

[Dirk Pranke]

Committed r88287: <http://trac.webkit.org/changeset/88287>