Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix a regression that breaks scalapb when jvm_flags are set on the toolchain #806

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions manual_test/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
This directory contains tests that require extra setup such as extra bazel flags.
43 changes: 43 additions & 0 deletions manual_test/scala_test_jvm_flags/BUILD
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
load("//scala:scala_toolchain.bzl", "scala_toolchain")
load("//scala:scala.bzl", "scala_test")

scala_toolchain(
name = "failing_toolchain_impl",
# This will fail because 1M isn't enough
scala_test_jvm_flags = ["-Xmx1M"],
visibility = ["//visibility:public"],
)

toolchain(
name = "failing_scala_toolchain",
toolchain = "failing_toolchain_impl",
toolchain_type = "@io_bazel_rules_scala//scala:toolchain_type",
visibility = ["//visibility:public"],
)

scala_toolchain(
name = "passing_toolchain_impl",
# This will pass because 1G is enough
scala_test_jvm_flags = ["-Xmx1G"],
visibility = ["//visibility:public"],
)

toolchain(
name = "passing_scala_toolchain",
toolchain = "passing_toolchain_impl",
toolchain_type = "@io_bazel_rules_scala//scala:toolchain_type",
visibility = ["//visibility:public"],
)

scala_test(
name = "empty_test",
srcs = ["EmptyTest.scala"],
)

scala_test(
name = "empty_overriding_test",
srcs = ["EmptyTest.scala"],
# This overrides the option passed in on the toolchain, and should BUILD, even if
# the `failing_scala_toolchain` is used.
jvm_flags = ["-Xmx1G"],
)
9 changes: 9 additions & 0 deletions manual_test/scala_test_jvm_flags/EmptyTest.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
package test_expect_failure.scala_test_jvm_flags

import org.scalatest.FunSuite

class EmptyTest extends FunSuite {
test("empty test") {
assert(true)
}
}
61 changes: 61 additions & 0 deletions manual_test/scalac_jvm_opts/BUILD
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
load("//scala:scala_toolchain.bzl", "scala_toolchain")
load("//scala:scala.bzl", "scala_library")
load(
"//scala_proto:scala_proto.bzl",
"scalapb_proto_library",
)

scala_toolchain(
name = "failing_toolchain_impl",
# This will fail because 1M isn't enough
scalac_jvm_flags = ["-Xmx1M"],
visibility = ["//visibility:public"],
)

toolchain(
name = "failing_scala_toolchain",
toolchain = "failing_toolchain_impl",
toolchain_type = "@io_bazel_rules_scala//scala:toolchain_type",
visibility = ["//visibility:public"],
)

scala_toolchain(
name = "passing_toolchain_impl",
# This will pass because 1G is enough
scalac_jvm_flags = ["-Xmx1G"],
visibility = ["//visibility:public"],
)

toolchain(
name = "passing_scala_toolchain",
toolchain = "passing_toolchain_impl",
toolchain_type = "@io_bazel_rules_scala//scala:toolchain_type",
visibility = ["//visibility:public"],
)

scala_library(
name = "empty_build",
srcs = ["Empty.scala"],
)

scala_library(
name = "empty_overriding_build",
srcs = ["Empty.scala"],
# This overrides the option passed in on the toolchain, and should BUILD, even if
# the `failing_scala_toolchain` is used.
scalac_jvm_flags = ["-Xmx1G"],
)

proto_library(
name = "test",
srcs = ["test.proto"],
visibility = ["//visibility:public"],
)

# This is a regression test for a bug that broke compiling scalapb targets when
# `scalac_jvm_flags` was set on the toolchain.
scalapb_proto_library(
name = "proto",
visibility = ["//visibility:public"],
deps = [":test"],
)
3 changes: 3 additions & 0 deletions manual_test/scalac_jvm_opts/Empty.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
package test_expect_failure.scalac_jvm_opts

class Empty
8 changes: 8 additions & 0 deletions manual_test/scalac_jvm_opts/test.proto
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
syntax = "proto2";

option java_package = "test.proto";

message TestResponse1 {
optional uint32 c = 1;
optional bool d = 2;
}
6 changes: 5 additions & 1 deletion scala/private/rule_impls.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,11 @@ touch {statsfile}
)

def _expand_location(ctx, flags):
return [ctx.expand_location(f, ctx.attr.data) for f in flags]
if hasattr(ctx.attr, "data"):
data = ctx.attr.data
else:
data = []
return [ctx.expand_location(f, data) for f in flags]

