From 2c67576de8a0059391f7ce7f8f34a674fb687018 Mon Sep 17 00:00:00 2001 From: Ian O'Connell Date: Mon, 29 Jul 2019 16:17:43 -0600 Subject: [PATCH 01/10] Add test for host deps --- test/proto/BUILD | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/test/proto/BUILD b/test/proto/BUILD index ccc708a9a..2edc416ea 100644 --- a/test/proto/BUILD +++ b/test/proto/BUILD @@ -114,6 +114,13 @@ scalapb_proto_library( deps = [":test2"], ) +scala_binary( + name = "test_binary_to_ensure_no_host_deps", + main_class = "a.b.c", + visibility = ["//visibility:public"], + deps = [":test_proto_nogrpc"], +) + java_proto_library( name = "test_proto_java_lib", deps = [ From fccdd4f0dace86fc8f6360ba50266121f3015004 Mon Sep 17 00:00:00 2001 From: Ian O'Connell Date: Mon, 29 Jul 2019 16:23:04 -0600 Subject: [PATCH 02/10] Add a test hopefully to illustrate host deps --- test_rules_scala.sh | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/test_rules_scala.sh b/test_rules_scala.sh index b68d92083..986968cfd 100755 --- a/test_rules_scala.sh +++ b/test_rules_scala.sh @@ -887,6 +887,18 @@ test_scala_classpath_resources_expect_warning_on_namespace_conflict() { fi } +scala_pb_library_targets_do_not_have_host_deps() { + set -e + bazel build test/proto:test_binary_to_ensure_no_host_deps + LINES=$(find bazel-bin/test/proto/test_binary_to_ensure_no_host_deps.runfiles -name '*.jar' -exec readlink {} \; | grep 'bazel-out/host' | wc -l) + if [ "$LINES" != "0" ]; then + echo "Host deps exist in output of target" + echo "Possibly toolchains limitation?" + find bazel-bin/test/proto/test_binary_to_ensure_no_host_deps.runfiles -name '*.jar' -exec readlink {} \; | grep 'bazel-out/host' + exit 1 + fi +} + scala_binary_common_jar_is_exposed_in_build_event_protocol() { local target=$1 set +e @@ -1088,3 +1100,4 @@ $runner test_plus_one_deps_only_works_for_java_info_targets $runner bazel test //test/... --extra_toolchains="//test_expect_failure/plus_one_deps:plus_one_deps" $runner test_unused_dependency_fails_even_if_also_exists_in_plus_one_deps $runner test_coverage_on +$runner scala_pb_library_targets_do_not_have_host_deps From f3b4cf305d85069ef12549c852283abb3e3a58eb Mon Sep 17 00:00:00 2001 From: Ian O'Connell Date: Mon, 29 Jul 2019 16:32:47 -0600 Subject: [PATCH 03/10] Update test --- test_rules_scala.sh | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/test_rules_scala.sh b/test_rules_scala.sh index 986968cfd..ff65051bb 100755 --- a/test_rules_scala.sh +++ b/test_rules_scala.sh @@ -890,9 +890,12 @@ test_scala_classpath_resources_expect_warning_on_namespace_conflict() { scala_pb_library_targets_do_not_have_host_deps() { set -e bazel build test/proto:test_binary_to_ensure_no_host_deps - LINES=$(find bazel-bin/test/proto/test_binary_to_ensure_no_host_deps.runfiles -name '*.jar' -exec readlink {} \; | grep 'bazel-out/host' | wc -l) - if [ "$LINES" != "0" ]; then - echo "Host deps exist in output of target" + set +e + find bazel-bin/test/proto/test_binary_to_ensure_no_host_deps.runfiles -name '*.jar' -exec readlink {} \; | grep 'bazel-out/host' + RET=$? + set -e + if [ "$RET" == "0" ]; then + echo "Host deps exist in output of target:" echo "Possibly toolchains limitation?" find bazel-bin/test/proto/test_binary_to_ensure_no_host_deps.runfiles -name '*.jar' -exec readlink {} \; | grep 'bazel-out/host' exit 1 From 4fd0f7fab26230f5c9e2aad09a2c8923b9a972f2 Mon Sep 17 00:00:00 2001 From: Ian O'Connell Date: Mon, 29 Jul 2019 16:38:02 -0600 Subject: [PATCH 04/10] Change api usage to use binds --- scala_proto/BUILD | 15 +++++++++++++++ scala_proto/private/scalapb_aspect.bzl | 10 ++++++++-- scala_proto/scala_proto.bzl | 22 +++++++++++++++++++++- scala_proto/scala_proto_toolchain.bzl | 10 ---------- 4 files changed, 44 insertions(+), 13 deletions(-) diff --git a/scala_proto/BUILD b/scala_proto/BUILD index 08d8bd7d9..17a4f2e78 100644 --- a/scala_proto/BUILD +++ b/scala_proto/BUILD @@ -1,4 +1,6 @@ load("//scala_proto:scala_proto_toolchain.bzl", "scala_proto_toolchain") +load("//scala_proto:default_dep_sets.bzl", "DEFAULT_SCALAPB_COMPILE_DEPS", "DEFAULT_SCALAPB_GRPC_DEPS") + toolchain_type( name = "toolchain_type", @@ -36,3 +38,16 @@ toolchain( toolchain_type = "@io_bazel_rules_scala//scala_proto:toolchain_type", visibility = ["//visibility:public"], ) + + +java_library( + name="default_scalapb_compile_dependencies", + exports = DEFAULT_SCALAPB_COMPILE_DEPS, + visibility = ["//visibility:public"], +) + +java_library( + name="default_scalapb_grpc_dependencies", + exports = DEFAULT_SCALAPB_GRPC_DEPS, + visibility = ["//visibility:public"], +) diff --git a/scala_proto/private/scalapb_aspect.bzl b/scala_proto/private/scalapb_aspect.bzl index 2e372d26e..d58101102 100644 --- a/scala_proto/private/scalapb_aspect.bzl +++ b/scala_proto/private/scalapb_aspect.bzl @@ -133,11 +133,11 @@ def _scalapb_aspect_impl(target, ctx): toolchain = ctx.toolchains["@io_bazel_rules_scala//scala_proto:toolchain_type"] flags = [] - imps = [j[JavaInfo] for j in toolchain.implicit_compile_deps] + imps = [j[JavaInfo] for j in ctx.attr._implicit_compile_deps] if toolchain.with_grpc: flags.append("grpc") - imps.extend([j[JavaInfo] for j in toolchain.grpc_deps]) + imps.extend([j[JavaInfo] for j in ctx.attr._grpc_deps]) if toolchain.with_flat_package: flags.append("flat_package") @@ -224,6 +224,12 @@ scalapb_aspect = aspect( ], attrs = { "_protoc": attr.label(executable = True, cfg = "host", default = "@com_google_protobuf//:protoc"), + "_implicit_compile_deps": attr.label_list(cfg = "target", default = [ + "//external:io_bazel_rules_scala/dependency/proto/implicit_compile_deps", + ]), + "_grpc_deps": attr.label_list(cfg = "target", default = [ + "//external:io_bazel_rules_scala/dependency/proto/grpc_deps", + ]) }, toolchains = [ "@io_bazel_rules_scala//scala:toolchain_type", diff --git a/scala_proto/scala_proto.bzl b/scala_proto/scala_proto.bzl index f2ff80b0e..6357756df 100644 --- a/scala_proto/scala_proto.bzl +++ b/scala_proto/scala_proto.bzl @@ -14,10 +14,30 @@ load( "scalapb_aspect", ) +def register_default_proto_dependencies(): + print("here") + if native.existing_rule("io_bazel_rules_scala/dependency/proto/grpc_deps") == None: + native.bind( + name = "io_bazel_rules_scala/dependency/proto/grpc_deps", + actual = "@io_bazel_rules_scala//scala_proto:default_scalapb_compile_dependencies", + ) + + if native.existing_rule("io_bazel_rules_scala/dependency/proto/implicit_compile_deps") == None: + native.bind( + name = "io_bazel_rules_scala/dependency/proto/implicit_compile_deps", + actual = "@io_bazel_rules_scala//scala_proto:default_scalapb_grpc_dependencies", + ) + + + def scala_proto_repositories( scala_version = _default_scala_version(), maven_servers = ["http://central.maven.org/maven2"]): - return scala_proto_default_repositories(scala_version, maven_servers) + ret = scala_proto_default_repositories(scala_version, maven_servers) + register_default_proto_dependencies() + return ret + + def _scalapb_proto_library_impl(ctx): aspect_info = merge_scalapb_aspect_info( diff --git a/scala_proto/scala_proto_toolchain.bzl b/scala_proto/scala_proto_toolchain.bzl index 0082f2cd3..84e328a3c 100644 --- a/scala_proto/scala_proto_toolchain.bzl +++ b/scala_proto/scala_proto_toolchain.bzl @@ -7,8 +7,6 @@ def _scala_proto_toolchain_impl(ctx): with_single_line_to_string = ctx.attr.with_single_line_to_string, blacklisted_protos = ctx.attr.blacklisted_protos, code_generator = ctx.attr.code_generator, - grpc_deps=ctx.attr.grpc_deps, - implicit_compile_deps=ctx.attr.implicit_compile_deps, extra_generator_dependencies = ctx.attr.extra_generator_dependencies, scalac=ctx.attr.scalac, named_generators = ctx.attr.named_generators, @@ -39,14 +37,6 @@ scala_proto_toolchain = rule( "extra_generator_dependencies": attr.label_list( providers = [JavaInfo] ), - "grpc_deps": attr.label_list( - providers = [JavaInfo], - default = DEFAULT_SCALAPB_GRPC_DEPS - ), - "implicit_compile_deps": attr.label_list( - providers = [JavaInfo], - default = DEFAULT_SCALAPB_COMPILE_DEPS, - ), "scalac": attr.label( default = Label( "@io_bazel_rules_scala//src/java/io/bazel/rulesscala/scalac", From e403eaa74919b496bac45dcd632eda14f2633d68 Mon Sep 17 00:00:00 2001 From: Ian O'Connell Date: Tue, 30 Jul 2019 09:13:48 -0600 Subject: [PATCH 05/10] Remove errant print --- scala_proto/scala_proto.bzl | 1 - 1 file changed, 1 deletion(-) diff --git a/scala_proto/scala_proto.bzl b/scala_proto/scala_proto.bzl index 6357756df..9aa61c16d 100644 --- a/scala_proto/scala_proto.bzl +++ b/scala_proto/scala_proto.bzl @@ -15,7 +15,6 @@ load( ) def register_default_proto_dependencies(): - print("here") if native.existing_rule("io_bazel_rules_scala/dependency/proto/grpc_deps") == None: native.bind( name = "io_bazel_rules_scala/dependency/proto/grpc_deps", From cdef90ae690041378949b3e2bc7a4981e59ac7c9 Mon Sep 17 00:00:00 2001 From: Ian O'Connell Date: Tue, 30 Jul 2019 09:20:08 -0600 Subject: [PATCH 06/10] See if behavior is different on 0.28.1 --- .travis.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.travis.yml b/.travis.yml index d23b486b8..da28fa7b1 100644 --- a/.travis.yml +++ b/.travis.yml @@ -21,6 +21,7 @@ env: # we want to test the last release #- V=0.16.1 TEST_SCRIPT=test_lint.sh - V=0.23.1 TEST_SCRIPT=test_rules_scala + - V=0.28.1 TEST_SCRIPT=test_rules_scala #- V=0.14.1 TEST_SCRIPT=test_intellij_aspect.sh - V=0.23.1 TEST_SCRIPT=test_reproducibility From 2e0c8eca2f411a5640e26b372110a1250c56ba60 Mon Sep 17 00:00:00 2001 From: Ian O'Connell Date: Tue, 30 Jul 2019 09:42:18 -0600 Subject: [PATCH 07/10] incompatible_string_join_requires_strings: string.join(list) accepts invalid (non-string) list elements --- scala/private/rule_impls.bzl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scala/private/rule_impls.bzl b/scala/private/rule_impls.bzl index 6f38717cc..2639a49e7 100644 --- a/scala/private/rule_impls.bzl +++ b/scala/private/rule_impls.bzl @@ -177,7 +177,7 @@ def compile_scala( transitive_cjars_list = transitive_compile_jars.to_list() indirect_jars = _join_path(transitive_cjars_list) - indirect_targets = ",".join([labels[j.path] for j in transitive_cjars_list]) + indirect_targets = ",".join([str(labels[j.path]) for j in transitive_cjars_list]) current_target = str(target_label) From a5c3631cbb054ea1333e8db9b460b38e54d5ba46 Mon Sep 17 00:00:00 2001 From: Ian O'Connell Date: Tue, 30 Jul 2019 09:59:51 -0600 Subject: [PATCH 08/10] Add a to_list --- scala/private/rule_impls.bzl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scala/private/rule_impls.bzl b/scala/private/rule_impls.bzl index 2639a49e7..fdbaf8dcd 100644 --- a/scala/private/rule_impls.bzl +++ b/scala/private/rule_impls.bzl @@ -1191,7 +1191,7 @@ def scala_test_impl(ctx): rjars = depset([ coverage_replacements[jar] if jar in coverage_replacements else jar - for jar in rjars + for jar in rjars.to_list() ]) coverage_runfiles = ctx.files._jacocorunner + ctx.files._lcov_merger + coverage_replacements.values() From 31212eccfbde25bd32c4356bfc176cce07e4a5e2 Mon Sep 17 00:00:00 2001 From: Ian O'Connell Date: Tue, 30 Jul 2019 10:23:58 -0600 Subject: [PATCH 09/10] Another case of depset iterable --- scala/private/rule_impls.bzl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scala/private/rule_impls.bzl b/scala/private/rule_impls.bzl index fdbaf8dcd..8a40f55fc 100644 --- a/scala/private/rule_impls.bzl +++ b/scala/private/rule_impls.bzl @@ -655,7 +655,7 @@ def _write_executable_non_windows(ctx, executable, rjars, main_class, jvm_flags, ) ctx.actions.write(jacoco_metadata_file, "\n".join([ jar.short_path.replace("../", "external/") - for jar in rjars + for jar in rjars.to_list() ])) ctx.actions.expand_template( template = template, From 78d3435746a65a8a589c88b4515b3ccf2bd9ba97 Mon Sep 17 00:00:00 2001 From: Ian O'Connell Date: Tue, 30 Jul 2019 10:40:28 -0600 Subject: [PATCH 10/10] Windows ci can only support 0.28.0 via chocolaty right now --- .travis.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.travis.yml b/.travis.yml index da28fa7b1..cf4c9ca87 100644 --- a/.travis.yml +++ b/.travis.yml @@ -21,7 +21,7 @@ env: # we want to test the last release #- V=0.16.1 TEST_SCRIPT=test_lint.sh - V=0.23.1 TEST_SCRIPT=test_rules_scala - - V=0.28.1 TEST_SCRIPT=test_rules_scala + - V=0.28.0 TEST_SCRIPT=test_rules_scala #- V=0.14.1 TEST_SCRIPT=test_intellij_aspect.sh - V=0.23.1 TEST_SCRIPT=test_reproducibility