Make ActoolZip only print output if it actool fails or doesn't output its output file

--
MOS_MIGRATED_REVID=89705982

3 files changed, 130 insertions, 30 deletions

diff --git a/src/tools/xcode-common/java/com/google/devtools/build/xcode/actoolzip/ActoolZip.java b/src/tools/xcode-common/java/com/google/devtools/build/xcode/actoolzip/ActoolZip.java
index 0da4be3741..b028ae5947 100644
--- a/src/tools/xcode-common/java/com/google/devtools/build/xcode/actoolzip/ActoolZip.java
+++ b/src/tools/xcode-common/java/com/google/devtools/build/xcode/actoolzip/ActoolZip.java
@@ -15,12 +15,15 @@
 package com.google.devtools.build.xcode.actoolzip;
 
 import com.google.common.base.Function;
+import com.google.common.base.Optional;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.Iterables;
 import com.google.devtools.build.xcode.zippingoutput.Arguments;
 import com.google.devtools.build.xcode.zippingoutput.Wrapper;
 import com.google.devtools.build.xcode.zippingoutput.Wrappers;
+import com.google.devtools.build.xcode.zippingoutput.Wrappers.CommandFailedException;
+import com.google.devtools.build.xcode.zippingoutput.Wrappers.OutErr;
 
 import java.io.File;
 import java.io.IOException;
@@ -71,8 +74,16 @@ public class ActoolZip implements Wrapper {
   }
 
   public static void main(String[] args) throws IOException, InterruptedException {
-    replaceInfoPlistPath(args);
-    Wrappers.execute(args, new ActoolZip());
+    Optional<File> infoPlistPath = replaceInfoPlistPath(args);
+    try {
+      OutErr outErr = Wrappers.executeCapturingOutput(args, new ActoolZip());
+      if (infoPlistPath.isPresent() && !infoPlistPath.get().exists()) {
+        outErr.print();
+        System.exit(1);
+      }
+    } catch (CommandFailedException e) {
+      Wrappers.handleException(e);
+    }
   }
 
   /**
@@ -84,24 +95,30 @@ public class ActoolZip implements Wrapper {
    * file doesn't exist at the time of flag parsing.
    *
    * <p>Modifies args in-place.
+   *
+   * @return new value of the output-partial-info-plist flag.
    */
