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 Redirect {
715 final FileDescriptor fd;
716
717 RedirectPipeImpl() {
718 this.fd = new FileDescriptor();
719 }
720 public Type type() { return Type.PIPE; }
721 public String toString() { return type().toString();}
722 FileDescriptor getFd() { return fd; }
723 }
Can you add @Override to appropriate methods?
ok
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 descriptor. On Windows, it
is a handle, not a fd.
1140 * All input and output streams between the intermediate processes are
1141 * not accessible, those redirects should be {@link Redirect#PIPE
Redirect.PIPE}.
This was a little unclear to me until i read the documentation for throwing an
ISE*(IAE)* e.g. we need to talk about the process builder here.
ok, added a description of the redirects to the the previous paragraph
that is talking about first and last.
* The redirects for standard
* input of the first process and standard output of the last
process are
* initialized using the redirect settings of the respective
ProcessBuilder.
* All other {@code ProcessBuilder} redirects should be
* {@link Redirect#PIPE Redirect.PIPE}.
1153 * @apiNote
1154 * For example to count the unique imports for all the files in a
file hierarchy:
1155 *
1156 * <pre>{@code
1157 * String directory = "/home/duke/src";
1158 * List<Process> processes = ProcessBuilder.startPipeline(
1159 * new ProcessBuilder("find", directory, "-type", "f"),
1160 * new ProcessBuilder("xargs", "grep", "-h", "^import "),
1161 * new ProcessBuilder("awk", "{print $2;}"),
1162 * new ProcessBuilder("sort", "-u"));
1163 * Process last = processes.get(processes.size()-1);
1164 * try (InputStream is = last.getInputStream();
1165 * Reader isr = new InputStreamReader(is);
1166 * BufferedReader r = new BufferedReader(isr)) {
1167 * long count = r.lines().count();
1168 * }
1169 * }</pre>
Perhaps you might want to qualify that example saying something like “on a UNIX
compatible platform”.
+1
1214 processes.forEach(p -> p.destroyForcibly());
If you feel in anyway inclined you can use a method reference here.
ok
PipelineTest
—
Can you make the test method names more meaningful?
ok
Thanks, Roger
Paul.
On 5 Nov 2015, at 22:56, Roger Riggs <roger.ri...@oracle.com> wrote:
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 output with standard
output.
But otherwise standard error streams are not modified.
The API is the same as discussed on the earlier core-libs thread [1] and
addresses
the comments.
webrev:
http://cr.openjdk.java.net/~rriggs/webrev-pipeline-8132394/
javadoc of ProcessBuilder: only startPipeline is new:
http://cr.openjdk.java.net/~rriggs/pipedoc/
Thanks, Roger
p.s. The PIPE_CHANNEL redirection proposed by Peter Levert is complementary
and still has a bug or two to work out.
[1] http://mail.openjdk.java.net/pipermail/core-libs-dev/2015-July/034634.html
