Fix a regression that breaks scalapb when jvm_flags are set on the to…

…olchain (#806)

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

* Move passing manual tests to new directory

manual_test/README.md

@@ -0,0 +1 @@
+This directory contains tests that require extra setup such as extra bazel flags.

manual_test/scala_test_jvm_flags/BUILD

@@ -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"],
+)

manual_test/scala_test_jvm_flags/EmptyTest.scala

@@ -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)
+    }
+}

manual_test/scalac_jvm_opts/BUILD

@@ -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"],
+)

manual_test/scalac_jvm_opts/Empty.scala

@@ -0,0 +1,3 @@
+package test_expect_failure.scalac_jvm_opts
+
+class Empty

manual_test/scalac_jvm_opts/test.proto

@@ -0,0 +1,8 @@
+syntax = "proto2";
+
+option java_package = "test.proto";
+
+message TestResponse1 {
+  optional uint32 c = 1;
+  optional bool d = 2;
+}

scala/private/rule_impls.bzl

@@ -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])

test_expect_failure/scala_test_jvm_flags/BUILD

@@ -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"],
-)
+)

test_expect_failure/scalac_jvm_opts/BUILD

@@ -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",
@@ -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"],
-)
+)

test_rules_scala.sh

@@ -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() {
@@ -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