Skip to content

Commit 697e5f3

Browse files
ianoc-stripejohnynek
authored andcommitted
Due to limitations in the toolchains implementation this is required … (#801)
* Add test for host deps * Add a test hopefully to illustrate host deps * Update test * Change api usage to use binds * Remove errant print * See if behavior is different on 0.28.1 * incompatible_string_join_requires_strings: string.join(list) accepts invalid (non-string) list elements * Add a to_list * Another case of depset iterable * Windows ci can only support 0.28.0 via chocolaty right now
1 parent acac888 commit 697e5f3

File tree

8 files changed

+70
-16
lines changed

8 files changed

+70
-16
lines changed

.travis.yml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ env:
2121
# we want to test the last release
2222
#- V=0.16.1 TEST_SCRIPT=test_lint.sh
2323
- V=0.23.1 TEST_SCRIPT=test_rules_scala
24+
- V=0.28.0 TEST_SCRIPT=test_rules_scala
2425
#- V=0.14.1 TEST_SCRIPT=test_intellij_aspect.sh
2526
- V=0.23.1 TEST_SCRIPT=test_reproducibility
2627

scala/private/rule_impls.bzl

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -177,7 +177,7 @@ def compile_scala(
177177

178178
transitive_cjars_list = transitive_compile_jars.to_list()
179179
indirect_jars = _join_path(transitive_cjars_list)
180-
indirect_targets = ",".join([labels[j.path] for j in transitive_cjars_list])
180+
indirect_targets = ",".join([str(labels[j.path]) for j in transitive_cjars_list])
181181

182182
current_target = str(target_label)
183183

@@ -655,7 +655,7 @@ def _write_executable_non_windows(ctx, executable, rjars, main_class, jvm_flags,
655655
)
656656
ctx.actions.write(jacoco_metadata_file, "\n".join([
657657
jar.short_path.replace("../", "external/")
658-
for jar in rjars
658+
for jar in rjars.to_list()
659659
]))
660660
ctx.actions.expand_template(
661661
template = template,
@@ -1191,7 +1191,7 @@ def scala_test_impl(ctx):
11911191

11921192
rjars = depset([
11931193
coverage_replacements[jar] if jar in coverage_replacements else jar
1194-
for jar in rjars
1194+
for jar in rjars.to_list()
11951195
])
11961196
coverage_runfiles = ctx.files._jacocorunner + ctx.files._lcov_merger + coverage_replacements.values()
11971197

scala_proto/BUILD

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,6 @@
11
load("//scala_proto:scala_proto_toolchain.bzl", "scala_proto_toolchain")
2+
load("//scala_proto:default_dep_sets.bzl", "DEFAULT_SCALAPB_COMPILE_DEPS", "DEFAULT_SCALAPB_GRPC_DEPS")
3+
24

35
toolchain_type(
46
name = "toolchain_type",
@@ -36,3 +38,16 @@ toolchain(
3638
toolchain_type = "@io_bazel_rules_scala//scala_proto:toolchain_type",
3739
visibility = ["//visibility:public"],
3840
)
41+
42+
43+
java_library(
44+
name="default_scalapb_compile_dependencies",
45+
exports = DEFAULT_SCALAPB_COMPILE_DEPS,
46+
visibility = ["//visibility:public"],
47+
)
48+
49+
java_library(
50+
name="default_scalapb_grpc_dependencies",
51+
exports = DEFAULT_SCALAPB_GRPC_DEPS,
52+
visibility = ["//visibility:public"],
53+
)

scala_proto/private/scalapb_aspect.bzl

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -133,11 +133,11 @@ def _scalapb_aspect_impl(target, ctx):
133133

134134
toolchain = ctx.toolchains["@io_bazel_rules_scala//scala_proto:toolchain_type"]
135135
flags = []
136-
imps = [j[JavaInfo] for j in toolchain.implicit_compile_deps]
136+
imps = [j[JavaInfo] for j in ctx.attr._implicit_compile_deps]
137137

138138
if toolchain.with_grpc:
139139
flags.append("grpc")
140-
imps.extend([j[JavaInfo] for j in toolchain.grpc_deps])
140+
imps.extend([j[JavaInfo] for j in ctx.attr._grpc_deps])
141141

142142
if toolchain.with_flat_package:
143143
flags.append("flat_package")
@@ -224,6 +224,12 @@ scalapb_aspect = aspect(
224224
],
225225
attrs = {
226226
"_protoc": attr.label(executable = True, cfg = "host", default = "@com_google_protobuf//:protoc"),
227+
"_implicit_compile_deps": attr.label_list(cfg = "target", default = [
228+
"//external:io_bazel_rules_scala/dependency/proto/implicit_compile_deps",
229+
]),
230+
"_grpc_deps": attr.label_list(cfg = "target", default = [
231+
"//external:io_bazel_rules_scala/dependency/proto/grpc_deps",
232+
])
227233
},
228234
toolchains = [
229235
"@io_bazel_rules_scala//scala:toolchain_type",

scala_proto/scala_proto.bzl

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,10 +14,29 @@ load(
1414
"scalapb_aspect",
1515
)
1616

17+
def register_default_proto_dependencies():
18+
if native.existing_rule("io_bazel_rules_scala/dependency/proto/grpc_deps") == None:
19+
native.bind(
20+
name = "io_bazel_rules_scala/dependency/proto/grpc_deps",
21+
actual = "@io_bazel_rules_scala//scala_proto:default_scalapb_compile_dependencies",
22+
)
23+
24+
if native.existing_rule("io_bazel_rules_scala/dependency/proto/implicit_compile_deps") == None:
25+
native.bind(
26+
name = "io_bazel_rules_scala/dependency/proto/implicit_compile_deps",
27+
actual = "@io_bazel_rules_scala//scala_proto:default_scalapb_grpc_dependencies",
28+
)
29+
30+
31+
1732
def scala_proto_repositories(
1833
scala_version = _default_scala_version(),
1934
maven_servers = ["http://central.maven.org/maven2"]):
20-
return scala_proto_default_repositories(scala_version, maven_servers)
35+
ret = scala_proto_default_repositories(scala_version, maven_servers)
36+
register_default_proto_dependencies()
37+
return ret
38+
39+
2140

2241
def _scalapb_proto_library_impl(ctx):
2342
aspect_info = merge_scalapb_aspect_info(

scala_proto/scala_proto_toolchain.bzl

Lines changed: 0 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -7,8 +7,6 @@ def _scala_proto_toolchain_impl(ctx):
77
with_single_line_to_string = ctx.attr.with_single_line_to_string,
88
blacklisted_protos = ctx.attr.blacklisted_protos,
99
code_generator = ctx.attr.code_generator,
10-
grpc_deps=ctx.attr.grpc_deps,
11-
implicit_compile_deps=ctx.attr.implicit_compile_deps,
1210
extra_generator_dependencies = ctx.attr.extra_generator_dependencies,
1311
scalac=ctx.attr.scalac,
1412
named_generators = ctx.attr.named_generators,
@@ -39,14 +37,6 @@ scala_proto_toolchain = rule(
3937
"extra_generator_dependencies": attr.label_list(
4038
providers = [JavaInfo]
4139
),
42-
"grpc_deps": attr.label_list(
43-
providers = [JavaInfo],
44-
default = DEFAULT_SCALAPB_GRPC_DEPS
45-
),
46-
"implicit_compile_deps": attr.label_list(
47-
providers = [JavaInfo],
48-
default = DEFAULT_SCALAPB_COMPILE_DEPS,
49-
),
5040
"scalac": attr.label(
5141
default = Label(
5242
"@io_bazel_rules_scala//src/java/io/bazel/rulesscala/scalac",

test/proto/BUILD

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -114,6 +114,13 @@ scalapb_proto_library(
114114
deps = [":test2"],
115115
)
116116

117+
scala_binary(
118+
name = "test_binary_to_ensure_no_host_deps",
119+
main_class = "a.b.c",
120+
visibility = ["//visibility:public"],
121+
deps = [":test_proto_nogrpc"],
122+
)
123+
117124
java_proto_library(
118125
name = "test_proto_java_lib",
119126
deps = [

test_rules_scala.sh

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -887,6 +887,21 @@ test_scala_classpath_resources_expect_warning_on_namespace_conflict() {
887887
fi
888888
}
889889

890+
scala_pb_library_targets_do_not_have_host_deps() {
891+
set -e
892+
bazel build test/proto:test_binary_to_ensure_no_host_deps
893+
set +e
894+
find bazel-bin/test/proto/test_binary_to_ensure_no_host_deps.runfiles -name '*.jar' -exec readlink {} \; | grep 'bazel-out/host'
895+
RET=$?
896+
set -e
897+
if [ "$RET" == "0" ]; then
898+
echo "Host deps exist in output of target:"
899+
echo "Possibly toolchains limitation?"
900+
find bazel-bin/test/proto/test_binary_to_ensure_no_host_deps.runfiles -name '*.jar' -exec readlink {} \; | grep 'bazel-out/host'
901+
exit 1
902+
fi
903+
}
904+
890905
scala_binary_common_jar_is_exposed_in_build_event_protocol() {
891906
local target=$1
892907
set +e
@@ -1088,3 +1103,4 @@ $runner test_plus_one_deps_only_works_for_java_info_targets
10881103
$runner bazel test //test/... --extra_toolchains="//test_expect_failure/plus_one_deps:plus_one_deps"
10891104
$runner test_unused_dependency_fails_even_if_also_exists_in_plus_one_deps
10901105
$runner test_coverage_on
1106+
$runner scala_pb_library_targets_do_not_have_host_deps

0 commit comments

Comments
 (0)