bazelbuild / rules_scala

Scala rules for Bazel
Apache License 2.0
363 stars 275 forks source link

javacopts order differs from java_library #1550

Closed patrick-premont closed 3 weeks ago

patrick-premont commented 7 months ago

The order of javacopts parameters passed to javac differs from the order seen in java_library.

This can result in different options taking precedence within the same toolchain based on whether it is used from scala_library or java_library.

In the example below --release and -source/-target are being reordered leading to a java_library with Java 8 classes and a scala_library with Java 21 classes.

BUILD.bazel

load("@io_bazel_rules_scala//scala:scala.bzl", "scala_library")
load("@bazel_tools//tools/jdk:default_java_toolchain.bzl", "default_java_toolchain")

default_java_toolchain(
    name = "custom_toolchain",
    package_configuration = [":custom_package_config"],
    source_version = "21",
    target_version = "21",
)

java_package_configuration(
    name = "custom_package_config",
    javacopts = ["--release 8"],
    packages = [":this_package"],
)

package_group(
    name = "this_package",
    packages = ["//"],
)

genrule(
    name = "generate_example_java",
    outs = ["Example.java"],
    cmd = "echo 'public class Example {}' > $@",
)

scala_library(
    name = "scala_library",
    srcs = ["Example.java"],
)

java_library(
    name = "java_library",
    srcs = ["Example.java"],
)

WORKSPACE.bazel

# WORKSPACE
load("@bazel_tools//tools/build_defs/repo:http.bzl", "http_archive")

register_toolchains("//:custom_toolchain_definition")

# The rest of this file is taken from the README.md of rules_scala, with version 6.4.0
http_archive(
    name = "bazel_skylib",
    sha256 = "b8a1527901774180afc798aeb28c4634bdccf19c4d98e7bdd1ce79d1fe9aaad7",
    urls = [
        "https://mirror.bazel.build/github.com/bazelbuild/bazel-skylib/releases/download/1.4.1/bazel-skylib-1.4.1.tar.gz",
        "https://github.com/bazelbuild/bazel-skylib/releases/download/1.4.1/bazel-skylib-1.4.1.tar.gz",
    ],
)

# See https://github.com/bazelbuild/rules_scala/releases for up to date version information.
http_archive(
    name = "io_bazel_rules_scala",
    sha256 = "9a23058a36183a556a9ba7229b4f204d3e68c8c6eb7b28260521016b38ef4e00",
    strip_prefix = "rules_scala-6.4.0",
    url = "https://github.com/bazelbuild/rules_scala/releases/download/v6.4.0/rules_scala-v6.4.0.tar.gz",
)

load("@io_bazel_rules_scala//:scala_config.bzl", "scala_config")

# Stores Scala version and other configuration
# 2.12 is a default version, other versions can be use by passing them explicitly:
# scala_config(scala_version = "2.11.12")
# Scala 3 requires extras...
#   3.2 should be supported on master. Please note that Scala artifacts for version (3.2.2) are not defined in
#   Rules Scala, they need to be provided by your WORKSPACE. You can use external loader like
#   https://github.com/bazelbuild/rules_jvm_external
scala_config()

load("@io_bazel_rules_scala//scala:scala.bzl", "rules_scala_setup", "rules_scala_toolchain_deps_repositories")

# loads other rules Rules Scala depends on
rules_scala_setup()

# Loads Maven deps like Scala compiler and standard libs. On production projects you should consider
# defining a custom deps toolchains to use your project libs instead
rules_scala_toolchain_deps_repositories(fetch_sources = True)

load("@rules_proto//proto:repositories.bzl", "rules_proto_dependencies", "rules_proto_toolchains")

rules_proto_dependencies()

rules_proto_toolchains()

load("@io_bazel_rules_scala//scala:toolchains.bzl", "scala_register_toolchains")

scala_register_toolchains()

# optional: setup ScalaTest toolchain and dependencies
load("@io_bazel_rules_scala//testing:scalatest.bzl", "scalatest_repositories", "scalatest_toolchain")

scalatest_repositories()

scalatest_toolchain()

Running

Then running

bazel build --java_language_version=21 --toolchain_resolution_debug=. //... 2>&1 | grep "jdk:toolchain_type ->" 

we can see that custom_toolchain is selected for both java_library and scala_library.

INFO: ToolchainResolution: Target platform @@local_config_platform//:host: Selected execution platform @@local_config_platform//:host, type @@bazel_tools//tools/jdk:toolchain_type -> toolchain //:custom_toolchain
INFO: ToolchainResolution: Target platform @@local_config_platform//:host: Selected execution platform @@local_config_platform//:host, type @@io_bazel_rules_scala//scala:toolchain_type -> toolchain @@io_bazel_rules_scala//scala:default_toolchain_impl, type @@bazel_tools//tools/jdk:toolchain_type -> toolchain //:custom_toolchain

And the class major versions in each jar are those of Java 21 (65) and Java 8(52):

> javap -verbose -classpath bazel-bin/scala_library_java.jar Example | grep major
  major version: 65
