rules_jvm_external icon indicating copy to clipboard operation
rules_jvm_external copied to clipboard

The `docs` target from `java_export` fails with Protobuf dependencies

Open c16a opened this issue 1 year ago • 8 comments

When a java_library, and java_export target both have protobuf dependencies, only the java_library target successfully builds. The java_export target throws the below error for every conceivable class in com.google.protobuf

package com.google.protobuf does not exist

I'm guessing this was earlier reported in https://github.com/bazelbuild/rules_jvm_external/issues/462

Bazel version

Build label: 7.0.0
Build target: @@//src/main/java/com/google/devtools/build/lib/bazel:BazelServer
Build time: Mon Dec 11 16:52:42 2023 (1702313562)
Build timestamp: 1702313562
Build timestamp as int: 1702313562

.bazelrc

build --enable_bzlmod=true
query --enable_bzlmod=true
build --java_language_version=21
build --java_runtime_version=21
build --tool_java_runtime_version=21
build --cxxopt=-std=c++17 --host_cxxopt=-std=c++17
build --javacopt="-XepDisableAllChecks"

MODULE.bazel

module(name = "sample")

bazel_dep(
    name = "rules_proto",
    version = "6.0.0-rc1",
)
bazel_dep(
    name = "rules_jvm_external",
    version = "6.0",
)
bazel_dep(
    name = "rules_java",
    version = "7.3.2",
)
bazel_dep(
    name = "protobuf", 
    version = "23.1"
)

Last working configuration

This issue does not occur with the below configuration (same .bazelrc as before)

Bazel version

Build label: 6.4.0
Build target: bazel-out/darwin_arm64-opt/bin/src/main/java/com/google/devtools/build/lib/bazel/BazelServer_deploy.jar
Build time: Thu Oct 19 17:08:20 2023 (1697735300)
Build timestamp: 1697735300
Build timestamp as int: 1697735300

MODULE.bazel

module(name = "sample")

bazel_dep(
    name = "rules_proto",
    version = "6.0.0-rc1",
)
bazel_dep(
    name = "rules_jvm_external",
    version = "5.3",
)
bazel_dep(
    name = "rules_java",
    version = "7.2.0",
)
bazel_dep(
    name = "protobuf", 
    version = "23.1"
)

Minimal reproduction

A minimal reproduction can be found at https://github.com/c16a/rules_jvm_external_javadoc_protobuf. The main branch reproduces the issue, but the working branch compiles both the java_library and java_export targets as expected.

c16a avatar Jan 24 '24 02:01 c16a

You need to add a dependency on the protobuf jar to your java_export.

The code compiles because java_proto_library uses a version of protobuf pulled from rules_proto which compiles the generated source in an aspect, but it doesn't declare the version of protobuf as a dependency on the generated target. This meant that we used to generate pom.xml files that didn't list the protobuf dependency, which lead to problems for projects that consumed the output of the java_export target.

The javadoc fails to compile because the symbols from protobuf are needed, and they're not being found. Adding the dep on to your java_export target is one way to fix this. The other fix is to use a macro that adds the protobuf dep to your java_proto_library targets automatically.

shs96c avatar Jan 24 '24 09:01 shs96c

@shs96c No luck, I've tried adding this. I've update the reproduction repo as well. Stil fails. Am I adding the correct dependencies?

load("@com_google_protobuf//:protobuf_deps.bzl", "PROTOBUF_MAVEN_ARTIFACTS")
load("@rules_jvm_external//:defs.bzl", "java_export", "artifact")

java_export(
    name = "export",
    srcs = [
        "Main.java"
    ],
    javacopts = ["--release 11"],
    deps = [
        ":java_proto",
        "@com_google_protobuf//:protobuf_java",
        "@com_google_protobuf//:protobuf_java_util",
    ] + [artifact(x) for x in PROTOBUF_MAVEN_ARTIFACTS],
    maven_coordinates = "org:sample:1.0.0"
)

c16a avatar Jan 24 '24 14:01 c16a

What happens when you add the @maven//com_google_protobuf_protobuf_java dependency?

shs96c avatar Jan 24 '24 15:01 shs96c

Sadly, still fails.

load("@rules_jvm_external//:defs.bzl", "java_export", "artifact")
load("@com_google_protobuf//:protobuf_deps.bzl", "PROTOBUF_MAVEN_ARTIFACTS")

java_export(
    name = "export",
    srcs = [
        "Main.java"
    ],
    javacopts = ["--release 11"],
    deps = [
        ":java_proto",
        "@maven//:com_google_protobuf_protobuf_java",
        "@com_google_protobuf//:protobuf_java",
        "@com_google_protobuf//:protobuf_java_util",
    ] + [artifact(x) for x in PROTOBUF_MAVEN_ARTIFACTS],
    maven_coordinates = "org:sample:1.0.0"
)

c16a avatar Jan 24 '24 15:01 c16a

@shs96c can we please reopen this?

c16a avatar Jan 26 '24 15:01 c16a

We also encountered this same issue while trying to migrate from WORKSPACE to bzlmod: https://github.com/google/cel-java/pull/326. This specifically breaks in 6.0 but works with 5.3 pinned.

l46kok avatar Apr 20 '24 03:04 l46kok

Looking into this further, I think the issue is specific to 6.0 with bzlmod. This works as expected with WORKSPACE based repository.

It looks like MVS is making maven.install ignore dependencies loaded from maven.install when an alternative dependency is available from bazel_dep. The problem in this scenario is that the alternative dep wouldn't have maven coordinates set, thus it wouldn't make it to the generated pom.xml. This appears to be the case even when the maven target is set directly as deps to java_export.

For example, with the following bzlmod setup (only the relevant portions are shown):

bazel_dep(name = "googleapis", repo_name="com_google_googleapis", version = "0.0.0-20240326-1c8d509c5")
...

maven.install(
    artifacts = [
        "com.google.protobuf:protobuf-java-util:3.24.4",
        "com.google.protobuf:protobuf-java:3.24.4",
        "com.google.guava:guava-testlib:33.0.0-jre",
        "com.google.guava:guava:33.0.0-jre",
        "com.google.code.findbugs:annotations:3.0.1",
        ...
    ]
)

Printing all loaded dependencies shows that only the protobuf is not loaded through rules_jvm_external (thus, its maven coordinates are missing from its tags).

<merged target @@protobuf~//java/core:core>
<merged target @@protobuf~//java/core:lite_runtime_only>
<merged target @@rules_jvm_external~~maven~maven//:com_google_code_findbugs_jsr305>
<merged target @@rules_jvm_external~~maven~maven//:com_google_errorprone_error_prone_annotations>
<merged target @@rules_jvm_external~~maven~maven//:com_google_guava_failureaccess>
<merged target @@rules_jvm_external~~maven~maven//:com_google_guava_listenablefuture>
<merged target @@rules_jvm_external~~maven~maven//:com_google_j2objc_j2objc_annotations>
...

Removing directive bazel_dep(name = "googleapis", repo_name="com_google_googleapis", version = "0.0.0-20240326-1c8d509c5") makes the protobuf dependency load from rules_jvm_external as expected.

l46kok avatar Apr 22 '24 16:04 l46kok