address review

bazelbuild · Sep 12, 2016 · 30b0efc · 30b0efc
1 parent b28dcd1
commit 30b0efc
Show file tree

Hide file tree

Showing 2 changed files with 65 additions and 67 deletions.
diff --git a/src/java/io/bazel/rulesscala/scalac/CompileOptions.java b/src/java/io/bazel/rulesscala/scalac/CompileOptions.java
@@ -1,11 +1,5 @@
 package io.bazel.rulesscala.scalac;
 
-import java.io.File;
-import java.io.FileNotFoundException;
-import java.io.IOException;
-import java.nio.file.FileSystems;
-import java.nio.file.Files;
-import java.nio.file.Path;
 import java.util.ArrayList;
 import java.util.HashMap;
 import java.util.List;
@@ -18,14 +12,12 @@ public class CompileOptions {
   final public String[] pluginArgs;
   final public String classpath;
   final public String[] files;
-  final public Path outputPath;
   final public String[] sourceJars;
   final public boolean iJarEnabled;
   final public String ijarOutput;
   final public String ijarCmdPath;
-  final public Path tmpPath;
 
-  public CompileOptions(List<String> args) throws IOException, FileNotFoundException {
+  public CompileOptions(List<String> args) {
     Map<String, String> argMap = buildArgMap(args);
 
     outputName = getOrError(argMap, "JarOutput", "Missing required arg JarOutput");
@@ -34,21 +26,9 @@ public CompileOptions(List<String> args) throws IOException, FileNotFoundExcepti
     scalaOpts = getOrEmpty(argMap, "ScalacOpts").split(",");
     pluginArgs = buildPluginArgs(getOrEmpty(argMap, "Plugins"));
     classpath = getOrError(argMap, "Classpath", "Must supply the classpath arg");
-    outputPath = FileSystems.getDefault().getPath(outputName);
+    files = getOrEmpty(argMap, "Files").split(",");
 
     sourceJars = getOrEmpty(argMap, "SourceJars").split(",");
-    List<File> sourceFiles = new ArrayList<File>();
-
-    for(String jarPath : sourceJars) {
-      if(jarPath.length() > 0){
-        Path tmpPath = Files.createTempDirectory(outputPath.getParent(), "tmp");
-        sourceFiles.addAll(extractJar(jarPath, tmpPath.toString()));
-      }
-    }
-    files = appendToString(getOrEmpty(argMap, "Files").split(","), sourceFiles);
-    if(files.length == 0) {
-      throw new RuntimeException("Must have input files from either source jars or local files.");
-    }
     iJarEnabled = booleanGetOrFalse(argMap, "EnableIjar");
     if(iJarEnabled) {
      ijarOutput = getOrError(argMap, "ijarOutput", "Missing required arg ijarOutput when ijar enabled");
@@ -58,18 +38,6 @@ public CompileOptions(List<String> args) throws IOException, FileNotFoundExcepti
       ijarOutput = null;
       ijarCmdPath = null;
     }
-    tmpPath = Files.createTempDirectory(outputPath.getParent(),"tmp");
-  }
-
-  private static <T> String[] appendToString(String[] init, List<T> rest) {
-    String[] tmp = new String[init.length + rest.size()];
-    System.arraycopy(init, 0, tmp, 0, init.length);
-    int baseIdx = init.length;
-    for(T t : rest) {
-      tmp[baseIdx] = t.toString();
-      baseIdx += 1;
-    }
-    return tmp;
   }
 
   private static HashMap<String, String> buildArgMap(List<String> lines) {
@@ -129,33 +97,4 @@ public static String[] buildPluginArgs(String packedPlugins) {
     }
     return result;
   }
-  private static List<File> extractJar(String jarPath,
-      String outputFolder) throws IOException, FileNotFoundException {
-
-    List<File> outputPaths = new ArrayList<File>();
-    java.util.jar.JarFile jar = new java.util.jar.JarFile(jarPath);
-    java.util.Enumeration e = jar.entries();
-    while (e.hasMoreElements()) {
-      java.util.jar.JarEntry file = (java.util.jar.JarEntry) e.nextElement();
-      File f = new File(outputFolder + java.io.File.separator + file.getName());
-
-      if (file.isDirectory()) { // if its a directory, create it
-        f.mkdirs();
-        continue;
-      }
-
-      File parent = f.getParentFile();
-      parent.mkdirs();
-      outputPaths.add(f);
-
-      java.io.InputStream is = jar.getInputStream(file); // get the input stream
-      java.io.FileOutputStream fos = new java.io.FileOutputStream(f);
-      while (is.available() > 0) {  // write contents of 'is' to 'fos'
-        fos.write(is.read());
-      }
-      fos.close();
-      is.close();
-    }
-    return outputPaths;
-  }
 }
diff --git a/src/java/io/bazel/rulesscala/scalac/ScalaCInvoker.java b/src/java/io/bazel/rulesscala/scalac/ScalaCInvoker.java
@@ -31,6 +31,7 @@
 import java.io.IOException;
 import java.io.PrintStream;
 import java.lang.reflect.Field;
+import java.nio.file.FileSystems;
 import java.nio.file.Files;
 import java.nio.file.Path;
 import java.nio.file.Paths;
@@ -135,6 +136,62 @@ private static void runPersistentWorker(WorkerOptions workerOptions) throws IOEx
     }
   }
 