> javap -verbose -classpath bazel-bin/libjava_library.jar Example | grep major
  major version: 52

Then to see the contents of the params files we can add an option badoption to custom_package_config.

For java we can run

bazel build --java_language_version=21 --verbose_failures //:java_library
cat bazel-out/darwin_arm64-fastbuild/bin/libjava_library.jar-0.params

and we get this order:

--output
bazel-out/darwin_arm64-fastbuild/bin/libjava_library.jar
--native_header_output
bazel-out/darwin_arm64-fastbuild/bin/libjava_library-native-header.jar
--output_manifest_proto
bazel-out/darwin_arm64-fastbuild/bin/libjava_library.jar_manifest_proto
--compress_jar
--output_deps_proto
bazel-out/darwin_arm64-fastbuild/bin/libjava_library.jdeps
--bootclasspath
bazel-out/darwin_arm64-fastbuild/bin/external/rules_java~7.1.0/toolchains/platformclasspath.jar
--system
external/rules_java~7.1.0~toolchains~local_jdk
--sources
bazel-out/darwin_arm64-fastbuild/bin/Example.java
--javacopts
-source
21
-target
21
-XDskipDuplicateBridges=true
-XDcompilePolicy=simple
-g
-parameters
-Xep:ReturnValueIgnored:OFF
-Xep:IgnoredPureGetter:OFF
-Xep:EmptyTopLevelDeclaration:OFF
-Xep:LenientFormatStringValidation:OFF
-Xep:ReturnMissingNullable:OFF
-Xep:UseCorrectAssertInTests:OFF
--release
8
badoption
--
--target_label
//:java_library
--strict_java_deps
ERROR
--experimental_fix_deps_tool
add_dep

For scala we run

bazel build --java_language_version=21 --verbose_failures //:scala_library
cat bazel-out/darwin_arm64-fastbuild/bin/scala_library_java.jar-0.params

and get

--output
bazel-out/darwin_arm64-fastbuild/bin/scala_library_java.jar
--native_header_output
bazel-out/darwin_arm64-fastbuild/bin/scala_library_java-native-header.jar
--output_manifest_proto
bazel-out/darwin_arm64-fastbuild/bin/scala_library_java.jar_manifest_proto
--compress_jar
--output_deps_proto
bazel-out/darwin_arm64-fastbuild/bin/scala_library_java.jdeps
--bootclasspath
bazel-out/darwin_arm64-fastbuild/bin/external/rules_java~7.1.0/toolchains/platformclasspath.jar
--system
external/rules_java~7.1.0~toolchains~local_jdk
--sources
bazel-out/darwin_arm64-fastbuild/bin/Example.java
--javacopts
--release
8
badoption
-source
21
-target
21
-XDskipDuplicateBridges=true
-XDcompilePolicy=simple
-g
-parameters
-Xep:ReturnValueIgnored:OFF
-Xep:IgnoredPureGetter:OFF
-Xep:EmptyTopLevelDeclaration:OFF
-Xep:LenientFormatStringValidation:OFF
-Xep:ReturnMissingNullable:OFF
-Xep:UseCorrectAssertInTests:OFF
--
--target_label
//:scala_library
--strict_java_deps
ERROR
--direct_dependencies
bazel-out/darwin_arm64-fastbuild/bin/external/io_bazel_rules_scala_scala_library/io_bazel_rules_scala_scala_library.stamp/scala-library-2.12.18-stamped.jar
bazel-out/darwin_arm64-fastbuild/bin/external/io_bazel_rules_scala_scala_reflect/io_bazel_rules_scala_scala_reflect.stamp/scala-reflect-2.12.18-stamped.jar
bazel-out/darwin_arm64-fastbuild/bin/scala_library-ijar.jar
--experimental_fix_deps_tool
add_dep

We can see that --release 8 occurs before -source 21 -target 21 in the scala library only.

thomasbao12 commented 4 weeks ago

@patrick-premont Did you end up finding a resolution? I believe the issue is here:

https://sourcegraph.com/github.com/bazelbuild/rules_scala/-/blob/scala/private/rule_impls.bzl?L150

thomasbao12 commented 4 weeks ago

We fixed by building the io_rules_scala from source and patching like this:

diff --git a/scala/private/rule_impls.bzl b/scala/private/rule_impls.bzl
index fa73ebc..1c5779a 100644
--- a/scala/private/rule_impls.bzl
+++ b/scala/private/rule_impls.bzl
@@ -157,10 +157,9 @@ def compile_java(ctx, source_jars, source_files, output, extra_javac_opts, provi
         output = output,
         javac_opts = expand_location(
             ctx,
-            extra_javac_opts +
             java_common.default_javac_opts(
                 java_toolchain = java_toolchain,
-            ),
+            ) + extra_javac_opts,
         ),
         deps = providers_of_dependencies,
         #exports can be empty since the manually created provider exposes exports
thomasbao12 commented 4 weeks ago

I created a fix https://github.com/bazelbuild/rules_scala/pull/1613