From b196489f316ef90307a3824e9b6c16b20e20775b Mon Sep 17 00:00:00 2001 From: Diego Marquez Date: Fri, 30 Aug 2024 21:04:45 -0400 Subject: [PATCH] chore: generate poms with grpc dependencies as test scoped (#3072) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes https://github.com/googleapis/google-cloud-java/issues/10037 ☕ ### Overview This will modify the behavior of library generation at the postprocessing stage. The main change is that generated _cloud_ poms (i.e. `google-cloud-${service}/pom.xml`) will now have the dependencies `proto-google-common-protos` and `grpc-google-iam-v1` declared as `test` if the libraries belong to a monorepo. ### Integration test It will be failing until we update the test branch. Once this approach is validated, I will proceed with updating the branch. ### Approach #### For new libraries Via template in `owlbot/teplates/poms/cloud_pom.xml.j2`. New libraries will have these two dependencies declared as test scope if they are part of a monorepo. #### For existing libraries The script `fix_poms.py` will now modify the existing poms by setting the two dependencies as test-scoped, only if they are part of a monorepo. ### Confirmation that it works * java-bigquerystorage: no effects since these dependencies are [not found in the pom](https://github.com/search?q=repo%3Agoogleapis%2Fjava-bigquerystorage%20path%3Agoogle-cloud-bigquerystorage%2Fpom.xml%20grpc-google&type=code) * java-datastore: [no relevant changes](https://github.com/googleapis/java-datastore/pull/1530/files) since the dependencies are [not found in the pom](https://github.com/googleapis/java-datastore/blob/main/google-cloud-datastore/pom.xml) * java-firestore: [no relevant changes](https://github.com/googleapis/java-firestore/pull/1764) since the one targeted dependency is [already declared as test scoped](https://github.com/googleapis/java-firestore/blob/main/google-cloud-firestore/pom.xml#L160-L164) * java-logging: [no relevant changes](https://github.com/googleapis/java-logging/pull/1660)] since the dependencies are [not declared in the pom](https://github.com/googleapis/java-logging/blob/main/google-cloud-logging/pom.xml) * java-pubsub: no changes. Relevant dependency [already declared as test scoped](https://github.com/googleapis/java-pubsub/blob/2b15b0a3f43c9ccef663fedf0398375f58fd9183/google-cloud-pubsub/pom.xml#L135-L139) * java-pubsublite: [no relevant changes](https://github.com/googleapis/java-pubsublite/pull/1692) since there are [no relevant dependencies declared in the pom](https://github.com/googleapis/java-pubsublite/blob/main/google-cloud-pubsublite/pom.xml) * java-spanner: ignored - dependency kept as is * java-storage: ignored - kept as is * **google-cloud-java: [91 poms affected](https://github.com/googleapis/google-cloud-java/pull/11031/files) by converting dependencies to test-scoped (PR only contains pom changes for simplicity + outdated googleapis_committish)** * We had a special situation with `java-dataproc` because it references `google-iam-v1` via `grpc-google-iam-v1`. However this is incorrect because what this library really needs is the proto classes. The fix for this was to treat this library as any other library and add `proto-google-iam-v1` as a dependency. --- library_generation/owlbot/src/fix_poms.py | 47 ++++++++++++++++++- .../owlbot/templates/poms/cloud_pom.xml.j2 | 18 ++++++- .../java-admanager/ad-manager/pom-golden.xml | 2 + 3 files changed, 63 insertions(+), 4 deletions(-) diff --git a/library_generation/owlbot/src/fix_poms.py b/library_generation/owlbot/src/fix_poms.py index b88cd3d6bb2..9ee7514a4de 100644 --- a/library_generation/owlbot/src/fix_poms.py +++ b/library_generation/owlbot/src/fix_poms.py @@ -15,6 +15,8 @@ import sys import glob import json +from xml.etree.ElementTree import ElementTree + from lxml import etree import os import re @@ -92,7 +94,10 @@ def _is_cloud_client(existing_modules: List[module.Module]) -> bool: def update_cloud_pom( - filename: str, proto_modules: List[module.Module], grpc_modules: List[module.Module] + filename: str, + proto_modules: List[module.Module], + grpc_modules: List[module.Module], + is_monorepo: bool, ): tree = etree.parse(filename) root = tree.getroot() @@ -104,6 +109,9 @@ def update_cloud_pom( if m.find("{http://maven.apache.org/POM/4.0.0}artifactId") is not None ] + if is_monorepo: + _set_test_scoped_deps(dependencies) + try: grpc_index = _find_dependency_index( dependencies, "com.google.api.grpc", "grpc-" @@ -169,6 +177,39 @@ def update_cloud_pom( tree.write(filename, pretty_print=True, xml_declaration=True, encoding="utf-8") +def _set_test_scoped_deps(dependencies: list[ElementTree]) -> None: + """ + As of July 2024, we have two dependencies that should be declared as + test-scoped in a monorepo: grpc-google-common-protos and grpc-google-iam-v1. + HW libraries are treated as usual + :param dependencies: List of XML Objects representing a + """ + TEST_SCOPED_DEPENDENCIES = ["grpc-google-common-protos", "grpc-google-iam-v1"] + print( + 'converting dependencies "grpc-google-common-protos" and "grpc-google-iam-v1" to test-scoped' + ) + for d in dependencies: + artifact_query = "{http://maven.apache.org/POM/4.0.0}artifactId" + scope_query = "{http://maven.apache.org/POM/4.0.0}scope" + current_scope = d.find(scope_query) + artifact_id_elem = d.find(artifact_query) + if artifact_id_elem is None: + continue + artifact_id = artifact_id_elem.text + is_test_scoped = ( + current_scope.text == "test" if current_scope is not None else False + ) + if artifact_id in TEST_SCOPED_DEPENDENCIES and not is_test_scoped: + new_scope = etree.Element(scope_query) + new_scope.text = "test" + if current_scope is not None: + d.replace(current_scope, new_scope) + else: + d.append(new_scope) + new_scope.tail = "\n " + new_scope.getprevious().tail = "\n " + + def update_parent_pom(filename: str, modules: List[module.Module]): tree = etree.parse(filename) root = tree.getroot() @@ -492,7 +533,9 @@ def main(versions_file, monorepo): if os.path.isfile(f"{artifact_id}/pom.xml"): print("updating modules in cloud pom.xml") if artifact_id not in excluded_poms_list: - update_cloud_pom(f"{artifact_id}/pom.xml", proto_modules, grpc_modules) + update_cloud_pom( + f"{artifact_id}/pom.xml", proto_modules, grpc_modules, monorepo + ) elif artifact_id not in excluded_poms_list: print("creating missing cloud pom.xml") templates.render( diff --git a/library_generation/owlbot/templates/poms/cloud_pom.xml.j2 b/library_generation/owlbot/templates/poms/cloud_pom.xml.j2 index ed1f6fbfea8..d0ae8cd7c56 100644 --- a/library_generation/owlbot/templates/poms/cloud_pom.xml.j2 +++ b/library_generation/owlbot/templates/poms/cloud_pom.xml.j2 @@ -66,22 +66,36 @@ com.google.api.grpc - grpc-google-common-protos + proto-google-iam-v1 +{%- if not monorepo %} com.google.api.grpc - proto-google-iam-v1 + grpc-google-common-protos com.google.api.grpc grpc-google-iam-v1 +{%- endif %} org.threeten threetenbp +{%- if monorepo %} + + com.google.api.grpc + grpc-google-common-protos + test + + + com.google.api.grpc + grpc-google-iam-v1 + test + +{%- endif %} junit junit diff --git a/library_generation/test/resources/test-owlbot/java-admanager/ad-manager/pom-golden.xml b/library_generation/test/resources/test-owlbot/java-admanager/ad-manager/pom-golden.xml index 106b55c8c5e..4c8a8a8343c 100644 --- a/library_generation/test/resources/test-owlbot/java-admanager/ad-manager/pom-golden.xml +++ b/library_generation/test/resources/test-owlbot/java-admanager/ad-manager/pom-golden.xml @@ -64,6 +64,7 @@ com.google.api.grpc grpc-google-common-protos + test com.google.api.grpc @@ -72,6 +73,7 @@ com.google.api.grpc grpc-google-iam-v1 + test org.threeten