+  static private String[] extractSourceJars(CompileOptions opts, Path tmpParent) throws IOException {
+    List<File> sourceFiles = new ArrayList<File>();
+
+    for(String jarPath : opts.sourceJars) {
+      if (jarPath.length() > 0){
+        Path tmpPath = Files.createTempDirectory(tmpParent, "tmp");
+        sourceFiles.addAll(extractJar(jarPath, tmpPath.toString()));
+      }
+    }
+    String[] files = appendToString(opts.files, sourceFiles);
+    if(files.length == 0) {
+      throw new RuntimeException("Must have input files from either source jars or local files.");
+    }
+    return files;
+  }
+
+  private static List<File> extractJar(String jarPath,
+      String outputFolder) throws IOException, FileNotFoundException {
+
+    List<File> outputPaths = new ArrayList<File>();
+    java.util.jar.JarFile jar = new java.util.jar.JarFile(jarPath);
+    java.util.Enumeration e = jar.entries();
+    while (e.hasMoreElements()) {
+      java.util.jar.JarEntry file = (java.util.jar.JarEntry) e.nextElement();
+      File f = new File(outputFolder + java.io.File.separator + file.getName());
+
+      if (file.isDirectory()) { // if its a directory, create it
+        f.mkdirs();
+        continue;
+      }
+
+      File parent = f.getParentFile();
+      parent.mkdirs();
+      outputPaths.add(f);
+
+      java.io.InputStream is = jar.getInputStream(file); // get the input stream
+      java.io.FileOutputStream fos = new java.io.FileOutputStream(f);
+      while (is.available() > 0) {  // write contents of 'is' to 'fos'
+        fos.write(is.read());
+      }
+      fos.close();
+      is.close();
+    }
+    return outputPaths;
+  }
+
+  static <T> String[] appendToString(String[] init, List<T> rest) {
+    String[] tmp = new String[init.length + rest.size()];
+    System.arraycopy(init, 0, tmp, 0, init.length);
+    int baseIdx = init.length;
+    for(T t : rest) {
+      tmp[baseIdx] = t.toString();
+      baseIdx += 1;
+    }
+    return tmp;
+  }
   public static String[] merge(String[]... arrays) {
     int totalLength = 0;
     for(String[] arr:arrays){
@@ -170,18 +227,20 @@ private static void processRequest(List<String> args) throws Exception {
       }
       CompileOptions ops = new CompileOptions(args);
 
+      Path outputPath = FileSystems.getDefault().getPath(ops.outputName);
+      Path tmpPath = Files.createTempDirectory(outputPath.getParent(), "tmp");
       String[] constParams = {
         "-classpath",
         ops.classpath,
         "-d",
-        ops.tmpPath.toString()
+        tmpPath.toString()
         };
 
       String[] compilerArgs = merge(
         ops.scalaOpts,
         ops.pluginArgs,
         constParams,
-        ops.files);
+        extractSourceJars(ops, outputPath.getParent()));
 
       MainClass comp = new MainClass();
       long start = System.currentTimeMillis();
@@ -199,8 +258,8 @@ private static void processRequest(List<String> args) throws Exception {
         String[] jarCreatorArgs = {
           "-m",
           ops.manifestPath,
-          ops.outputPath.toString(),
-          ops.tmpPath.toString()
+          outputPath.toString(),
+          tmpPath.toString()
         };
         JarCreator.buildJar(jarCreatorArgs);