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

feat: dynamically determine protobuf version in build.gradle #1753

Merged
merged 15 commits into from
Jun 8, 2023
Merged

feat: dynamically determine protobuf version in build.gradle #1753

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
5aefe55
feat: dynamically determine protobuf version in build.gradle
JoeWang1127 Jun 6, 2023
3176c3c
add code comment
JoeWang1127 Jun 7, 2023
04c513c
merge main
JoeWang1127 Jun 7, 2023
d297e27
add a method to sync versions from googleapis
JoeWang1127 Jun 7, 2023
09c9895
add a seperate dict for synced version
JoeWang1127 Jun 7, 2023
982d818
restore protobuf version in dependencies.properties
JoeWang1127 Jun 7, 2023
1370b23
add synced version after reading properties
JoeWang1127 Jun 7, 2023
d123a20
combine two properties
JoeWang1127 Jun 7, 2023
22d3d0c
remove protobuf version in dependencies.properties
JoeWang1127 Jun 7, 2023
7b2b7f2
empty commit
JoeWang1127 Jun 7, 2023
e265b65
Revert "remove protobuf version in dependencies.properties"
JoeWang1127 Jun 7, 2023
64cfb1b
add comment for protobuf version
JoeWang1127 Jun 7, 2023
530ea55
make a copy of properties
JoeWang1127 Jun 8, 2023
3eaa5aa
change comment
JoeWang1127 Jun 8, 2023
0ba592e
change comments
JoeWang1127 Jun 8, 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
2 changes: 2 additions & 0 deletions gax-java/dependencies.properties
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@ version.gax_httpjson=0.114.1-SNAPSHOT
# Versions for dependencies which actual artifacts differ between Bazel and Gradle.
# Gradle build depends on prebuilt maven artifacts, while Bazel build depends on Bazel workspaces
# with the sources.
# The protobuf version is only used for generating gradle files for showcase module,
# not for self-service clients (from googleapis project).
version.com_google_protobuf=3.23.2
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is this used by showcase? I think the protobuf runtime version in Showcase module extends from gapic-generator-java-parent-pom

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Correct me if I'm wrong.

Looking at the ci error log, showcase golden tests eventually invoke bazelisk run //showcase:verify_gapic.

bazelisk run //showcase:verify_gapic will fail if version.com_google_protobuf=3.23.2 is removed from dependencies.properties.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I see, I didn't notice that we are making sure showcase modules are compilable right after generation, they are still verified with the generated gradle files though.

version.google_java_format=1.15.0
version.io_grpc=1.54.0
Expand Down
10 changes: 8 additions & 2 deletions rules_java_gapic/java_gapic_pkg.bzl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,14 +13,20 @@
# limitations under the License.

load("@com_google_api_gax_java_properties//:dependencies.properties.bzl", "PROPERTIES")
load("@com_google_protobuf//:protobuf_version.bzl", "PROTOBUF_JAVA_VERSION")

def _wrapPropertyNamesInBraces(properties):
JoeWang1127 marked this conversation as resolved.
Show resolved Hide resolved
wrappedProperties = {}
for k, v in properties.items():
wrappedProperties["{{%s}}" % k] = v
return wrappedProperties

_PROPERTIES = _wrapPropertyNamesInBraces(PROPERTIES)
# Before this replacement, there was a problem (e.g., b/284292352) when
# the version of protobuf defined in googleapis is higher than protobuf
# defined in gax-java/dependencies.properties, use this replacement to
# sync the two versions.
SYNCED_PROPERTIES = PROPERTIES | {"version.com_google_protobuf": PROTOBUF_JAVA_VERSION}
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@suztomo is this the idea of "making a copy"?

Copy link
Member

@suztomo suztomo Jun 8, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

This is better than what I was thinking with "dict.copy()".

_PROPERTIES = _wrapPropertyNamesInBraces(SYNCED_PROPERTIES)

# ========================================================================
# General packaging helpers.
Expand Down Expand Up @@ -63,7 +69,7 @@ def _gapic_pkg_tar_impl(ctx):
for f in dep.files.to_list():
deps.append(f)

samples =[]
samples = []
for s in ctx.attr.samples:
for f in s.files.to_list():
samples.append(f)
Expand Down