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

javacopts order differs from java_library #1550

Closed
patrick-premont opened this issue Mar 6, 2024 · 3 comments · Fixed by #1613
Closed

javacopts order differs from java_library #1550

patrick-premont opened this issue Mar 6, 2024 · 3 comments · Fixed by #1613

Comments

@patrick-premont
Copy link

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
Copy link
Contributor

thomasbao12 commented Sep 8, 2024

@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
Copy link
Contributor

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
Copy link
Contributor

I created a fix #1613

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants