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

java_package_configuration is ignored when rules_java is overridden #19815

Closed
davido opened this issue Oct 14, 2023 · 4 comments
Closed

java_package_configuration is ignored when rules_java is overridden #19815

davido opened this issue Oct 14, 2023 · 4 comments

Comments

@davido
Copy link
Contributor

davido commented Oct 14, 2023

Description of the bug:

Gerrit Code Review has custom java_package_configuration, with error-prone specific parameters:

tools/BUILD:

default_java_toolchain(
    name = "error_prone_warnings_toolchain_java17",
    configuration = dict(),
    java_runtime = "@rules_java_builtin//toolchains:remotejdk_17",
    package_configuration = [
        ":error_prone",
    ],
    source_version = "17",
    target_version = "17",
    visibility = ["//visibility:public"],
)

java_package_configuration(
    name = "error_prone",
    javacopts = [
        "-XepDisableWarningsInGeneratedCode",
        # The XepDisableWarningsInGeneratedCode disables only warnings, but
        # not errors. We should manually exclude all files generated by
        # AutoValue; such files always start AutoValue_..., $AutoValue_...,
        # $$AutoValue_... or AutoValueGson_...
        # XepExcludedPaths is a regexp. If you need more paths - use | as
        # separator. This issue is related to the number of escapes of $ char:
        # https://github.com/bazelbuild/bazel/issues/19647
        "-XepExcludedPaths:.*/\\\\$?\\\\$?AutoValue(Gson)?_.*\\.java",
        "-Xep:BanJNDI:WARN",
[...]
    ],
    packages = ["error_prone_packages"],
)

All is fine, when rules_java version shipped with Bazel is used.

However, if we are trying to override even the same rules_java version used on Bazel@HEAD 3.6.1,
java package configuration seems to be ignored:

WORKSPACE:

http_archive(
    name = "rules_java",
    urls = [
        "https://github.com/bazelbuild/rules_java/releases/download/6.3.1/rules_java-6.3.1.tar.gz",
    ],
    sha256 = "117a1227cdaf813a20a1bba78a9f2d8fb30841000c33e2f2d2a640bd224c9282",
)

load("@rules_java//java:repositories.bzl", "rules_java_dependencies", "rules_java_toolchains")
rules_java_dependencies()
rules_java_toolchains()

Then all the custom error-prone options are ignored, and we are seeing error-prone errors reported:

