grpc-java icon indicating copy to clipboard operation
grpc-java copied to clipboard

Include com_google_protobuf_javalite to MODULE.bazel to fix bzlmod querying graph in end-user repo

Open firov opened this issue 2 months ago • 4 comments

Query in end-user repository fails with:

bazel query --notool_deps --noimplicit_deps "deps(//...)" --output graph

ERROR: Evaluation of query failed: preloading transitive closure failed: no such package '@@[unknown repo 'com_google_protobuf_javalite' requested from @https://github.com/grpc-java~]//': The repository '@@[unknown repo 'com_google_protobuf_javalite' requested from @https://github.com/grpc-java~]' could not be resolved: No repository visible as '@com_google_protobuf_javalite' from repository '@https://github.com/grpc-java~'

It's because com_google_protobuf_javalite is missing in use_repo in MODULE.bazel and completely not imported in repositories.bzl when using bzlmod. I noticed that actually com_google_protobuf_javalite is an archive with protobuf repository, which we already have, so i just change remapping.

firov avatar May 03 '24 10:05 firov

com_google_protobuf_javalite was load-bearing in previous versions of Bazel, as java_lite_proto_library used it. I can totally believe it isn't needed in newer versions, but we'd need to make sure it isn't needed by versions of Bazel we still support.

The easier approach is to just define com_google_protobuf_javalite as a duplicate of com_google_protobuf_java, which is what we've been doing for a while. (In the olden days, protobuf had a separate branch for lite, but it was later integrated into the master branch.)

ejona86 avatar May 06 '24 17:05 ejona86

The easier approach is to just define com_google_protobuf_javalite as a duplicate of com_google_protobuf_java, which is what we've been doing for a while.

It won't work in bzlmod because of module strict visibility, it must be accessible from grpc-java itself. So currently we just patch it on module import.

but we'd need to make sure it isn't needed by versions of Bazel we still support.

I don't remove it completely, I thought this remapping will do the trick https://github.com/grpc/grpc-java/pull/11147/files#diff-c8bacb964b6cf475b85b8fa254a2769dcf34f28b813437048f877aa3ffa3d71bL65-R65 But i can imaging situation in workspace variant when you reference it directly and don't import by yourself. But I think it's ok to just replace in end user code when referencing to protobuf as it is the same library.

Currently in bzlmod perspective trunk is not correct as it has a reference to repository com_google_protobuf_javalite and does not define repository with that name. So other possible fix is remove if not bzlmod and not for com_google_protobuf_javalite and add it to use_repo macros in MODULE.bazel.

firov avatar May 06 '24 18:05 firov

It won't work in bzlmod because of module strict visibility, it must be accessible from grpc-java itself.

I was meaning to change grpc-java to add the repo. (So changing not bzlmod and add an add_repo sounds fine.)

I don't remove it completely, I thought this remapping will do the trick

The problem is we need to make sure to use the same library as java_lite_proto_library. So it is really a question of java_lite_proto_library's behavior and then we match it (with tweaks for migration when it changes).

ejona86 avatar May 06 '24 18:05 ejona86

I was meaning to change grpc-java to add the repo. (So changing not bzlmod and add an add_repo sounds fine.)

Okay, let's go this way.

The problem is we need to make sure to use the same library as java_lite_proto_library. So it is really a question of java_lite_proto_library's behavior and then we match it (with tweaks for migration when it changes).

Ah i got it now, if user redefines it in workspace, we need to use his version.

firov avatar May 07 '24 09:05 firov

Hi folks, just hit this issue. Can this be merged, or is there a different way in which it can be fixed on my end?

ar3s3ru avatar May 15 '24 11:05 ar3s3ru

Hi folks, just hit this issue. Can this be merged, or is there a different way in which it can be fixed on my end?

Hey, as i see we are waiting for second review. For now you can import this pr as a patch, if you are using version from central registry, something like this:

archive_override(
    module_name = "grpc-java",
    integrity = "sha256-MM/JVMIXRJOCJgGnS4doN5hsRE2bnpFwgplLXjQ0jzQ=",
    patch_strip = 1,
    patches = [
        "//:patches/grpc-java/add_module_bazel.patch",
        "//:patches/grpc-java/maven_lockfile.patch",
        "//:patches/grpc-java/update_labels.patch",
        "//:patches/grpc-java/javalite.patch"
    ],
    strip_prefix = "grpc-java-1.62.2",
    urls = [
        "https://github.com/grpc/grpc-java/archive/refs/tags/v1.62.2.tar.gz",
    ],
)

where first 3 patches you can take from registry https://github.com/bazelbuild/bazel-central-registry/tree/main/modules/grpc-java/1.62.2/patches And 4th is from pr

firov avatar May 15 '24 11:05 firov