def _join_path(args, sep = ","):
return sep.join([f.path for f in args])
Expand Down
10 changes: 1 addition & 9 deletions test_expect_failure/scala_test_jvm_flags/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -32,12 +32,4 @@ toolchain(
scala_test(
name = "empty_test",
srcs = ["EmptyTest.scala"],
)

scala_test(
name = "empty_overriding_test",
srcs = ["EmptyTest.scala"],
# This overrides the option passed in on the toolchain, and should BUILD, even if
# the `failing_scala_toolchain` is used.
jvm_flags = ["-Xmx1G"],
)
)
28 changes: 5 additions & 23 deletions test_expect_failure/scalac_jvm_opts/BUILD
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
load("//scala:scala_toolchain.bzl", "scala_toolchain")
load("//scala:scala.bzl", "scala_library")
load(
"//scala_proto:scala_proto.bzl",
"scalapb_proto_library",
)

scala_toolchain(
name = "failing_toolchain_impl",
Expand All @@ -8,36 +12,14 @@ scala_toolchain(
visibility = ["//visibility:public"],
)

scala_toolchain(
name = "passing_toolchain_impl",
# This will pass because 1G is enough
scalac_jvm_flags = ["-Xmx1G"],
visibility = ["//visibility:public"],
)

toolchain(
name = "failing_scala_toolchain",
toolchain = "failing_toolchain_impl",
toolchain_type = "@io_bazel_rules_scala//scala:toolchain_type",
visibility = ["//visibility:public"],
)

toolchain(
name = "passing_scala_toolchain",
toolchain = "passing_toolchain_impl",
toolchain_type = "@io_bazel_rules_scala//scala:toolchain_type",
visibility = ["//visibility:public"],
)

scala_library(
name = "empty_build",
srcs = ["Empty.scala"],
)

scala_library(
name = "empty_overriding_build",
srcs = ["Empty.scala"],
# This overrides the option passed in on the toolchain, and should BUILD, even if
# the `failing_scala_toolchain` is used.
scalac_jvm_flags = ["-Xmx1G"],
)
)
13 changes: 9 additions & 4 deletions test_rules_scala.sh
Original file line number Diff line number Diff line change
Expand Up @@ -830,23 +830,27 @@ test_scalac_jvm_flags_from_scala_toolchain_fails() {
}

test_scalac_jvm_flags_from_scala_toolchain_passes() {
bazel build --extra_toolchains="//test_expect_failure/scalac_jvm_opts:passing_scala_toolchain" //test_expect_failure/scalac_jvm_opts:empty_build
bazel build --extra_toolchains="//manual_test/scalac_jvm_opts:passing_scala_toolchain" //manual_test/scalac_jvm_opts:empty_build
}

test_scalac_jvm_flags_on_target_overrides_toolchain_passes() {
bazel build --extra_toolchains="//test_expect_failure/scalac_jvm_opts:failing_scala_toolchain" //test_expect_failure/scalac_jvm_opts:empty_overriding_build
bazel build --extra_toolchains="//manual_test/scalac_jvm_opts:failing_scala_toolchain" //manual_test/scalac_jvm_opts:empty_overriding_build
}

test_scalac_jvm_flags_work_with_scalapb() {
bazel build --extra_toolchains="//manual_test/scalac_jvm_opts:passing_scala_toolchain" //manual_test/scalac_jvm_opts:proto
}

test_scala_test_jvm_flags_from_scala_toolchain_fails() {
action_should_fail test --extra_toolchains="//test_expect_failure/scala_test_jvm_flags:failing_scala_toolchain" //test_expect_failure/scala_test_jvm_flags:empty_test
}

test_scala_test_jvm_flags_from_scala_toolchain_passes() {
bazel test --extra_toolchains="//test_expect_failure/scala_test_jvm_flags:passing_scala_toolchain" //test_expect_failure/scala_test_jvm_flags:empty_test
bazel test --extra_toolchains="//manual_test/scala_test_jvm_flags:passing_scala_toolchain" //manual_test/scala_test_jvm_flags:empty_test
}

test_scala_test_jvm_flags_on_target_overrides_toolchain_passes() {
bazel test --extra_toolchains="//test_expect_failure/scala_test_jvm_flags:failing_scala_toolchain" //test_expect_failure/scala_test_jvm_flags:empty_overriding_test
bazel test --extra_toolchains="//manual_test/scala_test_jvm_flags:failing_scala_toolchain" //manual_test/scala_test_jvm_flags:empty_overriding_test
}

test_unused_dependency_checker_mode_set_in_rule() {
Expand Down Expand Up @@ -1131,6 +1135,7 @@ $runner scala_pb_library_targets_do_not_have_host_deps
$runner test_scalac_jvm_flags_on_target_overrides_toolchain_passes
$runner test_scalac_jvm_flags_from_scala_toolchain_passes
$runner test_scalac_jvm_flags_from_scala_toolchain_fails
$runner test_scalac_jvm_flags_work_with_scalapb
$runner test_scala_test_jvm_flags_on_target_overrides_toolchain_passes
$runner test_scala_test_jvm_flags_from_scala_toolchain_passes
$runner test_scala_test_jvm_flags_from_scala_toolchain_fails