Update to handle rename of java_names.h to names.h in protobuf upstream

[jvolkman]

Protobuf 3.21 renamed google/protobuf/compiler/java/java_names.h to google/protobuf/compiler/java/names.h

Fixes #9220

[linux-foundation-easycla]

The committers listed above are authorized under a signed CLA.

• ✅ login: jvolkman / name: Jeremy Volkman (a839fbd)

[sanjaypujare]

This change seems dependent on protobuf version 3.21 - but protobufVersion in ./build.gradle (line 61) still says 3.19.2. Shouldn't that be changed as well along with required amount of testing? cc @ejona86

[jvolkman]

Happy to update the gradle file in this PR, but I'm not sure I'm the right person to perform the required amount of testing (unless it's automated).

[YifeiZhuang]

> Task :grpc-all:checkUpperBoundDeps FAILED

> Task :grpc-context:compileJava
FAILURE: Build failed with an exception.

* Where:
3 actionable tasks: 3 executed
Build file '/home/runner/work/grpc-java/grpc-java/build.gradle' line: 569

* What went wrong:
Execution failed for task ':grpc-all:checkUpperBoundDeps'.
> Maven version skew: com.google.errorprone:error_prone_annotations (2.10.0 != 2.11.0) Bad version dependency path: [project ':grpc-all', io.grpc:grpc-api:1.48.0-SNAPSHOT] Run './gradlew :grpc-all:dependencies --configuration runtimeClasspath' to diagnose

[jvolkman]

On second look it seems this is quite a more involved change than I anticipated since version 3.19.2 is referenced in a number of places.

➜  grpc-java git:(jvolkman/fix-java-names-h) grep -HR 3\.19\.2 *
buildscripts/make_dependencies.sh:PROTOBUF_VERSION=3.19.2
buildscripts/make_dependencies.bat:set PROTOBUF_VER=3.19.2
COMPILING.md:The codegen plugin is C++ code and requires protobuf 3.19.2 or later.
COMPILING.md:$ PROTOBUF_VERSION=3.19.2
examples/example-xds/build.gradle:def protocVersion = '3.19.2'
examples/build.gradle:def protobufVersion = '3.19.2'
examples/example-hostname/build.gradle:def protobufVersion = '3.19.2'
examples/example-hostname/pom.xml:    <protoc.version>3.19.2</protoc.version>
examples/android/clientcache/app/build.gradle:    protoc { artifact = 'com.google.protobuf:protoc:3.19.2' }
examples/android/helloworld/app/build.gradle:    protoc { artifact = 'com.google.protobuf:protoc:3.19.2' }
examples/android/strictmode/app/build.gradle:    protoc { artifact = 'com.google.protobuf:protoc:3.19.2' }
examples/android/routeguide/app/build.gradle:    protoc { artifact = 'com.google.protobuf:protoc:3.19.2' }
examples/pom.xml:    <protobuf.version>3.19.2</protobuf.version>
examples/pom.xml:    <protoc.version>3.19.2</protoc.version>
examples/example-tls/pom.xml:    <protoc.version>3.19.2</protoc.version>
examples/example-tls/build.gradle:def protocVersion = '3.19.2'
examples/example-jwt-auth/pom.xml:    <protobuf.version>3.19.2</protobuf.version>
examples/example-jwt-auth/pom.xml:    <protoc.version>3.19.2</protoc.version>
examples/example-jwt-auth/build.gradle:def protobufVersion = '3.19.2'
examples/example-orca/build.gradle:def protocVersion = '3.19.2'
examples/example-alts/build.gradle:def protocVersion = '3.19.2'
examples/example-gauth/build.gradle:def protobufVersion = '3.19.2'
examples/example-gauth/pom.xml:    <protobuf.version>3.19.2</protobuf.version>
README.md:        <protocArtifact>com.google.protobuf:protoc:3.19.2:exe:${os.detected.classifier}</protocArtifact>
README.md:    artifact = "com.google.protobuf:protoc:3.19.2"
README.md:    artifact = "com.google.protobuf:protoc:3.19.2"
repositories.bzl:        strip_prefix = "protobuf-3.19.2",
repositories.bzl:        urls = ["https://github.com/protocolbuffers/protobuf/archive/v3.19.2.zip"],
repositories.bzl:        strip_prefix = "protobuf-3.19.2",
repositories.bzl:        urls = ["https://github.com/protocolbuffers/protobuf/archive/v3.19.2.zip"],

[ejona86]

Upgrading proto is indeed a bit more involved. 80c3be0 is an example of upgrading it.

@jvolkman how did you notice this? It looks like you may be using Bazel? It looks like this requires a lock-step upgrade, which is bad form. It is probably worthwhile to file a bug on Protobuf to request a forwarding header file (that maybe includes a warning) that could be removed after a release or two releases. I heard C core was also impacted by the renames and I think protobuf went back and properly added back the old header names which just #include the new names. @jvolkman, if you are using Bazel, then feel free to make a protobuf issue and CC me.

But I think we can solve this a bit more simply. Since this is a java-specific header, we can still reference PROTOBUF_VERSION. #if PROTOBUF_VERSION >= 3021000, then we do the new include, otherwise the old include. We'll need to add a protobuf import before the protobuf java import.

[jvolkman]

@ejona86 we are indeed using Bazel, and I happened to push an update with a switch based on GOOGLE_PROTOBUF_VERSION just as you commented.

[ejona86]

Switch java_names.h/names.h based on GOOGLE_PROTOBUF_VERSION

And the commit shows up just when I make my comment, although apparently there is a GOOGLE_PROTOBUF_VERSION in addition to PROTOBUF_VERSION.

[ejona86]

I really am not too sure what is in /stubs/, but this seems fine.

[jvolkman]

I really am not too sure what is in /stubs/, but this seems fine.

Yeah, I agree. But the top comment in stubs/common.h says

// Contains basic types and utilities used by the rest of the library.

so it seemed appropriate.

[YifeiZhuang]

But I think we can solve this a bit more simply. Since this is a java-specific header, we can still reference PROTOBUF_VERSION. #if PROTOBUF_VERSION >= 3021000, then we do the new include, otherwise the old include. We'll need to add a protobuf import before the protobuf java import.

The solution looks great(hacky)! I was thinking a proper way (if in gradle) is to resolve the dependency version, isn't it?

[ejona86]

I was thinking a proper way (if in gradle) is to resolve the dependency version, isn't it?

@YifeiZhuang, well, the user here isn't using the specific dependency versions that we are defining. They are using Bazel and choosing a different Protobuf version. So Gradle isn't used at all.

[YifeiZhuang]

I was thinking a proper way (if in gradle) is to resolve the dependency version, isn't it?

@YifeiZhuang, well, the user here isn't using the specific dependency versions that we are defining. They are using Bazel and choosing a different Protobuf version. So Gradle isn't used at all.

no i guess my question was why we can not specify a version around here:

grpc-java/compiler/BUILD.bazel

Line 14 in d4fa0ec

"@com_google_protobuf//:protoc_lib",

so that the customer can perfectly use a newer version but the :grpc-compiler would use an old version. Not something so important to know tho.

[ejona86]

@YifeiZhuang, we specify a version in repositories.bzl:

grpc-java/repositories.bzl

Line 114 in d4fa0ec

urls = ["https://github.com/protocolbuffers/protobuf/archive/v3.12.0.zip"],

However, that is only a recommendation. The user is free to define "com_google_protobuf" themselves and then our version won't be used:

grpc-java/repositories.bzl

Lines 86 to 87 in d4fa0ec

	if not native.existing_rule("com_google_protobuf"):
	com_google_protobuf()

Bazel dependencies are shared by the entire workspace, so we can't use a different version from the rest of the user's application. That's okay. We normally allow users to upgrade dependencies; it's just less frequently done for the protoc plugin.

[ejona86]

@jvolkman, thank you!

[suztomo]

Any plan for release gRPC with this fix soon? GAX-java repository is facing the compilation error. (No hard deadline but we're thinking whether to revert Protobuf versions in some artifacts, or wait for gRPC release)

CC: @Neenu1995

[ejona86]

@suztomo, the fix here was included in 1.47.0.

[suztomo]

Thank you. Indeed I see v1.47.0's https://github.com/grpc/grpc-java/blob/v1.47.0/compiler/src/java_plugin/cpp/java_generator.cpp#L32 has the fix in this pull request.