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.

Sign up for GitHub

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

fix(bazel): generate java assembly for type-only #169

Merged
merged 3 commits into from
Aug 4, 2023
Merged

fix(bazel): generate java assembly for type-only #169

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
9b30a09
fix(bazel): generate java assembly for type-only
noahdietz Aug 4, 2023
43b84ef
remove java grpc import
noahdietz Aug 4, 2023
4200365
Merge branch 'main' into java-types-assembly
noahdietz Aug 4, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 7 additions & 5 deletions bazel/src/main/java/com/google/api/codegen/bazel/BUILD.bazel.raw_api.mustache
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ proto_library(
##############################################################################
load(
"@com_google_googleapis_imports//:imports.bzl",
"java_grpc_library",
"java_proto_library",
)

Expand All @@ -32,10 +31,13 @@ java_proto_library(
deps = [":{{name}}_proto"],
)

java_grpc_library(
name = "{{name}}_java_grpc",
srcs = [":{{name}}_proto"],
deps = [":{{name}}_java_proto"],
# Open Source Packages
java_gapic_assembly_gradle_pkg(
name = "{{type_only_assembly_name}}-java",
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is consistent with the rest of the file so I think it's good in the context of the PR.
However, do we want to keep the it consistent with the names of regular packages? I don't see anything wrong with google-cloud-alloydb-connectors-v1alpha-java for types only packages. It may not matter for Bazel bot but the benefit I see is that

  1. Developers do not have to be aware of the differences between type-only and a regular package.
  2. It's a more predictable name when we want to test it locally.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Type only packages don't always have a version in them, sometimes they are just google/shopping/type. In this case it would like google-shopping-type-java which isn't bad either.

Why don't we just use this name type_only_assembly_name variable here, and I will change the code in a separate PR to update the format of its value. It will impact other languages as well so best to keep in a separate PR.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, agree that it should be a different PR even if we want to change it.

deps = [
":{{name}}_proto",
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's probably my own lack of understanding, why do we need this for Java but not for other languages?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No idea tbh. I just saw it in the same target for GAPIC libraries and copied.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps because the protos are included in the generated code under the proto- directory?

":{{name}}_java_proto",
],
)

##############################################################################
Expand Down
12 changes: 7 additions & 5 deletions bazel/src/test/data/googleapis/google/example/library/type/BUILD.bazel.baseline
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@ proto_library(
##############################################################################
load(
"@com_google_googleapis_imports//:imports.bzl",
"java_grpc_library",
"java_proto_library",
)

Expand All @@ -34,10 +33,13 @@ java_proto_library(
deps = [":types_proto"],
)

java_grpc_library(
name = "types_java_grpc",
srcs = [":types_proto"],
deps = [":types_java_proto"],
# Open Source Packages
java_gapic_assembly_gradle_pkg(
name = "library-types-java",
deps = [
":types_proto",
":types_java_proto",
],
)

##############################################################################
Expand Down