-  private static void replaceInfoPlistPath(String[] args) {
+  private static Optional<File> replaceInfoPlistPath(String[] args) {
     String flag = "output-partial-info-plist";
     Set<String> flagOptions = ImmutableSet.of(
         "-" + flag,
         "--" + flag);
+    Optional<File> newPath = Optional.absent();
     for (int i = 0; i < args.length; ++i) {
       for (String flagOption : flagOptions) {
         String arg = args[i];
         String flagEquals = flagOption + "=";
         if (arg.startsWith(flagEquals)) {
-          args[i] = flagEquals + new File(arg.substring(flagEquals.length())).getAbsolutePath();
+          newPath = Optional.of(new File(arg.substring(flagEquals.length())));
+          args[i] = flagEquals + newPath.get().getAbsolutePath();
         }
         if (arg.equals(flagOption) && i + 1 < args.length) {
-          args[i + 1] = new File(args[i + 1]).getAbsolutePath();
+          newPath = Optional.of(new File(args[i + 1]));
+          args[i + 1] = newPath.get().getAbsolutePath();
         }
       }
     }
+    return newPath;
   }
 
   @Override
diff --git a/src/tools/xcode-common/java/com/google/devtools/build/xcode/ibtoolzip/IbtoolZip.java b/src/tools/xcode-common/java/com/google/devtools/build/xcode/ibtoolzip/IbtoolZip.java
index 5022a0cdd7..892053b1f6 100644
--- a/src/tools/xcode-common/java/com/google/devtools/build/xcode/ibtoolzip/IbtoolZip.java
+++ b/src/tools/xcode-common/java/com/google/devtools/build/xcode/ibtoolzip/IbtoolZip.java
@@ -48,7 +48,7 @@ public class IbtoolZip implements Wrapper {
   }
 
   public static void main(String[] args) throws IOException, InterruptedException {
-    Wrappers.execute(args, new IbtoolZip());
+    Wrappers.executePipingOutput(args, new IbtoolZip());
   }
 
   @Override
diff --git a/src/tools/xcode-common/java/com/google/devtools/build/xcode/zippingoutput/Wrappers.java b/src/tools/xcode-common/java/com/google/devtools/build/xcode/zippingoutput/Wrappers.java
index 282ce42641..7d4df044d7 100644
--- a/src/tools/xcode-common/java/com/google/devtools/build/xcode/zippingoutput/Wrappers.java
+++ b/src/tools/xcode-common/java/com/google/devtools/build/xcode/zippingoutput/Wrappers.java
@@ -15,17 +15,23 @@
 package com.google.devtools.build.xcode.zippingoutput;
 
 import com.google.common.collect.ImmutableList;
+import com.google.common.io.CharStreams;
 import com.google.devtools.build.singlejar.ZipCombiner;
 import com.google.devtools.build.xcode.zip.ZipInputEntry;
 
 import java.io.IOException;
+import java.io.InputStream;
+import java.io.InputStreamReader;
 import java.io.OutputStream;
+import java.nio.charset.StandardCharsets;
 import java.nio.file.FileSystem;
 import java.nio.file.FileSystems;
 import java.nio.file.Files;
 import java.nio.file.Path;
 import java.nio.file.Paths;
 
+import javax.annotation.Nullable;
+
 /** Utility code for working with {@link Wrapper}s. */
 public class Wrappers {
   private Wrappers() {
@@ -33,39 +39,82 @@ public class Wrappers {
   }
 
   /**
-   * Runs the given wrapper using command-line arguments passed to the {@code main} method. Calling
-   * this method should be the last thing you do in {@code main}, because it may exit prematurely
-   * with {@link System#exit(int)}.
+   * Executes the command specified by argsArray and wrapper, writing the output directly to this
+   * Java process's stdout/stderr. Calling this method should be the last thing you do in
+   * {@code main}, because it may exit prematurely with {@link System#exit(int)}.
    */
-  public static void execute(String[] argsArray, Wrapper wrapper)
+  public static void executePipingOutput(String[] argsArray, Wrapper wrapper)
       throws IOException, InterruptedException {
+    try {
+      execute(argsArray, wrapper, true);
+    } catch (CommandFailedException e) {
+      handleException(e);
+    }
+  }
+
+  /**
+   * Runs the given wrapper using command-line arguments passed to the {@code main} method, and
+   * returns the stdout and stderr of the process.
+   *
+   * @throws CommandFailedException if the command did not successfully run
+   */
+  public static OutErr executeCapturingOutput(String[] argsArray, Wrapper wrapper)
+      throws CommandFailedException, IOException, InterruptedException {
+    return execute(argsArray, wrapper, false);
+  }
+
+  /**
+   * Outputs stdout and stderr, then exits with a non-zero status code.
+   */
+  public static void handleException(CommandFailedException e) {
+    e.outErr.print();
+    System.err.print(e.getMessage());
+    System.exit(1);
+  }
+
+  @Nullable
+  private static OutErr execute(String[] argsArray, Wrapper wrapper, boolean printOutput)
+      throws CommandFailedException, IOException, InterruptedException {
     FileSystem filesystem = FileSystems.getDefault();
     ArgumentsParsing argsParsing = ArgumentsParsing.parse(
         filesystem, argsArray, wrapper.name(), wrapper.subtoolName());
     for (String error : argsParsing.error().asSet()) {
-      System.err.printf(error);
-      System.exit(1);
+      throw new CommandFailedException(error, OutErr.empty());
+    }
+    if (!argsParsing.arguments().isPresent()) {
+      throw new IllegalStateException("No arguments or error present");
     }
+    Arguments args = argsParsing.arguments().get();
     Path tempDir = getTempDir(filesystem);
-    for (Arguments args : argsParsing.arguments().asSet()) {
-      Path outputDir = Files.createTempDirectory(tempDir, "ZippingOutput");
-      Path rootedOutputDir = outputDir.resolve(args.bundleRoot());
-      Files.createDirectories(
-          wrapper.outputDirectoryMustExist() ? rootedOutputDir : rootedOutputDir.getParent());
-
-      ImmutableList<String> subCommandArguments =
-          ImmutableList.copyOf(wrapper.subCommand(args, rootedOutputDir.toString()));
-      Process subProcess = new ProcessBuilder(subCommandArguments).inheritIO().start();
-      int exit = subProcess.waitFor();
-      if (exit != 0) {
-        System.exit(exit);
-      }
-
-      try (OutputStream out = Files.newOutputStream(Paths.get(args.outputZip()));
-          ZipCombiner combiner = new ZipCombiner(out)) {
-        ZipInputEntry.addAll(combiner, ZipInputEntry.fromDirectory(outputDir));
-      }
+    Path outputDir = Files.createTempDirectory(tempDir, "ZippingOutput");
+    Path rootedOutputDir = outputDir.resolve(args.bundleRoot());
+    Files.createDirectories(
+        wrapper.outputDirectoryMustExist() ? rootedOutputDir : rootedOutputDir.getParent());
+
+    ImmutableList<String> subCommandArguments =
+        ImmutableList.copyOf(wrapper.subCommand(args, rootedOutputDir.toString()));
+    ProcessBuilder processBuilder = new ProcessBuilder(subCommandArguments);
+    if (printOutput) {
+      processBuilder = processBuilder.inheritIO();
+    }
+    Process subProcess = processBuilder.start();
+    int exit = subProcess.waitFor();
+    OutErr outErr = new OutErr(
+        streamToString(subProcess.getInputStream()),
+        streamToString(subProcess.getErrorStream()));
+    if (exit != 0) {
+      throw new CommandFailedException("", outErr);
+    }
+
+    try (OutputStream out = Files.newOutputStream(Paths.get(args.outputZip()));
+        ZipCombiner combiner = new ZipCombiner(out)) {
+      ZipInputEntry.addAll(combiner, ZipInputEntry.fromDirectory(outputDir));
     }
+    return outErr;
+  }
+
+  private static String streamToString(InputStream stream) throws IOException {
+    return CharStreams.toString(new InputStreamReader(stream, StandardCharsets.UTF_8));
   }
 
   private static Path getTempDir(FileSystem filesystem) {
@@ -75,4 +124,38 @@ public class Wrappers {
     }
     return filesystem.getPath(tempDir);
   }
+
+  /** Thrown if command exception fails for some reason. */
+  public static class CommandFailedException extends Exception {
+    private OutErr outErr;
+
+    public CommandFailedException(String message, OutErr outErr) {
+      super(message);
+      this.outErr = outErr;
+    }
+
+    public OutErr outErr() {
+      return outErr;
+    }
+  }
+
+  /** Stdout and stderr of a process. */
+  public static class OutErr {
+    public final String stdout;
+    public final String stderr;
+
+    public OutErr(String stdout, String stderr) {
+      this.stdout = stdout;
+      this.stderr = stderr;
+    }
+
+    public static OutErr empty() {
+      return new OutErr("", "");
+    }
+
+    public void print() {
+      System.out.print(stdout);
+      System.err.print(stderr);
+    }
+  }
 }