Use java_common.run_ijar (#538)

* Use java_common.run_ijar

* run lint fix

* try to fix a test

* fix up the tests

* try one more time

Author: johnynek

scala/private/rule_impls.bzl

@@ -70,12 +70,9 @@ def _add_resources_cmd(ctx):
     res_cmd.extend([line])
   return "".join(res_cmd)
 
-def _build_nosrc_jar(ctx, buildijar):
+def _build_nosrc_jar(ctx):
   resources = _add_resources_cmd(ctx)
   ijar_cmd = ""
-  if buildijar:
-    ijar_cmd = "\ncp {jar_output} {ijar_output}\n".format(
-        jar_output = ctx.outputs.jar.path, ijar_output = ctx.outputs.ijar.path)
 
   # this ensures the file is not empty
   resources += "META-INF/MANIFEST.MF=%s\n" % ctx.outputs.manifest.path
@@ -97,8 +94,6 @@ touch {statsfile}
   )
 
   outs = [ctx.outputs.jar, ctx.outputs.statsfile]
-  if buildijar:
-    outs.extend([ctx.outputs.ijar])
 
   inputs = ctx.files.resources + [
       ctx.outputs.manifest, ctx.executable._zipper, zipper_arg_path
@@ -132,31 +127,11 @@ def _expand_location(ctx, flags):
 def _join_path(args, sep = ","):
   return sep.join([f.path for f in args])
 
-def compile_scala(ctx,
-                  target_label,
-                  output,
-                  manifest,
-                  statsfile,
-                  sources,
-                  cjars,
-                  all_srcjars,
-                  buildijar,
-                  transitive_compile_jars,
-                  plugins,
-                  resource_strip_prefix,
-                  resources,
-                  resource_jars,
-                  labels,
-                  in_scalacopts,
-                  print_compile_time,
-                  expect_java_output,
-                  scalac_jvm_flags = []):
-  ijar_output_path = ""
-  ijar_cmd_path = ""
-  if buildijar:
-    ijar_output_path = ctx.outputs.ijar.path
-    ijar_cmd_path = ctx.executable._ijar.path
-
+def compile_scala(ctx, target_label, output, manifest, statsfile, sources,
+                  cjars, all_srcjars, transitive_compile_jars, plugins,
+                  resource_strip_prefix, resources, resource_jars, labels,
+                  in_scalacopts, print_compile_time, expect_java_output,
+                  scalac_jvm_flags):
   # look for any plugins:
   plugins = _collect_plugin_paths(plugins)
   dependency_analyzer_plugin_jars = []
@@ -204,10 +179,7 @@ CurrentTarget: {current_target}
   scalac_args = """
 Classpath: {cp}
 ClasspathResourceSrcs: {classpath_resource_src}
-EnableIjar: {enableijar}
 Files: {files}
-IjarCmdPath: {ijar_cmd_path}
-IjarOutput: {ijar_out}
 JarOutput: {out}
 Manifest: {manifest}
 Plugins: {plugin_arg}
@@ -232,9 +204,6 @@ StatsfileOutput: {statsfile_output}
       cp = compiler_classpath,
       classpath_resource_src = _join_path(classpath_resources),
       files = _join_path(sources),
-      enableijar = buildijar,
-      ijar_out = ijar_output_path,
-      ijar_cmd_path = ijar_cmd_path,
       srcjars = _join_path(all_srcjars.to_list()),
       # the resource paths need to be aligned in order
       resource_src = ",".join([f.path for f in resources]),
@@ -254,12 +223,10 @@ StatsfileOutput: {statsfile_output}
       output = argfile, content = scalac_args + optional_scalac_args)
 
   outs = [output, statsfile]
-  if buildijar:
-    outs.extend([ctx.outputs.ijar])
-  ins = (
-      compiler_classpath_jars.to_list() + all_srcjars.to_list() + list(sources)
-      + plugins_list + dependency_analyzer_plugin_jars + classpath_resources +
-      resources + resource_jars + [manifest, ctx.executable._ijar, argfile])
+  ins = (compiler_classpath_jars.to_list() + all_srcjars.to_list() +
+         list(sources) + plugins_list + dependency_analyzer_plugin_jars +
+         classpath_resources + resources + resource_jars + [manifest, argfile])
+
   ctx.actions.run(
       inputs = ins,
       outputs = outs,
@@ -334,7 +301,7 @@ def _compile_or_empty(ctx, manifest, jars, srcjars, buildijar,
                       implicit_junit_deps_needed_for_java_compilation):
   # We assume that if a srcjar is present, it is not empty
   if len(ctx.files.srcs) + len(srcjars.to_list()) == 0:
-    _build_nosrc_jar(ctx, buildijar)
+    _build_nosrc_jar(ctx)
     #  no need to build ijar when empty
     return struct(
         ijar = ctx.outputs.jar,
@@ -355,28 +322,30 @@ def _compile_or_empty(ctx, manifest, jars, srcjars, buildijar,
         f for f in ctx.files.srcs if f.basename.endswith(_scala_extension)
     ] + java_srcs
     compile_scala(ctx, ctx.label, ctx.outputs.jar, manifest,
-                  ctx.outputs.statsfile, sources, jars, all_srcjars, buildijar,
+                  ctx.outputs.statsfile, sources, jars, all_srcjars,
                   transitive_compile_jars, ctx.attr.plugins,
                   ctx.attr.resource_strip_prefix, ctx.files.resources,
                   ctx.files.resource_jars, jars2labels, ctx.attr.scalacopts,
                   ctx.attr.print_compile_time, ctx.attr.expect_java_output,
                   ctx.attr.scalac_jvm_flags)
-    # compile the java now
-    if buildijar:
-      scala_output = ctx.outputs.ijar
-    else:
-      scala_output = ctx.outputs.jar
-    java_jar = try_to_compile_java_jar(
-        ctx, scala_output, all_srcjars, java_srcs,
-        implicit_junit_deps_needed_for_java_compilation)
 
-    ijar = None
+    # build ijar if needed
     if buildijar:
-      ijar = ctx.outputs.ijar
+      ijar = java_common.run_ijar(
+          ctx.actions,
+          jar = ctx.outputs.jar,
+          target_label = ctx.label,
+          java_toolchain = ctx.attr._java_toolchain)
     else:
       #  macro code needs to be available at compile-time,
       #  so set ijar == jar
       ijar = ctx.outputs.jar
+
+    # compile the java now
+    java_jar = try_to_compile_java_jar(
+        ctx, ijar, all_srcjars, java_srcs,
+        implicit_junit_deps_needed_for_java_compilation)
+
     full_jars = [ctx.outputs.jar]
     ijars = [ijar]
     if java_jar:

scala/scala.bzl

@@ -25,11 +25,6 @@ _implicit_deps = {
         cfg = "host",
         default = Label("@bazel_tools//tools/jdk:singlejar"),
         allow_files = True),
-    "_ijar": attr.label(
-        executable = True,
-        cfg = "host",
-        default = Label("@bazel_tools//tools/jdk:ijar"),
-        allow_files = True),
     "_scalac": attr.label(
         executable = True,
         cfg = "host",
@@ -146,9 +141,6 @@ _common_outputs = {
 
 _library_outputs = {}
 _library_outputs.update(_common_outputs)
-_library_outputs.update({
-    "ijar": "%{name}_ijar.jar",
-})
 
 _scala_library_attrs = {}
 _scala_library_attrs.update(_implicit_deps)

src/java/io/bazel/rulesscala/scalac/CompileOptions.java

@@ -14,9 +14,6 @@ public class CompileOptions {
   public final String classpath;
   public final String[] files;
   public final String[] sourceJars;
-  public final boolean iJarEnabled;
-  public final String ijarOutput;
-  public final String ijarCmdPath;
   public final String[] javaFiles;
   public final Map<String, Resource> resourceFiles;
   public final String resourceStripPrefix;
@@ -49,16 +46,6 @@ public CompileOptions(List<String> args) {
     }
 
     sourceJars = getCommaList(argMap, "SourceJars");
-    iJarEnabled = booleanGetOrFalse(argMap, "EnableIjar");
-    if (iJarEnabled) {
-      ijarOutput =
-          getOrError(argMap, "IjarOutput", "Missing required arg ijarOutput when ijar enabled");
-      ijarCmdPath =
-          getOrError(argMap, "IjarCmdPath", "Missing required arg ijarCmdPath when ijar enabled");
-    } else {
-      ijarOutput = null;
-      ijarCmdPath = null;
-    }
     resourceFiles = getResources(argMap);
     resourceStripPrefix = getOrEmpty(argMap, "ResourceStripPrefix");
     resourceJars = getCommaList(argMap, "ResourceJars");

src/java/io/bazel/rulesscala/scalac/ScalacProcessor.java

@@ -79,19 +79,6 @@ public void processRequest(List<String> args) throws Exception {
       /** Now build the output jar */
       String[] jarCreatorArgs = {"-m", ops.manifestPath, outputPath.toString(), tmpPath.toString()};
       JarCreator.main(jarCreatorArgs);
-
-      /** Now build the output ijar */
-      if (ops.iJarEnabled) {
-        Process iostat =
-            new ProcessBuilder()
-                .command(ops.ijarCmdPath, ops.outputName, ops.ijarOutput)
-                .inheritIO()
-                .start();
-        int exitCode = iostat.waitFor();
-        if (exitCode != 0) {
-          throw new RuntimeException("ijar process failed!");
-        }
-      }
     } finally {
       removeTmp(tmpPath);
     }

test_rules_scala.sh

@@ -176,11 +176,9 @@ test_scala_library_expect_failure_on_missing_direct_deps_warn_mode() {
 
 test_scala_library_expect_failure_on_missing_direct_java() {
   dependency_target='//test_expect_failure/missing_direct_deps/internal_deps:transitive_dependency'
-  #since bazel 0.12.0 the labels are only emmitted if ijar is in play 
-  dependency_file='test_expect_failure/missing_direct_deps/internal_deps/transitive_dependency_ijar.jar'
   test_target='//test_expect_failure/missing_direct_deps/internal_deps:transitive_dependency_java_user'
 
-  expected_message="$dependency_file.*$test_target"
+  expected_message="$dependency_target.*$test_target"
 
   test_expect_failure_or_warning_on_missing_direct_deps_with_expected_message "${expected_message}" $test_target "--strict_java_deps=error"
 }
@@ -194,7 +192,7 @@ test_scala_library_expect_failure_on_java_in_src_jar_when_disabled() {
 }
 
 test_scala_library_expect_better_failure_message_on_missing_transitive_dependency_labels_from_other_jvm_rules() {
-  transitive_target='.*transitive_dependency_ijar.jar'
+  transitive_target='.*transitive_dependency-ijar.jar'
   direct_target='//test_expect_failure/missing_direct_deps/internal_deps:direct_java_provider_dependency'
   test_target='//test_expect_failure/missing_direct_deps/internal_deps:dependent_on_some_java_provider'
 
@@ -205,10 +203,9 @@ test_scala_library_expect_better_failure_message_on_missing_transitive_dependenc
 
 test_scala_library_expect_failure_on_missing_direct_deps_warn_mode_java() {
   dependency_target='//test_expect_failure/missing_direct_deps/internal_deps:transitive_dependency'
-  dependency_file='test_expect_failure/missing_direct_deps/internal_deps/transitive_dependency_ijar.jar'
   test_target='//test_expect_failure/missing_direct_deps/internal_deps:transitive_dependency_java_user'
 
-  local expected_message="$dependency_file.*$test_target"
+  local expected_message="$dependency_target.*$test_target"
 
   test_expect_failure_or_warning_on_missing_direct_deps_with_expected_message "${expected_message}" ${test_target} "--strict_java_deps=warn" "ne"
 }