Re: RFR 9: 8132394 : (process) ProcessBuilder support for a pipeline of processes

2015-11-12 Thread Paul Sandoz
> On 12 Nov 2015, at 15:50, Roger Riggs wrote: > > Hi Paul, > > > On 11/12/2015 9:45 AM, Paul Sandoz wrote: >> One more thing i forgot to mention, if there is ever a chance that >> ProcessBuilder is changed from final to non-final we should List> ProcessBuilder>. > I

Re: RFR 9: 8132394 : (process) ProcessBuilder support for a pipeline of processes

2015-11-12 Thread Paul Sandoz
> On 11 Nov 2015, at 19:11, Roger Riggs wrote: > > Hi Paul, > > Updated Webrev to use List instead of ProcessHandle... varargs. > > http://cr.openjdk.java.net/~rriggs/webrev-pipeline-8132394/ > +1 Paul.

Re: RFR 9: 8132394 : (process) ProcessBuilder support for a pipeline of processes

2015-11-12 Thread Roger Riggs
Hi Paul, On 11/12/2015 9:45 AM, Paul Sandoz wrote: One more thing i forgot to mention, if there is ever a chance that ProcessBuilder is changed from final to non-final we should Listextends ProcessBuilder>. I don't see that happening. If so would the change from List to ListProcessBuilder>

Re: RFR 9: 8132394 : (process) ProcessBuilder support for a pipeline of processes

2015-11-12 Thread Roger Riggs
Hi Paul, Thanks. One followup comment comparing varargs to List. If there is any concern about concurrency, the callee still has to make a copy of the incoming List/array before using/check it to get a consistent view. Only the caller can rely on the immutability of the argument (if it

Re: RFR 9: 8132394 : (process) ProcessBuilder support for a pipeline of processes

2015-11-12 Thread Paul Sandoz
> On 12 Nov 2015, at 15:24, Roger Riggs wrote: > > Hi Paul, > > Thanks. > One more thing i forgot to mention, if there is ever a chance that ProcessBuilder is changed from final to non-final we should List. > One followup comment comparing varargs to List. > > If

Re: RFR 9: 8132394 : (process) ProcessBuilder support for a pipeline of processes

2015-11-11 Thread Roger Riggs
Hi Paul, Updated Webrev to use List instead of ProcessHandle... varargs. http://cr.openjdk.java.net/~rriggs/webrev-pipeline-8132394/ comments below... On 11/10/2015 4:49 AM, Paul Sandoz wrote: On 9 Nov 2015, at 19:55, Roger Riggs wrote: Hi Paul, Alan, What are

Re: RFR 9: 8132394 : (process) ProcessBuilder support for a pipeline of processes

2015-11-10 Thread Paul Sandoz
> On 9 Nov 2015, at 19:07, Roger Riggs wrote: >> >> Why do you need to use a FileDescriptor rather than an int? AFAICT >> FileDescriptor is used as a box to the underlying descriptor that is >> accessed via shared secrets. > It is a cleaner encapsulation for the file

Re: RFR 9: 8132394 : (process) ProcessBuilder support for a pipeline of processes

2015-11-10 Thread Paul Sandoz
> On 9 Nov 2015, at 19:55, Roger Riggs wrote: > > Hi Paul, Alan, > > What are the chances that varargs will be updated at some point in the future > to use > Lists instead of arrays? It would be a bit of thrash to try to avoid varargs > if varargs > is likely to be

Re: RFR 9: 8132394 : (process) ProcessBuilder support for a pipeline of processes

2015-11-09 Thread Paul Sandoz
> On 9 Nov 2015, at 12:20, Alan Bateman wrote: > > > > On 05/11/2015 21:56, Roger Riggs wrote: >> Please review the new ProcessBuilder.startPipeline API, implementation, and >> tests. >> >> : >> >> javadoc of ProcessBuilder: only startPipeline is new: >>

Re: RFR 9: 8132394 : (process) ProcessBuilder support for a pipeline of processes

2015-11-09 Thread Roger Riggs
Hi Alan, Thanks for comments, On 11/9/2015 6:20 AM, Alan Bateman wrote: On 05/11/2015 21:56, Roger Riggs wrote: Please review the new ProcessBuilder.startPipeline API, implementation, and tests. : javadoc of ProcessBuilder: only startPipeline is new:

Re: RFR 9: 8132394 : (process) ProcessBuilder support for a pipeline of processes

2015-11-09 Thread Roger Riggs
Hi Paul, Thanks for the review, On 11/9/2015 4:32 AM, Paul Sandoz wrote: Hi Roger, ProcessBuilder — 711 * The FileDescriptor is used as the standard input of the next Process 712 * begin started. s/begin/to be started ? ok 714 static class RedirectPipeImpl extends

Re: RFR 9: 8132394 : (process) ProcessBuilder support for a pipeline of processes

2015-11-09 Thread Paul Sandoz
Hi Roger, ProcessBuilder — 711 * The FileDescriptor is used as the standard input of the next Process 712 * begin started. s/begin/to be started ? 714 static class RedirectPipeImpl extends Redirect { 715 final FileDescriptor fd; 716 717 RedirectPipeImpl() {

Re: RFR 9: 8132394 : (process) ProcessBuilder support for a pipeline of processes

2015-11-09 Thread Alan Bateman
On 05/11/2015 21:56, Roger Riggs wrote: Please review the new ProcessBuilder.startPipeline API, implementation, and tests. : javadoc of ProcessBuilder: only startPipeline is new: http://cr.openjdk.java.net/~rriggs/pipedoc/ I skimmed over the javadoc and it looks quite good. The

RFR 9: 8132394 : (process) ProcessBuilder support for a pipeline of processes

2015-11-05 Thread Roger Riggs
Please review the new ProcessBuilder.startPipeline API, implementation, and tests. The new method starts a Process for each ProcessBuilder, creating a pipeline of processes linked by their standard output and standard input streams. Each builder can use redirectErrorsream to coalesce error