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

The docs target from java_export fails with Protobuf dependencies #1048

Open
c16a opened this issue Jan 24, 2024 · 7 comments
Open

The docs target from java_export fails with Protobuf dependencies #1048

c16a opened this issue Jan 24, 2024 · 7 comments

Comments

@c16a
Copy link

c16a commented Jan 24, 2024

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 #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 c16a changed the title java_export's `docs" target fails with Protobuf dependencies java_export's docs target fails with Protobuf dependencies Jan 24, 2024
@c16a c16a changed the title java_export's docs target fails with Protobuf dependencies The docs target from java_export fails with Protobuf dependencies Jan 24, 2024
@shs96c
Copy link
Collaborator

shs96c commented Jan 24, 2024

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 shs96c closed this as completed Jan 24, 2024
@c16a
Copy link
Author

c16a commented Jan 24, 2024

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

@shs96c
Copy link
Collaborator

shs96c commented Jan 24, 2024

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

@c16a
Copy link
Author

c16a commented Jan 24, 2024

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

c16a commented Jan 26, 2024

@shs96c can we please reopen this?

@shs96c shs96c reopened this Jan 26, 2024
@l46kok
Copy link

l46kok commented Apr 20, 2024

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

@l46kok
Copy link

l46kok commented Apr 22, 2024

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.

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

No branches or pull requests

3 participants