From 6271290a313269fc9c2f9ca52c9ef8d52a314cae Mon Sep 17 00:00:00 2001 From: Ittai Zeidman Date: Sat, 13 Jul 2019 20:54:06 +0300 Subject: [PATCH 1/6] expose source_jar in JavaInfo --- scala/private/rule_impls.bzl | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/scala/private/rule_impls.bzl b/scala/private/rule_impls.bzl index 964ad5fdc..fcf738896 100644 --- a/scala/private/rule_impls.bzl +++ b/scala/private/rule_impls.bzl @@ -401,7 +401,7 @@ def _compile_or_empty( if len(ctx.files.srcs) + len(srcjars.to_list()) == 0: _build_nosrc_jar(ctx) - scala_compilation_provider = _create_scala_compilation_provider(ctx, ctx.outputs.jar, deps_providers) + scala_compilation_provider = _create_scala_compilation_provider(ctx, ctx.outputs.jar, None, deps_providers) # no need to build ijar when empty return struct( @@ -476,7 +476,8 @@ def _compile_or_empty( # so set ijar == jar ijar = ctx.outputs.jar - scala_compilation_provider = _create_scala_compilation_provider(ctx, ijar, deps_providers) + source_jar = _pack_source_jar(ctx) + scala_compilation_provider = _create_scala_compilation_provider(ctx, ijar, source_jar, deps_providers) # compile the java now java_jar = try_to_compile_java_jar( @@ -513,7 +514,7 @@ def _compile_or_empty( merged_provider = merged_provider ) -def _create_scala_compilation_provider(ctx, ijar, deps_providers): +def _create_scala_compilation_provider(ctx, ijar, source_jar, deps_providers): exports = [] if hasattr(ctx.attr, "exports"): exports = [dep[JavaInfo] for dep in ctx.attr.exports] @@ -523,6 +524,7 @@ def _create_scala_compilation_provider(ctx, ijar, deps_providers): return JavaInfo( output_jar = ctx.outputs.jar, compile_jar = ijar, + source_jar = source_jar, deps = deps_providers, exports = exports, runtime_deps = runtime_deps, @@ -962,9 +964,7 @@ def _scala_binary_common( rjars, #calling rules need this for the classpath in the launcher ) -def _pack_source_jars(ctx): - source_jars = [] - +def _pack_source_jar(ctx): # collect .scala sources and pack a source jar for Scala scala_sources = [ f @@ -986,10 +986,12 @@ def _pack_source_jars(ctx): java_toolchain = find_java_toolchain(ctx, ctx.attr._java_toolchain), host_javabase = find_java_runtime_toolchain(ctx, ctx.attr._host_javabase), ) - if scala_source_jar: - source_jars.append(scala_source_jar) - return source_jars + return scala_source_jar + +def _pack_source_jars(ctx): + source_jar = _pack_source_jar(ctx) + return [source_jar] if source_jar else [] def scala_binary_impl(ctx): scalac_provider = _scalac_provider(ctx) From 9f97ada5a54749d869d477e2817710a4c1378a14 Mon Sep 17 00:00:00 2001 From: Ittai Zeidman Date: Sat, 13 Jul 2019 20:54:31 +0300 Subject: [PATCH 2/6] nit: clarify conditional --- scala/scala_import.bzl | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/scala/scala_import.bzl b/scala/scala_import.bzl index 52285acd2..5fcb02f3c 100644 --- a/scala/scala_import.bzl +++ b/scala/scala_import.bzl @@ -25,11 +25,10 @@ def _scala_import_impl(ctx): jars2labels, ) #last to override the label of the export compile jars to the current target - current_target_providers = [_new_java_info(ctx, jar) for jar in current_target_compile_jars] - - # Handle the case with no jars. - # TODO(#8867): Migrate away from the placeholder jar hack when #8867 is fixed. - if not current_target_providers: + if current_target_compile_jars: + current_target_providers = [_new_java_info(ctx, jar) for jar in current_target_compile_jars] + else: + # TODO(#8867): Migrate away from the placeholder jar hack when #8867 is fixed. current_target_providers = [_new_java_info(ctx, ctx.file._placeholder_jar)] return struct( From 2673eb8faeeaaead5bfc305885ee86c6e67d3ff5 Mon Sep 17 00:00:00 2001 From: Ittai Zeidman Date: Sat, 13 Jul 2019 20:54:41 +0300 Subject: [PATCH 3/6] nit: replace one hack with another --- .../missing_direct_deps/internal_deps/custom-jvm-rule.bzl | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/test_expect_failure/missing_direct_deps/internal_deps/custom-jvm-rule.bzl b/test_expect_failure/missing_direct_deps/internal_deps/custom-jvm-rule.bzl index 5241c48c9..82a21d97d 100644 --- a/test_expect_failure/missing_direct_deps/internal_deps/custom-jvm-rule.bzl +++ b/test_expect_failure/missing_direct_deps/internal_deps/custom-jvm-rule.bzl @@ -1,6 +1,7 @@ #This rule is an example for a jvm rule that doesn't support Jars2Labels def _custom_jvm_impl(ctx): - jar = ctx.file.main + # TODO(#8867): Migrate away from the placeholder jar hack when #8867 is fixed. + jar = ctx.file._placeholder_jar provider = JavaInfo(output_jar = jar, compile_jar = jar, deps = [target[JavaInfo] for target in ctx.attr.deps] @@ -10,7 +11,10 @@ def _custom_jvm_impl(ctx): custom_jvm = rule( implementation = _custom_jvm_impl, attrs = { - "main": attr.label(allow_single_file = True), #just used so we'll be able to build the JavaInfo with a "main" jar "deps": attr.label_list(), + "_placeholder_jar": attr.label( + allow_single_file = True, + default = Label("@io_bazel_rules_scala//scala:libPlaceHolderClassToCreateEmptyJarForScalaImport.jar"), + ), }, ) From d9f9b993ea01267c5af222931d1c545be2660704 Mon Sep 17 00:00:00 2001 From: Ittai Zeidman Date: Sat, 13 Jul 2019 20:54:51 +0300 Subject: [PATCH 4/6] nit: replace concat with append --- twitter_scrooge/twitter_scrooge.bzl | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/twitter_scrooge/twitter_scrooge.bzl b/twitter_scrooge/twitter_scrooge.bzl index eacb47c3d..41a99b2c2 100644 --- a/twitter_scrooge/twitter_scrooge.bzl +++ b/twitter_scrooge/twitter_scrooge.bzl @@ -372,7 +372,8 @@ def _scrooge_scala_library_impl(ctx): ) if ctx.attr.exports: exports = [exp[JavaInfo] for exp in ctx.attr.exports] - all_java = java_common.merge(_concat_lists(exports, [aspect_info.java_info])) + exports.append(aspect_info.java_info) + all_java = java_common.merge(exports) else: all_java = aspect_info.java_info From 12cb974a71a2b8b9c2b2c6f2db68f1c0abdb710c Mon Sep 17 00:00:00 2001 From: Ittai Zeidman Date: Sat, 13 Jul 2019 21:18:46 +0300 Subject: [PATCH 5/6] remove "main" attr usage it's no longer needed --- test_expect_failure/missing_direct_deps/internal_deps/BUILD | 1 - 1 file changed, 1 deletion(-) diff --git a/test_expect_failure/missing_direct_deps/internal_deps/BUILD b/test_expect_failure/missing_direct_deps/internal_deps/BUILD index e37e371e4..b249991d3 100644 --- a/test_expect_failure/missing_direct_deps/internal_deps/BUILD +++ b/test_expect_failure/missing_direct_deps/internal_deps/BUILD @@ -44,7 +44,6 @@ scala_library( custom_jvm( name = "direct_java_provider_dependency", - main = "//test/src/main/resources/scalarules/test:hellos-and-byes.jar", #just any jar would do deps = ["transitive_dependency"], ) From f6c2e104a268223c9bc6de8212338559122804f3 Mon Sep 17 00:00:00 2001 From: Ittai Zeidman Date: Sat, 13 Jul 2019 21:45:20 +0300 Subject: [PATCH 6/6] add comment --- scala/private/rule_impls.bzl | 1 + 1 file changed, 1 insertion(+) diff --git a/scala/private/rule_impls.bzl b/scala/private/rule_impls.bzl index fcf738896..97ee2c36a 100644 --- a/scala/private/rule_impls.bzl +++ b/scala/private/rule_impls.bzl @@ -991,6 +991,7 @@ def _pack_source_jar(ctx): def _pack_source_jars(ctx): source_jar = _pack_source_jar(ctx) + #_pack_source_jar may return None if java_common.pack_sources returned None (and it can) return [source_jar] if source_jar else [] def scala_binary_impl(ctx):