$ bazeldev build --config=java17 -s java/com/google/gerrit/auth
java/com/google/gerrit/auth/ldap/Helper.java:390: error: [BanJNDI] Using JNDI may deserialize user input via the `Serializable` API which is extremely dangerous
              ctx.getAttributes(compositeGroupName, schema.accountMemberFieldArray)
                               ^
    (see https://errorprone.info/bugpattern/BanJNDI)
Target //java/com/google/gerrit/auth:auth failed to build
Use --verbose_failures to see the command lines of failed build steps.
INFO: Elapsed time: 60.154s, Critical Path: 58.75s
INFO: 69 processes: 3 internal, 33 linux-sandbox, 33 worker.
ERROR: Build did NOT complete successfully

Even though the severity level for those bug patterns was demoted to warning, e.g.: -Xep:BanJNDI:WARN.

What are we trying to achieve?

We try to use JDK 21 added recently, in rules_java.
I've conducted a custom rules_java release and am trying to integrate it like this in gerrit:

WORKSPACE:

http_archive(
    name = "rules_java",
    urls = [
        "https://github.com/davido/rules_java/archive/refs/tags/6.5.2.tar.gz",
    ],
    strip_prefix = "rules_java-6.5.2",
    sha256 = "07a8785349bdaa63bf42654d0d6cde189a250b2ca59d120c3a96376c30bf6b8b",
)

load("@rules_java//java:repositories.bzl", "rules_java_dependencies", "rules_java_toolchains")
rules_java_dependencies()
rules_java_toolchains()

While I am able to build some basic targets using JDK 21 and produce major java byte version 65:

$> bazeldev build --java_language_version=21 \
--java_runtime_version=remotejdk_21 \
--tool_java_language_version=21 \
--tool_java_runtime_version=remotejdk_21 \
java/com/google/gerrit/entities

It's failing to build other targets with overridden rules_java version, because error-prone parameters are ignored:
bazeldev build java/com/google/gerrit/auth.

Which category does this issue belong to?

Java Rules

What's the simplest, easiest way to reproduce this bug? Please provide a minimal example if possible.

Fetch this CL form Gerrit Review: [1], and run:

$> bazelisk build java/com/google/gerrit/auth
[...]
java/com/google/gerrit/auth/ldap/Helper.java:390: error: [BanJNDI] Using JNDI may deserialize user input via the `Serializable` API which is extremely dangerous
              ctx.getAttributes(compositeGroupName, schema.accountMemberFieldArray)
                               ^
    (see https://errorprone.info/bugpattern/BanJNDI)
Target //java/com/google/gerrit/auth:auth failed to build
Use --verbose_failures to see the command lines of failed build steps.
INFO: Elapsed time: 19.403s, Critical Path: 10.64s
INFO: 142 processes: 34 disk cache hit, 12 internal, 66 linux-sandbox, 30 worker.
ERROR: Build did NOT complete successfully

[1] https://gerrit-review.googlesource.com/c/gerrit/+/387837

Which operating system are you running Bazel on?

Linux

What is the output of bazel info release?

Bazel@HEAD or 7.0.0-pre.20230926.1

If bazel info release returns development version or (@non-git), tell us how you built Bazel.

bazelisk build src:bazel-bin-dev

What's the output of git remote get-url origin; git rev-parse master; git rev-parse HEAD ?

No response

Is this a regression? If yes, please try to identify the Bazel commit where the bug was introduced.

It works as expected when rules_java shipped with Bazel is used.

Have you found anything relevant by searching the web?

No response

Any other information, logs, or outputs that you want to share?

No response

@hvadehra
Copy link
Member

  1. Order of registering toolchains matters. As currently written in your change, the rules_java toolchains take precedence over your custom toolchains, so the default jdk21 toolchain gets picked (one without your custom package_config). You need to move the rules_java_toolchains() after the register_toolchains("//tools:error_prone_warnings_toolchain_java21_definition")
  2. @rules_java_builtin is only for internal bazel usage and won't get resolved in your repo. Please use @rules_java//toolchains:remotejdk_21

@davido
Copy link
Contributor Author

davido commented Oct 17, 2023

@hvadehra Thanks for clarifying.

  1. Order of registering toolchains matters.

Changing the order solved the problem.

  1. @rules_java_builtin is only for internal bazel usage and won't get resolved in your repo. Please use @rules_java//toolchains:remotejdk_21

@rules_java//toolchains:remotejdk_17 and @rules_java//toolchains:remotejdk_21 can be referenced, only if I consume a custom version of rules_java in WORKSPACE file.

If we just use default rules_java on Bazel@7.0.0-pre.20230926.1 we still have to use @rules_java_builtin, right?

default_java_toolchain(
    name = "error_prone_warnings_toolchain_java17",
    configuration = dict(),
    java_runtime = "@rules_java_builtin//toolchains:remotejdk_17",
    package_configuration = [
        ":error_prone",
    ],
    source_version = "17",
    target_version = "17",
    visibility = ["//visibility:public"],
)

Reproducer: https://gerrit-review.googlesource.com/c/gerrit/+/387360

Before the upgrade of Bazel from 6.x to 7.x, we used: @bazel_tools//tools/jdk:remotejdk_17.

@hvadehra
Copy link
Member

Do I understand correctly that referencing @rules_java_builtin is working for you? I would have thought that repo name would not work.

If you wish to use the version of @rules_java embedded in Bazel, I believe you should continue to use @bazel_tools - this internally redirects to the embedded @rules_java.

@davido
Copy link
Contributor Author

davido commented Oct 17, 2023

Do I understand correctly that referencing @rules_java_builtin is working for you?

Yeah, it works. But you are right, I was able to revert to @bazel_tools and it worked.
I think we can close this issue now. Thanks again for your help.

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

No branches or pull requests

5 participants