Implement an abstraction layer over java.lang.Process so that the Windows implementation can eventually be plugged in.

--
MOS_MIGRATED_REVID=126404913

5 files changed, 382 insertions, 65 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/shell/Command.java b/src/main/java/com/google/devtools/build/lib/shell/Command.java
index d4a69dc903..e5a958c185 100644
--- a/src/main/java/com/google/devtools/build/lib/shell/Command.java
+++ b/src/main/java/com/google/devtools/build/lib/shell/Command.java
@@ -15,12 +15,13 @@
 package com.google.devtools.build.lib.shell;
 
 
+import com.google.common.collect.ImmutableList;
+import com.google.devtools.build.lib.shell.SubprocessBuilder.StreamAction;
+
 import java.io.File;
 import java.io.IOException;
 import java.io.InputStream;
 import java.io.OutputStream;
-import java.lang.ProcessBuilder.Redirect;
-import java.util.Collections;
 import java.util.List;
 import java.util.Map;
 import java.util.logging.Level;
@@ -125,8 +126,6 @@ public final class Command {
    */
   public static final byte[] NO_INPUT = new byte[0];
 
-  private static final String[] EMPTY_STRING_ARRAY = new String[0];
-
   /**
    * Pass this to {@link #execute(byte[], KillableObserver, boolean)} to
    * indicate that you do not wish to observe / kill the underlying
@@ -143,25 +142,11 @@ public final class Command {
     }
   };
 
-  private final ProcessBuilder processBuilder;
+  private final SubprocessBuilder subprocessBuilder;
 
   // Start of public API -----------------------------------------------------
 
   /**
-   * Creates a new {@link Command} that will execute a command line that
-   * is described by a {@link ProcessBuilder}. Command line elements,
-   * environment, and working directory are taken from this object. The
-   * command line is executed exactly as given, without a shell.
-   *
-   * @param processBuilder {@link ProcessBuilder} describing command line
-   *  to execute
-   */
-  public Command(final ProcessBuilder processBuilder) {
-    this(processBuilder.command().toArray(EMPTY_STRING_ARRAY),
-         processBuilder.environment(),
-         processBuilder.directory());
-  }
-
   /**
    * Creates a new {@link Command} for the given command line elements. The
    * command line is executed exactly as given, without a shell.
@@ -214,22 +199,17 @@ public final class Command {
       commandLineElements[0] = new File(workingDirectory, commandLineElements[0]).getAbsolutePath();
     }
 
-    this.processBuilder =
-      new ProcessBuilder(commandLineElements);
-    if (environmentVariables != null) {
-      // TODO(bazel-team) remove next line eventually; it is here to mimic old
-      // Runtime.exec() behavior
-      this.processBuilder.environment().clear();
-      this.processBuilder.environment().putAll(environmentVariables);
-    }
-    this.processBuilder.directory(workingDirectory);
+    this.subprocessBuilder = new SubprocessBuilder();
+    subprocessBuilder.setArgv(ImmutableList.copyOf(commandLineElements));
+    subprocessBuilder.setEnv(environmentVariables);
+    subprocessBuilder.setWorkingDirectory(workingDirectory);
   }
 
   /**
    * @return raw command line elements to be executed
    */
   public String[] getCommandLineElements() {
-    final List<String> elements = processBuilder.command();
+    final List<String> elements = subprocessBuilder.getArgv();
     return elements.toArray(new String[elements.size()]);
   }
 
@@ -237,7 +217,7 @@ public final class Command {
    * @return (unmodifiable) {@link Map} view of command's environment variables
    */
   public Map<String, String> getEnvironmentVariables() {
-    return Collections.unmodifiableMap(processBuilder.environment());
+    return subprocessBuilder.getEnv();
   }
 
   /**
@@ -245,7 +225,7 @@ public final class Command {
    *         working directory is used
    */
   public File getWorkingDirectory() {
-    return processBuilder.directory();
+    return subprocessBuilder.getWorkingDirectory();
   }
 
   /**
@@ -446,31 +426,23 @@ public final class Command {
       throws CommandException {
     nullCheck(stdinInput, "stdinInput");
     nullCheck(observer, "observer");
-    processBuilder.redirectOutput(redirectToFileOrDevNull(stdOut));
-    processBuilder.redirectError(redirectToFileOrDevNull(stdErr));
+    if (stdOut == null) {
+      subprocessBuilder.setStdout(StreamAction.DISCARD);
+    } else {
+      subprocessBuilder.setStdout(stdOut);
+    }
+
+    if (stdErr == null) {
+      subprocessBuilder.setStderr(StreamAction.DISCARD);
+    } else {
+      subprocessBuilder.setStderr(stdErr);
+    }
     return doExecute(
             new ByteArrayInputSource(stdinInput), observer, null, killSubprocessOnInterrupt, false)
         .get();
   }
 
   /**
-   * Returns a {@link ProcessBuilder.Redirect} that writes process output to {@code file} or to
-   * /dev/null in case {@code file} is null. If {@code file} exists, it is deleted before
-   * redirecting to it.
-   */
-  private Redirect redirectToFileOrDevNull(File file) {
-    if (file == null) {
-      return Redirect.to(new File("/dev/null"));
-    }
-    // We need to use Redirect.appendTo() here, because on older Linux kernels writes are otherwise
-    // not atomic and might result in lost log messages: https://lkml.org/lkml/2014/3/3/308
-    if (file.exists()) {
-      file.delete();
-    }
-    return Redirect.appendTo(file);
-  }
-
-  /**
    * Execute this command with given input to stdin; this stream is closed when the process
    * terminates, and exceptions raised when closing this stream are ignored. This call blocks until
    * the process completes or an error occurs. The caller provides {@link OutputStream} instances
@@ -706,7 +678,7 @@ public final class Command {
 
     logCommand();
 
-    final Process process = startProcess();
+    final Subprocess process = startProcess();
 
     if (outErrConsumers != null) {
       outErrConsumers.logConsumptionStrategy();
@@ -745,10 +717,10 @@ public final class Command {
     };
   }
 
-  private Process startProcess()
+  private Subprocess startProcess()
     throws ExecFailedException {
     try {
-      return processBuilder.start();
+      return subprocessBuilder.start();
     } catch (IOException ioe) {
       throw new ExecFailedException(this, ioe);
     }
@@ -809,8 +781,7 @@ public final class Command {
     }
   }
 
-  private static void processInput(final InputSource stdinInput,
-                                   final Process process) {
+  private static void processInput(InputSource stdinInput, Subprocess process) {
     if (log.isLoggable(Level.FINER)) {
       log.finer(stdinInput.toLogString("stdin"));
     }
@@ -836,15 +807,15 @@ public final class Command {
     }
   }
 
-  private static Killable observeProcess(final Process process,
-                                         final KillableObserver observer) {
+  private static Killable observeProcess(Subprocess process,
+      final KillableObserver observer) {
     final Killable processKillable = new ProcessKillable(process);
     observer.startObserving(processKillable);
     return processKillable;
   }
 
   private CommandResult waitForProcessToComplete(
-    final Process process,
+    final Subprocess process,
     final KillableObserver observer,
     final Killable processKillable,
     final Consumers.OutErrConsumers outErr,
@@ -903,7 +874,7 @@ public final class Command {
     }
   }
 
-  private static TerminationStatus waitForProcess(Process process,
+  private static TerminationStatus waitForProcess(Subprocess process,
                                        boolean killSubprocessOnInterrupt) {
     boolean wasInterrupted = false;
     try {
@@ -941,15 +912,15 @@ public final class Command {
   public String toDebugString() {
     StringBuilder message = new StringBuilder(128);
     message.append("Executing (without brackets):");
-    for (final String arg : processBuilder.command()) {
+    for (String arg : subprocessBuilder.getArgv()) {
       message.append(" [");
       message.append(arg);
       message.append(']');
     }
     message.append("; environment: ");
-    message.append(processBuilder.environment());
-    final File workingDirectory = processBuilder.directory();
+    message.append(subprocessBuilder.getEnv());
     message.append("; working dir: ");
+    File workingDirectory = subprocessBuilder.getWorkingDirectory();
     message.append(workingDirectory == null ?
                    "(current)" :
                    workingDirectory.toString());
diff --git a/src/main/java/com/google/devtools/build/lib/shell/JavaSubprocessFactory.java b/src/main/java/com/google/devtools/build/lib/shell/JavaSubprocessFactory.java
new file mode 100644
index 0000000000..c09a24a704
--- /dev/null
+++ b/src/main/java/com/google/devtools/build/lib/shell/JavaSubprocessFactory.java
@@ -0,0 +1,119 @@
+// Copyright 2016 The Bazel Authors. All rights reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//    http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.devtools.build.lib.shell;
+
+import com.google.devtools.build.lib.shell.SubprocessBuilder.StreamAction;
+
+import java.io.File;
+import java.io.IOException;
+import java.io.InputStream;
+import java.io.OutputStream;
+import java.lang.ProcessBuilder.Redirect;
+
+/**
+ * A subprocess factory that uses {@link java.lang.ProcessBuilder}.
+ */
+public class JavaSubprocessFactory implements Subprocess.Factory {
+
+  /**
+   * A subprocess backed by a {@link java.lang.Process}.
+   */
+  private static class JavaSubprocess implements Subprocess {
+    private final Process process;
+
+    private JavaSubprocess(Process process) {
+      this.process = process;
+    }
+
+    @Override
+    public boolean destroy() {
+      process.destroy();
+      return true;
+    }
+
+    @Override
+    public int exitValue() {
+      return process.exitValue();
+    }
+
+    @Override
+    public int waitFor() throws InterruptedException {
+      return process.waitFor();
+    }
+
+    @Override
+    public OutputStream getOutputStream() {
+      return process.getOutputStream();
+    }
+
+    @Override
+    public InputStream getErrorStream() {
+      return process.getErrorStream();
+    }
+
+    @Override
+    public InputStream getInputStream() {
+      return process.getInputStream();
+    }
+  }
+
+  public static final JavaSubprocessFactory INSTANCE = new JavaSubprocessFactory();
+
+  private JavaSubprocessFactory() {
+    // We are a singleton
+  }
+
+  @Override
+  public Subprocess create(SubprocessBuilder params) throws IOException {
+    ProcessBuilder builder = new ProcessBuilder();
+    builder.command(params.getArgv());
+    if (params.getEnv() != null) {
+      builder.environment().clear();
+      builder.environment().putAll(params.getEnv());
+    }
+
+    builder.redirectOutput(getRedirect(params.getStdout(), params.getStdoutFile()));
+    builder.redirectError(getRedirect(params.getStderr(), params.getStderrFile()));
+    builder.directory(params.getWorkingDirectory());
+
+    return new JavaSubprocess(builder.start());
+  }
+
+  /**
+   * Returns a {@link ProcessBuilder.Redirect} appropriate for the parameters. If a file redirected
+   * to exists, deletes the file before redirecting to it.
+   */
+  private Redirect getRedirect(StreamAction action, File file) throws IOException {
+    switch (action) {
+      case DISCARD:
+        return Redirect.to(new File("/dev/null"));
+
+      case REDIRECT:
+        // We need to use Redirect.appendTo() here, because on older Linux kernels writes are
+        // otherwise not atomic and might result in lost log messages:
+        // https://lkml.org/lkml/2014/3/3/308
+        if (file.exists()) {
+          file.delete();
+        }
+        return Redirect.appendTo(file);
+
+      case STREAM:
+        return Redirect.PIPE;
+
+      default:
+        throw new IllegalStateException();
+    }
+  }
+}
diff --git a/src/main/java/com/google/devtools/build/lib/shell/ProcessKillable.java b/src/main/java/com/google/devtools/build/lib/shell/ProcessKillable.java
index 114f1bbf5d..76118df165 100644
--- a/src/main/java/com/google/devtools/build/lib/shell/ProcessKillable.java
+++ b/src/main/java/com/google/devtools/build/lib/shell/ProcessKillable.java
@@ -20,14 +20,14 @@ package com.google.devtools.build.lib.shell;
  */
 final class ProcessKillable implements Killable {
 
-  private final Process process;
+  private final Subprocess process;
 
-  ProcessKillable(final Process process) {
+  ProcessKillable(Subprocess process) {
     this.process = process;
   }
 
   /**
-   * Calls {@link Process#destroy()}.
+   * Calls {@link Subprocess#destroy()}.
    */
   @Override
   public void kill() {
diff --git a/src/main/java/com/google/devtools/build/lib/shell/Subprocess.java b/src/main/java/com/google/devtools/build/lib/shell/Subprocess.java
new file mode 100644
index 0000000000..99e55776ff
--- /dev/null
+++ b/src/main/java/com/google/devtools/build/lib/shell/Subprocess.java
@@ -0,0 +1,68 @@
+// Copyright 2016 The Bazel Authors. All rights reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//    http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.devtools.build.lib.shell;
+
+import java.io.IOException;
+import java.io.InputStream;
+import java.io.OutputStream;
+
+/**
+ * A process started by Bazel.
+ */
+public interface Subprocess {
+
+  /**
+   * Something that can create subprocesses.
+   */
+  interface Factory {
+
+    /**
+     * Create a subprocess according to the specified parameters.
+     */
+    Subprocess create(SubprocessBuilder params) throws IOException;
+  }
+
+  /**
+   * Kill the process.
+   */
+  boolean destroy();
+
+  /**
+   * Returns the exit value of the process.
+   *
+   * <p>Throws {@code IllegalThreadStateException} if the process has not terminated yet.
+   */
+  int exitValue();
+
+  /**
+   * Waits for the process to finish.
+   */
+  int waitFor() throws InterruptedException;
+
+  /**
+   * Returns a stream into which data can be written that the process will get on its stdin.
+   */
+  OutputStream getOutputStream();
+
+  /**
+   * Returns a stream from which the stdout of the process can be read.
+   */
+  InputStream getInputStream();
+
+  /**
+   * Returns a stream from which the stderr of the process can be read.
+   */
+  InputStream getErrorStream();
+}
diff --git a/src/main/java/com/google/devtools/build/lib/shell/SubprocessBuilder.java b/src/main/java/com/google/devtools/build/lib/shell/SubprocessBuilder.java
new file mode 100644
index 0000000000..0b6ecfb2f5
--- /dev/null
+++ b/src/main/java/com/google/devtools/build/lib/shell/SubprocessBuilder.java
@@ -0,0 +1,159 @@
+// Copyright 2016 The Bazel Authors. All rights reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//    http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.devtools.build.lib.shell;
+
+import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableMap;
+
+import java.io.File;
+import java.io.IOException;
+import java.util.Map;
+
+/**
+ * A builder class that starts a subprocess.
+ */
+public class SubprocessBuilder {
+  /**
+   * What to do with an output stream of the process.
+   */
+  public enum StreamAction {
+    /** Redirect to a file */
+    REDIRECT,
+
+    /** Discard. */
+    DISCARD,
+
+    /** Stream back to the parent process using an output stream. */
+    STREAM };
+
+  private ImmutableList<String> argv;
+  private ImmutableMap<String, String> env;
+  private StreamAction stdoutAction;
+  private File stdoutFile;
+  private StreamAction stderrAction;
+  private File stderrFile;
+  private File workingDirectory;
+
+  public SubprocessBuilder() {
+    stdoutAction = StreamAction.STREAM;
+    stderrAction = StreamAction.STREAM;
+  }
+
+  public ImmutableList<String> getArgv() {
+    return argv;
+  }
+
+  /**
+   * Sets the argv, including argv[0], that is, the binary to execute.
+   */
+  public SubprocessBuilder setArgv(Iterable<String> argv) {
+    this.argv = ImmutableList.copyOf(argv);
+    return this;
+  }
+
+  public ImmutableMap<String, String> getEnv() {
+    return env;
+  }
+
+  /**
+   * Sets the environment passed to the child process. If null, inherit the environment of the
+   * server.
+   */
+  public SubprocessBuilder setEnv(Map<String, String> env) {
+    this.env = env == null
+        ? null : ImmutableMap.copyOf(env);
+    return this;
+  }
+
+  public StreamAction getStdout() {
+    return stdoutAction;
+  }
+
+  public File getStdoutFile() {
+    return stdoutFile;
+  }
+
+  /**
+   * Tells the object what to do with stdout: either stream as a {@code InputStream} or discard.
+   * 
+   * <p>It can also be redirected to a file using {@link #setStdout(File)}.
+   */
+  public SubprocessBuilder setStdout(StreamAction action) {
+    if (action == StreamAction.REDIRECT) {
+      throw new IllegalStateException();
+    }
+    this.stdoutAction = action;
+    this.stdoutFile = null;
+    return this;
+  }
+  
+  /**
+   * Sets the file stdout is appended to. If null, the stdout will be available as an input stream
+   * on the resulting object representing the process.
+   */
+  public SubprocessBuilder setStdout(File file) {
+    this.stdoutAction = StreamAction.REDIRECT;
+    this.stdoutFile = file;
+    return this;
+  }
+
+  public StreamAction getStderr() {
+    return stderrAction;
+  }
+
+  public File getStderrFile() {
+    return stderrFile;
+  }
+
+  /**
+   * Tells the object what to do with stderr: either stream as a {@code InputStream} or discard.
+   * 
+   * <p>It can also be redirected to a file using {@link #setStderr(File)}.
+   */
+  public SubprocessBuilder setStderr(StreamAction action) {
+    if (action == StreamAction.REDIRECT) {
+      throw new IllegalStateException();
+    }
+    this.stderrAction = action;
+    this.stderrFile = null;
+    return this;
+  }
+  
+  /**
+   * Sets the file stderr is appended to. If null, the stderr will be available as an input stream
+   * on the resulting object representing the process.
+   */
+  public SubprocessBuilder setStderr(File file) {
+    this.stderrAction = StreamAction.REDIRECT;
+    this.stderrFile = file;
+    return this;
+  }
+
+  public File getWorkingDirectory() {
+    return workingDirectory;
+  }
+
+  /**
+   * Sets the current working directory. If null, it will be that of this process.
+   */
+  public SubprocessBuilder setWorkingDirectory(File workingDirectory) {
+    this.workingDirectory = workingDirectory;
+    return this;
+  }
+
+  public Subprocess start() throws IOException {
+    return JavaSubprocessFactory.INSTANCE.create(this);
+  }
+}