From ee0922a0b9a0b2a21400a5ac778043d8d1e6faa3 Mon Sep 17 00:00:00 2001 From: Oscar Boykin Date: Mon, 25 Jun 2018 15:17:03 -1000 Subject: [PATCH 1/5] Use java_common.run_ijar --- scala/private/rule_impls.bzl | 55 ++++++------------- scala/scala.bzl | 8 --- .../rulesscala/scalac/CompileOptions.java | 13 ----- .../rulesscala/scalac/ScalacProcessor.java | 13 ----- 4 files changed, 17 insertions(+), 72 deletions(-) diff --git a/scala/private/rule_impls.bzl b/scala/private/rule_impls.bzl index 38d61d0d2..b3f02741a 100644 --- a/scala/private/rule_impls.bzl +++ b/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,15 +127,9 @@ def _expand_location(ctx, flags): def _join_path(args, sep = ","): return sep.join([f.path for f in args]) -def _compile_scala(ctx, output, sources, cjars, all_srcjars, buildijar, +def _compile_scala(ctx, output, sources, cjars, all_srcjars, transitive_compile_jars, resource_strip_prefix, resources, resource_jars, labels, in_scalacopts): - ijar_output_path = "" - ijar_cmd_path = "" - if buildijar: - ijar_output_path = ctx.outputs.ijar.path - ijar_cmd_path = ctx.executable._ijar.path - # look for any plugins: plugins = _collect_plugin_paths(ctx.attr.plugins) dependency_analyzer_plugin_jars = [] @@ -187,10 +176,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} @@ -215,9 +201,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]), @@ -237,13 +220,10 @@ StatsfileOutput: {statsfile_output} output = argfile, content = scalac_args + optional_scalac_args) outs = [output, ctx.outputs.statsfile] - if buildijar: - outs.extend([ctx.outputs.ijar]) - ins = ( - compiler_classpath_jars.to_list() + all_srcjars.to_list() + - list(sources) + ctx.files.plugins + dependency_analyzer_plugin_jars + - classpath_resources + resources + resource_jars + ctx.files._java_runtime - + [ctx.outputs.manifest, ctx.executable._ijar, argfile]) + ins = (compiler_classpath_jars.to_list() + all_srcjars.to_list() + + list(sources) + ctx.files.plugins + dependency_analyzer_plugin_jars + + classpath_resources + resources + resource_jars + + ctx.files._java_runtime + [ctx.outputs.manifest, argfile]) ctx.actions.run( inputs = ins, outputs = outs, @@ -319,7 +299,7 @@ def _compile_or_empty(ctx, jars, srcjars, buildijar, transitive_compile_jars, 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, @@ -332,26 +312,25 @@ def _compile_or_empty(ctx, jars, srcjars, buildijar, transitive_compile_jars, all_srcjars = depset(in_srcjars, transitive = [srcjars]) java_srcs = _java_filetype.filter(ctx.files.srcs) sources = _scala_filetype.filter(ctx.files.srcs) + java_srcs - _compile_scala(ctx, ctx.outputs.jar, sources, jars, all_srcjars, buildijar, + _compile_scala(ctx, ctx.outputs.jar, sources, jars, all_srcjars, transitive_compile_jars, ctx.attr.resource_strip_prefix, ctx.files.resources, ctx.files.resource_jars, jars2labels, ctx.attr.scalacopts) # 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 - 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 + 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: diff --git a/scala/scala.bzl b/scala/scala.bzl index cc5504a0c..28c47065c 100644 --- a/scala/scala.bzl +++ b/scala/scala.bzl @@ -23,11 +23,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", @@ -144,9 +139,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) diff --git a/src/java/io/bazel/rulesscala/scalac/CompileOptions.java b/src/java/io/bazel/rulesscala/scalac/CompileOptions.java index 2665cd959..52abbe0c8 100644 --- a/src/java/io/bazel/rulesscala/scalac/CompileOptions.java +++ b/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 resourceFiles; public final String resourceStripPrefix; @@ -49,16 +46,6 @@ public CompileOptions(List 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"); diff --git a/src/java/io/bazel/rulesscala/scalac/ScalacProcessor.java b/src/java/io/bazel/rulesscala/scalac/ScalacProcessor.java index a65783fda..0287a2669 100644 --- a/src/java/io/bazel/rulesscala/scalac/ScalacProcessor.java +++ b/src/java/io/bazel/rulesscala/scalac/ScalacProcessor.java @@ -79,19 +79,6 @@ public void processRequest(List 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); } From 664cb768a0b4ce66a6deb52a1be7a1e8719c5695 Mon Sep 17 00:00:00 2001 From: Oscar Boykin Date: Wed, 27 Jun 2018 09:09:29 -1000 Subject: [PATCH 2/5] run lint fix --- scala/private/rule_impls.bzl | 24 +++++------------------- 1 file changed, 5 insertions(+), 19 deletions(-) diff --git a/scala/private/rule_impls.bzl b/scala/private/rule_impls.bzl index ec20b9f01..3349e1166 100644 --- a/scala/private/rule_impls.bzl +++ b/scala/private/rule_impls.bzl @@ -127,23 +127,10 @@ 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, - transitive_compile_jars, - plugins, - resource_strip_prefix, - resources, - resource_jars, - labels, - in_scalacopts, - print_compile_time, - expect_java_output, +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) @@ -238,8 +225,7 @@ StatsfileOutput: {statsfile_output} outs = [output, statsfile] 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]) + classpath_resources + resources + resource_jars + [manifest, argfile]) ctx.actions.run( inputs = ins, From c9a70a4a4c36851fccd4f13c468f624dfb7aa40c Mon Sep 17 00:00:00 2001 From: Oscar Boykin Date: Wed, 27 Jun 2018 09:58:13 -1000 Subject: [PATCH 3/5] try to fix a test --- scala/private/rule_impls.bzl | 4 +++- test_rules_scala.sh | 4 +--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/scala/private/rule_impls.bzl b/scala/private/rule_impls.bzl index 3349e1166..bb1642d9d 100644 --- a/scala/private/rule_impls.bzl +++ b/scala/private/rule_impls.bzl @@ -329,7 +329,7 @@ def _compile_or_empty(ctx, manifest, jars, srcjars, buildijar, ctx.attr.print_compile_time, ctx.attr.expect_java_output, ctx.attr.scalac_jvm_flags) - # compile the java now + # build ijar if needed if buildijar: ijar = java_common.run_ijar( ctx.actions, @@ -340,6 +340,8 @@ def _compile_or_empty(ctx, manifest, jars, srcjars, buildijar, # 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) diff --git a/test_rules_scala.sh b/test_rules_scala.sh index 41b18a2f3..1b2ad3937 100755 --- a/test_rules_scala.sh +++ b/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=".*$test_target" test_expect_failure_or_warning_on_missing_direct_deps_with_expected_message "${expected_message}" $test_target "--strict_java_deps=error" } From e11597d92e55d7ace479fd7aafa9173802fc75b3 Mon Sep 17 00:00:00 2001 From: Oscar Boykin Date: Wed, 27 Jun 2018 10:38:48 -1000 Subject: [PATCH 4/5] fix up the tests --- test_rules_scala.sh | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/test_rules_scala.sh b/test_rules_scala.sh index 1b2ad3937..3eece51a6 100755 --- a/test_rules_scala.sh +++ b/test_rules_scala.sh @@ -178,7 +178,7 @@ test_scala_library_expect_failure_on_missing_direct_java() { dependency_target='//test_expect_failure/missing_direct_deps/internal_deps:transitive_dependency' test_target='//test_expect_failure/missing_direct_deps/internal_deps:transitive_dependency_java_user' - expected_message=".*$test_target" + expected_message=".*$dependency_target" test_expect_failure_or_warning_on_missing_direct_deps_with_expected_message "${expected_message}" $test_target "--strict_java_deps=error" } @@ -192,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' @@ -203,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_expect_failure_or_warning_on_missing_direct_deps_with_expected_message "${expected_message}" ${test_target} "--strict_java_deps=warn" "ne" } From adf0cd96a89c081aaccca6621c72f7287b69d818 Mon Sep 17 00:00:00 2001 From: Oscar Boykin Date: Wed, 27 Jun 2018 10:50:03 -1000 Subject: [PATCH 5/5] try one more time --- test_rules_scala.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test_rules_scala.sh b/test_rules_scala.sh index 3eece51a6..e8a8bcb7a 100755 --- a/test_rules_scala.sh +++ b/test_rules_scala.sh @@ -178,7 +178,7 @@ test_scala_library_expect_failure_on_missing_direct_java() { dependency_target='//test_expect_failure/missing_direct_deps/internal_deps:transitive_dependency' test_target='//test_expect_failure/missing_direct_deps/internal_deps:transitive_dependency_java_user' - expected_message=".*$dependency_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" } @@ -205,7 +205,7 @@ test_scala_library_expect_failure_on_missing_direct_deps_warn_mode_java() { dependency_target='//test_expect_failure/missing_direct_deps/internal_deps:transitive_dependency' test_target='//test_expect_failure/missing_direct_deps/internal_deps:transitive_dependency_java_user' - local expected_message=".*$dependency_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" }