From 8de1c01d5810e6fb2a713672ef0174364744f7cb Mon Sep 17 00:00:00 2001 From: diegomarquezp Date: Thu, 11 Jul 2024 13:26:19 +0000 Subject: [PATCH 01/63] chore: bake the grpc plugin in the hermetic library generation image --- .../library_generation.Dockerfile | 12 +++++- library_generation/generate_library.sh | 4 +- .../test/generate_library_unit_tests.sh | 40 +++++++++++++++++++ library_generation/utils/utilities.sh | 27 ++++++++++--- 4 files changed, 75 insertions(+), 8 deletions(-) diff --git a/.cloudbuild/library_generation/library_generation.Dockerfile b/.cloudbuild/library_generation/library_generation.Dockerfile index 2629c092aa..63385f136a 100644 --- a/.cloudbuild/library_generation/library_generation.Dockerfile +++ b/.cloudbuild/library_generation/library_generation.Dockerfile @@ -19,7 +19,9 @@ SHELL [ "/bin/bash", "-c" ] ARG OWLBOT_CLI_COMMITTISH=ac84fa5c423a0069bbce3d2d869c9730c8fdf550 ARG PROTOC_VERSION=25.3 +ARG GRPC_VERSION=1.62.2 ENV HOME=/home +ENV OS_ARCHITECTURE="linux-x86_64" # install OS tools RUN apt-get update && apt-get install -y \ @@ -32,11 +34,19 @@ COPY library_generation /src # install protoc WORKDIR /protoc RUN source /src/utils/utilities.sh \ - && download_protoc "${PROTOC_VERSION}" "linux-x86_64" + && download_protoc "${PROTOC_VERSION}" "${OS_ARCHITECTURE}" # we indicate protoc is available in the container via env vars ENV DOCKER_PROTOC_LOCATION=/protoc ENV DOCKER_PROTOC_VERSION="${PROTOC_VERSION}" +# install grpc +WORKDIR /grpc +RUN source /src/utils/utilities.sh \ + && download_grpc_plugin "${GRPC_VERSION}" "${OS_ARCHITECTURE}" +# similar to protoc, we indicate grpc is available in the container via env vars +ENV DOCKER_GRPC_LOCATION="/grpc/protoc-gen-grpc-java-${GRPC_VERSION}-${OS_ARCHITECTURE}.exe" +ENV DOCKER_GRPC_VERSION="${GRPC_VERSION}" + # use python 3.11 (the base image has several python versions; here we define the default one) RUN rm $(which python3) RUN ln -s $(which python3.11) /usr/local/bin/python diff --git a/library_generation/generate_library.sh b/library_generation/generate_library.sh index ecf8c64a50..8988c57b50 100755 --- a/library_generation/generate_library.sh +++ b/library_generation/generate_library.sh @@ -183,13 +183,15 @@ case "${proto_path}" in ;; esac # download gapic-generator-java, protobuf and grpc plugin. +# this function will create the environment variables "protoc_path" and +# "grpc_path", to be used in the protoc calls below download_tools "${gapic_generator_version}" "${protoc_version}" "${grpc_version}" "${os_architecture}" ##################### Section 1 ##################### # generate grpc-*/ ##################################################### if [[ ! "${transport}" == "rest" ]]; then # do not need to generate grpc-* if the transport is `rest`. - "${protoc_path}"/protoc "--plugin=protoc-gen-rpc-plugin=protoc-gen-grpc-java-${grpc_version}-${os_architecture}.exe" \ + "${protoc_path}"/protoc "--plugin=protoc-gen-rpc-plugin=${grpc_path}" \ "--rpc-plugin_out=:${temp_destination_path}/java_grpc.jar" \ ${proto_files} # Do not quote because this variable should not be treated as one long string. # unzip java_grpc.jar to grpc-*/src/main/java diff --git a/library_generation/test/generate_library_unit_tests.sh b/library_generation/test/generate_library_unit_tests.sh index 5384d6982c..b14865033a 100755 --- a/library_generation/test/generate_library_unit_tests.sh +++ b/library_generation/test/generate_library_unit_tests.sh @@ -28,10 +28,21 @@ get_grpc_version_failed_with_invalid_generator_version_test() { assertEquals 1 $((res)) } +get_grpc_version_succeed_docker_env_var_test() { + local version_with_docker + local version_without_docker + export DOCKER_GRPC_VERSION="9.9.9" + # get_grpc_version should prioritize DOCKER_GRPC_VERSION + version_with_docker=$(get_grpc_version "2.24.0") + assertEquals "${DOCKER_GRPC_VERSION}" "${version_with_docker}" + unset DOCKER_GRPC_VERSION +} + get_protoc_version_succeed_docker_env_var_test() { local version_with_docker local version_without_docker export DOCKER_PROTOC_VERSION="9.9.9" + # get_protoc_version should prioritize DOCKER_PROTOC_VERSION version_with_docker=$(get_protoc_version "2.24.0") assertEquals "${DOCKER_PROTOC_VERSION}" "${version_with_docker}" unset DOCKER_PROTOC_VERSION @@ -162,6 +173,8 @@ download_tools_succeed_with_baked_protoc() { local test_ggj_version="2.40.0" local test_grpc_version="1.64.0" + # we expect download_tools to decide to use DOCKER_PROTOC_LOCATION because + # the protoc version we want to download is the same as DOCKER_PROTOC_VERSION download_tools "${test_ggj_version}" "99.99" "${test_grpc_version}" "linux-x86_64" assertEquals "${protoc_bin_folder}" "${protoc_path}" @@ -169,6 +182,31 @@ download_tools_succeed_with_baked_protoc() { unset DOCKER_PROTOC_LOCATION unset DOCKER_PROTOC_VERSION unset output_folder + unset protoc_path +} + +download_tools_succeed_with_baked_grpc() { + # This test has the same structure as + # download_tools_succeed_with_baked_protoc, but meant to test grpc. + local test_dir=$(mktemp -d) + pushd "${test_dir}" + export DOCKER_GRPC_LOCATION=$(mktemp -d) + export DOCKER_GRPC_VERSION="99.99" + export output_folder=$(get_output_folder) + mkdir "${output_folder}" + + local test_ggj_version="2.40.0" + local test_protoc_version="1.64.0" + # we expect download_tools to decide to use DOCKER_GRPC_LOCATION because + # the protoc version we want to download is the same as DOCKER_GRPC_VERSION + download_tools "${test_ggj_version}" "${test_protoc_version}" "99.99" "linux-x86_64" + assertEquals "${DOCKER_GRPC_LOCATION}" "${grpc_path}" + + rm -rdf "${output_folder}" + unset DOCKER_GRPC_LOCATION + unset DOCKER_GRPC_VERSION + unset output_folder + unset grpc_path } download_grpc_plugin_succeed_with_valid_version_linux_test() { @@ -293,6 +331,7 @@ test_list=( extract_folder_name_test get_grpc_version_succeed_with_valid_generator_version_test get_grpc_version_failed_with_invalid_generator_version_test + get_grpc_version_succeed_docker_env_var_test get_protoc_version_succeed_docker_env_var_test get_protoc_version_succeed_with_valid_generator_version_test get_protoc_version_failed_with_invalid_generator_version_test @@ -307,6 +346,7 @@ test_list=( download_protoc_failed_with_invalid_version_linux_test download_protoc_failed_with_invalid_arch_test download_tools_succeed_with_baked_protoc + download_tools_succeed_with_baked_grpc download_grpc_plugin_succeed_with_valid_version_linux_test download_grpc_plugin_succeed_with_valid_version_macos_test download_grpc_plugin_failed_with_invalid_version_linux_test diff --git a/library_generation/utils/utilities.sh b/library_generation/utils/utilities.sh index 2e26ed6027..12ea9ac7e3 100755 --- a/library_generation/utils/utilities.sh +++ b/library_generation/utils/utilities.sh @@ -105,6 +105,11 @@ download_gapic_generator_pom_parent() { get_grpc_version() { local gapic_generator_version=$1 local grpc_version + if [[ -n "${DOCKER_GRPC_VERSION}" ]]; then + >&2 echo "Using grpc version baked into the container: ${DOCKER_GRPC_VERSION}" + echo "${DOCKER_GRPC_VERSION}" + return + fi pushd "${output_folder}" > /dev/null # get grpc version from gapic-generator-java-pom-parent/pom.xml download_gapic_generator_pom_parent "${gapic_generator_version}" @@ -147,7 +152,15 @@ download_tools() { export protoc_path=$(download_protoc "${protoc_version}" "${os_architecture}") fi - download_grpc_plugin "${grpc_version}" "${os_architecture}" + # similar case with grpc + if [[ "${DOCKER_GRPC_VERSION}" == "${grpc_version}" ]]; then + # if the specified protoc_version matches the one baked in the docker + # container, we just point grpc_path to its location. + export grpc_path="${DOCKER_GRPC_LOCATION}" + else + export grpc_path=$(download_grpc_plugin "${grpc_version}" "${os_architecture}") + fi + popd } @@ -198,13 +211,15 @@ download_protoc() { download_grpc_plugin() { local grpc_version=$1 local os_architecture=$2 - if [ ! -f "protoc-gen-grpc-java-${grpc_version}-${os_architecture}.exe" ]; then + grpc_filename="protoc-gen-grpc-java-${grpc_version}-${os_architecture}.exe" + if [ ! -f "${grpc_filename}" ]; then # download protoc-gen-grpc-java plugin from Google maven central mirror. download_from \ - "https://maven-central.storage-download.googleapis.com/maven2/io/grpc/protoc-gen-grpc-java/${grpc_version}/protoc-gen-grpc-java-${grpc_version}-${os_architecture}.exe" \ - "protoc-gen-grpc-java-${grpc_version}-${os_architecture}.exe" - chmod +x "protoc-gen-grpc-java-${grpc_version}-${os_architecture}.exe" + "https://maven-central.storage-download.googleapis.com/maven2/io/grpc/protoc-gen-grpc-java/${grpc_version}/${grpc_filename}" \ + "${grpc_filename}" + chmod +x "${grpc_filename}" fi + echo "$(pwd)/${grpc_filename}" } download_from() { @@ -282,7 +297,7 @@ get_proto_path_from_preprocessed_sources() { pushd "${sources}" > /dev/null local proto_library=$(find . -maxdepth 1 -type d -name 'proto-*' | sed 's/\.\///') local found_libraries=$(echo "${proto_library}" | wc -l) - if [ -z ${proto_library} ]; then + if [[ -z ${proto_library} ]]; then echo "no proto libraries found in the supplied sources path" exit 1 elif [ ${found_libraries} -gt 1 ]; then From 15b2fc51ab724e6f3e39857ada3b9842eff1e306 Mon Sep 17 00:00:00 2001 From: diegomarquezp Date: Thu, 11 Jul 2024 13:40:36 +0000 Subject: [PATCH 02/63] improve comments --- library_generation/generate_library.sh | 4 ++-- .../test/generate_library_unit_tests.sh | 7 +++++-- library_generation/utils/utilities.sh | 18 ++++++++++++++++++ 3 files changed, 25 insertions(+), 4 deletions(-) diff --git a/library_generation/generate_library.sh b/library_generation/generate_library.sh index 8988c57b50..1b113ba11b 100755 --- a/library_generation/generate_library.sh +++ b/library_generation/generate_library.sh @@ -183,8 +183,8 @@ case "${proto_path}" in ;; esac # download gapic-generator-java, protobuf and grpc plugin. -# this function will create the environment variables "protoc_path" and -# "grpc_path", to be used in the protoc calls below +# the download_tools function will create the environment variables "protoc_path" +# and "grpc_path", to be used in the protoc calls below. download_tools "${gapic_generator_version}" "${protoc_version}" "${grpc_version}" "${os_architecture}" ##################### Section 1 ##################### # generate grpc-*/ diff --git a/library_generation/test/generate_library_unit_tests.sh b/library_generation/test/generate_library_unit_tests.sh index b14865033a..0261a041c3 100755 --- a/library_generation/test/generate_library_unit_tests.sh +++ b/library_generation/test/generate_library_unit_tests.sh @@ -174,7 +174,10 @@ download_tools_succeed_with_baked_protoc() { local test_ggj_version="2.40.0" local test_grpc_version="1.64.0" # we expect download_tools to decide to use DOCKER_PROTOC_LOCATION because - # the protoc version we want to download is the same as DOCKER_PROTOC_VERSION + # the protoc version we want to download is the same as DOCKER_PROTOC_VERSION. + # Note that `protoc_bin_folder` is just the expected formatted value that + # download_tools will format using DOCKER_PROTOC_VERSION (via + # download_protoc). download_tools "${test_ggj_version}" "99.99" "${test_grpc_version}" "linux-x86_64" assertEquals "${protoc_bin_folder}" "${protoc_path}" @@ -187,7 +190,7 @@ download_tools_succeed_with_baked_protoc() { download_tools_succeed_with_baked_grpc() { # This test has the same structure as - # download_tools_succeed_with_baked_protoc, but meant to test grpc. + # download_tools_succeed_with_baked_protoc, but meant for the grpc plugin. local test_dir=$(mktemp -d) pushd "${test_dir}" export DOCKER_GRPC_LOCATION=$(mktemp -d) diff --git a/library_generation/utils/utilities.sh b/library_generation/utils/utilities.sh index 12ea9ac7e3..a93df59b42 100755 --- a/library_generation/utils/utilities.sh +++ b/library_generation/utils/utilities.sh @@ -102,6 +102,10 @@ download_gapic_generator_pom_parent() { download_generator_artifact "${gapic_generator_version}" "gapic-generator-java-pom-parent-${gapic_generator_version}.pom" "gapic-generator-java-pom-parent" } +# This function returns the version of the grpc plugin to generate the libraries. If +# DOCKER_GRPC_VERSION is set, this will be the version. Otherwise, it will be +# computed from the gapic-generator-pom-parent artifact at the specified +# gapic_generator_version. get_grpc_version() { local gapic_generator_version=$1 local grpc_version @@ -118,6 +122,10 @@ get_grpc_version() { echo "${grpc_version}" } +# This function returns the version of protoc to generate the libraries. If +# DOCKER_PROTOC_VERSION is set, this will be the version. Otherwise, it will be +# computed from the gapic-generator-pom-parent artifact at the specified +# gapic_generator_version. get_protoc_version() { local gapic_generator_version=$1 local protoc_version @@ -134,6 +142,16 @@ get_protoc_version() { echo "${protoc_version}" } +# Given the versions of the gapic generator, protoc and the protoc-grpc plugin, +# this function will download each one of the tools and create the environment +# variables "protoc_path" and "grpc_path" which are expected upstream. Note that +# if the specified versions of protoc and grpc match DOCKER_PROTOC_VERSION and +# DOCKER_GRPC_VERSION respectively, this function will instead set "protoc_path" +# and "grpc_path" to DOCKER_PROTOC_PATH and DOCKER_GRPC_PATH respectively (no +# download), since the docker image will have downloaded these tools beforehand. +# For the case of gapic-generator-java, no env var will be exported for the +# upstream flow, but instead it will be assigned a default filename that will be +# referenced by the file `library_generation/gapic-generator-java-wrapper`. download_tools() { local gapic_generator_version=$1 local protoc_version=$2 From 0a95554b4a4ecb2d2b5ca9c968aac82ba27e9887 Mon Sep 17 00:00:00 2001 From: Diego Marquez Date: Mon, 22 Jul 2024 16:02:12 -0400 Subject: [PATCH 03/63] Update library_generation/utils/utilities.sh Co-authored-by: Blake Li --- library_generation/utils/utilities.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library_generation/utils/utilities.sh b/library_generation/utils/utilities.sh index a93df59b42..72fefec76a 100755 --- a/library_generation/utils/utilities.sh +++ b/library_generation/utils/utilities.sh @@ -172,7 +172,7 @@ download_tools() { # similar case with grpc if [[ "${DOCKER_GRPC_VERSION}" == "${grpc_version}" ]]; then - # if the specified protoc_version matches the one baked in the docker + # if the specified grpc_version matches the one baked in the docker # container, we just point grpc_path to its location. export grpc_path="${DOCKER_GRPC_LOCATION}" else From 034edf985b1e7fc6076984712f4ffb2de01732db Mon Sep 17 00:00:00 2001 From: diegomarquezp Date: Mon, 22 Jul 2024 20:17:48 +0000 Subject: [PATCH 04/63] add grpc to renovate.json --- renovate.json | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/renovate.json b/renovate.json index 473aa6f717..1ea28fa0fe 100644 --- a/renovate.json +++ b/renovate.json @@ -41,6 +41,18 @@ "depNameTemplate": "protocolbuffers/protobuf", "extractVersionTemplate": "^v(?.*)$" }, + { + "customType": "regex", + "fileMatch": [ + "^\\.cloudbuild/library_generation/library_generation\\.Dockerfile$" + ], + "matchStrings": [ + "ARG GRPC_VERSION=[\"']?(?.+?)[\"']?\\s+" + ], + "datasourceTemplate": "github-releases", + "depNameTemplate": "grpc/grpc", + "extractVersionTemplate": "^v(?.*)$" + }, { "customType": "regex", "fileMatch": [ From 9795e23af1aeab77d6c9557000c311e21f972978 Mon Sep 17 00:00:00 2001 From: diegomarquezp Date: Mon, 22 Jul 2024 20:30:17 +0000 Subject: [PATCH 05/63] add GRPC_VERSION to renovate config --- renovate.json | 20 +++++--------------- 1 file changed, 5 insertions(+), 15 deletions(-) diff --git a/renovate.json b/renovate.json index 1ea28fa0fe..f0ad762025 100644 --- a/renovate.json +++ b/renovate.json @@ -32,7 +32,7 @@ { "customType": "regex", "fileMatch": [ - "^\\.cloudbuild/library_generation/library_generation\\.Dockerfile$" + "^\\.cloudbuild/library_generation/library_generation\\.Dockerfile$" ], "matchStrings": [ "ARG PROTOC_VERSION=[\"']?(?.+?)[\"']?\\s+" @@ -41,18 +41,6 @@ "depNameTemplate": "protocolbuffers/protobuf", "extractVersionTemplate": "^v(?.*)$" }, - { - "customType": "regex", - "fileMatch": [ - "^\\.cloudbuild/library_generation/library_generation\\.Dockerfile$" - ], - "matchStrings": [ - "ARG GRPC_VERSION=[\"']?(?.+?)[\"']?\\s+" - ], - "datasourceTemplate": "github-releases", - "depNameTemplate": "grpc/grpc", - "extractVersionTemplate": "^v(?.*)$" - }, { "customType": "regex", "fileMatch": [ @@ -67,10 +55,12 @@ { "customType": "regex", "fileMatch": [ - "^gax-java/dependencies\\.properties$" + "^gax-java/dependencies\\.properties$", + "^\\.cloudbuild/library_generation/library_generation\\.Dockerfile$" ], "matchStrings": [ - "version\\.io_grpc=(?.+?)\\n" + "version\\.io_grpc=(?.+?)\\n", + "ARG GRPC_VERSION=[\"']?(?.+?)[\"']?\\s+" ], "depNameTemplate": "io.grpc:grpc-core", "datasourceTemplate": "maven" From 89fc562b0e971b55205ebfedce8efb3c541c3251 Mon Sep 17 00:00:00 2001 From: diegomarquezp Date: Wed, 24 Jul 2024 20:30:15 +0000 Subject: [PATCH 06/63] feat: bake gapic-generator-java into the hermetic docker image --- .../library_generation.Dockerfile | 25 ++++++++++++++++++- library_generation/generate_library.sh | 19 ++++++++++---- library_generation/model/generation_config.py | 4 +-- library_generation/utils/utilities.sh | 14 ++++++++++- 4 files changed, 53 insertions(+), 9 deletions(-) diff --git a/.cloudbuild/library_generation/library_generation.Dockerfile b/.cloudbuild/library_generation/library_generation.Dockerfile index 63385f136a..8cd32203d7 100644 --- a/.cloudbuild/library_generation/library_generation.Dockerfile +++ b/.cloudbuild/library_generation/library_generation.Dockerfile @@ -12,11 +12,25 @@ # See the License for the specific language governing permissions and # limitations under the License. +# install gapic-generator-java in a separate layer so we don't overload the image +# with the transferred source code and jars +FROM gcr.io/cloud-devrel-public-resources/java21 AS ggj-build + +WORKDIR /sdk-platform-java +COPY . . +# {x-version-update-start:gapic-generator-java:current} +ENV DOCKER_GAPIC_GENERATOR_VERSION="2.42.1-SNAPSHOT" + +RUN mvn install -DskipTests -Dclirr.skip -Dcheckstyle.skip +RUN ls "/root/.m2/repository/com/google/api/gapic-generator-java/" +RUN cp "/root/.m2/repository/com/google/api/gapic-generator-java/${DOCKER_GAPIC_GENERATOR_VERSION}/gapic-generator-java-${DOCKER_GAPIC_GENERATOR_VERSION}.jar" . + # build from the root of this repo: FROM gcr.io/cloud-devrel-public-resources/python SHELL [ "/bin/bash", "-c" ] + ARG OWLBOT_CLI_COMMITTISH=ac84fa5c423a0069bbce3d2d869c9730c8fdf550 ARG PROTOC_VERSION=25.3 ARG GRPC_VERSION=1.62.2 @@ -47,7 +61,16 @@ RUN source /src/utils/utilities.sh \ ENV DOCKER_GRPC_LOCATION="/grpc/protoc-gen-grpc-java-${GRPC_VERSION}-${OS_ARCHITECTURE}.exe" ENV DOCKER_GRPC_VERSION="${GRPC_VERSION}" -# use python 3.11 (the base image has several python versions; here we define the default one) + +# we transfer gapic-generator-java from the previous stage. +# here we redeclare this env var since they are not preserved between stages +ENV DOCKER_GAPIC_GENERATOR_VERSION="2.42.1-SNAPSHOT" +# {x-version-update-end:gapic-generator-java:current} +ENV DOCKER_GAPIC_GENERATOR_LOCATION="/gapic-generator-java/gapic-generator-java-${DOCKER_GAPIC_GENERATOR_VERSION}.jar" +COPY --from=ggj-build "/root/.m2/repository/com/google/api/gapic-generator-java/${DOCKER_GAPIC_GENERATOR_VERSION}/gapic-generator-java-${DOCKER_GAPIC_GENERATOR_VERSION}.jar" "${DOCKER_GAPIC_GENERATOR_LOCATION}" +RUN chmod 755 "${DOCKER_GAPIC_GENERATOR_LOCATION}" + +# use python 3.11 (the base image has several python versions; here we define the default one) RUN rm $(which python3) RUN ln -s $(which python3.11) /usr/local/bin/python RUN ln -s $(which python3.11) /usr/local/bin/python3 diff --git a/library_generation/generate_library.sh b/library_generation/generate_library.sh index 1b113ba11b..8fe9026d62 100755 --- a/library_generation/generate_library.sh +++ b/library_generation/generate_library.sh @@ -1,6 +1,7 @@ #!/usr/bin/env bash set -eo pipefail +set -x # parse input parameters while [[ $# -gt 0 ]]; do @@ -16,8 +17,6 @@ case $key in ;; --gapic_generator_version) gapic_generator_version="$2" - # export this variable so that it can be used in gapic-generator-java-wrapper.sh - export gapic_generator_version shift ;; --protoc_version) @@ -77,9 +76,16 @@ script_dir=$(dirname "$(readlink -f "$0")") source "${script_dir}"/utils/utilities.sh output_folder="$(get_output_folder)" -if [ -z "${gapic_generator_version}" ]; then - echo 'missing required argument --gapic_generator_version' - exit 1 +if [[ -z "${gapic_generator_version}" ]] ; then + if [[ -n "${DOCKER_GAPIC_GENERATOR_VERSION}" ]]; then + # we fall back to the docker env var so it's handled downstream in + # utilities.sh. This will make the jar embedded in the image to be + # transferred to output_folder + gapic_generator_version="${DOCKER_GAPIC_GENERATOR_VERSION}" + else + echo 'missing required argument --gapic_generator_version' + exit 1 + fi fi if [ -z "${protoc_version}" ]; then @@ -126,6 +132,9 @@ if [ -z "${os_architecture}" ]; then os_architecture=$(detect_os_architecture) fi +# export this variable so that it can be used in gapic-generator-java-wrapper.sh +export gapic_generator_version + temp_destination_path="${output_folder}/temp_preprocessed" mkdir -p "${output_folder}/${destination_path}" if [ -d "${temp_destination_path}" ]; then diff --git a/library_generation/model/generation_config.py b/library_generation/model/generation_config.py index 8a2b0829c4..719c97c85f 100644 --- a/library_generation/model/generation_config.py +++ b/library_generation/model/generation_config.py @@ -147,8 +147,8 @@ def from_yaml(path_to_yaml: str) -> GenerationConfig: parsed_libraries.append(new_library) parsed_config = GenerationConfig( - gapic_generator_version=__required( - config, GAPIC_GENERATOR_VERSION, REPO_LEVEL_PARAMETER + gapic_generator_version=__optional( + config, GAPIC_GENERATOR_VERSION, None ), googleapis_commitish=__required( config, "googleapis_commitish", REPO_LEVEL_PARAMETER diff --git a/library_generation/utils/utilities.sh b/library_generation/utils/utilities.sh index 72fefec76a..2e7a6a7f2c 100755 --- a/library_generation/utils/utilities.sh +++ b/library_generation/utils/utilities.sh @@ -1,6 +1,7 @@ #!/usr/bin/env bash set -eo pipefail +set -x utilities_script_dir=$(dirname "$(realpath "${BASH_SOURCE[0]}")") # Utility functions used in `generate_library.sh` and showcase generation. @@ -158,7 +159,6 @@ download_tools() { local grpc_version=$3 local os_architecture=$4 pushd "${output_folder}" - download_generator_artifact "${gapic_generator_version}" "gapic-generator-java-${gapic_generator_version}.jar" # the variable protoc_path is used in generate_library.sh. It is explicitly # exported to make clear that it is used outside this utilities file. @@ -179,6 +179,18 @@ download_tools() { export grpc_path=$(download_grpc_plugin "${grpc_version}" "${os_architecture}") fi + # similar case with gapic-generator-java + if [[ "${DOCKER_GAPIC_GENERATOR_VERSION}" == "${gapic_generator_version}" ]]; then + # if the specified gapic_generator_version matches the one baked in the docker + # container, we copy the generator jar into the output folder so it can be + # picked up by gapic-generator-java-wrapper + cp "${DOCKER_GAPIC_GENERATOR_LOCATION}" "${output_folder}" + else + download_generator_artifact \ + "${gapic_generator_version}" \ + "gapic-generator-java-${gapic_generator_version}.jar" + fi + popd } From 1f2ea9e70a1da41eb4cee8e309033e84e2b23fea Mon Sep 17 00:00:00 2001 From: diegomarquezp Date: Wed, 24 Jul 2024 20:53:50 +0000 Subject: [PATCH 07/63] add tests --- library_generation/model/generation_config.py | 4 +-- .../test/generate_library_unit_tests.sh | 29 +++++++++++++++++++ .../model/generation_config_unit_tests.py | 8 ----- library_generation/test/test_utilities.sh | 13 +++++++++ library_generation/utils/utilities.sh | 1 + 5 files changed, 44 insertions(+), 11 deletions(-) diff --git a/library_generation/model/generation_config.py b/library_generation/model/generation_config.py index 719c97c85f..33b58c8f45 100644 --- a/library_generation/model/generation_config.py +++ b/library_generation/model/generation_config.py @@ -147,9 +147,7 @@ def from_yaml(path_to_yaml: str) -> GenerationConfig: parsed_libraries.append(new_library) parsed_config = GenerationConfig( - gapic_generator_version=__optional( - config, GAPIC_GENERATOR_VERSION, None - ), + gapic_generator_version=__optional(config, GAPIC_GENERATOR_VERSION, None), googleapis_commitish=__required( config, "googleapis_commitish", REPO_LEVEL_PARAMETER ), diff --git a/library_generation/test/generate_library_unit_tests.sh b/library_generation/test/generate_library_unit_tests.sh index 0261a041c3..977a5836a7 100755 --- a/library_generation/test/generate_library_unit_tests.sh +++ b/library_generation/test/generate_library_unit_tests.sh @@ -212,6 +212,34 @@ download_tools_succeed_with_baked_grpc() { unset grpc_path } +download_tools_succeed_with_baked_generator() { + # This test has the same structure as + # download_tools_succeed_with_baked_protoc, but meant for + # gapic-generator-java. + local test_dir=$(mktemp -d) + pushd "${test_dir}" + generator_folder=$(mktemp -d) + touch "${generator_folder}/gapic-generator-java.fakejar" + export DOCKER_GAPIC_GENERATOR_LOCATION="${generator_folder}/gapic-generator-java.fakejar" + export DOCKER_GAPIC_GENERATOR_VERSION="99.99" + export output_folder=$(get_output_folder) + mkdir "${output_folder}" + + local test_protoc_version="1.64.0" + local test_grpc_version="1.64.0" + # we expect download_tools to decide to use DOCKER_GAPIC_GENERATOR_LOCATION because + # the protoc version we want to download is the same as DOCKER_GRPC_VERSION + log=$(download_tools "99.99" "${test_protoc_version}" "${test_grpc_version}" "linux-x86_64" 2>&1 1>/dev/null) + + assertContains "Using gapic-generator-java version baked into the container: 99.99" "${log}" + + rm -rdf "${output_folder}" + unset DOCKER_GRPC_LOCATION + unset DOCKER_GRPC_VERSION + unset output_folder + unset grpc_path +} + download_grpc_plugin_succeed_with_valid_version_linux_test() { download_grpc_plugin "1.55.1" "linux-x86_64" assertFileOrDirectoryExists "protoc-gen-grpc-java-1.55.1-linux-x86_64.exe" @@ -350,6 +378,7 @@ test_list=( download_protoc_failed_with_invalid_arch_test download_tools_succeed_with_baked_protoc download_tools_succeed_with_baked_grpc + download_tools_succeed_with_baked_generator download_grpc_plugin_succeed_with_valid_version_linux_test download_grpc_plugin_succeed_with_valid_version_macos_test download_grpc_plugin_failed_with_invalid_version_linux_test diff --git a/library_generation/test/model/generation_config_unit_tests.py b/library_generation/test/model/generation_config_unit_tests.py index 258b0db415..4f4d16edbd 100644 --- a/library_generation/test/model/generation_config_unit_tests.py +++ b/library_generation/test/model/generation_config_unit_tests.py @@ -160,14 +160,6 @@ def test_validate_with_duplicate_library_name_raise_exception(self): ], ) - def test_from_yaml_without_gapic_generator_version_raise_exception(self): - self.assertRaisesRegex( - ValueError, - "Repo level parameter, gapic_generator_version", - from_yaml, - f"{test_config_dir}/config_without_generator.yaml", - ) - def test_from_yaml_without_googleapis_commitish_raise_exception(self): self.assertRaisesRegex( ValueError, diff --git a/library_generation/test/test_utilities.sh b/library_generation/test/test_utilities.sh index 007dc8e6d9..f9611360ed 100755 --- a/library_generation/test/test_utilities.sh +++ b/library_generation/test/test_utilities.sh @@ -41,6 +41,19 @@ assertEquals() { __test_failed "${ut}" } +assertContains() { + local expected_substring=$1 + local test_string=$2 + + if grep -q "$substring" <<< "$test_string"; then + __test_succeed + return + fi + + echo "Error: expected test string to contain '${expected_substring}'. Test string was '${test_string}' instead." + __test_failed "${ut}" +} + assertFileOrDirectoryExists() { local expected_file=$1 if [ -d "${expected_file}" ]|| [ -f "${expected_file}" ]; then diff --git a/library_generation/utils/utilities.sh b/library_generation/utils/utilities.sh index 2e7a6a7f2c..7c8528af0b 100755 --- a/library_generation/utils/utilities.sh +++ b/library_generation/utils/utilities.sh @@ -184,6 +184,7 @@ download_tools() { # if the specified gapic_generator_version matches the one baked in the docker # container, we copy the generator jar into the output folder so it can be # picked up by gapic-generator-java-wrapper + >&2 echo "Using gapic-generator-java version baked into the container: ${DOCKER_GAPIC_GENERATOR_VERSION}" cp "${DOCKER_GAPIC_GENERATOR_LOCATION}" "${output_folder}" else download_generator_artifact \ From 8af8becf14b136117c65c353130d90a4990b71de Mon Sep 17 00:00:00 2001 From: diegomarquezp Date: Thu, 25 Jul 2024 12:35:47 +0000 Subject: [PATCH 08/63] remove generator version from generation config --- generation_config.yaml | 1 - 1 file changed, 1 deletion(-) diff --git a/generation_config.yaml b/generation_config.yaml index d9439fb7db..e894663ac0 100644 --- a/generation_config.yaml +++ b/generation_config.yaml @@ -1,4 +1,3 @@ -gapic_generator_version: 2.42.1-SNAPSHOT # {x-version-update:gapic-generator-java:current} googleapis_commitish: 7160b0c31e9b99fe896d1fa29b6fe6cfbf42588f # the libraries are ordered with respect to library name, which is # java-{library.library_name} or java-{library.api-shortname} when From 16494032133631dd6de5d04de32d01a93fb045b2 Mon Sep 17 00:00:00 2001 From: diegomarquezp Date: Wed, 31 Jul 2024 18:30:00 +0000 Subject: [PATCH 09/63] expand unit test to test file --- .../test/generate_library_unit_tests.sh | 18 +++++++++++++++++- library_generation/test/test_utilities.sh | 2 +- 2 files changed, 18 insertions(+), 2 deletions(-) diff --git a/library_generation/test/generate_library_unit_tests.sh b/library_generation/test/generate_library_unit_tests.sh index 977a5836a7..90128d2dfa 100755 --- a/library_generation/test/generate_library_unit_tests.sh +++ b/library_generation/test/generate_library_unit_tests.sh @@ -231,7 +231,23 @@ download_tools_succeed_with_baked_generator() { # the protoc version we want to download is the same as DOCKER_GRPC_VERSION log=$(download_tools "99.99" "${test_protoc_version}" "${test_grpc_version}" "linux-x86_64" 2>&1 1>/dev/null) - assertContains "Using gapic-generator-java version baked into the container: 99.99" "${log}" + # the assertion functions are designed to be called only once per function. + # Here we do two manual tests that are then coverged into a single + # pseudo-boolean + has_expected_log="false" + has_expected_file="false" + is_output_correct="false" + if grep -q "Using gapic-generator-java version baked into the container: 99.99" <<< "${log}"; then + has_expected_log="true" + fi + if [[ -f "${output_folder}/gapic-generator-java.fakejar" ]]; then + has_expected_file="true" + fi + if [[ "${has_expected_log}" == "true" ]] && [[ "${has_expected_file}" == "true" ]]; then + is_output_correct="true" + fi + + assertEquals "true" "${is_output_correct}" rm -rdf "${output_folder}" unset DOCKER_GRPC_LOCATION diff --git a/library_generation/test/test_utilities.sh b/library_generation/test/test_utilities.sh index f9611360ed..3393070436 100755 --- a/library_generation/test/test_utilities.sh +++ b/library_generation/test/test_utilities.sh @@ -27,7 +27,7 @@ __test_failed() { } -############# Functions used in test execution ############# +############# Functions used in test execution. They can only be called once per test ############# assertEquals() { local expected=$1 From 34c5355df900a4750a5046fc269cf2b52cb06cc8 Mon Sep 17 00:00:00 2001 From: diegomarquezp Date: Wed, 31 Jul 2024 18:31:09 +0000 Subject: [PATCH 10/63] remove unused function --- library_generation/test/test_utilities.sh | 13 ------------- 1 file changed, 13 deletions(-) diff --git a/library_generation/test/test_utilities.sh b/library_generation/test/test_utilities.sh index 3393070436..bbcdee1ebc 100755 --- a/library_generation/test/test_utilities.sh +++ b/library_generation/test/test_utilities.sh @@ -41,19 +41,6 @@ assertEquals() { __test_failed "${ut}" } -assertContains() { - local expected_substring=$1 - local test_string=$2 - - if grep -q "$substring" <<< "$test_string"; then - __test_succeed - return - fi - - echo "Error: expected test string to contain '${expected_substring}'. Test string was '${test_string}' instead." - __test_failed "${ut}" -} - assertFileOrDirectoryExists() { local expected_file=$1 if [ -d "${expected_file}" ]|| [ -f "${expected_file}" ]; then From 651af1e591a68153c2c5727c153144a4ae227693 Mon Sep 17 00:00:00 2001 From: diegomarquezp Date: Wed, 31 Jul 2024 18:32:34 +0000 Subject: [PATCH 11/63] remove xtrace --- library_generation/generate_library.sh | 1 - library_generation/utils/utilities.sh | 1 - 2 files changed, 2 deletions(-) diff --git a/library_generation/generate_library.sh b/library_generation/generate_library.sh index 8fe9026d62..b4019c368e 100755 --- a/library_generation/generate_library.sh +++ b/library_generation/generate_library.sh @@ -1,7 +1,6 @@ #!/usr/bin/env bash set -eo pipefail -set -x # parse input parameters while [[ $# -gt 0 ]]; do diff --git a/library_generation/utils/utilities.sh b/library_generation/utils/utilities.sh index 7c8528af0b..6174db3aef 100755 --- a/library_generation/utils/utilities.sh +++ b/library_generation/utils/utilities.sh @@ -1,7 +1,6 @@ #!/usr/bin/env bash set -eo pipefail -set -x utilities_script_dir=$(dirname "$(realpath "${BASH_SOURCE[0]}")") # Utility functions used in `generate_library.sh` and showcase generation. From 4ef977b488192cfeb2fa960783845a89a0c1c16d Mon Sep 17 00:00:00 2001 From: diegomarquezp Date: Wed, 31 Jul 2024 19:08:00 +0000 Subject: [PATCH 12/63] adjust to other env var provided --- .github/scripts/hermetic_library_generation.sh | 3 --- library_generation/generate_library.sh | 11 ++--------- library_generation/model/generation_config.py | 10 ++++++---- .../test/model/generation_config_unit_tests.py | 6 +++--- 4 files changed, 11 insertions(+), 19 deletions(-) diff --git a/.github/scripts/hermetic_library_generation.sh b/.github/scripts/hermetic_library_generation.sh index e69e8aa223..41f7cd2b84 100755 --- a/.github/scripts/hermetic_library_generation.sh +++ b/.github/scripts/hermetic_library_generation.sh @@ -90,9 +90,6 @@ fi git show "${target_branch}":"${generation_config}" > "${baseline_generation_config}" config_diff=$(diff "${generation_config}" "${baseline_generation_config}" || true) -generator_version=$(mvn help:evaluate -Dexpression=project.version -q -DforceStdout -pl gapic-generator-java) -echo "Local generator version: ${generator_version}" - # install generator locally since we're using a SNAPSHOT version. mvn -V -B -ntp clean install -DskipTests diff --git a/library_generation/generate_library.sh b/library_generation/generate_library.sh index b4019c368e..1ed76fd525 100755 --- a/library_generation/generate_library.sh +++ b/library_generation/generate_library.sh @@ -76,15 +76,8 @@ source "${script_dir}"/utils/utilities.sh output_folder="$(get_output_folder)" if [[ -z "${gapic_generator_version}" ]] ; then - if [[ -n "${DOCKER_GAPIC_GENERATOR_VERSION}" ]]; then - # we fall back to the docker env var so it's handled downstream in - # utilities.sh. This will make the jar embedded in the image to be - # transferred to output_folder - gapic_generator_version="${DOCKER_GAPIC_GENERATOR_VERSION}" - else - echo 'missing required argument --gapic_generator_version' - exit 1 - fi + echo 'missing required argument --gapic_generator_version' + exit 1 fi if [ -z "${protoc_version}" ]; then diff --git a/library_generation/model/generation_config.py b/library_generation/model/generation_config.py index bc3ff6d440..526c8f8a46 100644 --- a/library_generation/model/generation_config.py +++ b/library_generation/model/generation_config.py @@ -25,7 +25,7 @@ COMMON_PROTOS_LIBRARY_NAME = "common-protos" GAPIC_GENERATOR_VERSION = "gapic_generator_version" LIBRARIES_BOM_VERSION = "libraries_bom_version" -GENERATOR_VERSION_ENV_KEY = "GENERATOR_VERSION" +GENERATOR_VERSION_ENV_KEY = "DOCKER_GAPIC_GENERATOR_VERSION" class GenerationConfig: @@ -90,9 +90,11 @@ def __set_generator_version(gapic_generator_version: Optional[str]) -> str: gapic_generator_version = os.getenv(GENERATOR_VERSION_ENV_KEY) if not gapic_generator_version: raise ValueError( - f"Environment variable {GENERATOR_VERSION_ENV_KEY}" - f" is not set when the generator version is not" - f" specified in the generation config." + f"gapic_generator_version was not specified in the " + f"configuration yaml and the fall-back env var " + f"{GENERATOR_VERSION_ENV_KEY} was not found. At " + f"least one of them must be set to proceed with the " + f"generation." ) return gapic_generator_version diff --git a/library_generation/test/model/generation_config_unit_tests.py b/library_generation/test/model/generation_config_unit_tests.py index f6a2f2a2d8..f3efe73390 100644 --- a/library_generation/test/model/generation_config_unit_tests.py +++ b/library_generation/test/model/generation_config_unit_tests.py @@ -56,20 +56,20 @@ def test_generation_config_default_value(self): def test_generation_config_with_generator_version_env_raise_exception(self): self.assertRaisesRegex( ValueError, - "Environment variable GENERATOR_VERSION is not set", + "the fall-back env var DOCKER_GAPIC_GENERATOR_VERSION was not found", GenerationConfig, googleapis_commitish="", libraries=[], ) def test_generation_config_set_generator_version_from_env(self): - os.environ["GENERATOR_VERSION"] = "1.2.3" + os.environ["DOCKER_GAPIC_GENERATOR_VERSION"] = "1.2.3" config = GenerationConfig( googleapis_commitish="", libraries=[], ) self.assertEqual("1.2.3", config.gapic_generator_version) - os.environ.pop("GENERATOR_VERSION") + os.environ.pop("DOCKER_GAPIC_GENERATOR_VERSION") def test_from_yaml_succeeds(self): config = from_yaml(f"{test_config_dir}/generation_config.yaml") From 6e20135d9777c221ee5c116b093a9c338ad24156 Mon Sep 17 00:00:00 2001 From: diegomarquezp Date: Wed, 31 Jul 2024 20:47:13 +0000 Subject: [PATCH 13/63] update docker image generator versoin --- .../library_generation/library_generation.Dockerfile | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/.cloudbuild/library_generation/library_generation.Dockerfile b/.cloudbuild/library_generation/library_generation.Dockerfile index efeda824fc..42a4a630a1 100644 --- a/.cloudbuild/library_generation/library_generation.Dockerfile +++ b/.cloudbuild/library_generation/library_generation.Dockerfile @@ -19,10 +19,10 @@ FROM gcr.io/cloud-devrel-public-resources/java21 AS ggj-build WORKDIR /sdk-platform-java COPY . . # {x-version-update-start:gapic-generator-java:current} -ENV DOCKER_GAPIC_GENERATOR_VERSION="2.42.1-SNAPSHOT" +ENV DOCKER_GAPIC_GENERATOR_VERSION="2.43.1-SNAPSHOT" RUN mvn install -DskipTests -Dclirr.skip -Dcheckstyle.skip -RUN ls "/root/.m2/repository/com/google/api/gapic-generator-java/" +RUN ls "/root/.m2/repository/com/google/api/gapic-generator-java/${DOCKER_GAPIC_GENERATOR_VERSION}" RUN cp "/root/.m2/repository/com/google/api/gapic-generator-java/${DOCKER_GAPIC_GENERATOR_VERSION}/gapic-generator-java-${DOCKER_GAPIC_GENERATOR_VERSION}.jar" . # build from the root of this repo: @@ -64,7 +64,7 @@ ENV DOCKER_GRPC_VERSION="${GRPC_VERSION}" # we transfer gapic-generator-java from the previous stage. # here we redeclare this env var since they are not preserved between stages -ENV DOCKER_GAPIC_GENERATOR_VERSION="2.42.1-SNAPSHOT" +ENV DOCKER_GAPIC_GENERATOR_VERSION="2.43.1-SNAPSHOT" # {x-version-update-end:gapic-generator-java:current} ENV DOCKER_GAPIC_GENERATOR_LOCATION="/gapic-generator-java/gapic-generator-java-${DOCKER_GAPIC_GENERATOR_VERSION}.jar" COPY --from=ggj-build "/root/.m2/repository/com/google/api/gapic-generator-java/${DOCKER_GAPIC_GENERATOR_VERSION}/gapic-generator-java-${DOCKER_GAPIC_GENERATOR_VERSION}.jar" "${DOCKER_GAPIC_GENERATOR_LOCATION}" From a5c18b2920bfd349119ed95a75da785d1fa3f23a Mon Sep 17 00:00:00 2001 From: diegomarquezp Date: Wed, 7 Aug 2024 19:26:49 +0000 Subject: [PATCH 14/63] remove old generator_version, rename config key variable --- .github/scripts/hermetic_library_generation.sh | 1 - library_generation/model/generation_config.py | 4 ++-- library_generation/utils/commit_message_formatter.py | 4 ++-- 3 files changed, 4 insertions(+), 5 deletions(-) diff --git a/.github/scripts/hermetic_library_generation.sh b/.github/scripts/hermetic_library_generation.sh index 41f7cd2b84..cf29c87b3f 100755 --- a/.github/scripts/hermetic_library_generation.sh +++ b/.github/scripts/hermetic_library_generation.sh @@ -104,7 +104,6 @@ docker run \ -u "$(id -u):$(id -g)" \ -v "$(pwd):${workspace_name}" \ -v "$HOME"/.m2:/home/.m2 \ - -e GENERATOR_VERSION="${generator_version}" \ gcr.io/cloud-devrel-public-resources/java-library-generation:"${image_tag}" \ --baseline-generation-config-path="${workspace_name}/${baseline_generation_config}" \ --current-generation-config-path="${workspace_name}/${generation_config}" diff --git a/library_generation/model/generation_config.py b/library_generation/model/generation_config.py index 526c8f8a46..e7c1012d12 100644 --- a/library_generation/model/generation_config.py +++ b/library_generation/model/generation_config.py @@ -23,7 +23,7 @@ LIBRARY_LEVEL_PARAMETER = "Library level parameter" GAPIC_LEVEL_PARAMETER = "GAPIC level parameter" COMMON_PROTOS_LIBRARY_NAME = "common-protos" -GAPIC_GENERATOR_VERSION = "gapic_generator_version" +GAPIC_GENERATOR_VERSION_CONFIG_KEY = "gapic_generator_version" LIBRARIES_BOM_VERSION = "libraries_bom_version" GENERATOR_VERSION_ENV_KEY = "DOCKER_GAPIC_GENERATOR_VERSION" @@ -173,7 +173,7 @@ def from_yaml(path_to_yaml: str) -> GenerationConfig: googleapis_commitish=__required( config, "googleapis_commitish", REPO_LEVEL_PARAMETER ), - gapic_generator_version=__optional(config, GAPIC_GENERATOR_VERSION, None), + gapic_generator_version=__optional(config, GAPIC_GENERATOR_VERSION_CONFIG_KEY, None), grpc_version=__optional(config, "grpc_version", None), protoc_version=__optional(config, "protoc_version", None), libraries_bom_version=__optional(config, LIBRARIES_BOM_VERSION, None), diff --git a/library_generation/utils/commit_message_formatter.py b/library_generation/utils/commit_message_formatter.py index 5b75db51a0..33d0d4d322 100644 --- a/library_generation/utils/commit_message_formatter.py +++ b/library_generation/utils/commit_message_formatter.py @@ -16,12 +16,12 @@ from library_generation.model.config_change import ConfigChange, ChangeType from library_generation.model.generation_config import ( - GAPIC_GENERATOR_VERSION, + GAPIC_GENERATOR_VERSION_CONFIG_KEY, LIBRARIES_BOM_VERSION, ) PARAM_TO_COMMIT_MESSAGE = { - GAPIC_GENERATOR_VERSION: "fix(deps): update the Java code generator (gapic-generator-java) to", + GAPIC_GENERATOR_VERSION_CONFIG_KEY: "fix(deps): update the Java code generator (gapic-generator-java) to", LIBRARIES_BOM_VERSION: "chore: update the libraries_bom version to", } From 89e16c1256927a806651ac10a4f36f3b1c4b3e0a Mon Sep 17 00:00:00 2001 From: diegomarquezp Date: Wed, 7 Aug 2024 19:35:54 +0000 Subject: [PATCH 15/63] improve comment --- library_generation/utils/utilities.sh | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/library_generation/utils/utilities.sh b/library_generation/utils/utilities.sh index 6174db3aef..76d39105c6 100755 --- a/library_generation/utils/utilities.sh +++ b/library_generation/utils/utilities.sh @@ -149,9 +149,14 @@ get_protoc_version() { # DOCKER_GRPC_VERSION respectively, this function will instead set "protoc_path" # and "grpc_path" to DOCKER_PROTOC_PATH and DOCKER_GRPC_PATH respectively (no # download), since the docker image will have downloaded these tools beforehand. +# # For the case of gapic-generator-java, no env var will be exported for the # upstream flow, but instead it will be assigned a default filename that will be -# referenced by the file `library_generation/gapic-generator-java-wrapper`. +# referenced by the file `library_generation/gapic-generator-java-wrapper`. Note +# that the wrapped jar can be either downloaded from Maven or be obtained from +# the docker container. If the generator version matches +# DOCKER_GAPIC_GENERATOR_VERSION, then the baked jar will be transferred into +# the output folder so the wrapper can pick it up. download_tools() { local gapic_generator_version=$1 local protoc_version=$2 From 1cf63eafdcc7c4dd6cca235f0e2e7ecb3fc48ac8 Mon Sep 17 00:00:00 2001 From: diegomarquezp Date: Thu, 8 Aug 2024 19:54:19 +0000 Subject: [PATCH 16/63] adapt tests --- library_generation/model/generation_config.py | 25 ++++++++----------- .../generate_pr_description_unit_tests.py | 9 ------- .../test/generate_repo_unit_tests.py | 1 - .../test/model/config_change_unit_tests.py | 1 - .../model/generation_config_unit_tests.py | 6 ----- .../baseline_generation_config.yaml | 1 - .../current_generation_config.yaml | 1 - .../java-bigtable/generation_config.yaml | 1 - .../config_without_api_description.yaml | 1 - .../config_without_api_shortname.yaml | 1 - .../config_without_gapics_key.yaml | 1 - .../config_without_gapics_value.yaml | 1 - .../config_without_googleapis.yaml | 1 - .../test-config/config_without_libraries.yaml | 2 +- .../config_without_library_value.yaml | 1 - .../config_without_name_pretty.yaml | 1 - .../config_without_product_docs.yaml | 1 - .../config_without_proto_path.yaml | 1 - .../config_without_temp_excludes.yaml | 1 - .../test-config/generation_config.yaml | 1 - ...on_config_with_duplicate_library_name.yaml | 1 - .../test/utilities_unit_tests.py | 1 - .../commit_message_formatter_unit_tests.py | 5 +--- ...generation_config_comparator_unit_tests.py | 16 ------------ 24 files changed, 12 insertions(+), 69 deletions(-) diff --git a/library_generation/model/generation_config.py b/library_generation/model/generation_config.py index e7c1012d12..2f93d49b8e 100644 --- a/library_generation/model/generation_config.py +++ b/library_generation/model/generation_config.py @@ -26,6 +26,7 @@ GAPIC_GENERATOR_VERSION_CONFIG_KEY = "gapic_generator_version" LIBRARIES_BOM_VERSION = "libraries_bom_version" GENERATOR_VERSION_ENV_KEY = "DOCKER_GAPIC_GENERATOR_VERSION" +GENERATOR_LOCATION_ENV_KEY = "DOCKER_GAPIC_GENERATOR_LOCATION" class GenerationConfig: @@ -37,7 +38,6 @@ def __init__( self, googleapis_commitish: str, libraries: list[LibraryConfig], - gapic_generator_version: Optional[str] = None, libraries_bom_version: Optional[str] = None, grpc_version: Optional[str] = None, protoc_version: Optional[str] = None, @@ -46,9 +46,7 @@ def __init__( self.libraries_bom_version = ( libraries_bom_version if libraries_bom_version else "" ) - self.gapic_generator_version = GenerationConfig.__set_generator_version( - gapic_generator_version - ) + self.gapic_generator_version = GenerationConfig.__get_generator_version() self.libraries = libraries self.grpc_version = grpc_version self.protoc_version = protoc_version @@ -82,19 +80,17 @@ def contains_common_protos(self) -> bool: return self.__contains_common_protos @staticmethod - def __set_generator_version(gapic_generator_version: Optional[str]) -> str: - if gapic_generator_version is not None: - return gapic_generator_version - # if the generator version is not set through generation config, - # get it from environment variable. + def __get_generator_version() -> str: gapic_generator_version = os.getenv(GENERATOR_VERSION_ENV_KEY) if not gapic_generator_version: raise ValueError( - f"gapic_generator_version was not specified in the " - f"configuration yaml and the fall-back env var " - f"{GENERATOR_VERSION_ENV_KEY} was not found. At " - f"least one of them must be set to proceed with the " - f"generation." + f"The env var {GENERATOR_VERSION_ENV_KEY} was not found." + f"This variable is required to decide which generator to run" + ) + if not os.getenv(GENERATOR_LOCATION_ENV_KEY): + raise ValueError( + f"The env var {GENERATOR_LOCATION_ENV_KEY} was not found." + f"This variable is required to determine the generator jar location" ) return gapic_generator_version @@ -173,7 +169,6 @@ def from_yaml(path_to_yaml: str) -> GenerationConfig: googleapis_commitish=__required( config, "googleapis_commitish", REPO_LEVEL_PARAMETER ), - gapic_generator_version=__optional(config, GAPIC_GENERATOR_VERSION_CONFIG_KEY, None), grpc_version=__optional(config, "grpc_version", None), protoc_version=__optional(config, "protoc_version", None), libraries_bom_version=__optional(config, LIBRARIES_BOM_VERSION, None), diff --git a/library_generation/test/generate_pr_description_unit_tests.py b/library_generation/test/generate_pr_description_unit_tests.py index b727cc3da9..9d4cfcc9b7 100644 --- a/library_generation/test/generate_pr_description_unit_tests.py +++ b/library_generation/test/generate_pr_description_unit_tests.py @@ -69,7 +69,6 @@ def test_get_commit_messages_with_same_current_and_baseline_returns_empty_messag def test_generate_pr_description_with_no_change_in_config(self): commit_sha = "36441693dddaf0ed73951ad3a15c215a332756aa" config = GenerationConfig( - gapic_generator_version="", googleapis_commitish=commit_sha, libraries_bom_version="", # use empty libraries to make sure no qualified commit between @@ -99,14 +98,12 @@ def test_generate_pr_description_does_not_create_pr_description_without_qualifie config_change=ConfigChange( change_to_libraries={}, baseline_config=GenerationConfig( - gapic_generator_version="", googleapis_commitish=old_commit_sha, # use empty libraries to make sure no qualified commit between # two commit sha. libraries=[], ), current_config=GenerationConfig( - gapic_generator_version="", googleapis_commitish=new_commit_sha, # use empty libraries to make sure no qualified commit between # two commit sha. @@ -135,10 +132,6 @@ def test_generate_pr_description_with_combined_message( config_change=ConfigChange( change_to_libraries={ ChangeType.REPO_LEVEL_CHANGE: [ - LibraryChange( - changed_param="gapic_generator_version", - current_value="1.2.3", - ), LibraryChange( changed_param="libraries_bom_version", current_value="2.3.4" ), @@ -146,12 +139,10 @@ def test_generate_pr_description_with_combined_message( ChangeType.GOOGLEAPIS_COMMIT: [], }, baseline_config=GenerationConfig( - gapic_generator_version="", googleapis_commitish=baseline_commit_sha, libraries=[library], ), current_config=GenerationConfig( - gapic_generator_version="1.2.3", googleapis_commitish=documentai_commit_sha, libraries_bom_version="2.3.4", libraries=[library], diff --git a/library_generation/test/generate_repo_unit_tests.py b/library_generation/test/generate_repo_unit_tests.py index 6085c237a6..046c05fc00 100644 --- a/library_generation/test/generate_repo_unit_tests.py +++ b/library_generation/test/generate_repo_unit_tests.py @@ -43,7 +43,6 @@ def test_get_target_library_given_null_returns_all_libraries(self): @staticmethod def __get_an_empty_generation_config() -> GenerationConfig: return GenerationConfig( - gapic_generator_version="", googleapis_commitish="", libraries=[], ) diff --git a/library_generation/test/model/config_change_unit_tests.py b/library_generation/test/model/config_change_unit_tests.py index 147753ec99..022370dbf7 100644 --- a/library_generation/test/model/config_change_unit_tests.py +++ b/library_generation/test/model/config_change_unit_tests.py @@ -291,7 +291,6 @@ def __get_a_gen_config( if libraries is None: libraries = [] return GenerationConfig( - gapic_generator_version="", googleapis_commitish=googleapis_commitish, grpc_version="", protoc_version="", diff --git a/library_generation/test/model/generation_config_unit_tests.py b/library_generation/test/model/generation_config_unit_tests.py index f3efe73390..714113ea41 100644 --- a/library_generation/test/model/generation_config_unit_tests.py +++ b/library_generation/test/model/generation_config_unit_tests.py @@ -47,7 +47,6 @@ class GenerationConfigTest(unittest.TestCase): def test_generation_config_default_value(self): config = GenerationConfig( - gapic_generator_version="", googleapis_commitish="", libraries=[], ) @@ -122,7 +121,6 @@ def test_get_proto_path_to_library_name_success(self): def test_is_monorepo_with_one_library_returns_false(self): config = GenerationConfig( - gapic_generator_version="", googleapis_commitish="", libraries=[library_1], ) @@ -130,7 +128,6 @@ def test_is_monorepo_with_one_library_returns_false(self): def test_is_monorepo_with_two_libraries_returns_true(self): config = GenerationConfig( - gapic_generator_version="", googleapis_commitish="", libraries=[library_1, library_2], ) @@ -138,7 +135,6 @@ def test_is_monorepo_with_two_libraries_returns_true(self): def test_contains_common_protos_with_common_protos_returns_true(self): config = GenerationConfig( - gapic_generator_version="", googleapis_commitish="", libraries=[library_1, library_2, common_protos_library], ) @@ -146,7 +142,6 @@ def test_contains_common_protos_with_common_protos_returns_true(self): def test_contains_common_protos_without_common_protos_returns_false(self): config = GenerationConfig( - gapic_generator_version="", googleapis_commitish="", libraries=[library_1, library_2], ) @@ -157,7 +152,6 @@ def test_validate_with_duplicate_library_name_raise_exception(self): ValueError, "the same library name", GenerationConfig, - gapic_generator_version="", googleapis_commitish="", libraries=[ LibraryConfig( diff --git a/library_generation/test/resources/integration/google-cloud-java/baseline_generation_config.yaml b/library_generation/test/resources/integration/google-cloud-java/baseline_generation_config.yaml index 9a27dd381d..5a4037165a 100644 --- a/library_generation/test/resources/integration/google-cloud-java/baseline_generation_config.yaml +++ b/library_generation/test/resources/integration/google-cloud-java/baseline_generation_config.yaml @@ -1,4 +1,3 @@ -gapic_generator_version: 2.38.1 protoc_version: 25.2 googleapis_commitish: a17d4caf184b050d50cacf2b0d579ce72c31ce74 libraries_bom_version: 26.37.0 diff --git a/library_generation/test/resources/integration/google-cloud-java/current_generation_config.yaml b/library_generation/test/resources/integration/google-cloud-java/current_generation_config.yaml index adc1d170d6..4119df8782 100644 --- a/library_generation/test/resources/integration/google-cloud-java/current_generation_config.yaml +++ b/library_generation/test/resources/integration/google-cloud-java/current_generation_config.yaml @@ -1,4 +1,3 @@ -gapic_generator_version: 2.38.1 protoc_version: 25.2 googleapis_commitish: 4ce0ff67a3d4509be641cbe47a35844ddc1268fc libraries_bom_version: 26.37.0 diff --git a/library_generation/test/resources/integration/java-bigtable/generation_config.yaml b/library_generation/test/resources/integration/java-bigtable/generation_config.yaml index c6912c8125..7865e9a685 100644 --- a/library_generation/test/resources/integration/java-bigtable/generation_config.yaml +++ b/library_generation/test/resources/integration/java-bigtable/generation_config.yaml @@ -1,4 +1,3 @@ -gapic_generator_version: 2.37.0 protoc_version: 25.2 googleapis_commitish: 9868a57470a969ffa1d21194a5c05d7a6e4e98cc libraries: diff --git a/library_generation/test/resources/test-config/config_without_api_description.yaml b/library_generation/test/resources/test-config/config_without_api_description.yaml index 79ff135067..b93bab256f 100644 --- a/library_generation/test/resources/test-config/config_without_api_description.yaml +++ b/library_generation/test/resources/test-config/config_without_api_description.yaml @@ -1,4 +1,3 @@ -gapic_generator_version: 2.34.0 libraries: - api_shortname: apigeeconnect GAPICs: diff --git a/library_generation/test/resources/test-config/config_without_api_shortname.yaml b/library_generation/test/resources/test-config/config_without_api_shortname.yaml index ec8206be61..28253d55d3 100644 --- a/library_generation/test/resources/test-config/config_without_api_shortname.yaml +++ b/library_generation/test/resources/test-config/config_without_api_shortname.yaml @@ -1,4 +1,3 @@ -gapic_generator_version: 2.34.0 libraries: - GAPICs: - proto_path: google/cloud/apigeeconnect/v1 diff --git a/library_generation/test/resources/test-config/config_without_gapics_key.yaml b/library_generation/test/resources/test-config/config_without_gapics_key.yaml index 739a4d9239..a06cf1277a 100644 --- a/library_generation/test/resources/test-config/config_without_gapics_key.yaml +++ b/library_generation/test/resources/test-config/config_without_gapics_key.yaml @@ -1,3 +1,2 @@ -gapic_generator_version: 2.34.0 libraries: - api_shortname: apigeeconnect diff --git a/library_generation/test/resources/test-config/config_without_gapics_value.yaml b/library_generation/test/resources/test-config/config_without_gapics_value.yaml index ec49e4a669..9477c31ce3 100644 --- a/library_generation/test/resources/test-config/config_without_gapics_value.yaml +++ b/library_generation/test/resources/test-config/config_without_gapics_value.yaml @@ -1,4 +1,3 @@ -gapic_generator_version: 2.34.0 libraries: - api_shortname: apigeeconnect GAPICs: diff --git a/library_generation/test/resources/test-config/config_without_googleapis.yaml b/library_generation/test/resources/test-config/config_without_googleapis.yaml index e5a00ca4ee..c78b8b9700 100644 --- a/library_generation/test/resources/test-config/config_without_googleapis.yaml +++ b/library_generation/test/resources/test-config/config_without_googleapis.yaml @@ -1,4 +1,3 @@ -gapic_generator_version: 2.34.0 libraries: - api_shortname: apigeeconnect name_pretty: Apigee Connect diff --git a/library_generation/test/resources/test-config/config_without_libraries.yaml b/library_generation/test/resources/test-config/config_without_libraries.yaml index dbbe2ea318..b0b9636ce4 100644 --- a/library_generation/test/resources/test-config/config_without_libraries.yaml +++ b/library_generation/test/resources/test-config/config_without_libraries.yaml @@ -1 +1 @@ -gapic_generator_version: 2.34.0 +protoc_version: 3.25.2 diff --git a/library_generation/test/resources/test-config/config_without_library_value.yaml b/library_generation/test/resources/test-config/config_without_library_value.yaml index 174a293000..9c1dc55eac 100644 --- a/library_generation/test/resources/test-config/config_without_library_value.yaml +++ b/library_generation/test/resources/test-config/config_without_library_value.yaml @@ -1,2 +1 @@ -gapic_generator_version: 2.34.0 libraries: diff --git a/library_generation/test/resources/test-config/config_without_name_pretty.yaml b/library_generation/test/resources/test-config/config_without_name_pretty.yaml index f8612ad9ca..7565a4fca8 100644 --- a/library_generation/test/resources/test-config/config_without_name_pretty.yaml +++ b/library_generation/test/resources/test-config/config_without_name_pretty.yaml @@ -1,4 +1,3 @@ -gapic_generator_version: 2.34.0 libraries: - api_shortname: apigeeconnect api_description: "allows the Apigee hybrid management" diff --git a/library_generation/test/resources/test-config/config_without_product_docs.yaml b/library_generation/test/resources/test-config/config_without_product_docs.yaml index e3921d2c0d..a6c60dbf4c 100644 --- a/library_generation/test/resources/test-config/config_without_product_docs.yaml +++ b/library_generation/test/resources/test-config/config_without_product_docs.yaml @@ -1,4 +1,3 @@ -gapic_generator_version: 2.34.0 libraries: - api_shortname: apigeeconnect name_pretty: Apigee Connect diff --git a/library_generation/test/resources/test-config/config_without_proto_path.yaml b/library_generation/test/resources/test-config/config_without_proto_path.yaml index e37b0cef63..7bf5663a71 100644 --- a/library_generation/test/resources/test-config/config_without_proto_path.yaml +++ b/library_generation/test/resources/test-config/config_without_proto_path.yaml @@ -1,4 +1,3 @@ -gapic_generator_version: 2.34.0 libraries: - GAPICs: - random_key: diff --git a/library_generation/test/resources/test-config/config_without_temp_excludes.yaml b/library_generation/test/resources/test-config/config_without_temp_excludes.yaml index 0d1bb7deea..7225a1c711 100644 --- a/library_generation/test/resources/test-config/config_without_temp_excludes.yaml +++ b/library_generation/test/resources/test-config/config_without_temp_excludes.yaml @@ -1,4 +1,3 @@ -gapic_generator_version: 2.34.0 googleapis_commitish: 1a45bf7393b52407188c82e63101db7dc9c72026 libraries_bom_version: 26.37.0 libraries: diff --git a/library_generation/test/resources/test-config/generation_config.yaml b/library_generation/test/resources/test-config/generation_config.yaml index 168c8fd9a5..c9caf2943b 100644 --- a/library_generation/test/resources/test-config/generation_config.yaml +++ b/library_generation/test/resources/test-config/generation_config.yaml @@ -1,4 +1,3 @@ -gapic_generator_version: 2.34.0 protoc_version: 25.2 googleapis_commitish: 1a45bf7393b52407188c82e63101db7dc9c72026 libraries_bom_version: 26.37.0 diff --git a/library_generation/test/resources/test-config/generation_config_with_duplicate_library_name.yaml b/library_generation/test/resources/test-config/generation_config_with_duplicate_library_name.yaml index c5613f4308..8b48ec964f 100644 --- a/library_generation/test/resources/test-config/generation_config_with_duplicate_library_name.yaml +++ b/library_generation/test/resources/test-config/generation_config_with_duplicate_library_name.yaml @@ -1,4 +1,3 @@ -gapic_generator_version: 2.34.0 protoc_version: 25.2 googleapis_commitish: 1a45bf7393b52407188c82e63101db7dc9c72026 libraries_bom_version: 26.37.0 diff --git a/library_generation/test/utilities_unit_tests.py b/library_generation/test/utilities_unit_tests.py index 7d9f09ec2c..6a4a2d448c 100644 --- a/library_generation/test/utilities_unit_tests.py +++ b/library_generation/test/utilities_unit_tests.py @@ -361,7 +361,6 @@ def __get_a_gen_config( library.extra_versioned_modules = None return GenerationConfig( - gapic_generator_version="", googleapis_commitish="", libraries=libraries, ) diff --git a/library_generation/test/utils/commit_message_formatter_unit_tests.py b/library_generation/test/utils/commit_message_formatter_unit_tests.py index 16e3fffdfc..956ea280ab 100644 --- a/library_generation/test/utils/commit_message_formatter_unit_tests.py +++ b/library_generation/test/utils/commit_message_formatter_unit_tests.py @@ -29,7 +29,7 @@ from library_generation.utils.commit_message_formatter import wrap_override_commit gen_config = GenerationConfig( - gapic_generator_version="1.2.3", googleapis_commitish="123abc", libraries=[] + googleapis_commitish="123abc", libraries=[] ) @@ -169,9 +169,6 @@ def test_format_repo_level_change_success(self): config_change = ConfigChange( change_to_libraries={ ChangeType.REPO_LEVEL_CHANGE: [ - LibraryChange( - changed_param="gapic_generator_version", current_value="1.2.3" - ), LibraryChange( changed_param="libraries_bom_version", current_value="2.3.4" ), diff --git a/library_generation/test/utils/generation_config_comparator_unit_tests.py b/library_generation/test/utils/generation_config_comparator_unit_tests.py index 8d018a7c15..4222b78d0c 100644 --- a/library_generation/test/utils/generation_config_comparator_unit_tests.py +++ b/library_generation/test/utils/generation_config_comparator_unit_tests.py @@ -37,14 +37,12 @@ def setUp(self) -> None: gapic_configs=[], ) self.baseline_config = GenerationConfig( - gapic_generator_version="", googleapis_commitish="", grpc_version="", protoc_version="", libraries=[self.baseline_library], ) self.current_config = GenerationConfig( - gapic_generator_version="", googleapis_commitish="", grpc_version="", protoc_version="", @@ -71,20 +69,6 @@ def test_compare_config_googleapis_update(self): ) self.assertEqual({ChangeType.GOOGLEAPIS_COMMIT: []}, result.change_to_libraries) - def test_compare_config_generator_update(self): - self.baseline_config.gapic_generator_version = "1.2.3" - self.current_config.gapic_generator_version = "1.2.4" - result = compare_config( - baseline_config=self.baseline_config, - current_config=self.current_config, - ) - self.assertTrue( - len(result.change_to_libraries[ChangeType.REPO_LEVEL_CHANGE]) == 1 - ) - config_change = result.change_to_libraries[ChangeType.REPO_LEVEL_CHANGE][0] - self.assertEqual("gapic_generator_version", config_change.changed_param) - self.assertEqual("1.2.4", config_change.current_value) - def test_compare_config_libraries_bom_update(self): self.baseline_config.libraries_bom_version = "26.36.0" self.current_config.libraries_bom_version = "26.37.0" From 74b36d3caa0282a33c332d786cd616a5f21278a6 Mon Sep 17 00:00:00 2001 From: diegomarquezp Date: Thu, 8 Aug 2024 22:23:34 +0000 Subject: [PATCH 17/63] add docker environment for tests --- library_generation/test/test_utils.py | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/library_generation/test/test_utils.py b/library_generation/test/test_utils.py index 890cd95362..a792874817 100644 --- a/library_generation/test/test_utils.py +++ b/library_generation/test/test_utils.py @@ -14,6 +14,8 @@ import unittest from difflib import unified_diff from pathlib import Path +import os +from unittest.mock import patch from typing import List @@ -38,3 +40,19 @@ def cleanup(files: List[str]): path.unlink() elif path.is_dir(): path.rmdir() + +class SimulatedDockerEnvironmentTest(unittest.TestCase): + @classmethod + def setUpClass(cls): + cls.env_patcher = patch.dict(os.environ, { + "DOCKER_GAPIC_GENERATOR_VERSION": "test-version", + "DOCKER_GAPIC_GENERATOR_LOCATION": "test-location", + }) + cls.env_patcher.start() + + super().setUpClass() + + @classmethod + def tearDownClass(cls): + super().tearDownClass() + cls.env_patcher.stop() From e582817562ee327fd029aec8dd9dd47bf7e7cbaa Mon Sep 17 00:00:00 2001 From: diegomarquezp Date: Thu, 8 Aug 2024 22:23:56 +0000 Subject: [PATCH 18/63] adapt generate_pr_description --- library_generation/test/generate_pr_description_unit_tests.py | 3 ++- .../test/resources/goldens/pr_description-golden.txt | 3 --- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/library_generation/test/generate_pr_description_unit_tests.py b/library_generation/test/generate_pr_description_unit_tests.py index 9d4cfcc9b7..c4bd352d82 100644 --- a/library_generation/test/generate_pr_description_unit_tests.py +++ b/library_generation/test/generate_pr_description_unit_tests.py @@ -27,12 +27,13 @@ from library_generation.model.gapic_config import GapicConfig from library_generation.model.generation_config import GenerationConfig from library_generation.model.library_config import LibraryConfig +from library_generation.test.test_utils import SimulatedDockerEnvironmentTest script_dir = os.path.dirname(os.path.realpath(__file__)) resources_dir = os.path.join(script_dir, "resources", "goldens") -class GeneratePrDescriptionTest(unittest.TestCase): +class GeneratePrDescriptionTest(SimulatedDockerEnvironmentTest): def test_get_commit_messages_current_is_older_raise_exception(self): # committed on April 1st, 2024 current_commit = "36441693dddaf0ed73951ad3a15c215a332756aa" diff --git a/library_generation/test/resources/goldens/pr_description-golden.txt b/library_generation/test/resources/goldens/pr_description-golden.txt index 1a0f874936..1910f5d349 100644 --- a/library_generation/test/resources/goldens/pr_description-golden.txt +++ b/library_generation/test/resources/goldens/pr_description-golden.txt @@ -2,9 +2,6 @@ This pull request is generated with proto changes between [googleapis/googleapis BEGIN_COMMIT_OVERRIDE BEGIN_NESTED_COMMIT -fix(deps): update the Java code generator (gapic-generator-java) to 1.2.3 -END_NESTED_COMMIT -BEGIN_NESTED_COMMIT chore: update the libraries_bom version to 2.3.4 END_NESTED_COMMIT BEGIN_NESTED_COMMIT From 82e6b8e4acb95177a004b295d458e5c86e4aced1 Mon Sep 17 00:00:00 2001 From: diegomarquezp Date: Thu, 8 Aug 2024 22:25:28 +0000 Subject: [PATCH 19/63] adapt generate_repo --- library_generation/test/generate_repo_unit_tests.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/library_generation/test/generate_repo_unit_tests.py b/library_generation/test/generate_repo_unit_tests.py index 046c05fc00..ae2e69b6cb 100644 --- a/library_generation/test/generate_repo_unit_tests.py +++ b/library_generation/test/generate_repo_unit_tests.py @@ -17,9 +17,10 @@ from library_generation.generate_repo import get_target_libraries from library_generation.model.generation_config import GenerationConfig from library_generation.model.library_config import LibraryConfig +from library_generation.test.test_utils import SimulatedDockerEnvironmentTest -class GenerateRepoTest(unittest.TestCase): +class GenerateRepoTest(SimulatedDockerEnvironmentTest): def test_get_target_library_returns_selected_libraries(self): one_library = GenerateRepoTest.__get_an_empty_library_config() one_library.api_shortname = "one_library" From 3f0dc7f032621c4273045f5a25acf7c3d3266211 Mon Sep 17 00:00:00 2001 From: diegomarquezp Date: Thu, 8 Aug 2024 22:27:13 +0000 Subject: [PATCH 20/63] adapt cli --- library_generation/test/cli/entry_point_unit_tests.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/library_generation/test/cli/entry_point_unit_tests.py b/library_generation/test/cli/entry_point_unit_tests.py index 55fb583651..d99c42e1a8 100644 --- a/library_generation/test/cli/entry_point_unit_tests.py +++ b/library_generation/test/cli/entry_point_unit_tests.py @@ -18,9 +18,10 @@ script_dir = os.path.dirname(os.path.realpath(__file__)) test_resource_dir = os.path.join(script_dir, "..", "resources", "test-config") +from library_generation.test.test_utils import SimulatedDockerEnvironmentTest -class EntryPointTest(unittest.TestCase): +class EntryPointTest(SimulatedDockerEnvironmentTest): def test_entry_point_without_config_raise_file_exception(self): os.chdir(script_dir) runner = CliRunner() From b599d6f3b9d18c5152f1ec3832ef8c17d3eb5423 Mon Sep 17 00:00:00 2001 From: diegomarquezp Date: Thu, 8 Aug 2024 22:52:42 +0000 Subject: [PATCH 21/63] adapt model --- .../test/model/config_change_unit_tests.py | 3 ++- .../test/model/gapic_config_unit_tests.py | 3 ++- .../test/model/gapic_inputs_unit_tests.py | 3 ++- .../model/generation_config_unit_tests.py | 20 ++++++++++++++----- .../test/model/library_config_unit_tests.py | 3 ++- .../test/model/repo_config_unit_tests.py | 3 ++- 6 files changed, 25 insertions(+), 10 deletions(-) diff --git a/library_generation/test/model/config_change_unit_tests.py b/library_generation/test/model/config_change_unit_tests.py index 022370dbf7..e0de105832 100644 --- a/library_generation/test/model/config_change_unit_tests.py +++ b/library_generation/test/model/config_change_unit_tests.py @@ -19,9 +19,10 @@ from library_generation.model.gapic_config import GapicConfig from library_generation.model.generation_config import GenerationConfig from library_generation.model.library_config import LibraryConfig +from library_generation.test.test_utils import SimulatedDockerEnvironmentTest -class ConfigChangeTest(unittest.TestCase): +class ConfigChangeTest(SimulatedDockerEnvironmentTest): def test_get_changed_libraries_with_repo_level_change_returns_all_libraries_changed( self, ): diff --git a/library_generation/test/model/gapic_config_unit_tests.py b/library_generation/test/model/gapic_config_unit_tests.py index 64d8556648..4953afe483 100644 --- a/library_generation/test/model/gapic_config_unit_tests.py +++ b/library_generation/test/model/gapic_config_unit_tests.py @@ -14,9 +14,10 @@ import unittest from library_generation.model.gapic_config import GapicConfig +from library_generation.test.test_utils import SimulatedDockerEnvironmentTest -class GapicConfigTest(unittest.TestCase): +class GapicConfigTest(SimulatedDockerEnvironmentTest): def test_get_version_returns_none(self): self.assertIsNone(GapicConfig(proto_path="example/dir1/dir2").get_version()) diff --git a/library_generation/test/model/gapic_inputs_unit_tests.py b/library_generation/test/model/gapic_inputs_unit_tests.py index 210d321591..4d77fe827f 100644 --- a/library_generation/test/model/gapic_inputs_unit_tests.py +++ b/library_generation/test/model/gapic_inputs_unit_tests.py @@ -4,13 +4,14 @@ from parameterized import parameterized from library_generation.model.gapic_inputs import parse +from library_generation.test.test_utils import SimulatedDockerEnvironmentTest script_dir = os.path.dirname(os.path.realpath(__file__)) resources_dir = os.path.join(script_dir, "..", "resources") build_file = Path(os.path.join(resources_dir, "misc")).resolve() -class UtilitiesTest(unittest.TestCase): +class UtilitiesTest(SimulatedDockerEnvironmentTest): @parameterized.expand( [ ("BUILD_no_additional_protos.bazel", " "), diff --git a/library_generation/test/model/generation_config_unit_tests.py b/library_generation/test/model/generation_config_unit_tests.py index 714113ea41..c099bdcb01 100644 --- a/library_generation/test/model/generation_config_unit_tests.py +++ b/library_generation/test/model/generation_config_unit_tests.py @@ -16,6 +16,8 @@ from pathlib import Path from library_generation.model.generation_config import from_yaml, GenerationConfig from library_generation.model.library_config import LibraryConfig +from library_generation.test.test_utils import SimulatedDockerEnvironmentTest +from unittest.mock import patch script_dir = os.path.dirname(os.path.realpath(__file__)) resources_dir = os.path.join(script_dir, "..", "resources") @@ -44,7 +46,7 @@ ) -class GenerationConfigTest(unittest.TestCase): +class GenerationConfigTest(SimulatedDockerEnvironmentTest): def test_generation_config_default_value(self): config = GenerationConfig( googleapis_commitish="", @@ -52,27 +54,35 @@ def test_generation_config_default_value(self): ) self.assertEqual("", config.libraries_bom_version) + @patch.dict(os.environ, {}, clear=True) def test_generation_config_with_generator_version_env_raise_exception(self): self.assertRaisesRegex( ValueError, - "the fall-back env var DOCKER_GAPIC_GENERATOR_VERSION was not found", + "env var DOCKER_GAPIC_GENERATOR_VERSION was not found", GenerationConfig, googleapis_commitish="", libraries=[], ) + @patch.dict(os.environ, { + "DOCKER_GAPIC_GENERATOR_VERSION": "1.2.3", + "DOCKER_GAPIC_GENERATOR_LOCATION": "test-location", + }) def test_generation_config_set_generator_version_from_env(self): - os.environ["DOCKER_GAPIC_GENERATOR_VERSION"] = "1.2.3" config = GenerationConfig( googleapis_commitish="", libraries=[], ) self.assertEqual("1.2.3", config.gapic_generator_version) - os.environ.pop("DOCKER_GAPIC_GENERATOR_VERSION") + pass + @patch.dict(os.environ, { + "DOCKER_GAPIC_GENERATOR_VERSION": "1.2.3-env", + "DOCKER_GAPIC_GENERATOR_LOCATION": "test-location", + }) def test_from_yaml_succeeds(self): config = from_yaml(f"{test_config_dir}/generation_config.yaml") - self.assertEqual("2.34.0", config.gapic_generator_version) + self.assertEqual("1.2.3-env", config.gapic_generator_version) self.assertEqual(25.2, config.protoc_version) self.assertEqual( "1a45bf7393b52407188c82e63101db7dc9c72026", config.googleapis_commitish diff --git a/library_generation/test/model/library_config_unit_tests.py b/library_generation/test/model/library_config_unit_tests.py index 5d54737ced..bab3e3646c 100644 --- a/library_generation/test/model/library_config_unit_tests.py +++ b/library_generation/test/model/library_config_unit_tests.py @@ -15,9 +15,10 @@ from library_generation.model.gapic_config import GapicConfig from library_generation.model.library_config import LibraryConfig +from library_generation.test.test_utils import SimulatedDockerEnvironmentTest -class LibraryConfigTest(unittest.TestCase): +class LibraryConfigTest(SimulatedDockerEnvironmentTest): def test_get_library_returns_library_name(self): library = LibraryConfig( api_shortname="secret", diff --git a/library_generation/test/model/repo_config_unit_tests.py b/library_generation/test/model/repo_config_unit_tests.py index 12d28fe254..1bdaf83f0d 100644 --- a/library_generation/test/model/repo_config_unit_tests.py +++ b/library_generation/test/model/repo_config_unit_tests.py @@ -15,12 +15,13 @@ import unittest from library_generation.model.repo_config import RepoConfig +from library_generation.test.test_utils import SimulatedDockerEnvironmentTest script_dir = os.path.dirname(os.path.realpath(__file__)) versions_file = os.path.join(script_dir, "..", "resources", "misc", "versions.txt") -class RepoConfigTest(unittest.TestCase): +class RepoConfigTest(SimulatedDockerEnvironmentTest): def test_get_library_versions_with_existing_library(self): repo_config = RepoConfig( output_folder="test", libraries=dict(), versions_file=versions_file From ddb4ff91f6bf29bbea7bf1c4d49b59edfe2916bc Mon Sep 17 00:00:00 2001 From: diegomarquezp Date: Thu, 8 Aug 2024 23:05:34 +0000 Subject: [PATCH 22/63] adapt utils --- .../utils/commit_message_formatter_unit_tests.py | 14 ++++++-------- .../generation_config_comparator_unit_tests.py | 4 +++- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/library_generation/test/utils/commit_message_formatter_unit_tests.py b/library_generation/test/utils/commit_message_formatter_unit_tests.py index 956ea280ab..ed611f438e 100644 --- a/library_generation/test/utils/commit_message_formatter_unit_tests.py +++ b/library_generation/test/utils/commit_message_formatter_unit_tests.py @@ -27,13 +27,10 @@ ) from library_generation.utils.commit_message_formatter import wrap_googleapis_commit from library_generation.utils.commit_message_formatter import wrap_override_commit - -gen_config = GenerationConfig( - googleapis_commitish="123abc", libraries=[] -) +from library_generation.test.test_utils import SimulatedDockerEnvironmentTest -class CommitMessageFormatterTest(unittest.TestCase): +class CommitMessageFormatterTest(SimulatedDockerEnvironmentTest): def test_format_commit_message_should_add_library_name_for_conventional_commit( self, ): @@ -166,6 +163,10 @@ def test_commit_link_success(self): ) def test_format_repo_level_change_success(self): + + gen_config = GenerationConfig( + googleapis_commitish="123abc", libraries=[] + ) config_change = ConfigChange( change_to_libraries={ ChangeType.REPO_LEVEL_CHANGE: [ @@ -182,9 +183,6 @@ def test_format_repo_level_change_success(self): ) self.assertEqual( [ - "BEGIN_NESTED_COMMIT", - "fix(deps): update the Java code generator (gapic-generator-java) to 1.2.3", - "END_NESTED_COMMIT", "BEGIN_NESTED_COMMIT", "chore: update the libraries_bom version to 2.3.4", "END_NESTED_COMMIT", diff --git a/library_generation/test/utils/generation_config_comparator_unit_tests.py b/library_generation/test/utils/generation_config_comparator_unit_tests.py index 4222b78d0c..4b1e29e9f9 100644 --- a/library_generation/test/utils/generation_config_comparator_unit_tests.py +++ b/library_generation/test/utils/generation_config_comparator_unit_tests.py @@ -18,9 +18,11 @@ from library_generation.model.library_config import LibraryConfig from library_generation.utils.generation_config_comparator import ChangeType from library_generation.utils.generation_config_comparator import compare_config +from library_generation.test.test_utils import SimulatedDockerEnvironmentTest -class GenerationConfigComparatorTest(unittest.TestCase): + +class GenerationConfigComparatorTest(SimulatedDockerEnvironmentTest): def setUp(self) -> None: self.baseline_library = LibraryConfig( api_shortname="existing_library", From 6d35e7f4bcfdbd9699065d794ce856c2aca01b27 Mon Sep 17 00:00:00 2001 From: diegomarquezp Date: Thu, 8 Aug 2024 23:08:41 +0000 Subject: [PATCH 23/63] adapt utilities --- library_generation/test/utilities_unit_tests.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/library_generation/test/utilities_unit_tests.py b/library_generation/test/utilities_unit_tests.py index 6a4a2d448c..530d3426c5 100644 --- a/library_generation/test/utilities_unit_tests.py +++ b/library_generation/test/utilities_unit_tests.py @@ -28,6 +28,7 @@ from library_generation.model.library_config import LibraryConfig from library_generation.test.test_utils import FileComparator from library_generation.test.test_utils import cleanup +from library_generation.test.test_utils import SimulatedDockerEnvironmentTest script_dir = os.path.dirname(os.path.realpath(__file__)) resources_dir = os.path.join(script_dir, "resources") @@ -68,7 +69,7 @@ ) -class UtilitiesTest(unittest.TestCase): +class UtilitiesTest(SimulatedDockerEnvironmentTest): """ Unit tests for utilities.py """ From d08e739a3f4516d29aaa33107ed6305467e436b9 Mon Sep 17 00:00:00 2001 From: diegomarquezp Date: Thu, 8 Aug 2024 23:09:05 +0000 Subject: [PATCH 24/63] black --- .../model/generation_config_unit_tests.py | 22 ++++++++++++------- library_generation/test/test_utils.py | 12 ++++++---- .../commit_message_formatter_unit_tests.py | 4 +--- ...generation_config_comparator_unit_tests.py | 1 - 4 files changed, 23 insertions(+), 16 deletions(-) diff --git a/library_generation/test/model/generation_config_unit_tests.py b/library_generation/test/model/generation_config_unit_tests.py index c099bdcb01..1d0b49d936 100644 --- a/library_generation/test/model/generation_config_unit_tests.py +++ b/library_generation/test/model/generation_config_unit_tests.py @@ -64,10 +64,13 @@ def test_generation_config_with_generator_version_env_raise_exception(self): libraries=[], ) - @patch.dict(os.environ, { - "DOCKER_GAPIC_GENERATOR_VERSION": "1.2.3", - "DOCKER_GAPIC_GENERATOR_LOCATION": "test-location", - }) + @patch.dict( + os.environ, + { + "DOCKER_GAPIC_GENERATOR_VERSION": "1.2.3", + "DOCKER_GAPIC_GENERATOR_LOCATION": "test-location", + }, + ) def test_generation_config_set_generator_version_from_env(self): config = GenerationConfig( googleapis_commitish="", @@ -76,10 +79,13 @@ def test_generation_config_set_generator_version_from_env(self): self.assertEqual("1.2.3", config.gapic_generator_version) pass - @patch.dict(os.environ, { - "DOCKER_GAPIC_GENERATOR_VERSION": "1.2.3-env", - "DOCKER_GAPIC_GENERATOR_LOCATION": "test-location", - }) + @patch.dict( + os.environ, + { + "DOCKER_GAPIC_GENERATOR_VERSION": "1.2.3-env", + "DOCKER_GAPIC_GENERATOR_LOCATION": "test-location", + }, + ) def test_from_yaml_succeeds(self): config = from_yaml(f"{test_config_dir}/generation_config.yaml") self.assertEqual("1.2.3-env", config.gapic_generator_version) diff --git a/library_generation/test/test_utils.py b/library_generation/test/test_utils.py index a792874817..4c95070080 100644 --- a/library_generation/test/test_utils.py +++ b/library_generation/test/test_utils.py @@ -41,13 +41,17 @@ def cleanup(files: List[str]): elif path.is_dir(): path.rmdir() + class SimulatedDockerEnvironmentTest(unittest.TestCase): @classmethod def setUpClass(cls): - cls.env_patcher = patch.dict(os.environ, { - "DOCKER_GAPIC_GENERATOR_VERSION": "test-version", - "DOCKER_GAPIC_GENERATOR_LOCATION": "test-location", - }) + cls.env_patcher = patch.dict( + os.environ, + { + "DOCKER_GAPIC_GENERATOR_VERSION": "test-version", + "DOCKER_GAPIC_GENERATOR_LOCATION": "test-location", + }, + ) cls.env_patcher.start() super().setUpClass() diff --git a/library_generation/test/utils/commit_message_formatter_unit_tests.py b/library_generation/test/utils/commit_message_formatter_unit_tests.py index ed611f438e..dc3896d1f1 100644 --- a/library_generation/test/utils/commit_message_formatter_unit_tests.py +++ b/library_generation/test/utils/commit_message_formatter_unit_tests.py @@ -164,9 +164,7 @@ def test_commit_link_success(self): def test_format_repo_level_change_success(self): - gen_config = GenerationConfig( - googleapis_commitish="123abc", libraries=[] - ) + gen_config = GenerationConfig(googleapis_commitish="123abc", libraries=[]) config_change = ConfigChange( change_to_libraries={ ChangeType.REPO_LEVEL_CHANGE: [ diff --git a/library_generation/test/utils/generation_config_comparator_unit_tests.py b/library_generation/test/utils/generation_config_comparator_unit_tests.py index 4b1e29e9f9..5c3f21b04a 100644 --- a/library_generation/test/utils/generation_config_comparator_unit_tests.py +++ b/library_generation/test/utils/generation_config_comparator_unit_tests.py @@ -21,7 +21,6 @@ from library_generation.test.test_utils import SimulatedDockerEnvironmentTest - class GenerationConfigComparatorTest(SimulatedDockerEnvironmentTest): def setUp(self) -> None: self.baseline_library = LibraryConfig( From eea978d4f882ff353106c473fb36edc6bafb8c58 Mon Sep 17 00:00:00 2001 From: diegomarquezp Date: Fri, 9 Aug 2024 01:34:27 +0000 Subject: [PATCH 25/63] simplify generator fetching logic --- library_generation/generate_library.sh | 2 +- .../test/generate_library_unit_tests.sh | 21 +++++++----- library_generation/utils/utilities.sh | 34 ++++++++----------- 3 files changed, 29 insertions(+), 28 deletions(-) diff --git a/library_generation/generate_library.sh b/library_generation/generate_library.sh index 1ed76fd525..ebf7d3340b 100755 --- a/library_generation/generate_library.sh +++ b/library_generation/generate_library.sh @@ -186,7 +186,7 @@ esac # download gapic-generator-java, protobuf and grpc plugin. # the download_tools function will create the environment variables "protoc_path" # and "grpc_path", to be used in the protoc calls below. -download_tools "${gapic_generator_version}" "${protoc_version}" "${grpc_version}" "${os_architecture}" +download_tools "${protoc_version}" "${grpc_version}" "${os_architecture}" ##################### Section 1 ##################### # generate grpc-*/ ##################################################### diff --git a/library_generation/test/generate_library_unit_tests.sh b/library_generation/test/generate_library_unit_tests.sh index 90128d2dfa..c581071acf 100755 --- a/library_generation/test/generate_library_unit_tests.sh +++ b/library_generation/test/generate_library_unit_tests.sh @@ -167,23 +167,26 @@ download_tools_succeed_with_baked_protoc() { export DOCKER_PROTOC_LOCATION=$(mktemp -d) export DOCKER_PROTOC_VERSION="99.99" export output_folder=$(get_output_folder) + generator_folder=$(mktemp -d) + touch "${generator_folder}/gapic-generator-java.fakejar" + export DOCKER_GAPIC_GENERATOR_LOCATION="${generator_folder}/gapic-generator-java.fakejar" mkdir "${output_folder}" local protoc_bin_folder="${DOCKER_PROTOC_LOCATION}/protoc-99.99/bin" mkdir -p "${protoc_bin_folder}" - local test_ggj_version="2.40.0" local test_grpc_version="1.64.0" # we expect download_tools to decide to use DOCKER_PROTOC_LOCATION because # the protoc version we want to download is the same as DOCKER_PROTOC_VERSION. # Note that `protoc_bin_folder` is just the expected formatted value that # download_tools will format using DOCKER_PROTOC_VERSION (via # download_protoc). - download_tools "${test_ggj_version}" "99.99" "${test_grpc_version}" "linux-x86_64" + download_tools "99.99" "${test_grpc_version}" "linux-x86_64" assertEquals "${protoc_bin_folder}" "${protoc_path}" rm -rdf "${output_folder}" unset DOCKER_PROTOC_LOCATION unset DOCKER_PROTOC_VERSION + unset DOCKER_GAPIC_GENERATOR_LOCATION unset output_folder unset protoc_path } @@ -195,19 +198,22 @@ download_tools_succeed_with_baked_grpc() { pushd "${test_dir}" export DOCKER_GRPC_LOCATION=$(mktemp -d) export DOCKER_GRPC_VERSION="99.99" + generator_folder=$(mktemp -d) + touch "${generator_folder}/gapic-generator-java.fakejar" + export DOCKER_GAPIC_GENERATOR_LOCATION="${generator_folder}/gapic-generator-java.fakejar" export output_folder=$(get_output_folder) mkdir "${output_folder}" - local test_ggj_version="2.40.0" local test_protoc_version="1.64.0" # we expect download_tools to decide to use DOCKER_GRPC_LOCATION because # the protoc version we want to download is the same as DOCKER_GRPC_VERSION - download_tools "${test_ggj_version}" "${test_protoc_version}" "99.99" "linux-x86_64" + download_tools "${test_protoc_version}" "99.99" "linux-x86_64" assertEquals "${DOCKER_GRPC_LOCATION}" "${grpc_path}" rm -rdf "${output_folder}" unset DOCKER_GRPC_LOCATION unset DOCKER_GRPC_VERSION + unset DOCKER_GAPIC_GENERATOR_LOCATION unset output_folder unset grpc_path } @@ -221,7 +227,6 @@ download_tools_succeed_with_baked_generator() { generator_folder=$(mktemp -d) touch "${generator_folder}/gapic-generator-java.fakejar" export DOCKER_GAPIC_GENERATOR_LOCATION="${generator_folder}/gapic-generator-java.fakejar" - export DOCKER_GAPIC_GENERATOR_VERSION="99.99" export output_folder=$(get_output_folder) mkdir "${output_folder}" @@ -229,7 +234,7 @@ download_tools_succeed_with_baked_generator() { local test_grpc_version="1.64.0" # we expect download_tools to decide to use DOCKER_GAPIC_GENERATOR_LOCATION because # the protoc version we want to download is the same as DOCKER_GRPC_VERSION - log=$(download_tools "99.99" "${test_protoc_version}" "${test_grpc_version}" "linux-x86_64" 2>&1 1>/dev/null) + log=$(download_tools "${test_protoc_version}" "${test_grpc_version}" "linux-x86_64" 2>&1 1>/dev/null) # the assertion functions are designed to be called only once per function. # Here we do two manual tests that are then coverged into a single @@ -237,7 +242,7 @@ download_tools_succeed_with_baked_generator() { has_expected_log="false" has_expected_file="false" is_output_correct="false" - if grep -q "Using gapic-generator-java version baked into the container: 99.99" <<< "${log}"; then + if grep -q "Using gapic-generator-java version baked into the container" <<< "${log}"; then has_expected_log="true" fi if [[ -f "${output_folder}/gapic-generator-java.fakejar" ]]; then @@ -251,7 +256,7 @@ download_tools_succeed_with_baked_generator() { rm -rdf "${output_folder}" unset DOCKER_GRPC_LOCATION - unset DOCKER_GRPC_VERSION + unset DOCKER_GAPIC_GENERATOR_LOCATION unset output_folder unset grpc_path } diff --git a/library_generation/utils/utilities.sh b/library_generation/utils/utilities.sh index 76d39105c6..e223941a5d 100755 --- a/library_generation/utils/utilities.sh +++ b/library_generation/utils/utilities.sh @@ -153,15 +153,12 @@ get_protoc_version() { # For the case of gapic-generator-java, no env var will be exported for the # upstream flow, but instead it will be assigned a default filename that will be # referenced by the file `library_generation/gapic-generator-java-wrapper`. Note -# that the wrapped jar can be either downloaded from Maven or be obtained from -# the docker container. If the generator version matches -# DOCKER_GAPIC_GENERATOR_VERSION, then the baked jar will be transferred into -# the output folder so the wrapper can pick it up. +# that we assume an env var DOCKER_GAPIC_GENERATOR_LOCATION to point to the jar +# so we can copy it to our output folder download_tools() { - local gapic_generator_version=$1 - local protoc_version=$2 - local grpc_version=$3 - local os_architecture=$4 + local protoc_version=$1 + local grpc_version=$2 + local os_architecture=$3 pushd "${output_folder}" # the variable protoc_path is used in generate_library.sh. It is explicitly @@ -183,19 +180,18 @@ download_tools() { export grpc_path=$(download_grpc_plugin "${grpc_version}" "${os_architecture}") fi - # similar case with gapic-generator-java - if [[ "${DOCKER_GAPIC_GENERATOR_VERSION}" == "${gapic_generator_version}" ]]; then - # if the specified gapic_generator_version matches the one baked in the docker - # container, we copy the generator jar into the output folder so it can be - # picked up by gapic-generator-java-wrapper - >&2 echo "Using gapic-generator-java version baked into the container: ${DOCKER_GAPIC_GENERATOR_VERSION}" - cp "${DOCKER_GAPIC_GENERATOR_LOCATION}" "${output_folder}" + # prepare gapic-generator-java + # We will assume the generator location environment variable will point to the + # location of the generator JAR and then we will simply copy it. Although + # these env vars are already validated upstream in the python scripts, we + # still validate them here for testing purposes + if [[ -z "${DOCKER_GAPIC_GENERATOR_LOCATION}" ]]; then + >&2 echo "Env var DOCKER_GAPIC_GENERATOR_LOCATION must be set and pointing to the generator jar" + exit 1 else - download_generator_artifact \ - "${gapic_generator_version}" \ - "gapic-generator-java-${gapic_generator_version}.jar" + >&2 echo "Using gapic-generator-java version baked into the container" + cp "${DOCKER_GAPIC_GENERATOR_LOCATION}" "${output_folder}" fi - popd } From cdb137ec1283648a761aaff8aa1e9ad0890658a4 Mon Sep 17 00:00:00 2001 From: diegomarquezp Date: Fri, 9 Aug 2024 01:43:36 +0000 Subject: [PATCH 26/63] update documentation --- library_generation/DEVELOPMENT.md | 34 +++++++++++-------------------- library_generation/README.md | 2 -- 2 files changed, 12 insertions(+), 24 deletions(-) diff --git a/library_generation/DEVELOPMENT.md b/library_generation/DEVELOPMENT.md index 39992824fc..d7fbc49730 100644 --- a/library_generation/DEVELOPMENT.md +++ b/library_generation/DEVELOPMENT.md @@ -53,28 +53,6 @@ run them directly. This section explains how to run the entrypoint script In order to run the generation scripts directly, there are a few tools we need to install beforehand. -### Install synthtool - -It requires python 3.x to be installed. -You will need to specify a committish of the synthtool repo in order to have -your generation results matching exactly what the docker image would produce. -You can achieve this by inspecting `SYNTHTOOL_COMMITISH` in -`.cloudbuild/library_generation/library_generation.Dockerfile`. - -```bash -# obtained from .cloudbuild/library_generation/library_generation.Dockerfile -export SYNTHTOOL_COMMITTISH=6612ab8f3afcd5e292aecd647f0fa68812c9f5b5 -``` - -```bash -git clone https://github.com/googleapis/synthtool -cd synthtool -git checkout "${SYNTHTOOL_COMMITTISH}" -python -m pip install --require-hashes -r requirements.txt -python -m pip install --no-deps -e . -python -m synthtool --version -``` - ### Install the owl-bot CLI Requires node.js to be installed. @@ -93,6 +71,18 @@ owl-bot copy-code --version The key step is `npm link`, which will make the command available in you current shell session. +### Create the gapic-generator-java jar + +Run `cd sdk-platform-java && mvn install -DskipTests -Dclirr.skip +-Dcheckstyle.skip`. This will generate a jar located in +`~/.m2/repository/com/google/api/gapic-generator-java/{version}/gapic-generator-java-{version}.jar` + +Then `export` an environment variable +`DOCKER_GAPIC_GENERATOR_VERSION=${version}` and +`DOCKER_GAPIC_GENERATOR_LOCATION=path/to/generated/jar` + +These two env vars are necessary to simulate the docker image environment. + ## Running the script The entrypoint script (`library_generation/cli/entry_point.py`) allows you to update the target repository with the latest changes starting from the diff --git a/library_generation/README.md b/library_generation/README.md index bead08e290..fd6ea082e0 100644 --- a/library_generation/README.md +++ b/library_generation/README.md @@ -93,7 +93,6 @@ They are shared by library level parameters. | Name | Required | Notes | |:------------------------|:--------:|:---------------------------------------------| -| gapic_generator_version | No | set through env variable if not specified | | protoc_version | No | inferred from the generator if not specified | | grpc_version | No | inferred from the generator if not specified | | googleapis-commitish | Yes | | @@ -144,7 +143,6 @@ The GAPIC level parameters define how to generate a GAPIC library. ### An example of generation configuration ```yaml -gapic_generator_version: 2.34.0 protoc_version: 25.2 googleapis_commitish: 1a45bf7393b52407188c82e63101db7dc9c72026 libraries_bom_version: 26.37.0 From 9b50a733762f8402b84b3aa0c52fa4d8ea560b3d Mon Sep 17 00:00:00 2001 From: diegomarquezp Date: Fri, 9 Aug 2024 01:43:56 +0000 Subject: [PATCH 27/63] make generate_pr_desc executable --- library_generation/generate_pr_description.py | 0 1 file changed, 0 insertions(+), 0 deletions(-) mode change 100644 => 100755 library_generation/generate_pr_description.py diff --git a/library_generation/generate_pr_description.py b/library_generation/generate_pr_description.py old mode 100644 new mode 100755 From 4ac8caf4c2110b8f09249da488005608c86bb33c Mon Sep 17 00:00:00 2001 From: diegomarquezp Date: Fri, 9 Aug 2024 01:48:20 +0000 Subject: [PATCH 28/63] remove m2 mapping --- .github/scripts/hermetic_library_generation.sh | 1 - 1 file changed, 1 deletion(-) diff --git a/.github/scripts/hermetic_library_generation.sh b/.github/scripts/hermetic_library_generation.sh index cf29c87b3f..cb4b34e3fd 100755 --- a/.github/scripts/hermetic_library_generation.sh +++ b/.github/scripts/hermetic_library_generation.sh @@ -103,7 +103,6 @@ docker run \ --rm \ -u "$(id -u):$(id -g)" \ -v "$(pwd):${workspace_name}" \ - -v "$HOME"/.m2:/home/.m2 \ gcr.io/cloud-devrel-public-resources/java-library-generation:"${image_tag}" \ --baseline-generation-config-path="${workspace_name}/${baseline_generation_config}" \ --current-generation-config-path="${workspace_name}/${generation_config}" From 4a57a7d8d6908f76500e0614391db1a4007b3f88 Mon Sep 17 00:00:00 2001 From: diegomarquezp Date: Fri, 9 Aug 2024 02:18:03 +0000 Subject: [PATCH 29/63] remove gapic_generator_version --- .../gapic-generator-java-wrapper | 2 +- .../generate_composed_library.py | 1 - library_generation/generate_library.sh | 16 +--- library_generation/model/generation_config.py | 14 +--- .../scripts/update_generation_config.sh | 6 +- .../test/generate_library_unit_tests.sh | 73 +++++-------------- .../model/generation_config_unit_tests.py | 25 +------ .../utils/commit_message_formatter.py | 6 +- library_generation/utils/utilities.sh | 58 +++------------ 9 files changed, 38 insertions(+), 163 deletions(-) diff --git a/library_generation/gapic-generator-java-wrapper b/library_generation/gapic-generator-java-wrapper index 3e26a675c0..14d03aa6ea 100755 --- a/library_generation/gapic-generator-java-wrapper +++ b/library_generation/gapic-generator-java-wrapper @@ -3,4 +3,4 @@ set -e # Wrap gapic-generator-java.jar because protoc requires the plugin to be executable. -exec java -classpath "gapic-generator-java-${gapic_generator_version}.jar" com.google.api.generator.Main +exec java -classpath "gapic-generator-java.jar" com.google.api.generator.Main diff --git a/library_generation/generate_composed_library.py b/library_generation/generate_composed_library.py index 5b503450a9..2fd014f791 100755 --- a/library_generation/generate_composed_library.py +++ b/library_generation/generate_composed_library.py @@ -131,7 +131,6 @@ def __construct_tooling_arg(config: GenerationConfig) -> List[str]: :return: arguments containing tooling versions """ arguments = [] - arguments += util.create_argument("gapic_generator_version", config) arguments += util.create_argument("grpc_version", config) arguments += util.create_argument("protoc_version", config) diff --git a/library_generation/generate_library.sh b/library_generation/generate_library.sh index ebf7d3340b..c28b7f72a9 100755 --- a/library_generation/generate_library.sh +++ b/library_generation/generate_library.sh @@ -14,10 +14,6 @@ case $key in destination_path="$2" shift ;; - --gapic_generator_version) - gapic_generator_version="$2" - shift - ;; --protoc_version) protoc_version="$2" shift @@ -75,17 +71,12 @@ script_dir=$(dirname "$(readlink -f "$0")") source "${script_dir}"/utils/utilities.sh output_folder="$(get_output_folder)" -if [[ -z "${gapic_generator_version}" ]] ; then - echo 'missing required argument --gapic_generator_version' - exit 1 -fi - if [ -z "${protoc_version}" ]; then - protoc_version=$(get_protoc_version "${gapic_generator_version}") + protoc_version=$(get_protoc_version) fi if [ -z "${grpc_version}" ]; then - grpc_version=$(get_grpc_version "${gapic_generator_version}") + grpc_version=$(get_grpc_version) fi if [ -z "${proto_only}" ]; then @@ -124,9 +115,6 @@ if [ -z "${os_architecture}" ]; then os_architecture=$(detect_os_architecture) fi -# export this variable so that it can be used in gapic-generator-java-wrapper.sh -export gapic_generator_version - temp_destination_path="${output_folder}/temp_preprocessed" mkdir -p "${output_folder}/${destination_path}" if [ -d "${temp_destination_path}" ]; then diff --git a/library_generation/model/generation_config.py b/library_generation/model/generation_config.py index 2f93d49b8e..00129ff76b 100644 --- a/library_generation/model/generation_config.py +++ b/library_generation/model/generation_config.py @@ -23,7 +23,6 @@ LIBRARY_LEVEL_PARAMETER = "Library level parameter" GAPIC_LEVEL_PARAMETER = "GAPIC level parameter" COMMON_PROTOS_LIBRARY_NAME = "common-protos" -GAPIC_GENERATOR_VERSION_CONFIG_KEY = "gapic_generator_version" LIBRARIES_BOM_VERSION = "libraries_bom_version" GENERATOR_VERSION_ENV_KEY = "DOCKER_GAPIC_GENERATOR_VERSION" GENERATOR_LOCATION_ENV_KEY = "DOCKER_GAPIC_GENERATOR_LOCATION" @@ -42,11 +41,13 @@ def __init__( grpc_version: Optional[str] = None, protoc_version: Optional[str] = None, ): + # we confirm the necessary env var pointing to the generator jar + # location is set + GenerationConfig.__validate_generator_env() self.googleapis_commitish = googleapis_commitish self.libraries_bom_version = ( libraries_bom_version if libraries_bom_version else "" ) - self.gapic_generator_version = GenerationConfig.__get_generator_version() self.libraries = libraries self.grpc_version = grpc_version self.protoc_version = protoc_version @@ -80,19 +81,12 @@ def contains_common_protos(self) -> bool: return self.__contains_common_protos @staticmethod - def __get_generator_version() -> str: - gapic_generator_version = os.getenv(GENERATOR_VERSION_ENV_KEY) - if not gapic_generator_version: - raise ValueError( - f"The env var {GENERATOR_VERSION_ENV_KEY} was not found." - f"This variable is required to decide which generator to run" - ) + def __validate_generator_env() -> None: if not os.getenv(GENERATOR_LOCATION_ENV_KEY): raise ValueError( f"The env var {GENERATOR_LOCATION_ENV_KEY} was not found." f"This variable is required to determine the generator jar location" ) - return gapic_generator_version def __validate(self) -> None: seen_library_names = dict() diff --git a/library_generation/owlbot/templates/java_library/.github/scripts/update_generation_config.sh b/library_generation/owlbot/templates/java_library/.github/scripts/update_generation_config.sh index 561a313040..364694f484 100644 --- a/library_generation/owlbot/templates/java_library/.github/scripts/update_generation_config.sh +++ b/library_generation/owlbot/templates/java_library/.github/scripts/update_generation_config.sh @@ -1,7 +1,7 @@ #!/bin/bash set -e # This script should be run at the root of the repository. -# This script is used to update googleapis_commitish, gapic_generator_version, +# This script is used to update googleapis_commitish # and libraries_bom_version in generation configuration at the time of running # and create a pull request. @@ -94,10 +94,6 @@ popd rm -rf tmp-googleapis update_config "googleapis_commitish" "${latest_commit}" "${generation_config}" -# update gapic-generator-java version to the latest -latest_version=$(get_latest_released_version "com.google.api" "gapic-generator-java") -update_config "gapic_generator_version" "${latest_version}" "${generation_config}" - # update libraries-bom version to the latest latest_version=$(get_latest_released_version "com.google.cloud" "libraries-bom") update_config "libraries_bom_version" "${latest_version}" "${generation_config}" diff --git a/library_generation/test/generate_library_unit_tests.sh b/library_generation/test/generate_library_unit_tests.sh index c581071acf..273065c233 100755 --- a/library_generation/test/generate_library_unit_tests.sh +++ b/library_generation/test/generate_library_unit_tests.sh @@ -15,17 +15,13 @@ extract_folder_name_test() { assertEquals "google-cloud-aiplatform-v1-java" "${folder_name}" } -get_grpc_version_succeed_with_valid_generator_version_test() { - local actual_version - actual_version=$(get_grpc_version "2.24.0") - rm "gapic-generator-java-pom-parent-2.24.0.pom" - assertEquals "1.56.1" "${actual_version}" -} - -get_grpc_version_failed_with_invalid_generator_version_test() { - local res=0 - $(get_grpc_version "1.99.0") || res=$? - assertEquals 1 $((res)) +get_grpc_version_fails_with_no_env_var_test() { + # the absence of DOCKER_GRPC_VERSION will make this function to fail + exit_code=0 + ( + get_grpc_version + ) || exit_code=$? + assertEquals 1 "${exit_code}" } get_grpc_version_succeed_docker_env_var_test() { @@ -33,7 +29,7 @@ get_grpc_version_succeed_docker_env_var_test() { local version_without_docker export DOCKER_GRPC_VERSION="9.9.9" # get_grpc_version should prioritize DOCKER_GRPC_VERSION - version_with_docker=$(get_grpc_version "2.24.0") + version_with_docker=$(get_grpc_version) assertEquals "${DOCKER_GRPC_VERSION}" "${version_with_docker}" unset DOCKER_GRPC_VERSION } @@ -43,22 +39,18 @@ get_protoc_version_succeed_docker_env_var_test() { local version_without_docker export DOCKER_PROTOC_VERSION="9.9.9" # get_protoc_version should prioritize DOCKER_PROTOC_VERSION - version_with_docker=$(get_protoc_version "2.24.0") + version_with_docker=$(get_protoc_version) assertEquals "${DOCKER_PROTOC_VERSION}" "${version_with_docker}" unset DOCKER_PROTOC_VERSION } -get_protoc_version_succeed_with_valid_generator_version_test() { - local actual_version - actual_version=$(get_protoc_version "2.24.0") - assertEquals "23.2" "${actual_version}" - rm "gapic-generator-java-pom-parent-2.24.0.pom" -} - -get_protoc_version_failed_with_invalid_generator_version_test() { - local res=0 - $(get_protoc_version "1.99.0") || res=$? - assertEquals 1 $((res)) +get_protoc_version_fails_with_no_env_var_test() { + # the absence of DOCKER_PROTOC_VERSION will make this function to fail + exit_code=0 + ( + get_protoc_version + ) || exit_code=$? + assertEquals 1 "${exit_code}" } get_gapic_opts_with_rest_test() { @@ -108,28 +100,6 @@ remove_grpc_version_test() { rm "${destination_path}/QueryServiceGrpc.java" } -download_generator_success_with_valid_version_test() { - local version="2.24.0" - local artifact="gapic-generator-java-${version}.jar" - download_generator_artifact "${version}" "${artifact}" - assertFileOrDirectoryExists "${artifact}" - rm "${artifact}" -} - -download_generator_failed_with_invalid_version_test() { - # The download function will exit the shell - # if download failed. Test the exit code instead of - # downloaded file (there will be no downloaded file). - # Use $() to execute the function in subshell so that - # the other tests can continue executing in the current - # shell. - local res=0 - local version="1.99.0" - local artifact="gapic-generator-java-${version}.jar" - $(download_generator_artifact "${version}" "${artifact}") || res=$? - assertEquals 1 $((res)) -} - download_protoc_succeed_with_valid_version_linux_test() { download_protoc "23.2" "linux-x86_64" assertFileOrDirectoryExists "protoc-23.2" @@ -292,7 +262,6 @@ generate_library_failed_with_invalid_generator_version() { bash "${script_dir}"/../generate_library.sh \ -p google/cloud/alloydb/v1 \ -d ../"${destination}" \ - --gapic_generator_version 1.99.0 \ --protoc_version 23.2 \ --grpc_version 1.55.1 \ --transport grpc+rest \ @@ -309,7 +278,6 @@ generate_library_failed_with_invalid_protoc_version() { bash "${script_dir}"/../generate_library.sh \ -p google/cloud/alloydb/v1 \ -d ../"${destination}" \ - --gapic_generator_version 2.24.0 \ --protoc_version 22.99 \ --grpc_version 1.55.1 \ --transport grpc+rest \ @@ -326,7 +294,6 @@ generate_library_failed_with_invalid_grpc_version() { bash "${script_dir}"/../generate_library.sh \ -p google/cloud/alloydb/v1 \ -d ../output/"${destination}" \ - --gapic_generator_version 2.24.0 \ --grpc_version 0.99.0 \ --transport grpc+rest \ --rest_numeric_enums true || res=$? @@ -381,18 +348,14 @@ get_proto_path_from_preprocessed_sources_multiple_proto_dirs_fails() { # One line per test. test_list=( extract_folder_name_test - get_grpc_version_succeed_with_valid_generator_version_test - get_grpc_version_failed_with_invalid_generator_version_test + get_grpc_version_fails_with_no_env_var_test get_grpc_version_succeed_docker_env_var_test get_protoc_version_succeed_docker_env_var_test - get_protoc_version_succeed_with_valid_generator_version_test - get_protoc_version_failed_with_invalid_generator_version_test + get_protoc_version_fails_with_no_env_var_test get_gapic_opts_with_rest_test get_gapic_opts_without_rest_test get_gapic_opts_with_non_default_test remove_grpc_version_test - download_generator_success_with_valid_version_test - download_generator_failed_with_invalid_version_test download_protoc_succeed_with_valid_version_linux_test download_protoc_succeed_with_valid_version_macos_test download_protoc_failed_with_invalid_version_linux_test diff --git a/library_generation/test/model/generation_config_unit_tests.py b/library_generation/test/model/generation_config_unit_tests.py index 1d0b49d936..5a42b44421 100644 --- a/library_generation/test/model/generation_config_unit_tests.py +++ b/library_generation/test/model/generation_config_unit_tests.py @@ -58,37 +58,14 @@ def test_generation_config_default_value(self): def test_generation_config_with_generator_version_env_raise_exception(self): self.assertRaisesRegex( ValueError, - "env var DOCKER_GAPIC_GENERATOR_VERSION was not found", + "env var DOCKER_GAPIC_GENERATOR_LOCATION was not found", GenerationConfig, googleapis_commitish="", libraries=[], ) - @patch.dict( - os.environ, - { - "DOCKER_GAPIC_GENERATOR_VERSION": "1.2.3", - "DOCKER_GAPIC_GENERATOR_LOCATION": "test-location", - }, - ) - def test_generation_config_set_generator_version_from_env(self): - config = GenerationConfig( - googleapis_commitish="", - libraries=[], - ) - self.assertEqual("1.2.3", config.gapic_generator_version) - pass - - @patch.dict( - os.environ, - { - "DOCKER_GAPIC_GENERATOR_VERSION": "1.2.3-env", - "DOCKER_GAPIC_GENERATOR_LOCATION": "test-location", - }, - ) def test_from_yaml_succeeds(self): config = from_yaml(f"{test_config_dir}/generation_config.yaml") - self.assertEqual("1.2.3-env", config.gapic_generator_version) self.assertEqual(25.2, config.protoc_version) self.assertEqual( "1a45bf7393b52407188c82e63101db7dc9c72026", config.googleapis_commitish diff --git a/library_generation/utils/commit_message_formatter.py b/library_generation/utils/commit_message_formatter.py index 33d0d4d322..4da40496da 100644 --- a/library_generation/utils/commit_message_formatter.py +++ b/library_generation/utils/commit_message_formatter.py @@ -15,13 +15,9 @@ from git import Commit from library_generation.model.config_change import ConfigChange, ChangeType -from library_generation.model.generation_config import ( - GAPIC_GENERATOR_VERSION_CONFIG_KEY, - LIBRARIES_BOM_VERSION, -) +from library_generation.model.generation_config import LIBRARIES_BOM_VERSION PARAM_TO_COMMIT_MESSAGE = { - GAPIC_GENERATOR_VERSION_CONFIG_KEY: "fix(deps): update the Java code generator (gapic-generator-java) to", LIBRARIES_BOM_VERSION: "chore: update the libraries_bom version to", } diff --git a/library_generation/utils/utilities.sh b/library_generation/utils/utilities.sh index e223941a5d..e5879fd910 100755 --- a/library_generation/utils/utilities.sh +++ b/library_generation/utils/utilities.sh @@ -97,49 +97,34 @@ remove_grpc_version() { sed -i.bak 's/value = \"by gRPC proto compiler.*/value = \"by gRPC proto compiler\",/g' {} \; -exec rm {}.bak \; } -download_gapic_generator_pom_parent() { - local gapic_generator_version=$1 - download_generator_artifact "${gapic_generator_version}" "gapic-generator-java-pom-parent-${gapic_generator_version}.pom" "gapic-generator-java-pom-parent" -} - # This function returns the version of the grpc plugin to generate the libraries. If -# DOCKER_GRPC_VERSION is set, this will be the version. Otherwise, it will be -# computed from the gapic-generator-pom-parent artifact at the specified -# gapic_generator_version. +# DOCKER_GRPC_VERSION is set, this will be the version. Otherwise, the script +# will exit since this is a necessary env var get_grpc_version() { - local gapic_generator_version=$1 local grpc_version if [[ -n "${DOCKER_GRPC_VERSION}" ]]; then >&2 echo "Using grpc version baked into the container: ${DOCKER_GRPC_VERSION}" echo "${DOCKER_GRPC_VERSION}" return + else + >&2 echo "Cannot infer grpc version because DOCKER_GRPC_VERSION is not set" + exit 1 fi - pushd "${output_folder}" > /dev/null - # get grpc version from gapic-generator-java-pom-parent/pom.xml - download_gapic_generator_pom_parent "${gapic_generator_version}" - grpc_version=$(grep grpc.version "gapic-generator-java-pom-parent-${gapic_generator_version}.pom" | sed 's/\(.*\)<\/grpc\.version>/\1/' | sed 's/^[[:space:]]*//;s/[[:space:]]*$//') - popd > /dev/null - echo "${grpc_version}" } # This function returns the version of protoc to generate the libraries. If -# DOCKER_PROTOC_VERSION is set, this will be the version. Otherwise, it will be -# computed from the gapic-generator-pom-parent artifact at the specified -# gapic_generator_version. +# DOCKER_PROTOC_VERSION is set, this will be the version. Otherwise, the script +# will exit since this is a necessary env var get_protoc_version() { - local gapic_generator_version=$1 local protoc_version if [[ -n "${DOCKER_PROTOC_VERSION}" ]]; then >&2 echo "Using protoc version baked into the container: ${DOCKER_PROTOC_VERSION}" echo "${DOCKER_PROTOC_VERSION}" return + else + >&2 echo "Cannot infer protoc version because DOCKER_GRPC_VERSION is not set" + exit 1 fi - pushd "${output_folder}" > /dev/null - # get protobuf version from gapic-generator-java-pom-parent/pom.xml - download_gapic_generator_pom_parent "${gapic_generator_version}" - protoc_version=$(grep protobuf.version "gapic-generator-java-pom-parent-${gapic_generator_version}.pom" | sed 's/\(.*\)<\/protobuf\.version>/\1/' | sed 's/^[[:space:]]*//;s/[[:space:]]*$//' | cut -d "." -f2-) - popd > /dev/null - echo "${protoc_version}" } # Given the versions of the gapic generator, protoc and the protoc-grpc plugin, @@ -195,29 +180,6 @@ download_tools() { popd } -download_generator_artifact() { - local gapic_generator_version=$1 - local artifact=$2 - local project=${3:-"gapic-generator-java"} - if [ ! -f "${artifact}" ]; then - # first, try to fetch the generator locally - local local_fetch_successful - local_fetch_successful=$(copy_from "$HOME/.m2/repository/com/google/api/${project}/${gapic_generator_version}/${artifact}" \ - "${artifact}") - if [[ "${local_fetch_successful}" == "false" ]];then - # download gapic-generator-java artifact from Google maven central mirror if not - # found locally - >&2 echo "${artifact} not found locally. Attempting a download from Maven Central" - download_from \ - "https://maven-central.storage-download.googleapis.com/maven2/com/google/api/${project}/${gapic_generator_version}/${artifact}" \ - "${artifact}" - >&2 echo "${artifact} found and downloaded from Maven Central" - else - >&2 echo "${artifact} found copied from local repository (~/.m2)" - fi - fi -} - download_protoc() { local protoc_version=$1 local os_architecture=$2 From 11646942935ea5c0654a8d4b6ec2ff13e551dcee Mon Sep 17 00:00:00 2001 From: diegomarquezp Date: Fri, 9 Aug 2024 02:20:11 +0000 Subject: [PATCH 30/63] remove unnecessary version in Dockerfile --- .../library_generation/library_generation.Dockerfile | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/.cloudbuild/library_generation/library_generation.Dockerfile b/.cloudbuild/library_generation/library_generation.Dockerfile index 42a4a630a1..adbf9d6227 100644 --- a/.cloudbuild/library_generation/library_generation.Dockerfile +++ b/.cloudbuild/library_generation/library_generation.Dockerfile @@ -20,10 +20,12 @@ WORKDIR /sdk-platform-java COPY . . # {x-version-update-start:gapic-generator-java:current} ENV DOCKER_GAPIC_GENERATOR_VERSION="2.43.1-SNAPSHOT" +# {x-version-update-end:gapic-generator-java:current} RUN mvn install -DskipTests -Dclirr.skip -Dcheckstyle.skip RUN ls "/root/.m2/repository/com/google/api/gapic-generator-java/${DOCKER_GAPIC_GENERATOR_VERSION}" -RUN cp "/root/.m2/repository/com/google/api/gapic-generator-java/${DOCKER_GAPIC_GENERATOR_VERSION}/gapic-generator-java-${DOCKER_GAPIC_GENERATOR_VERSION}.jar" . +RUN cp "/root/.m2/repository/com/google/api/gapic-generator-java/${DOCKER_GAPIC_GENERATOR_VERSION}/gapic-generator-java-${DOCKER_GAPIC_GENERATOR_VERSION}.jar" \ + "./gapic-generator-java.jar" # build from the root of this repo: FROM gcr.io/cloud-devrel-public-resources/python @@ -63,11 +65,8 @@ ENV DOCKER_GRPC_VERSION="${GRPC_VERSION}" # we transfer gapic-generator-java from the previous stage. -# here we redeclare this env var since they are not preserved between stages -ENV DOCKER_GAPIC_GENERATOR_VERSION="2.43.1-SNAPSHOT" -# {x-version-update-end:gapic-generator-java:current} -ENV DOCKER_GAPIC_GENERATOR_LOCATION="/gapic-generator-java/gapic-generator-java-${DOCKER_GAPIC_GENERATOR_VERSION}.jar" -COPY --from=ggj-build "/root/.m2/repository/com/google/api/gapic-generator-java/${DOCKER_GAPIC_GENERATOR_VERSION}/gapic-generator-java-${DOCKER_GAPIC_GENERATOR_VERSION}.jar" "${DOCKER_GAPIC_GENERATOR_LOCATION}" +ENV DOCKER_GAPIC_GENERATOR_LOCATION="/gapic-generator-java/gapic-generator-java.jar" +COPY --from=ggj-build "/sdk-platform-java/gapic-generator-java.jar" "${DOCKER_GAPIC_GENERATOR_LOCATION}" RUN chmod 755 "${DOCKER_GAPIC_GENERATOR_LOCATION}" # use python 3.11 (the base image has several python versions; here we define the default one) From 5a4cfdc618d3faef348549583e422590b298406f Mon Sep 17 00:00:00 2001 From: diegomarquezp Date: Tue, 27 Aug 2024 19:25:48 +0000 Subject: [PATCH 31/63] do not use env vars --- .../library_generation.Dockerfile | 11 ++++---- .../gapic-generator-java-wrapper | 5 ++-- library_generation/model/generation_config.py | 13 ---------- .../model/generation_config_unit_tests.py | 10 -------- library_generation/utils/utilities.sh | 25 +++++++++++-------- 5 files changed, 24 insertions(+), 40 deletions(-) diff --git a/.cloudbuild/library_generation/library_generation.Dockerfile b/.cloudbuild/library_generation/library_generation.Dockerfile index adbf9d6227..7920534a3c 100644 --- a/.cloudbuild/library_generation/library_generation.Dockerfile +++ b/.cloudbuild/library_generation/library_generation.Dockerfile @@ -23,7 +23,6 @@ ENV DOCKER_GAPIC_GENERATOR_VERSION="2.43.1-SNAPSHOT" # {x-version-update-end:gapic-generator-java:current} RUN mvn install -DskipTests -Dclirr.skip -Dcheckstyle.skip -RUN ls "/root/.m2/repository/com/google/api/gapic-generator-java/${DOCKER_GAPIC_GENERATOR_VERSION}" RUN cp "/root/.m2/repository/com/google/api/gapic-generator-java/${DOCKER_GAPIC_GENERATOR_VERSION}/gapic-generator-java-${DOCKER_GAPIC_GENERATOR_VERSION}.jar" \ "./gapic-generator-java.jar" @@ -64,10 +63,12 @@ ENV DOCKER_GRPC_LOCATION="/grpc/protoc-gen-grpc-java-${GRPC_VERSION}-${OS_ARCHIT ENV DOCKER_GRPC_VERSION="${GRPC_VERSION}" -# we transfer gapic-generator-java from the previous stage. -ENV DOCKER_GAPIC_GENERATOR_LOCATION="/gapic-generator-java/gapic-generator-java.jar" -COPY --from=ggj-build "/sdk-platform-java/gapic-generator-java.jar" "${DOCKER_GAPIC_GENERATOR_LOCATION}" -RUN chmod 755 "${DOCKER_GAPIC_GENERATOR_LOCATION}" +# Here we transfer gapic-generator-java from the previous stage. +# Note that the destination is a well-known location that will be assumed at runtime +# We hard-code the location string to avoid making it configurable (via ARG) as +# well as to avoid it making it overridable at runtime (via ENV). +COPY --from=ggj-build "/sdk-platform-java/gapic-generator-java.jar" "${HOME}/.library_generation/gapic-generator-java.jar" +RUN chmod 755 "${HOME}/.library_generation/gapic-generator-java.jar" # use python 3.11 (the base image has several python versions; here we define the default one) RUN rm $(which python3) diff --git a/library_generation/gapic-generator-java-wrapper b/library_generation/gapic-generator-java-wrapper index 14d03aa6ea..3f1e7bd2e7 100755 --- a/library_generation/gapic-generator-java-wrapper +++ b/library_generation/gapic-generator-java-wrapper @@ -1,6 +1,7 @@ #!/usr/bin/env bash - set -e +wrapper_dir=$(dirname "$(realpath "${BASH_SOURCE[0]}")") +source "${wrapper_dir}/utils/utilities.sh" # Wrap gapic-generator-java.jar because protoc requires the plugin to be executable. -exec java -classpath "gapic-generator-java.jar" com.google.api.generator.Main +exec java -classpath "$(get_gapic_generator_location)" com.google.api.generator.Main diff --git a/library_generation/model/generation_config.py b/library_generation/model/generation_config.py index 00129ff76b..43aaa642c7 100644 --- a/library_generation/model/generation_config.py +++ b/library_generation/model/generation_config.py @@ -24,8 +24,6 @@ GAPIC_LEVEL_PARAMETER = "GAPIC level parameter" COMMON_PROTOS_LIBRARY_NAME = "common-protos" LIBRARIES_BOM_VERSION = "libraries_bom_version" -GENERATOR_VERSION_ENV_KEY = "DOCKER_GAPIC_GENERATOR_VERSION" -GENERATOR_LOCATION_ENV_KEY = "DOCKER_GAPIC_GENERATOR_LOCATION" class GenerationConfig: @@ -41,9 +39,6 @@ def __init__( grpc_version: Optional[str] = None, protoc_version: Optional[str] = None, ): - # we confirm the necessary env var pointing to the generator jar - # location is set - GenerationConfig.__validate_generator_env() self.googleapis_commitish = googleapis_commitish self.libraries_bom_version = ( libraries_bom_version if libraries_bom_version else "" @@ -80,14 +75,6 @@ def contains_common_protos(self) -> bool: break return self.__contains_common_protos - @staticmethod - def __validate_generator_env() -> None: - if not os.getenv(GENERATOR_LOCATION_ENV_KEY): - raise ValueError( - f"The env var {GENERATOR_LOCATION_ENV_KEY} was not found." - f"This variable is required to determine the generator jar location" - ) - def __validate(self) -> None: seen_library_names = dict() for library in self.libraries: diff --git a/library_generation/test/model/generation_config_unit_tests.py b/library_generation/test/model/generation_config_unit_tests.py index 5a42b44421..4ca8d0c319 100644 --- a/library_generation/test/model/generation_config_unit_tests.py +++ b/library_generation/test/model/generation_config_unit_tests.py @@ -54,16 +54,6 @@ def test_generation_config_default_value(self): ) self.assertEqual("", config.libraries_bom_version) - @patch.dict(os.environ, {}, clear=True) - def test_generation_config_with_generator_version_env_raise_exception(self): - self.assertRaisesRegex( - ValueError, - "env var DOCKER_GAPIC_GENERATOR_LOCATION was not found", - GenerationConfig, - googleapis_commitish="", - libraries=[], - ) - def test_from_yaml_succeeds(self): config = from_yaml(f"{test_config_dir}/generation_config.yaml") self.assertEqual(25.2, config.protoc_version) diff --git a/library_generation/utils/utilities.sh b/library_generation/utils/utilities.sh index e5879fd910..75ca316938 100755 --- a/library_generation/utils/utilities.sh +++ b/library_generation/utils/utilities.sh @@ -2,6 +2,8 @@ set -eo pipefail utilities_script_dir=$(dirname "$(realpath "${BASH_SOURCE[0]}")") +# The $HOME variable is always set in the OS env as per POSIX specification. +GAPIC_GENERATOR_LOCATION="${HOME}/.library_generation/gapic-generator-java.jar" # Utility functions used in `generate_library.sh` and showcase generation. extract_folder_name() { @@ -165,17 +167,17 @@ download_tools() { export grpc_path=$(download_grpc_plugin "${grpc_version}" "${os_architecture}") fi - # prepare gapic-generator-java - # We will assume the generator location environment variable will point to the - # location of the generator JAR and then we will simply copy it. Although - # these env vars are already validated upstream in the python scripts, we - # still validate them here for testing purposes - if [[ -z "${DOCKER_GAPIC_GENERATOR_LOCATION}" ]]; then - >&2 echo "Env var DOCKER_GAPIC_GENERATOR_LOCATION must be set and pointing to the generator jar" + # Here we check whether the jar is stored in the expected location. + # The docker image will prepare the jar in this location. Developers must + # prepare their environment by creating + # $HOME/.library_generation/gapic_generator_java.jar + # This check is meant to ensure integrity of the downstream workflow. (i.e. + # ensure the generator wrapper succeeds) + if [[ ! -f "${GAPIC_GENERATOR_LOCATION}" ]]; then + >&2 echo "File ${GAPIC_GENERATOR_LOCATION} not found in the " + >&2 echo "filesystem. Please configure your environment and store the " + >&2 echo "generator jar in this location" exit 1 - else - >&2 echo "Using gapic-generator-java version baked into the container" - cp "${DOCKER_GAPIC_GENERATOR_LOCATION}" "${output_folder}" fi popd } @@ -376,3 +378,6 @@ download_googleapis_files_and_folders() { cp -r grafeas "${output_folder}" } +get_gapic_generator_location() { + echo "${GAPIC_GENERATOR_LOCATION}" +} From ab7a68490e3d8921417de7238edf5688dc8ed0d6 Mon Sep 17 00:00:00 2001 From: diegomarquezp Date: Tue, 27 Aug 2024 20:15:26 +0000 Subject: [PATCH 32/63] update ggj version --- .cloudbuild/library_generation/library_generation.Dockerfile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.cloudbuild/library_generation/library_generation.Dockerfile b/.cloudbuild/library_generation/library_generation.Dockerfile index 9e54cb9ead..ef788284a9 100644 --- a/.cloudbuild/library_generation/library_generation.Dockerfile +++ b/.cloudbuild/library_generation/library_generation.Dockerfile @@ -19,7 +19,7 @@ FROM gcr.io/cloud-devrel-public-resources/java21 AS ggj-build WORKDIR /sdk-platform-java COPY . . # {x-version-update-start:gapic-generator-java:current} -ENV DOCKER_GAPIC_GENERATOR_VERSION="2.43.1-SNAPSHOT" +ENV DOCKER_GAPIC_GENERATOR_VERSION="2.44.1-SNAPSHOT" # {x-version-update-end:gapic-generator-java:current} RUN mvn install -DskipTests -Dclirr.skip -Dcheckstyle.skip From ddaeaf14a26c0fd64b0736a7ed305f9abfa88909 Mon Sep 17 00:00:00 2001 From: diegomarquezp Date: Tue, 27 Aug 2024 21:01:36 +0000 Subject: [PATCH 33/63] adapt shell tests --- .../test/generate_library_unit_tests.sh | 58 +++++++------------ 1 file changed, 22 insertions(+), 36 deletions(-) diff --git a/library_generation/test/generate_library_unit_tests.sh b/library_generation/test/generate_library_unit_tests.sh index 273065c233..b67e9e7ad6 100755 --- a/library_generation/test/generate_library_unit_tests.sh +++ b/library_generation/test/generate_library_unit_tests.sh @@ -5,7 +5,14 @@ set -xeo pipefail # Unit tests against ../utilities.sh script_dir=$(dirname "$(readlink -f "$0")") source "${script_dir}"/test_utilities.sh -source "${script_dir}"/../utils/utilities.sh + +# we simulate a properly prepared environemnt (i.e. generator jar) in its +# well-known location. Tests confirming the opposite case will make sure this +# environment is restored +readonly SIMULATED_HOME=$(mktemp -d) +mkdir "${SIMULATED_HOME}/.library_generation" +touch "${SIMULATED_HOME}/.library_generation/gapic-generator-java.jar" +HOME="${SIMULATED_HOME}" source "${script_dir}"/../utils/utilities.sh # Unit tests extract_folder_name_test() { @@ -132,14 +139,9 @@ download_tools_succeed_with_baked_protoc() { # the version passed to `download_protoc`, then we will not download protoc # but simply have the variable `protoc_path` pointing to DOCKER_PROTOC_LOCATION # (which we manually created in this test) - local test_dir=$(mktemp -d) - pushd "${test_dir}" export DOCKER_PROTOC_LOCATION=$(mktemp -d) export DOCKER_PROTOC_VERSION="99.99" export output_folder=$(get_output_folder) - generator_folder=$(mktemp -d) - touch "${generator_folder}/gapic-generator-java.fakejar" - export DOCKER_GAPIC_GENERATOR_LOCATION="${generator_folder}/gapic-generator-java.fakejar" mkdir "${output_folder}" local protoc_bin_folder="${DOCKER_PROTOC_LOCATION}/protoc-99.99/bin" mkdir -p "${protoc_bin_folder}" @@ -156,7 +158,6 @@ download_tools_succeed_with_baked_protoc() { rm -rdf "${output_folder}" unset DOCKER_PROTOC_LOCATION unset DOCKER_PROTOC_VERSION - unset DOCKER_GAPIC_GENERATOR_LOCATION unset output_folder unset protoc_path } @@ -164,13 +165,8 @@ download_tools_succeed_with_baked_protoc() { download_tools_succeed_with_baked_grpc() { # This test has the same structure as # download_tools_succeed_with_baked_protoc, but meant for the grpc plugin. - local test_dir=$(mktemp -d) - pushd "${test_dir}" export DOCKER_GRPC_LOCATION=$(mktemp -d) export DOCKER_GRPC_VERSION="99.99" - generator_folder=$(mktemp -d) - touch "${generator_folder}/gapic-generator-java.fakejar" - export DOCKER_GAPIC_GENERATOR_LOCATION="${generator_folder}/gapic-generator-java.fakejar" export output_folder=$(get_output_folder) mkdir "${output_folder}" @@ -183,50 +179,40 @@ download_tools_succeed_with_baked_grpc() { rm -rdf "${output_folder}" unset DOCKER_GRPC_LOCATION unset DOCKER_GRPC_VERSION - unset DOCKER_GAPIC_GENERATOR_LOCATION unset output_folder unset grpc_path } -download_tools_succeed_with_baked_generator() { +download_tools_fails_without_baked_generator() { # This test has the same structure as # download_tools_succeed_with_baked_protoc, but meant for # gapic-generator-java. local test_dir=$(mktemp -d) pushd "${test_dir}" - generator_folder=$(mktemp -d) - touch "${generator_folder}/gapic-generator-java.fakejar" - export DOCKER_GAPIC_GENERATOR_LOCATION="${generator_folder}/gapic-generator-java.fakejar" export output_folder=$(get_output_folder) mkdir "${output_folder}" local test_protoc_version="1.64.0" local test_grpc_version="1.64.0" - # we expect download_tools to decide to use DOCKER_GAPIC_GENERATOR_LOCATION because - # the protoc version we want to download is the same as DOCKER_GRPC_VERSION - log=$(download_tools "${test_protoc_version}" "${test_grpc_version}" "linux-x86_64" 2>&1 1>/dev/null) + local jar_location="${SIMULATED_HOME}/.library_generation/gapic-generator-java.jar" + # we expect the function to fail because the generator jar is not found in + # its well-known location. To achieve this, we temporarily remove the fake + # generator jar + rm "${jar_location}" + log=$(mktemp) + ( + download_tools "${test_protoc_version}" "${test_grpc_version}" "linux-x86_64" > /dev/null 2>"${log}" + ) || echo '' # expected - # the assertion functions are designed to be called only once per function. - # Here we do two manual tests that are then coverged into a single - # pseudo-boolean has_expected_log="false" - has_expected_file="false" - is_output_correct="false" - if grep -q "Using gapic-generator-java version baked into the container" <<< "${log}"; then + if grep -q "Please configure your environment" < "${log}"; then has_expected_log="true" fi - if [[ -f "${output_folder}/gapic-generator-java.fakejar" ]]; then - has_expected_file="true" - fi - if [[ "${has_expected_log}" == "true" ]] && [[ "${has_expected_file}" == "true" ]]; then - is_output_correct="true" - fi - - assertEquals "true" "${is_output_correct}" + assertEquals "true" "${has_expected_log}" rm -rdf "${output_folder}" + touch "${jar_location}" unset DOCKER_GRPC_LOCATION - unset DOCKER_GAPIC_GENERATOR_LOCATION unset output_folder unset grpc_path } @@ -362,7 +348,7 @@ test_list=( download_protoc_failed_with_invalid_arch_test download_tools_succeed_with_baked_protoc download_tools_succeed_with_baked_grpc - download_tools_succeed_with_baked_generator + download_tools_fails_without_baked_generator download_grpc_plugin_succeed_with_valid_version_linux_test download_grpc_plugin_succeed_with_valid_version_macos_test download_grpc_plugin_failed_with_invalid_version_linux_test From 62b281de89b776656dcc835df8deaa3d0270c4cd Mon Sep 17 00:00:00 2001 From: diegomarquezp Date: Thu, 29 Aug 2024 20:08:41 +0000 Subject: [PATCH 34/63] restore hermetic_library_generation script --- .github/scripts/hermetic_library_generation.sh | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/.github/scripts/hermetic_library_generation.sh b/.github/scripts/hermetic_library_generation.sh index f7974b7bc0..bfcbb4a7c5 100755 --- a/.github/scripts/hermetic_library_generation.sh +++ b/.github/scripts/hermetic_library_generation.sh @@ -81,6 +81,9 @@ fi git show "${target_branch}":"${generation_config}" > "${baseline_generation_config}" config_diff=$(diff "${generation_config}" "${baseline_generation_config}" || true) +generator_version=$(mvn help:evaluate -Dexpression=project.version -q -DforceStdout -pl gapic-generator-java) +echo "Local generator version: ${generator_version}" + # install generator locally since we're using a SNAPSHOT version. mvn -V -B -ntp clean install -DskipTests @@ -94,6 +97,8 @@ docker run \ --rm \ -u "$(id -u):$(id -g)" \ -v "$(pwd):${workspace_name}" \ + -v "$HOME"/.m2:/home/.m2 \ + -e GENERATOR_VERSION="${generator_version}" \ gcr.io/cloud-devrel-public-resources/java-library-generation:"${image_tag}" \ --baseline-generation-config-path="${workspace_name}/${baseline_generation_config}" \ --current-generation-config-path="${workspace_name}/${generation_config}" From e0de3c95cc2d8f07d071e3f365600e4a759cc796 Mon Sep 17 00:00:00 2001 From: diegomarquezp Date: Thu, 29 Aug 2024 20:09:05 +0000 Subject: [PATCH 35/63] explain how to setup the generator jar in development guide --- library_generation/DEVELOPMENT.md | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/library_generation/DEVELOPMENT.md b/library_generation/DEVELOPMENT.md index d7fbc49730..d2556dcd9a 100644 --- a/library_generation/DEVELOPMENT.md +++ b/library_generation/DEVELOPMENT.md @@ -77,11 +77,17 @@ Run `cd sdk-platform-java && mvn install -DskipTests -Dclirr.skip -Dcheckstyle.skip`. This will generate a jar located in `~/.m2/repository/com/google/api/gapic-generator-java/{version}/gapic-generator-java-{version}.jar` -Then `export` an environment variable -`DOCKER_GAPIC_GENERATOR_VERSION=${version}` and -`DOCKER_GAPIC_GENERATOR_LOCATION=path/to/generated/jar` +Then `mv` the jar into the well-known location of the jar. The +generation scripts will assume the jar is there. + +```shell +mv /path/to/jar "${HOME}/.library_generation/gapic-generator-java.jar" +``` + +Note that this relies on the `HOME` en var which is always +defined as per +[POSIX env var definition](https://pubs.opengroup.org/onlinepubs/9699919799/basedefs/V1_chap08.html). -These two env vars are necessary to simulate the docker image environment. ## Running the script The entrypoint script (`library_generation/cli/entry_point.py`) allows you to From 1bc545aa37f769749e61cfd03525df18accbae44 Mon Sep 17 00:00:00 2001 From: diegomarquezp Date: Thu, 29 Aug 2024 20:10:21 +0000 Subject: [PATCH 36/63] restore model --- library_generation/model/generation_config.py | 22 +++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/library_generation/model/generation_config.py b/library_generation/model/generation_config.py index 43aaa642c7..bc3ff6d440 100644 --- a/library_generation/model/generation_config.py +++ b/library_generation/model/generation_config.py @@ -23,7 +23,9 @@ LIBRARY_LEVEL_PARAMETER = "Library level parameter" GAPIC_LEVEL_PARAMETER = "GAPIC level parameter" COMMON_PROTOS_LIBRARY_NAME = "common-protos" +GAPIC_GENERATOR_VERSION = "gapic_generator_version" LIBRARIES_BOM_VERSION = "libraries_bom_version" +GENERATOR_VERSION_ENV_KEY = "GENERATOR_VERSION" class GenerationConfig: @@ -35,6 +37,7 @@ def __init__( self, googleapis_commitish: str, libraries: list[LibraryConfig], + gapic_generator_version: Optional[str] = None, libraries_bom_version: Optional[str] = None, grpc_version: Optional[str] = None, protoc_version: Optional[str] = None, @@ -43,6 +46,9 @@ def __init__( self.libraries_bom_version = ( libraries_bom_version if libraries_bom_version else "" ) + self.gapic_generator_version = GenerationConfig.__set_generator_version( + gapic_generator_version + ) self.libraries = libraries self.grpc_version = grpc_version self.protoc_version = protoc_version @@ -75,6 +81,21 @@ def contains_common_protos(self) -> bool: break return self.__contains_common_protos + @staticmethod + def __set_generator_version(gapic_generator_version: Optional[str]) -> str: + if gapic_generator_version is not None: + return gapic_generator_version + # if the generator version is not set through generation config, + # get it from environment variable. + gapic_generator_version = os.getenv(GENERATOR_VERSION_ENV_KEY) + if not gapic_generator_version: + raise ValueError( + f"Environment variable {GENERATOR_VERSION_ENV_KEY}" + f" is not set when the generator version is not" + f" specified in the generation config." + ) + return gapic_generator_version + def __validate(self) -> None: seen_library_names = dict() for library in self.libraries: @@ -150,6 +171,7 @@ def from_yaml(path_to_yaml: str) -> GenerationConfig: googleapis_commitish=__required( config, "googleapis_commitish", REPO_LEVEL_PARAMETER ), + gapic_generator_version=__optional(config, GAPIC_GENERATOR_VERSION, None), grpc_version=__optional(config, "grpc_version", None), protoc_version=__optional(config, "protoc_version", None), libraries_bom_version=__optional(config, LIBRARIES_BOM_VERSION, None), From 9022ba08a492db6cd028e8628a1ef64a00160ee8 Mon Sep 17 00:00:00 2001 From: diegomarquezp Date: Thu, 29 Aug 2024 20:11:55 +0000 Subject: [PATCH 37/63] restore commit message formatter --- library_generation/utils/commit_message_formatter.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/library_generation/utils/commit_message_formatter.py b/library_generation/utils/commit_message_formatter.py index 4da40496da..5b75db51a0 100644 --- a/library_generation/utils/commit_message_formatter.py +++ b/library_generation/utils/commit_message_formatter.py @@ -15,9 +15,13 @@ from git import Commit from library_generation.model.config_change import ConfigChange, ChangeType -from library_generation.model.generation_config import LIBRARIES_BOM_VERSION +from library_generation.model.generation_config import ( + GAPIC_GENERATOR_VERSION, + LIBRARIES_BOM_VERSION, +) PARAM_TO_COMMIT_MESSAGE = { + GAPIC_GENERATOR_VERSION: "fix(deps): update the Java code generator (gapic-generator-java) to", LIBRARIES_BOM_VERSION: "chore: update the libraries_bom version to", } From 6f1e20011c626faba9356454b56db0692765f389 Mon Sep 17 00:00:00 2001 From: diegomarquezp Date: Thu, 29 Aug 2024 20:20:42 +0000 Subject: [PATCH 38/63] restore usage of gapic_generator_version argument --- .../generate_composed_library.py | 1 + library_generation/generate_library.sh | 17 ++++- library_generation/utils/utilities.sh | 62 +++++++++++++++---- 3 files changed, 64 insertions(+), 16 deletions(-) diff --git a/library_generation/generate_composed_library.py b/library_generation/generate_composed_library.py index 2fd014f791..5b503450a9 100755 --- a/library_generation/generate_composed_library.py +++ b/library_generation/generate_composed_library.py @@ -131,6 +131,7 @@ def __construct_tooling_arg(config: GenerationConfig) -> List[str]: :return: arguments containing tooling versions """ arguments = [] + arguments += util.create_argument("gapic_generator_version", config) arguments += util.create_argument("grpc_version", config) arguments += util.create_argument("protoc_version", config) diff --git a/library_generation/generate_library.sh b/library_generation/generate_library.sh index c28b7f72a9..1b113ba11b 100755 --- a/library_generation/generate_library.sh +++ b/library_generation/generate_library.sh @@ -14,6 +14,12 @@ case $key in destination_path="$2" shift ;; + --gapic_generator_version) + gapic_generator_version="$2" + # export this variable so that it can be used in gapic-generator-java-wrapper.sh + export gapic_generator_version + shift + ;; --protoc_version) protoc_version="$2" shift @@ -71,12 +77,17 @@ script_dir=$(dirname "$(readlink -f "$0")") source "${script_dir}"/utils/utilities.sh output_folder="$(get_output_folder)" +if [ -z "${gapic_generator_version}" ]; then + echo 'missing required argument --gapic_generator_version' + exit 1 +fi + if [ -z "${protoc_version}" ]; then - protoc_version=$(get_protoc_version) + protoc_version=$(get_protoc_version "${gapic_generator_version}") fi if [ -z "${grpc_version}" ]; then - grpc_version=$(get_grpc_version) + grpc_version=$(get_grpc_version "${gapic_generator_version}") fi if [ -z "${proto_only}" ]; then @@ -174,7 +185,7 @@ esac # download gapic-generator-java, protobuf and grpc plugin. # the download_tools function will create the environment variables "protoc_path" # and "grpc_path", to be used in the protoc calls below. -download_tools "${protoc_version}" "${grpc_version}" "${os_architecture}" +download_tools "${gapic_generator_version}" "${protoc_version}" "${grpc_version}" "${os_architecture}" ##################### Section 1 ##################### # generate grpc-*/ ##################################################### diff --git a/library_generation/utils/utilities.sh b/library_generation/utils/utilities.sh index 75ca316938..245dec64f3 100755 --- a/library_generation/utils/utilities.sh +++ b/library_generation/utils/utilities.sh @@ -2,8 +2,6 @@ set -eo pipefail utilities_script_dir=$(dirname "$(realpath "${BASH_SOURCE[0]}")") -# The $HOME variable is always set in the OS env as per POSIX specification. -GAPIC_GENERATOR_LOCATION="${HOME}/.library_generation/gapic-generator-java.jar" # Utility functions used in `generate_library.sh` and showcase generation. extract_folder_name() { @@ -99,34 +97,49 @@ remove_grpc_version() { sed -i.bak 's/value = \"by gRPC proto compiler.*/value = \"by gRPC proto compiler\",/g' {} \; -exec rm {}.bak \; } +download_gapic_generator_pom_parent() { + local gapic_generator_version=$1 + download_generator_artifact "${gapic_generator_version}" "gapic-generator-java-pom-parent-${gapic_generator_version}.pom" "gapic-generator-java-pom-parent" +} + # This function returns the version of the grpc plugin to generate the libraries. If -# DOCKER_GRPC_VERSION is set, this will be the version. Otherwise, the script -# will exit since this is a necessary env var +# DOCKER_GRPC_VERSION is set, this will be the version. Otherwise, it will be +# computed from the gapic-generator-pom-parent artifact at the specified +# gapic_generator_version. get_grpc_version() { + local gapic_generator_version=$1 local grpc_version if [[ -n "${DOCKER_GRPC_VERSION}" ]]; then >&2 echo "Using grpc version baked into the container: ${DOCKER_GRPC_VERSION}" echo "${DOCKER_GRPC_VERSION}" return - else - >&2 echo "Cannot infer grpc version because DOCKER_GRPC_VERSION is not set" - exit 1 fi + pushd "${output_folder}" > /dev/null + # get grpc version from gapic-generator-java-pom-parent/pom.xml + download_gapic_generator_pom_parent "${gapic_generator_version}" + grpc_version=$(grep grpc.version "gapic-generator-java-pom-parent-${gapic_generator_version}.pom" | sed 's/\(.*\)<\/grpc\.version>/\1/' | sed 's/^[[:space:]]*//;s/[[:space:]]*$//') + popd > /dev/null + echo "${grpc_version}" } # This function returns the version of protoc to generate the libraries. If -# DOCKER_PROTOC_VERSION is set, this will be the version. Otherwise, the script -# will exit since this is a necessary env var +# DOCKER_PROTOC_VERSION is set, this will be the version. Otherwise, it will be +# computed from the gapic-generator-pom-parent artifact at the specified +# gapic_generator_version. get_protoc_version() { + local gapic_generator_version=$1 local protoc_version if [[ -n "${DOCKER_PROTOC_VERSION}" ]]; then >&2 echo "Using protoc version baked into the container: ${DOCKER_PROTOC_VERSION}" echo "${DOCKER_PROTOC_VERSION}" return - else - >&2 echo "Cannot infer protoc version because DOCKER_GRPC_VERSION is not set" - exit 1 fi + pushd "${output_folder}" > /dev/null + # get protobuf version from gapic-generator-java-pom-parent/pom.xml + download_gapic_generator_pom_parent "${gapic_generator_version}" + protoc_version=$(grep protobuf.version "gapic-generator-java-pom-parent-${gapic_generator_version}.pom" | sed 's/\(.*\)<\/protobuf\.version>/\1/' | sed 's/^[[:space:]]*//;s/[[:space:]]*$//' | cut -d "." -f2-) + popd > /dev/null + echo "${protoc_version}" } # Given the versions of the gapic generator, protoc and the protoc-grpc plugin, @@ -182,6 +195,29 @@ download_tools() { popd } +download_generator_artifact() { + local gapic_generator_version=$1 + local artifact=$2 + local project=${3:-"gapic-generator-java"} + if [ ! -f "${artifact}" ]; then + # first, try to fetch the generator locally + local local_fetch_successful + local_fetch_successful=$(copy_from "$HOME/.m2/repository/com/google/api/${project}/${gapic_generator_version}/${artifact}" \ + "${artifact}") + if [[ "${local_fetch_successful}" == "false" ]];then + # download gapic-generator-java artifact from Google maven central mirror if not + # found locally + >&2 echo "${artifact} not found locally. Attempting a download from Maven Central" + download_from \ + "https://maven-central.storage-download.googleapis.com/maven2/com/google/api/${project}/${gapic_generator_version}/${artifact}" \ + "${artifact}" + >&2 echo "${artifact} found and downloaded from Maven Central" + else + >&2 echo "${artifact} found copied from local repository (~/.m2)" + fi + fi +} + download_protoc() { local protoc_version=$1 local os_architecture=$2 @@ -380,4 +416,4 @@ download_googleapis_files_and_folders() { get_gapic_generator_location() { echo "${GAPIC_GENERATOR_LOCATION}" -} +} \ No newline at end of file From bd7177557ac4c265fb2d22f117ad354faa673b90 Mon Sep 17 00:00:00 2001 From: diegomarquezp Date: Fri, 30 Aug 2024 00:42:35 +0000 Subject: [PATCH 39/63] Revert "restore usage of gapic_generator_version argument" This reverts commit 6f1e20011c626faba9356454b56db0692765f389. --- .../generate_composed_library.py | 1 - library_generation/generate_library.sh | 17 +---- library_generation/utils/utilities.sh | 62 ++++--------------- 3 files changed, 16 insertions(+), 64 deletions(-) diff --git a/library_generation/generate_composed_library.py b/library_generation/generate_composed_library.py index 5b503450a9..2fd014f791 100755 --- a/library_generation/generate_composed_library.py +++ b/library_generation/generate_composed_library.py @@ -131,7 +131,6 @@ def __construct_tooling_arg(config: GenerationConfig) -> List[str]: :return: arguments containing tooling versions """ arguments = [] - arguments += util.create_argument("gapic_generator_version", config) arguments += util.create_argument("grpc_version", config) arguments += util.create_argument("protoc_version", config) diff --git a/library_generation/generate_library.sh b/library_generation/generate_library.sh index 1b113ba11b..c28b7f72a9 100755 --- a/library_generation/generate_library.sh +++ b/library_generation/generate_library.sh @@ -14,12 +14,6 @@ case $key in destination_path="$2" shift ;; - --gapic_generator_version) - gapic_generator_version="$2" - # export this variable so that it can be used in gapic-generator-java-wrapper.sh - export gapic_generator_version - shift - ;; --protoc_version) protoc_version="$2" shift @@ -77,17 +71,12 @@ script_dir=$(dirname "$(readlink -f "$0")") source "${script_dir}"/utils/utilities.sh output_folder="$(get_output_folder)" -if [ -z "${gapic_generator_version}" ]; then - echo 'missing required argument --gapic_generator_version' - exit 1 -fi - if [ -z "${protoc_version}" ]; then - protoc_version=$(get_protoc_version "${gapic_generator_version}") + protoc_version=$(get_protoc_version) fi if [ -z "${grpc_version}" ]; then - grpc_version=$(get_grpc_version "${gapic_generator_version}") + grpc_version=$(get_grpc_version) fi if [ -z "${proto_only}" ]; then @@ -185,7 +174,7 @@ esac # download gapic-generator-java, protobuf and grpc plugin. # the download_tools function will create the environment variables "protoc_path" # and "grpc_path", to be used in the protoc calls below. -download_tools "${gapic_generator_version}" "${protoc_version}" "${grpc_version}" "${os_architecture}" +download_tools "${protoc_version}" "${grpc_version}" "${os_architecture}" ##################### Section 1 ##################### # generate grpc-*/ ##################################################### diff --git a/library_generation/utils/utilities.sh b/library_generation/utils/utilities.sh index 245dec64f3..75ca316938 100755 --- a/library_generation/utils/utilities.sh +++ b/library_generation/utils/utilities.sh @@ -2,6 +2,8 @@ set -eo pipefail utilities_script_dir=$(dirname "$(realpath "${BASH_SOURCE[0]}")") +# The $HOME variable is always set in the OS env as per POSIX specification. +GAPIC_GENERATOR_LOCATION="${HOME}/.library_generation/gapic-generator-java.jar" # Utility functions used in `generate_library.sh` and showcase generation. extract_folder_name() { @@ -97,49 +99,34 @@ remove_grpc_version() { sed -i.bak 's/value = \"by gRPC proto compiler.*/value = \"by gRPC proto compiler\",/g' {} \; -exec rm {}.bak \; } -download_gapic_generator_pom_parent() { - local gapic_generator_version=$1 - download_generator_artifact "${gapic_generator_version}" "gapic-generator-java-pom-parent-${gapic_generator_version}.pom" "gapic-generator-java-pom-parent" -} - # This function returns the version of the grpc plugin to generate the libraries. If -# DOCKER_GRPC_VERSION is set, this will be the version. Otherwise, it will be -# computed from the gapic-generator-pom-parent artifact at the specified -# gapic_generator_version. +# DOCKER_GRPC_VERSION is set, this will be the version. Otherwise, the script +# will exit since this is a necessary env var get_grpc_version() { - local gapic_generator_version=$1 local grpc_version if [[ -n "${DOCKER_GRPC_VERSION}" ]]; then >&2 echo "Using grpc version baked into the container: ${DOCKER_GRPC_VERSION}" echo "${DOCKER_GRPC_VERSION}" return + else + >&2 echo "Cannot infer grpc version because DOCKER_GRPC_VERSION is not set" + exit 1 fi - pushd "${output_folder}" > /dev/null - # get grpc version from gapic-generator-java-pom-parent/pom.xml - download_gapic_generator_pom_parent "${gapic_generator_version}" - grpc_version=$(grep grpc.version "gapic-generator-java-pom-parent-${gapic_generator_version}.pom" | sed 's/\(.*\)<\/grpc\.version>/\1/' | sed 's/^[[:space:]]*//;s/[[:space:]]*$//') - popd > /dev/null - echo "${grpc_version}" } # This function returns the version of protoc to generate the libraries. If -# DOCKER_PROTOC_VERSION is set, this will be the version. Otherwise, it will be -# computed from the gapic-generator-pom-parent artifact at the specified -# gapic_generator_version. +# DOCKER_PROTOC_VERSION is set, this will be the version. Otherwise, the script +# will exit since this is a necessary env var get_protoc_version() { - local gapic_generator_version=$1 local protoc_version if [[ -n "${DOCKER_PROTOC_VERSION}" ]]; then >&2 echo "Using protoc version baked into the container: ${DOCKER_PROTOC_VERSION}" echo "${DOCKER_PROTOC_VERSION}" return + else + >&2 echo "Cannot infer protoc version because DOCKER_GRPC_VERSION is not set" + exit 1 fi - pushd "${output_folder}" > /dev/null - # get protobuf version from gapic-generator-java-pom-parent/pom.xml - download_gapic_generator_pom_parent "${gapic_generator_version}" - protoc_version=$(grep protobuf.version "gapic-generator-java-pom-parent-${gapic_generator_version}.pom" | sed 's/\(.*\)<\/protobuf\.version>/\1/' | sed 's/^[[:space:]]*//;s/[[:space:]]*$//' | cut -d "." -f2-) - popd > /dev/null - echo "${protoc_version}" } # Given the versions of the gapic generator, protoc and the protoc-grpc plugin, @@ -195,29 +182,6 @@ download_tools() { popd } -download_generator_artifact() { - local gapic_generator_version=$1 - local artifact=$2 - local project=${3:-"gapic-generator-java"} - if [ ! -f "${artifact}" ]; then - # first, try to fetch the generator locally - local local_fetch_successful - local_fetch_successful=$(copy_from "$HOME/.m2/repository/com/google/api/${project}/${gapic_generator_version}/${artifact}" \ - "${artifact}") - if [[ "${local_fetch_successful}" == "false" ]];then - # download gapic-generator-java artifact from Google maven central mirror if not - # found locally - >&2 echo "${artifact} not found locally. Attempting a download from Maven Central" - download_from \ - "https://maven-central.storage-download.googleapis.com/maven2/com/google/api/${project}/${gapic_generator_version}/${artifact}" \ - "${artifact}" - >&2 echo "${artifact} found and downloaded from Maven Central" - else - >&2 echo "${artifact} found copied from local repository (~/.m2)" - fi - fi -} - download_protoc() { local protoc_version=$1 local os_architecture=$2 @@ -416,4 +380,4 @@ download_googleapis_files_and_folders() { get_gapic_generator_location() { echo "${GAPIC_GENERATOR_LOCATION}" -} \ No newline at end of file +} From 0e90ddc147bd3ab09438a35b57dffb4f09882cb3 Mon Sep 17 00:00:00 2001 From: diegomarquezp Date: Fri, 30 Aug 2024 01:06:54 +0000 Subject: [PATCH 40/63] fix test --- library_generation/test/test_utils.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/library_generation/test/test_utils.py b/library_generation/test/test_utils.py index 4c95070080..c7ac19bdf6 100644 --- a/library_generation/test/test_utils.py +++ b/library_generation/test/test_utils.py @@ -48,8 +48,8 @@ def setUpClass(cls): cls.env_patcher = patch.dict( os.environ, { - "DOCKER_GAPIC_GENERATOR_VERSION": "test-version", - "DOCKER_GAPIC_GENERATOR_LOCATION": "test-location", + "GENERATOR_VERSION": "test-version", + "GENERATOR_LOCATION": "test-location", }, ) cls.env_patcher.start() From 25f2d7f49bc13d8f492b31d63068b7d5ffff1d5c Mon Sep 17 00:00:00 2001 From: diegomarquezp Date: Fri, 30 Aug 2024 01:07:58 +0000 Subject: [PATCH 41/63] restore readme --- library_generation/README.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/library_generation/README.md b/library_generation/README.md index fd6ea082e0..bead08e290 100644 --- a/library_generation/README.md +++ b/library_generation/README.md @@ -93,6 +93,7 @@ They are shared by library level parameters. | Name | Required | Notes | |:------------------------|:--------:|:---------------------------------------------| +| gapic_generator_version | No | set through env variable if not specified | | protoc_version | No | inferred from the generator if not specified | | grpc_version | No | inferred from the generator if not specified | | googleapis-commitish | Yes | | @@ -143,6 +144,7 @@ The GAPIC level parameters define how to generate a GAPIC library. ### An example of generation configuration ```yaml +gapic_generator_version: 2.34.0 protoc_version: 25.2 googleapis_commitish: 1a45bf7393b52407188c82e63101db7dc9c72026 libraries_bom_version: 26.37.0 From 902f656ae3500474334b7f417e9a26585d8019db Mon Sep 17 00:00:00 2001 From: diegomarquezp Date: Fri, 30 Aug 2024 01:08:56 +0000 Subject: [PATCH 42/63] restore templates --- .../.github/scripts/update_generation_config.sh | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/library_generation/owlbot/templates/java_library/.github/scripts/update_generation_config.sh b/library_generation/owlbot/templates/java_library/.github/scripts/update_generation_config.sh index 364694f484..561a313040 100644 --- a/library_generation/owlbot/templates/java_library/.github/scripts/update_generation_config.sh +++ b/library_generation/owlbot/templates/java_library/.github/scripts/update_generation_config.sh @@ -1,7 +1,7 @@ #!/bin/bash set -e # This script should be run at the root of the repository. -# This script is used to update googleapis_commitish +# This script is used to update googleapis_commitish, gapic_generator_version, # and libraries_bom_version in generation configuration at the time of running # and create a pull request. @@ -94,6 +94,10 @@ popd rm -rf tmp-googleapis update_config "googleapis_commitish" "${latest_commit}" "${generation_config}" +# update gapic-generator-java version to the latest +latest_version=$(get_latest_released_version "com.google.api" "gapic-generator-java") +update_config "gapic_generator_version" "${latest_version}" "${generation_config}" + # update libraries-bom version to the latest latest_version=$(get_latest_released_version "com.google.cloud" "libraries-bom") update_config "libraries_bom_version" "${latest_version}" "${generation_config}" From a06cd0b78e4e70bf41f8ead9050f725b001493fb Mon Sep 17 00:00:00 2001 From: diegomarquezp Date: Fri, 30 Aug 2024 01:16:54 +0000 Subject: [PATCH 43/63] restore pr description tests --- .../test/generate_pr_description_unit_tests.py | 12 ++++++++++-- .../test/resources/goldens/pr_description-golden.txt | 3 +++ 2 files changed, 13 insertions(+), 2 deletions(-) diff --git a/library_generation/test/generate_pr_description_unit_tests.py b/library_generation/test/generate_pr_description_unit_tests.py index c4bd352d82..b727cc3da9 100644 --- a/library_generation/test/generate_pr_description_unit_tests.py +++ b/library_generation/test/generate_pr_description_unit_tests.py @@ -27,13 +27,12 @@ from library_generation.model.gapic_config import GapicConfig from library_generation.model.generation_config import GenerationConfig from library_generation.model.library_config import LibraryConfig -from library_generation.test.test_utils import SimulatedDockerEnvironmentTest script_dir = os.path.dirname(os.path.realpath(__file__)) resources_dir = os.path.join(script_dir, "resources", "goldens") -class GeneratePrDescriptionTest(SimulatedDockerEnvironmentTest): +class GeneratePrDescriptionTest(unittest.TestCase): def test_get_commit_messages_current_is_older_raise_exception(self): # committed on April 1st, 2024 current_commit = "36441693dddaf0ed73951ad3a15c215a332756aa" @@ -70,6 +69,7 @@ def test_get_commit_messages_with_same_current_and_baseline_returns_empty_messag def test_generate_pr_description_with_no_change_in_config(self): commit_sha = "36441693dddaf0ed73951ad3a15c215a332756aa" config = GenerationConfig( + gapic_generator_version="", googleapis_commitish=commit_sha, libraries_bom_version="", # use empty libraries to make sure no qualified commit between @@ -99,12 +99,14 @@ def test_generate_pr_description_does_not_create_pr_description_without_qualifie config_change=ConfigChange( change_to_libraries={}, baseline_config=GenerationConfig( + gapic_generator_version="", googleapis_commitish=old_commit_sha, # use empty libraries to make sure no qualified commit between # two commit sha. libraries=[], ), current_config=GenerationConfig( + gapic_generator_version="", googleapis_commitish=new_commit_sha, # use empty libraries to make sure no qualified commit between # two commit sha. @@ -133,6 +135,10 @@ def test_generate_pr_description_with_combined_message( config_change=ConfigChange( change_to_libraries={ ChangeType.REPO_LEVEL_CHANGE: [ + LibraryChange( + changed_param="gapic_generator_version", + current_value="1.2.3", + ), LibraryChange( changed_param="libraries_bom_version", current_value="2.3.4" ), @@ -140,10 +146,12 @@ def test_generate_pr_description_with_combined_message( ChangeType.GOOGLEAPIS_COMMIT: [], }, baseline_config=GenerationConfig( + gapic_generator_version="", googleapis_commitish=baseline_commit_sha, libraries=[library], ), current_config=GenerationConfig( + gapic_generator_version="1.2.3", googleapis_commitish=documentai_commit_sha, libraries_bom_version="2.3.4", libraries=[library], diff --git a/library_generation/test/resources/goldens/pr_description-golden.txt b/library_generation/test/resources/goldens/pr_description-golden.txt index 1910f5d349..1a0f874936 100644 --- a/library_generation/test/resources/goldens/pr_description-golden.txt +++ b/library_generation/test/resources/goldens/pr_description-golden.txt @@ -2,6 +2,9 @@ This pull request is generated with proto changes between [googleapis/googleapis BEGIN_COMMIT_OVERRIDE BEGIN_NESTED_COMMIT +fix(deps): update the Java code generator (gapic-generator-java) to 1.2.3 +END_NESTED_COMMIT +BEGIN_NESTED_COMMIT chore: update the libraries_bom version to 2.3.4 END_NESTED_COMMIT BEGIN_NESTED_COMMIT From 7194148c86ade09960311c972e449e9a2a344288 Mon Sep 17 00:00:00 2001 From: diegomarquezp Date: Fri, 30 Aug 2024 01:23:49 +0000 Subject: [PATCH 44/63] restore gen config units --- .../model/generation_config_unit_tests.py | 29 +++++++++++++++++-- .../test-config/generation_config.yaml | 1 + 2 files changed, 27 insertions(+), 3 deletions(-) diff --git a/library_generation/test/model/generation_config_unit_tests.py b/library_generation/test/model/generation_config_unit_tests.py index 4ca8d0c319..f6a2f2a2d8 100644 --- a/library_generation/test/model/generation_config_unit_tests.py +++ b/library_generation/test/model/generation_config_unit_tests.py @@ -16,8 +16,6 @@ from pathlib import Path from library_generation.model.generation_config import from_yaml, GenerationConfig from library_generation.model.library_config import LibraryConfig -from library_generation.test.test_utils import SimulatedDockerEnvironmentTest -from unittest.mock import patch script_dir = os.path.dirname(os.path.realpath(__file__)) resources_dir = os.path.join(script_dir, "..", "resources") @@ -46,16 +44,36 @@ ) -class GenerationConfigTest(SimulatedDockerEnvironmentTest): +class GenerationConfigTest(unittest.TestCase): def test_generation_config_default_value(self): config = GenerationConfig( + gapic_generator_version="", googleapis_commitish="", libraries=[], ) self.assertEqual("", config.libraries_bom_version) + def test_generation_config_with_generator_version_env_raise_exception(self): + self.assertRaisesRegex( + ValueError, + "Environment variable GENERATOR_VERSION is not set", + GenerationConfig, + googleapis_commitish="", + libraries=[], + ) + + def test_generation_config_set_generator_version_from_env(self): + os.environ["GENERATOR_VERSION"] = "1.2.3" + config = GenerationConfig( + googleapis_commitish="", + libraries=[], + ) + self.assertEqual("1.2.3", config.gapic_generator_version) + os.environ.pop("GENERATOR_VERSION") + def test_from_yaml_succeeds(self): config = from_yaml(f"{test_config_dir}/generation_config.yaml") + self.assertEqual("2.34.0", config.gapic_generator_version) self.assertEqual(25.2, config.protoc_version) self.assertEqual( "1a45bf7393b52407188c82e63101db7dc9c72026", config.googleapis_commitish @@ -104,6 +122,7 @@ def test_get_proto_path_to_library_name_success(self): def test_is_monorepo_with_one_library_returns_false(self): config = GenerationConfig( + gapic_generator_version="", googleapis_commitish="", libraries=[library_1], ) @@ -111,6 +130,7 @@ def test_is_monorepo_with_one_library_returns_false(self): def test_is_monorepo_with_two_libraries_returns_true(self): config = GenerationConfig( + gapic_generator_version="", googleapis_commitish="", libraries=[library_1, library_2], ) @@ -118,6 +138,7 @@ def test_is_monorepo_with_two_libraries_returns_true(self): def test_contains_common_protos_with_common_protos_returns_true(self): config = GenerationConfig( + gapic_generator_version="", googleapis_commitish="", libraries=[library_1, library_2, common_protos_library], ) @@ -125,6 +146,7 @@ def test_contains_common_protos_with_common_protos_returns_true(self): def test_contains_common_protos_without_common_protos_returns_false(self): config = GenerationConfig( + gapic_generator_version="", googleapis_commitish="", libraries=[library_1, library_2], ) @@ -135,6 +157,7 @@ def test_validate_with_duplicate_library_name_raise_exception(self): ValueError, "the same library name", GenerationConfig, + gapic_generator_version="", googleapis_commitish="", libraries=[ LibraryConfig( diff --git a/library_generation/test/resources/test-config/generation_config.yaml b/library_generation/test/resources/test-config/generation_config.yaml index c9caf2943b..168c8fd9a5 100644 --- a/library_generation/test/resources/test-config/generation_config.yaml +++ b/library_generation/test/resources/test-config/generation_config.yaml @@ -1,3 +1,4 @@ +gapic_generator_version: 2.34.0 protoc_version: 25.2 googleapis_commitish: 1a45bf7393b52407188c82e63101db7dc9c72026 libraries_bom_version: 26.37.0 From 41ab2da46c063d29e3d03156c17df252b4b0b4af Mon Sep 17 00:00:00 2001 From: diegomarquezp Date: Fri, 30 Aug 2024 01:28:35 +0000 Subject: [PATCH 45/63] restore gen config in unit tests --- library_generation/test/cli/entry_point_unit_tests.py | 3 +-- library_generation/test/generate_repo_unit_tests.py | 4 ++-- library_generation/test/model/config_change_unit_tests.py | 4 ++-- library_generation/test/model/gapic_config_unit_tests.py | 3 +-- library_generation/test/model/gapic_inputs_unit_tests.py | 3 +-- library_generation/test/model/library_config_unit_tests.py | 3 +-- library_generation/test/model/repo_config_unit_tests.py | 3 +-- .../google-cloud-java/baseline_generation_config.yaml | 1 + .../google-cloud-java/current_generation_config.yaml | 1 + .../integration/java-bigtable/generation_config.yaml | 1 + .../resources/test-config/config_without_api_description.yaml | 1 + .../resources/test-config/config_without_api_shortname.yaml | 1 + .../test/resources/test-config/config_without_gapics_key.yaml | 1 + .../resources/test-config/config_without_gapics_value.yaml | 1 + .../test/resources/test-config/config_without_googleapis.yaml | 1 + .../test/resources/test-config/config_without_libraries.yaml | 2 +- .../resources/test-config/config_without_library_value.yaml | 1 + .../resources/test-config/config_without_name_pretty.yaml | 1 + .../resources/test-config/config_without_product_docs.yaml | 1 + .../test/resources/test-config/config_without_proto_path.yaml | 1 + .../resources/test-config/config_without_temp_excludes.yaml | 1 + .../generation_config_with_duplicate_library_name.yaml | 1 + 22 files changed, 24 insertions(+), 15 deletions(-) diff --git a/library_generation/test/cli/entry_point_unit_tests.py b/library_generation/test/cli/entry_point_unit_tests.py index d99c42e1a8..55fb583651 100644 --- a/library_generation/test/cli/entry_point_unit_tests.py +++ b/library_generation/test/cli/entry_point_unit_tests.py @@ -18,10 +18,9 @@ script_dir = os.path.dirname(os.path.realpath(__file__)) test_resource_dir = os.path.join(script_dir, "..", "resources", "test-config") -from library_generation.test.test_utils import SimulatedDockerEnvironmentTest -class EntryPointTest(SimulatedDockerEnvironmentTest): +class EntryPointTest(unittest.TestCase): def test_entry_point_without_config_raise_file_exception(self): os.chdir(script_dir) runner = CliRunner() diff --git a/library_generation/test/generate_repo_unit_tests.py b/library_generation/test/generate_repo_unit_tests.py index ae2e69b6cb..6085c237a6 100644 --- a/library_generation/test/generate_repo_unit_tests.py +++ b/library_generation/test/generate_repo_unit_tests.py @@ -17,10 +17,9 @@ from library_generation.generate_repo import get_target_libraries from library_generation.model.generation_config import GenerationConfig from library_generation.model.library_config import LibraryConfig -from library_generation.test.test_utils import SimulatedDockerEnvironmentTest -class GenerateRepoTest(SimulatedDockerEnvironmentTest): +class GenerateRepoTest(unittest.TestCase): def test_get_target_library_returns_selected_libraries(self): one_library = GenerateRepoTest.__get_an_empty_library_config() one_library.api_shortname = "one_library" @@ -44,6 +43,7 @@ def test_get_target_library_given_null_returns_all_libraries(self): @staticmethod def __get_an_empty_generation_config() -> GenerationConfig: return GenerationConfig( + gapic_generator_version="", googleapis_commitish="", libraries=[], ) diff --git a/library_generation/test/model/config_change_unit_tests.py b/library_generation/test/model/config_change_unit_tests.py index e0de105832..147753ec99 100644 --- a/library_generation/test/model/config_change_unit_tests.py +++ b/library_generation/test/model/config_change_unit_tests.py @@ -19,10 +19,9 @@ from library_generation.model.gapic_config import GapicConfig from library_generation.model.generation_config import GenerationConfig from library_generation.model.library_config import LibraryConfig -from library_generation.test.test_utils import SimulatedDockerEnvironmentTest -class ConfigChangeTest(SimulatedDockerEnvironmentTest): +class ConfigChangeTest(unittest.TestCase): def test_get_changed_libraries_with_repo_level_change_returns_all_libraries_changed( self, ): @@ -292,6 +291,7 @@ def __get_a_gen_config( if libraries is None: libraries = [] return GenerationConfig( + gapic_generator_version="", googleapis_commitish=googleapis_commitish, grpc_version="", protoc_version="", diff --git a/library_generation/test/model/gapic_config_unit_tests.py b/library_generation/test/model/gapic_config_unit_tests.py index 4953afe483..64d8556648 100644 --- a/library_generation/test/model/gapic_config_unit_tests.py +++ b/library_generation/test/model/gapic_config_unit_tests.py @@ -14,10 +14,9 @@ import unittest from library_generation.model.gapic_config import GapicConfig -from library_generation.test.test_utils import SimulatedDockerEnvironmentTest -class GapicConfigTest(SimulatedDockerEnvironmentTest): +class GapicConfigTest(unittest.TestCase): def test_get_version_returns_none(self): self.assertIsNone(GapicConfig(proto_path="example/dir1/dir2").get_version()) diff --git a/library_generation/test/model/gapic_inputs_unit_tests.py b/library_generation/test/model/gapic_inputs_unit_tests.py index 4d77fe827f..210d321591 100644 --- a/library_generation/test/model/gapic_inputs_unit_tests.py +++ b/library_generation/test/model/gapic_inputs_unit_tests.py @@ -4,14 +4,13 @@ from parameterized import parameterized from library_generation.model.gapic_inputs import parse -from library_generation.test.test_utils import SimulatedDockerEnvironmentTest script_dir = os.path.dirname(os.path.realpath(__file__)) resources_dir = os.path.join(script_dir, "..", "resources") build_file = Path(os.path.join(resources_dir, "misc")).resolve() -class UtilitiesTest(SimulatedDockerEnvironmentTest): +class UtilitiesTest(unittest.TestCase): @parameterized.expand( [ ("BUILD_no_additional_protos.bazel", " "), diff --git a/library_generation/test/model/library_config_unit_tests.py b/library_generation/test/model/library_config_unit_tests.py index bab3e3646c..5d54737ced 100644 --- a/library_generation/test/model/library_config_unit_tests.py +++ b/library_generation/test/model/library_config_unit_tests.py @@ -15,10 +15,9 @@ from library_generation.model.gapic_config import GapicConfig from library_generation.model.library_config import LibraryConfig -from library_generation.test.test_utils import SimulatedDockerEnvironmentTest -class LibraryConfigTest(SimulatedDockerEnvironmentTest): +class LibraryConfigTest(unittest.TestCase): def test_get_library_returns_library_name(self): library = LibraryConfig( api_shortname="secret", diff --git a/library_generation/test/model/repo_config_unit_tests.py b/library_generation/test/model/repo_config_unit_tests.py index 1bdaf83f0d..12d28fe254 100644 --- a/library_generation/test/model/repo_config_unit_tests.py +++ b/library_generation/test/model/repo_config_unit_tests.py @@ -15,13 +15,12 @@ import unittest from library_generation.model.repo_config import RepoConfig -from library_generation.test.test_utils import SimulatedDockerEnvironmentTest script_dir = os.path.dirname(os.path.realpath(__file__)) versions_file = os.path.join(script_dir, "..", "resources", "misc", "versions.txt") -class RepoConfigTest(SimulatedDockerEnvironmentTest): +class RepoConfigTest(unittest.TestCase): def test_get_library_versions_with_existing_library(self): repo_config = RepoConfig( output_folder="test", libraries=dict(), versions_file=versions_file diff --git a/library_generation/test/resources/integration/google-cloud-java/baseline_generation_config.yaml b/library_generation/test/resources/integration/google-cloud-java/baseline_generation_config.yaml index 5a4037165a..9a27dd381d 100644 --- a/library_generation/test/resources/integration/google-cloud-java/baseline_generation_config.yaml +++ b/library_generation/test/resources/integration/google-cloud-java/baseline_generation_config.yaml @@ -1,3 +1,4 @@ +gapic_generator_version: 2.38.1 protoc_version: 25.2 googleapis_commitish: a17d4caf184b050d50cacf2b0d579ce72c31ce74 libraries_bom_version: 26.37.0 diff --git a/library_generation/test/resources/integration/google-cloud-java/current_generation_config.yaml b/library_generation/test/resources/integration/google-cloud-java/current_generation_config.yaml index 4119df8782..adc1d170d6 100644 --- a/library_generation/test/resources/integration/google-cloud-java/current_generation_config.yaml +++ b/library_generation/test/resources/integration/google-cloud-java/current_generation_config.yaml @@ -1,3 +1,4 @@ +gapic_generator_version: 2.38.1 protoc_version: 25.2 googleapis_commitish: 4ce0ff67a3d4509be641cbe47a35844ddc1268fc libraries_bom_version: 26.37.0 diff --git a/library_generation/test/resources/integration/java-bigtable/generation_config.yaml b/library_generation/test/resources/integration/java-bigtable/generation_config.yaml index 7865e9a685..c6912c8125 100644 --- a/library_generation/test/resources/integration/java-bigtable/generation_config.yaml +++ b/library_generation/test/resources/integration/java-bigtable/generation_config.yaml @@ -1,3 +1,4 @@ +gapic_generator_version: 2.37.0 protoc_version: 25.2 googleapis_commitish: 9868a57470a969ffa1d21194a5c05d7a6e4e98cc libraries: diff --git a/library_generation/test/resources/test-config/config_without_api_description.yaml b/library_generation/test/resources/test-config/config_without_api_description.yaml index b93bab256f..79ff135067 100644 --- a/library_generation/test/resources/test-config/config_without_api_description.yaml +++ b/library_generation/test/resources/test-config/config_without_api_description.yaml @@ -1,3 +1,4 @@ +gapic_generator_version: 2.34.0 libraries: - api_shortname: apigeeconnect GAPICs: diff --git a/library_generation/test/resources/test-config/config_without_api_shortname.yaml b/library_generation/test/resources/test-config/config_without_api_shortname.yaml index 28253d55d3..ec8206be61 100644 --- a/library_generation/test/resources/test-config/config_without_api_shortname.yaml +++ b/library_generation/test/resources/test-config/config_without_api_shortname.yaml @@ -1,3 +1,4 @@ +gapic_generator_version: 2.34.0 libraries: - GAPICs: - proto_path: google/cloud/apigeeconnect/v1 diff --git a/library_generation/test/resources/test-config/config_without_gapics_key.yaml b/library_generation/test/resources/test-config/config_without_gapics_key.yaml index a06cf1277a..739a4d9239 100644 --- a/library_generation/test/resources/test-config/config_without_gapics_key.yaml +++ b/library_generation/test/resources/test-config/config_without_gapics_key.yaml @@ -1,2 +1,3 @@ +gapic_generator_version: 2.34.0 libraries: - api_shortname: apigeeconnect diff --git a/library_generation/test/resources/test-config/config_without_gapics_value.yaml b/library_generation/test/resources/test-config/config_without_gapics_value.yaml index 9477c31ce3..ec49e4a669 100644 --- a/library_generation/test/resources/test-config/config_without_gapics_value.yaml +++ b/library_generation/test/resources/test-config/config_without_gapics_value.yaml @@ -1,3 +1,4 @@ +gapic_generator_version: 2.34.0 libraries: - api_shortname: apigeeconnect GAPICs: diff --git a/library_generation/test/resources/test-config/config_without_googleapis.yaml b/library_generation/test/resources/test-config/config_without_googleapis.yaml index c78b8b9700..e5a00ca4ee 100644 --- a/library_generation/test/resources/test-config/config_without_googleapis.yaml +++ b/library_generation/test/resources/test-config/config_without_googleapis.yaml @@ -1,3 +1,4 @@ +gapic_generator_version: 2.34.0 libraries: - api_shortname: apigeeconnect name_pretty: Apigee Connect diff --git a/library_generation/test/resources/test-config/config_without_libraries.yaml b/library_generation/test/resources/test-config/config_without_libraries.yaml index b0b9636ce4..dbbe2ea318 100644 --- a/library_generation/test/resources/test-config/config_without_libraries.yaml +++ b/library_generation/test/resources/test-config/config_without_libraries.yaml @@ -1 +1 @@ -protoc_version: 3.25.2 +gapic_generator_version: 2.34.0 diff --git a/library_generation/test/resources/test-config/config_without_library_value.yaml b/library_generation/test/resources/test-config/config_without_library_value.yaml index 9c1dc55eac..174a293000 100644 --- a/library_generation/test/resources/test-config/config_without_library_value.yaml +++ b/library_generation/test/resources/test-config/config_without_library_value.yaml @@ -1 +1,2 @@ +gapic_generator_version: 2.34.0 libraries: diff --git a/library_generation/test/resources/test-config/config_without_name_pretty.yaml b/library_generation/test/resources/test-config/config_without_name_pretty.yaml index 7565a4fca8..f8612ad9ca 100644 --- a/library_generation/test/resources/test-config/config_without_name_pretty.yaml +++ b/library_generation/test/resources/test-config/config_without_name_pretty.yaml @@ -1,3 +1,4 @@ +gapic_generator_version: 2.34.0 libraries: - api_shortname: apigeeconnect api_description: "allows the Apigee hybrid management" diff --git a/library_generation/test/resources/test-config/config_without_product_docs.yaml b/library_generation/test/resources/test-config/config_without_product_docs.yaml index a6c60dbf4c..e3921d2c0d 100644 --- a/library_generation/test/resources/test-config/config_without_product_docs.yaml +++ b/library_generation/test/resources/test-config/config_without_product_docs.yaml @@ -1,3 +1,4 @@ +gapic_generator_version: 2.34.0 libraries: - api_shortname: apigeeconnect name_pretty: Apigee Connect diff --git a/library_generation/test/resources/test-config/config_without_proto_path.yaml b/library_generation/test/resources/test-config/config_without_proto_path.yaml index 7bf5663a71..e37b0cef63 100644 --- a/library_generation/test/resources/test-config/config_without_proto_path.yaml +++ b/library_generation/test/resources/test-config/config_without_proto_path.yaml @@ -1,3 +1,4 @@ +gapic_generator_version: 2.34.0 libraries: - GAPICs: - random_key: diff --git a/library_generation/test/resources/test-config/config_without_temp_excludes.yaml b/library_generation/test/resources/test-config/config_without_temp_excludes.yaml index 7225a1c711..0d1bb7deea 100644 --- a/library_generation/test/resources/test-config/config_without_temp_excludes.yaml +++ b/library_generation/test/resources/test-config/config_without_temp_excludes.yaml @@ -1,3 +1,4 @@ +gapic_generator_version: 2.34.0 googleapis_commitish: 1a45bf7393b52407188c82e63101db7dc9c72026 libraries_bom_version: 26.37.0 libraries: diff --git a/library_generation/test/resources/test-config/generation_config_with_duplicate_library_name.yaml b/library_generation/test/resources/test-config/generation_config_with_duplicate_library_name.yaml index 8b48ec964f..c5613f4308 100644 --- a/library_generation/test/resources/test-config/generation_config_with_duplicate_library_name.yaml +++ b/library_generation/test/resources/test-config/generation_config_with_duplicate_library_name.yaml @@ -1,3 +1,4 @@ +gapic_generator_version: 2.34.0 protoc_version: 25.2 googleapis_commitish: 1a45bf7393b52407188c82e63101db7dc9c72026 libraries_bom_version: 26.37.0 From 1301b0f0c0f9be77b9a1d3043bb038dcf759f772 Mon Sep 17 00:00:00 2001 From: diegomarquezp Date: Fri, 30 Aug 2024 01:33:35 +0000 Subject: [PATCH 46/63] restore simulated docker env --- library_generation/test/test_utils.py | 22 ------------------- .../test/utilities_unit_tests.py | 4 ++-- .../commit_message_formatter_unit_tests.py | 15 +++++++++---- ...generation_config_comparator_unit_tests.py | 19 ++++++++++++++-- 4 files changed, 30 insertions(+), 30 deletions(-) diff --git a/library_generation/test/test_utils.py b/library_generation/test/test_utils.py index c7ac19bdf6..890cd95362 100644 --- a/library_generation/test/test_utils.py +++ b/library_generation/test/test_utils.py @@ -14,8 +14,6 @@ import unittest from difflib import unified_diff from pathlib import Path -import os -from unittest.mock import patch from typing import List @@ -40,23 +38,3 @@ def cleanup(files: List[str]): path.unlink() elif path.is_dir(): path.rmdir() - - -class SimulatedDockerEnvironmentTest(unittest.TestCase): - @classmethod - def setUpClass(cls): - cls.env_patcher = patch.dict( - os.environ, - { - "GENERATOR_VERSION": "test-version", - "GENERATOR_LOCATION": "test-location", - }, - ) - cls.env_patcher.start() - - super().setUpClass() - - @classmethod - def tearDownClass(cls): - super().tearDownClass() - cls.env_patcher.stop() diff --git a/library_generation/test/utilities_unit_tests.py b/library_generation/test/utilities_unit_tests.py index 530d3426c5..7d9f09ec2c 100644 --- a/library_generation/test/utilities_unit_tests.py +++ b/library_generation/test/utilities_unit_tests.py @@ -28,7 +28,6 @@ from library_generation.model.library_config import LibraryConfig from library_generation.test.test_utils import FileComparator from library_generation.test.test_utils import cleanup -from library_generation.test.test_utils import SimulatedDockerEnvironmentTest script_dir = os.path.dirname(os.path.realpath(__file__)) resources_dir = os.path.join(script_dir, "resources") @@ -69,7 +68,7 @@ ) -class UtilitiesTest(SimulatedDockerEnvironmentTest): +class UtilitiesTest(unittest.TestCase): """ Unit tests for utilities.py """ @@ -362,6 +361,7 @@ def __get_a_gen_config( library.extra_versioned_modules = None return GenerationConfig( + gapic_generator_version="", googleapis_commitish="", libraries=libraries, ) diff --git a/library_generation/test/utils/commit_message_formatter_unit_tests.py b/library_generation/test/utils/commit_message_formatter_unit_tests.py index dc3896d1f1..16e3fffdfc 100644 --- a/library_generation/test/utils/commit_message_formatter_unit_tests.py +++ b/library_generation/test/utils/commit_message_formatter_unit_tests.py @@ -27,10 +27,13 @@ ) from library_generation.utils.commit_message_formatter import wrap_googleapis_commit from library_generation.utils.commit_message_formatter import wrap_override_commit -from library_generation.test.test_utils import SimulatedDockerEnvironmentTest + +gen_config = GenerationConfig( + gapic_generator_version="1.2.3", googleapis_commitish="123abc", libraries=[] +) -class CommitMessageFormatterTest(SimulatedDockerEnvironmentTest): +class CommitMessageFormatterTest(unittest.TestCase): def test_format_commit_message_should_add_library_name_for_conventional_commit( self, ): @@ -163,11 +166,12 @@ def test_commit_link_success(self): ) def test_format_repo_level_change_success(self): - - gen_config = GenerationConfig(googleapis_commitish="123abc", libraries=[]) config_change = ConfigChange( change_to_libraries={ ChangeType.REPO_LEVEL_CHANGE: [ + LibraryChange( + changed_param="gapic_generator_version", current_value="1.2.3" + ), LibraryChange( changed_param="libraries_bom_version", current_value="2.3.4" ), @@ -181,6 +185,9 @@ def test_format_repo_level_change_success(self): ) self.assertEqual( [ + "BEGIN_NESTED_COMMIT", + "fix(deps): update the Java code generator (gapic-generator-java) to 1.2.3", + "END_NESTED_COMMIT", "BEGIN_NESTED_COMMIT", "chore: update the libraries_bom version to 2.3.4", "END_NESTED_COMMIT", diff --git a/library_generation/test/utils/generation_config_comparator_unit_tests.py b/library_generation/test/utils/generation_config_comparator_unit_tests.py index 5c3f21b04a..8d018a7c15 100644 --- a/library_generation/test/utils/generation_config_comparator_unit_tests.py +++ b/library_generation/test/utils/generation_config_comparator_unit_tests.py @@ -18,10 +18,9 @@ from library_generation.model.library_config import LibraryConfig from library_generation.utils.generation_config_comparator import ChangeType from library_generation.utils.generation_config_comparator import compare_config -from library_generation.test.test_utils import SimulatedDockerEnvironmentTest -class GenerationConfigComparatorTest(SimulatedDockerEnvironmentTest): +class GenerationConfigComparatorTest(unittest.TestCase): def setUp(self) -> None: self.baseline_library = LibraryConfig( api_shortname="existing_library", @@ -38,12 +37,14 @@ def setUp(self) -> None: gapic_configs=[], ) self.baseline_config = GenerationConfig( + gapic_generator_version="", googleapis_commitish="", grpc_version="", protoc_version="", libraries=[self.baseline_library], ) self.current_config = GenerationConfig( + gapic_generator_version="", googleapis_commitish="", grpc_version="", protoc_version="", @@ -70,6 +71,20 @@ def test_compare_config_googleapis_update(self): ) self.assertEqual({ChangeType.GOOGLEAPIS_COMMIT: []}, result.change_to_libraries) + def test_compare_config_generator_update(self): + self.baseline_config.gapic_generator_version = "1.2.3" + self.current_config.gapic_generator_version = "1.2.4" + result = compare_config( + baseline_config=self.baseline_config, + current_config=self.current_config, + ) + self.assertTrue( + len(result.change_to_libraries[ChangeType.REPO_LEVEL_CHANGE]) == 1 + ) + config_change = result.change_to_libraries[ChangeType.REPO_LEVEL_CHANGE][0] + self.assertEqual("gapic_generator_version", config_change.changed_param) + self.assertEqual("1.2.4", config_change.current_value) + def test_compare_config_libraries_bom_update(self): self.baseline_config.libraries_bom_version = "26.36.0" self.current_config.libraries_bom_version = "26.37.0" From c4933ea3597afa7fb81dbf31b9285701c6ae5f3e Mon Sep 17 00:00:00 2001 From: Diego Marquez Date: Thu, 29 Aug 2024 21:34:52 -0400 Subject: [PATCH 47/63] Update library_generation/test/generate_library_unit_tests.sh --- library_generation/test/generate_library_unit_tests.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/library_generation/test/generate_library_unit_tests.sh b/library_generation/test/generate_library_unit_tests.sh index b67e9e7ad6..7b7a6c3e47 100755 --- a/library_generation/test/generate_library_unit_tests.sh +++ b/library_generation/test/generate_library_unit_tests.sh @@ -6,8 +6,8 @@ set -xeo pipefail script_dir=$(dirname "$(readlink -f "$0")") source "${script_dir}"/test_utilities.sh -# we simulate a properly prepared environemnt (i.e. generator jar) in its -# well-known location. Tests confirming the opposite case will make sure this +# we simulate a properly prepared environment (i.e. generator jar in its +# well-known location). Tests confirming the opposite case will make sure this # environment is restored readonly SIMULATED_HOME=$(mktemp -d) mkdir "${SIMULATED_HOME}/.library_generation" From 6379c0d2920df36d2ada3b86c43f64015b5e9731 Mon Sep 17 00:00:00 2001 From: diegomarquezp Date: Fri, 30 Aug 2024 01:41:53 +0000 Subject: [PATCH 48/63] improve development guide --- library_generation/DEVELOPMENT.md | 43 +++++++++++++++++++------------ 1 file changed, 26 insertions(+), 17 deletions(-) diff --git a/library_generation/DEVELOPMENT.md b/library_generation/DEVELOPMENT.md index d2556dcd9a..d9411f9c0e 100644 --- a/library_generation/DEVELOPMENT.md +++ b/library_generation/DEVELOPMENT.md @@ -48,6 +48,32 @@ Although the scripts are designed to be run in a Docker container, you can also run them directly. This section explains how to run the entrypoint script (`library_generation/cli/entry_point.py`). +## Assumptions made by the scripts +### The Hermetic Build's well-known folder +Located in `${HOME}/.library_generation`, this folder is assumed by the scripts +to contain the generator JAR. Please note that this is a recent feature and only +this jar is expected to be there. Soon enough, more binaries such as the gRPC +and protoc plugins will also be stored here. Developers must make sure +this folder is properly configured before running the scripts locally. +Note that this relies on the `HOME` en var which is always +defined as per +[POSIX env var definition](https://pubs.opengroup.org/onlinepubs/9699919799/basedefs/V1_chap08.html). + +#### Put the gapic-generator-java jar in its well-known location + +Run `cd sdk-platform-java && mvn install -DskipTests -Dclirr.skip +-Dcheckstyle.skip`. This will generate a jar located in +`~/.m2/repository/com/google/api/gapic-generator-java/{version}/gapic-generator-java-{version}.jar` + +Then `mv` the jar into the well-known location of the jar. The +generation scripts will assume the jar is there. + +```shell +mv /path/to/jar "${HOME}/.library_generation/gapic-generator-java.jar" +``` + + + ## Installing prerequisites In order to run the generation scripts directly, there are a few tools we @@ -71,23 +97,6 @@ owl-bot copy-code --version The key step is `npm link`, which will make the command available in you current shell session. -### Create the gapic-generator-java jar - -Run `cd sdk-platform-java && mvn install -DskipTests -Dclirr.skip --Dcheckstyle.skip`. This will generate a jar located in -`~/.m2/repository/com/google/api/gapic-generator-java/{version}/gapic-generator-java-{version}.jar` - -Then `mv` the jar into the well-known location of the jar. The -generation scripts will assume the jar is there. - -```shell -mv /path/to/jar "${HOME}/.library_generation/gapic-generator-java.jar" -``` - -Note that this relies on the `HOME` en var which is always -defined as per -[POSIX env var definition](https://pubs.opengroup.org/onlinepubs/9699919799/basedefs/V1_chap08.html). - ## Running the script The entrypoint script (`library_generation/cli/entry_point.py`) allows you to From 16435c4c2ef219cec677dadcc18b2d9ee7c031d8 Mon Sep 17 00:00:00 2001 From: diegomarquezp Date: Fri, 30 Aug 2024 01:57:14 +0000 Subject: [PATCH 49/63] fix showcase tests --- .github/workflows/ci.yaml | 10 ++++++++++ showcase/scripts/generate_showcase.sh | 10 ---------- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index d18f9c9646..d09173b56d 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -232,6 +232,16 @@ jobs: - name: Showcase golden tests working-directory: showcase run: | + # The golden test directly calls + # library_generation/generate_library.sh, which expects the jar to be + # located in its well-known location. More info in + # library_generation/DEVELOPMENT.md + # Here we prepare the jar in such location + generator_version=$(grep "gapic-generator-java:" "../versions.txt" \ + | cut -d: -f3) # the 3rd field is the snapshot version + cp \ + "${HOME}/.m2/repository/com/google/api/gapic-generator-java/${generator_version}/gapic-generator-java-${generator_version}.jar" \ + "${HOME}/.library_generation/gapic-generator-java.jar" mvn test \ -P enable-golden-tests \ --batch-mode \ diff --git a/showcase/scripts/generate_showcase.sh b/showcase/scripts/generate_showcase.sh index d524aaa77f..a2834f3af6 100755 --- a/showcase/scripts/generate_showcase.sh +++ b/showcase/scripts/generate_showcase.sh @@ -14,14 +14,6 @@ readonly perform_cleanup=$1 cd "${SCRIPT_DIR}" mkdir -p "${SCRIPT_DIR}/output" -# takes a versions.txt file and returns its version -get_version_from_versions_txt() { - versions=$1 - key=$2 - version=$(grep "$key:" "${versions}" | cut -d: -f3) # 3rd field is snapshot - echo "${version}" -} - # clone gapic-showcase if [ ! -d schema ]; then if [ -d gapic-showcase ]; then @@ -46,7 +38,6 @@ if [ ! -d google ];then rm -rdf googleapis fi -ggj_version=$(get_version_from_versions_txt ../../versions.txt "gapic-generator-java") gapic_additional_protos="google/iam/v1/iam_policy.proto google/cloud/location/locations.proto" rest_numeric_enums="false" transport="grpc+rest" @@ -60,7 +51,6 @@ set +e bash "${SCRIPT_DIR}/../../library_generation/generate_library.sh" \ --proto_path "schema/google/showcase/v1beta1" \ --destination_path "showcase-output" \ - --gapic_generator_version "${ggj_version}" \ --gapic_additional_protos "${gapic_additional_protos}" \ --rest_numeric_enums "${rest_numeric_enums}" \ --gapic_yaml "${gapic_yaml}" \ From 8e0ddcc7a3f1e021df55d58d5728763ab1205703 Mon Sep 17 00:00:00 2001 From: diegomarquezp Date: Fri, 30 Aug 2024 02:00:48 +0000 Subject: [PATCH 50/63] fix showcase ii --- .github/workflows/ci.yaml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index d09173b56d..45cb63aeb8 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -239,6 +239,7 @@ jobs: # Here we prepare the jar in such location generator_version=$(grep "gapic-generator-java:" "../versions.txt" \ | cut -d: -f3) # the 3rd field is the snapshot version + mkdir -p "${HOME}/.library_generation" cp \ "${HOME}/.m2/repository/com/google/api/gapic-generator-java/${generator_version}/gapic-generator-java-${generator_version}.jar" \ "${HOME}/.library_generation/gapic-generator-java.jar" From 81f6748c0e932ea930de3fc34e97c1ad436392aa Mon Sep 17 00:00:00 2001 From: diegomarquezp Date: Fri, 30 Aug 2024 02:32:25 +0000 Subject: [PATCH 51/63] get protoc and grpc versions in showcase golden test --- showcase/scripts/generate_showcase.sh | 20 ++++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-) diff --git a/showcase/scripts/generate_showcase.sh b/showcase/scripts/generate_showcase.sh index a2834f3af6..f5558e7b04 100755 --- a/showcase/scripts/generate_showcase.sh +++ b/showcase/scripts/generate_showcase.sh @@ -14,6 +14,13 @@ readonly perform_cleanup=$1 cd "${SCRIPT_DIR}" mkdir -p "${SCRIPT_DIR}/output" +get_version_from_pom() { + target_pom="$1" + key="$2" + # prints the result to stdout + grep -e "<${key}>" "${target_pom}" | cut -d'>' -f2 | cut -d'<' -f1 +} + # clone gapic-showcase if [ ! -d schema ]; then if [ -d gapic-showcase ]; then @@ -22,8 +29,10 @@ if [ ! -d schema ]; then # looks at sdk-platform-java/showcase/gapic-showcase/pom.xml to extract the # version of gapic-showcase # see https://github.com/googleapis/gapic-showcase/releases - showcase_version=$(grep -e '' "${SCRIPT_DIR}/../gapic-showcase/pom.xml" | cut -d'>' -f2 | cut -d'<' -f1) - sparse_clone https://github.com/googleapis/gapic-showcase.git "schema/google/showcase/v1beta1" "v${showcase_version}" + showcase_version=$(get_version_from_pom \ + "${SCRIPT_DIR}/../gapic-showcase/pom.xml" "gapic-showcase.version" + ) + sparse_clone https://github.com/googleapis/gapic-showcase.git "schema/google/showcase/v1beta1" "v${showcase_version}" cd gapic-showcase mv schema ../output cd .. @@ -39,6 +48,11 @@ if [ ! -d google ];then fi gapic_additional_protos="google/iam/v1/iam_policy.proto google/cloud/location/locations.proto" + +path_to_generator_parent_pom="${SCRIPT_DIR}/../../gapic-generator-java-pom-parent/pom.xml" +protoc_version=$(get_version_from_pom "${path_to_generator_parent_pom}" "protobuf.version" \ + | cut -d. -f2-) +grpc_version=$(get_version_from_pom "${path_to_generator_parent_pom}" "grpc.version") rest_numeric_enums="false" transport="grpc+rest" gapic_yaml="" @@ -49,6 +63,8 @@ rm -rdf output/showcase-output mkdir output/showcase-output set +e bash "${SCRIPT_DIR}/../../library_generation/generate_library.sh" \ + --protoc_version "${protoc_version}" \ + --grpc_version "${grpc_version}" \ --proto_path "schema/google/showcase/v1beta1" \ --destination_path "showcase-output" \ --gapic_additional_protos "${gapic_additional_protos}" \ From 35ad97e915aede0dbd0dcb64fca2003ccc8e8117 Mon Sep 17 00:00:00 2001 From: diegomarquezp Date: Fri, 30 Aug 2024 17:44:16 +0000 Subject: [PATCH 52/63] fix utilities comment --- library_generation/utils/utilities.sh | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/library_generation/utils/utilities.sh b/library_generation/utils/utilities.sh index 75ca316938..d10daccf11 100755 --- a/library_generation/utils/utilities.sh +++ b/library_generation/utils/utilities.sh @@ -138,10 +138,9 @@ get_protoc_version() { # download), since the docker image will have downloaded these tools beforehand. # # For the case of gapic-generator-java, no env var will be exported for the -# upstream flow, but instead it will be assigned a default filename that will be -# referenced by the file `library_generation/gapic-generator-java-wrapper`. Note -# that we assume an env var DOCKER_GAPIC_GENERATOR_LOCATION to point to the jar -# so we can copy it to our output folder +# upstream flow. Instead, the jar must be located in the well-known location +# (${HOME}/.library_generation/gapic-generator-java.jar). More information in +# `library_generation/DEVELOPMENT.md` download_tools() { local protoc_version=$1 local grpc_version=$2 From d6844ce9acb987797857eca5ffc565524a76d473 Mon Sep 17 00:00:00 2001 From: diegomarquezp Date: Fri, 30 Aug 2024 22:03:06 +0000 Subject: [PATCH 53/63] install spring without relying on published versions --- .kokoro/presubmit/downstream-compatibility-spring.sh | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.kokoro/presubmit/downstream-compatibility-spring.sh b/.kokoro/presubmit/downstream-compatibility-spring.sh index 2c0ae2b62c..766eac7d9e 100755 --- a/.kokoro/presubmit/downstream-compatibility-spring.sh +++ b/.kokoro/presubmit/downstream-compatibility-spring.sh @@ -47,10 +47,12 @@ pushd spring-cloud-gcp/spring-cloud-generator # Generate showcase autoconfig +pushd spring-cloud-gcp/spring-cloud-generator ./scripts/generate-showcase.sh pushd showcase/showcase-spring-starter mvn verify popd # showcase/showcase-spring-starter popd # spring-cloud-gcp/spring-cloud-generator +popd # spring-cloud-gcp popd # gapic-generator-java/target From d776b329c3d5a7573cfce1e006a982eb8432947e Mon Sep 17 00:00:00 2001 From: diegomarquezp Date: Sat, 31 Aug 2024 00:28:19 +0000 Subject: [PATCH 54/63] use locked-in jar --- library_generation/test/integration_tests.py | 54 +++++++++++++++++-- .../test/resources/integration/.gitignore | 2 + .../integration/test_generator_coordinates | 1 + 3 files changed, 54 insertions(+), 3 deletions(-) create mode 100644 library_generation/test/resources/integration/.gitignore create mode 100644 library_generation/test/resources/integration/test_generator_coordinates diff --git a/library_generation/test/integration_tests.py b/library_generation/test/integration_tests.py index 5e5d219c61..2257f6aa8e 100644 --- a/library_generation/test/integration_tests.py +++ b/library_generation/test/integration_tests.py @@ -29,7 +29,9 @@ from library_generation.utils.utilities import sh_util as shell_call script_dir = os.path.dirname(os.path.realpath(__file__)) -golden_dir = os.path.join(script_dir, "resources", "integration", "golden") +config_dir = os.path.join(script_dir, "resources", "integration") +golden_dir = os.path.join(config_dir, "golden") +generator_jar_coordinates_file = os.path.join(config_dir, "test_generator_coordinates") repo_root_dir = os.path.join(script_dir, "..", "..") build_file = os.path.join( repo_root_dir, ".cloudbuild", "library_generation", "library_generation.Dockerfile" @@ -42,15 +44,24 @@ "google-cloud-java": "chore/test-hermetic-build", "java-bigtable": "chore/test-hermetic-build", } -config_dir = f"{script_dir}/resources/integration" baseline_config_name = "baseline_generation_config.yaml" current_config_name = "current_generation_config.yaml" +# This variable is used to override the jar created by building the image +# with our own downloaded jar in order to lock the integration test to use +# a constant version specified in +# library_generation/test/resources/integration/test_generator_coordinates +# This allows us to decouple the generation workflow testing with what the +# generator jar will actually generate. +# See library_generation/DEVELOPMENT.MD +WELL_KNOWN_GENERATOR_JAR_FILENAME = "gapic-generator-java.jar" + class IntegrationTest(unittest.TestCase): @classmethod def setUpClass(cls) -> None: - IntegrationTest.__build_image(docker_file=build_file, cwd=repo_root_dir) + cls.__download_generator_jar(coordinates_file=generator_jar_coordinates_file) + cls.__build_image(docker_file=build_file, cwd=repo_root_dir) @classmethod def setUp(cls) -> None: @@ -186,6 +197,41 @@ def __build_image(cls, docker_file: str, cwd: str): cwd=cwd, ) + @classmethod + def __download_generator_jar(cls, coordinates_file: str) -> None: + """ + Downloads the jar at the version specified in the + coordinates file + :param coordinates_file: path to the file containing the coordinates + """ + with open(coordinates_file, "r") as coordinates_file_handle: + # make this var available in the function scope + # nonlocal coordinates + coordinates = coordinates_file_handle.read() + # download the jar + subprocess.check_call( + [ + "mvn", + "dependency:copy", + f"-Dartifact={coordinates}", + f"-DoutputDirectory={config_dir}", + ] + ) + + # compute the filename of the downloaded jar + split_coordinates = coordinates.split(":") + artifact_id = split_coordinates[1] + version = split_coordinates[2] + jar_filename = f"{artifact_id}-{version}.jar" + + # rename the jar to its well-known filename defined at the top of this + # script file + source_jar_path = os.path.join(config_dir, jar_filename) + destination_jar_path = os.path.join( + config_dir, WELL_KNOWN_GENERATOR_JAR_FILENAME + ) + shutil.move(source_jar_path, destination_jar_path) + @classmethod def __remove_generated_files(cls): shutil.rmtree(f"{output_dir}", ignore_errors=True) @@ -247,6 +293,8 @@ def __run_entry_point_in_docker_container( f"{repo_location}:/workspace/repo", "-v", f"{config_location}:/workspace/config", + "-v", + f"{config_dir}/{WELL_KNOWN_GENERATOR_JAR_FILENAME}:/home/.library_generation/{WELL_KNOWN_GENERATOR_JAR_FILENAME}", "-w", "/workspace/repo", image_tag, diff --git a/library_generation/test/resources/integration/.gitignore b/library_generation/test/resources/integration/.gitignore new file mode 100644 index 0000000000..cb2fa3374a --- /dev/null +++ b/library_generation/test/resources/integration/.gitignore @@ -0,0 +1,2 @@ +# Ignore all downloaded jars. The integration test downloads a specific generator jar +*.jar diff --git a/library_generation/test/resources/integration/test_generator_coordinates b/library_generation/test/resources/integration/test_generator_coordinates new file mode 100644 index 0000000000..00af5f647f --- /dev/null +++ b/library_generation/test/resources/integration/test_generator_coordinates @@ -0,0 +1 @@ +com.google.api:gapic-generator-java:2.38.1 \ No newline at end of file From 1de293ff249999e775a9aa1b3cb030feac3a2caa Mon Sep 17 00:00:00 2001 From: diegomarquezp Date: Sat, 31 Aug 2024 00:33:19 +0000 Subject: [PATCH 55/63] fix spring test --- .kokoro/presubmit/downstream-compatibility-spring.sh | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/.kokoro/presubmit/downstream-compatibility-spring.sh b/.kokoro/presubmit/downstream-compatibility-spring.sh index 766eac7d9e..9ac6081ff2 100755 --- a/.kokoro/presubmit/downstream-compatibility-spring.sh +++ b/.kokoro/presubmit/downstream-compatibility-spring.sh @@ -34,7 +34,7 @@ git clone "https://github.com/GoogleCloudPlatform/spring-cloud-gcp.git" --depth= update_all_poms_dependency "spring-cloud-gcp" "gapic-generator-java-bom" "${GAPIC_GENERATOR_VERSION}" # Install spring-cloud-gcp modules -pushd spring-cloud-gcp/spring-cloud-generator +pushd spring-cloud-gcp ../mvnw \ -U \ --batch-mode \ @@ -47,12 +47,12 @@ pushd spring-cloud-gcp/spring-cloud-generator # Generate showcase autoconfig -pushd spring-cloud-gcp/spring-cloud-generator +pushd spring-cloud-generator ./scripts/generate-showcase.sh pushd showcase/showcase-spring-starter mvn verify popd # showcase/showcase-spring-starter -popd # spring-cloud-gcp/spring-cloud-generator +popd # spring-cloud-generator popd # spring-cloud-gcp popd # gapic-generator-java/target From 1895792ff273e9b98dcca20e6376decf68a28fc9 Mon Sep 17 00:00:00 2001 From: Diego Marquez Date: Fri, 30 Aug 2024 20:42:21 -0400 Subject: [PATCH 56/63] fix path to mvnw --- .kokoro/presubmit/downstream-compatibility-spring.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.kokoro/presubmit/downstream-compatibility-spring.sh b/.kokoro/presubmit/downstream-compatibility-spring.sh index 9ac6081ff2..0e2d760bb5 100755 --- a/.kokoro/presubmit/downstream-compatibility-spring.sh +++ b/.kokoro/presubmit/downstream-compatibility-spring.sh @@ -35,7 +35,7 @@ update_all_poms_dependency "spring-cloud-gcp" "gapic-generator-java-bom" "${GAPI # Install spring-cloud-gcp modules pushd spring-cloud-gcp -../mvnw \ +./mvnw \ -U \ --batch-mode \ --no-transfer-progress \ From 7d805e2888fc676dfcef5edfda9e9d577c28fe7a Mon Sep 17 00:00:00 2001 From: diegomarquezp Date: Sat, 31 Aug 2024 01:21:07 +0000 Subject: [PATCH 57/63] chmod the spring script to allow execution --- .kokoro/presubmit/downstream-compatibility-spring.sh | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/.kokoro/presubmit/downstream-compatibility-spring.sh b/.kokoro/presubmit/downstream-compatibility-spring.sh index 0e2d760bb5..286b304a92 100755 --- a/.kokoro/presubmit/downstream-compatibility-spring.sh +++ b/.kokoro/presubmit/downstream-compatibility-spring.sh @@ -48,6 +48,10 @@ pushd spring-cloud-gcp # Generate showcase autoconfig pushd spring-cloud-generator +# The script is not executable for non-owners. Here we manually chmod it. +# TODO(diegomarquezp): remove this line after +# https://github.com/GoogleCloudPlatform/spring-cloud-gcp/pull/3183 is merged +chmod 755 ./scripts/generate-showcase.sh ./scripts/generate-showcase.sh pushd showcase/showcase-spring-starter mvn verify From 935534ec0c3f5b6f79d854155ab5d6c95b06dcc8 Mon Sep 17 00:00:00 2001 From: Diego Marquez Date: Sat, 31 Aug 2024 11:12:06 -0400 Subject: [PATCH 58/63] Update library_generation/test/integration_tests.py Co-authored-by: Joe Wang <106995533+JoeWang1127@users.noreply.github.com> --- library_generation/test/integration_tests.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/library_generation/test/integration_tests.py b/library_generation/test/integration_tests.py index 2257f6aa8e..ebb8c66afa 100644 --- a/library_generation/test/integration_tests.py +++ b/library_generation/test/integration_tests.py @@ -53,7 +53,8 @@ # library_generation/test/resources/integration/test_generator_coordinates # This allows us to decouple the generation workflow testing with what the # generator jar will actually generate. -# See library_generation/DEVELOPMENT.MD +# See library_generation/DEVELOPMENT.md ("The Hermetic Build's +# well-known folder). WELL_KNOWN_GENERATOR_JAR_FILENAME = "gapic-generator-java.jar" From b01717a4e66c0c1ebd91d2c837f6df73057ae51e Mon Sep 17 00:00:00 2001 From: diegomarquezp Date: Sat, 31 Aug 2024 15:17:32 +0000 Subject: [PATCH 59/63] one sentence per line in DEVELOPMENT.md --- library_generation/DEVELOPMENT.md | 45 ++++++++++++++++--------------- 1 file changed, 24 insertions(+), 21 deletions(-) diff --git a/library_generation/DEVELOPMENT.md b/library_generation/DEVELOPMENT.md index d9411f9c0e..3226f2dc83 100644 --- a/library_generation/DEVELOPMENT.md +++ b/library_generation/DEVELOPMENT.md @@ -4,8 +4,7 @@ # Linting -When contributing, ensure your changes to python code have a valid -format. +When contributing, ensure your changes to python code have a valid format. ``` python -m pip install black @@ -30,9 +29,9 @@ python -m unittest test/integration_tests.py # Running the unit tests The unit tests of the hermetic build scripts are contained in several scripts, -corresponding to a specific component. Every unit test script ends with -`unit_tests.py`. To avoid them specifying them -individually, we can use the following command: +corresponding to a specific component. +Every unit test script ends with `unit_tests.py`. +To avoid them specifying them individually, we can use the following command: ```bash python -m unittest discover -s test/ -p "*unit_tests.py" @@ -45,28 +44,32 @@ python -m unittest discover -s test/ -p "*unit_tests.py" # Running the scripts in your local environment Although the scripts are designed to be run in a Docker container, you can also -run them directly. This section explains how to run the entrypoint script +run them directly. +This section explains how to run the entrypoint script (`library_generation/cli/entry_point.py`). ## Assumptions made by the scripts ### The Hermetic Build's well-known folder Located in `${HOME}/.library_generation`, this folder is assumed by the scripts -to contain the generator JAR. Please note that this is a recent feature and only -this jar is expected to be there. Soon enough, more binaries such as the gRPC -and protoc plugins will also be stored here. Developers must make sure -this folder is properly configured before running the scripts locally. -Note that this relies on the `HOME` en var which is always -defined as per +to contain the generator JAR. +Please note that this is a recent feature and only this jar is expected to be +there. +Soon enough, more binaries such as the gRPC and protoc plugins will also be +stored here. +Developers must make sure this folder is properly configured before running the +scripts locally. +Note that this relies on the `HOME` en var which is always defined as per [POSIX env var definition](https://pubs.opengroup.org/onlinepubs/9699919799/basedefs/V1_chap08.html). #### Put the gapic-generator-java jar in its well-known location Run `cd sdk-platform-java && mvn install -DskipTests -Dclirr.skip --Dcheckstyle.skip`. This will generate a jar located in +-Dcheckstyle.skip`. +This will generate a jar located in `~/.m2/repository/com/google/api/gapic-generator-java/{version}/gapic-generator-java-{version}.jar` -Then `mv` the jar into the well-known location of the jar. The -generation scripts will assume the jar is there. +Then `mv` the jar into the well-known location of the jar. +The generation scripts will assume the jar is there. ```shell mv /path/to/jar "${HOME}/.library_generation/gapic-generator-java.jar" @@ -137,9 +140,9 @@ This will create an `image-id` file at the root of the repo with the hash ID of the image. ## Run the docker image -The docker image will perform changes on its internal `/workspace` folder, to which you -need to map a folder on your host machine (i.e. map your downloaded repo to this -folder). +The docker image will perform changes on its internal `/workspace` folder, +to which you need to map a folder on your host machine (i.e. map your downloaded +repo to this folder). To run the docker container on the google-cloud-java repo, you must run: ```bash @@ -156,9 +159,9 @@ docker run -u "$(id -u)":"$(id -g)" -v/path/to/google-cloud-java:/workspace $(c ## Debug the created containers If you are working on changing the way the containers are created, you may want -to inspect the containers to check the setup. It would be convenient in such -case to have a text editor/viewer available. You can achieve this by modifying -the Dockerfile as follows: +to inspect the containers to check the setup. +It would be convenient in such case to have a text editor/viewer available. +You can achieve this by modifying the Dockerfile as follows: ```docker # install OS tools From 38b53a26dbb5331a05dd297cd2f367705bd4a05c Mon Sep 17 00:00:00 2001 From: Diego Marquez Date: Sat, 31 Aug 2024 11:19:56 -0400 Subject: [PATCH 60/63] Update .kokoro/presubmit/downstream-compatibility-spring.sh --- .kokoro/presubmit/downstream-compatibility-spring.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.kokoro/presubmit/downstream-compatibility-spring.sh b/.kokoro/presubmit/downstream-compatibility-spring.sh index 286b304a92..edc99d6cb7 100755 --- a/.kokoro/presubmit/downstream-compatibility-spring.sh +++ b/.kokoro/presubmit/downstream-compatibility-spring.sh @@ -50,7 +50,7 @@ pushd spring-cloud-gcp pushd spring-cloud-generator # The script is not executable for non-owners. Here we manually chmod it. # TODO(diegomarquezp): remove this line after -# https://github.com/GoogleCloudPlatform/spring-cloud-gcp/pull/3183 is merged +# https://github.com/GoogleCloudPlatform/spring-cloud-gcp/pull/3183 is merged and released. chmod 755 ./scripts/generate-showcase.sh ./scripts/generate-showcase.sh pushd showcase/showcase-spring-starter From 87b7425efdbc0e71bca183f43bb36a8239e4b70e Mon Sep 17 00:00:00 2001 From: diegomarquezp Date: Tue, 3 Sep 2024 19:35:53 +0000 Subject: [PATCH 61/63] use python for newer generate_library units --- .../test/generate_library_unit_tests.py | 96 +++++++++++++++++++ .../test/generate_library_unit_tests.sh | 55 ----------- library_generation/utils/utilities.py | 43 +++++++-- library_generation/utils/utilities.sh | 2 +- 4 files changed, 131 insertions(+), 65 deletions(-) create mode 100644 library_generation/test/generate_library_unit_tests.py diff --git a/library_generation/test/generate_library_unit_tests.py b/library_generation/test/generate_library_unit_tests.py new file mode 100644 index 0000000000..86a6a5d637 --- /dev/null +++ b/library_generation/test/generate_library_unit_tests.py @@ -0,0 +1,96 @@ +import subprocess +import unittest +import os +from library_generation.utils.utilities import ( + run_process_and_print_output as bash_call, + run_process_and_get_output_string as get_bash_call_output, +) + +script_dir = os.path.dirname(os.path.realpath(__file__)) + + +class GenerateLibraryUnitTests(unittest.TestCase): + """ + Confirms the correct behavior of `library_generation/utils/utilities.sh`. + + Note that there is an already existing, shell-based, test suite for + generate_library.sh, but these tests will soon be transferred to this one as + an effort to unify the implementation of the Hermetic Build scripts as + python-only. New tests for `utilities.sh` should be added in this file. + """ + + TEST_ARCHITECTURE = "linux-x86_64" + + def setUp(self): + # we create a simulated home folder that has a fake generator jar + # in its well-known location + self.simulated_home = get_bash_call_output("mktemp -d") + bash_call(f"mkdir {self.simulated_home}/.library_generation") + bash_call( + f"touch {self.simulated_home}/.library_generation/gapic-generator-java.jar" + ) + + # We create a per-test directory where all output files will be created into. + # Each folder will be deleted after its corresponding test finishes. + test_dir = get_bash_call_output("mktemp -d") + self.output_folder = self._run_command_and_get_sdout( + "get_output_folder", + cwd=test_dir, + ) + bash_call(f"mkdir {self.output_folder}") + + def tearDown(self): + bash_call(f"rm -rdf {self.simulated_home}") + + def _run_command(self, command, **kwargs): + env = os.environ.copy() + env["HOME"] = self.simulated_home + if "cwd" not in kwargs: + kwargs["cwd"] = self.output_folder + return bash_call( + [ + "bash", + "-exc", + f"source {script_dir}/../utils/utilities.sh " + f"&& {command}", + ], + exit_on_fail=False, + env=env, + **kwargs, + ) + + def _run_command_and_get_sdout(self, command, **kwargs): + return self._run_command( + command, stderr=subprocess.PIPE, **kwargs + ).stdout.decode()[:-1] + + def test_get_grpc_version_with_no_env_var_fails(self): + # the absence of DOCKER_GRPC_VERSION will make this function to fail + result = self._run_command("get_grpc_version") + self.assertEquals(1, result.returncode) + self.assertRegex(result.stdout.decode(), "DOCKER_GRPC_VERSION is not set") + + def test_get_protoc_version_with_no_env_var_fails(self): + # the absence of DOCKER_PROTOC_VERSION will make this function to fail + result = self._run_command("get_protoc_version") + self.assertEquals(1, result.returncode) + self.assertRegex(result.stdout.decode(), "DOCKER_PROTOC_VERSION is not set") + + def test_download_tools_without_baked_generator_fails(self): + # This test has the same structure as + # download_tools_succeed_with_baked_protoc, but meant for + # gapic-generator-java. + + test_protoc_version = "1.64.0" + test_grpc_version = "1.64.0" + jar_location = ( + f"{self.simulated_home}/.library_generation/gapic-generator-java.jar" + ) + # we expect the function to fail because the generator jar is not found in + # its well-known location. To achieve this, we temporarily remove the fake + # generator jar + bash_call(f"rm {jar_location}") + result = self._run_command( + f"download_tools {test_protoc_version} {test_grpc_version} {self.TEST_ARCHITECTURE}" + ) + self.assertEquals(1, result.returncode) + self.assertRegex(result.stdout.decode(), "Please configure your environment") diff --git a/library_generation/test/generate_library_unit_tests.sh b/library_generation/test/generate_library_unit_tests.sh index 7b7a6c3e47..2e4912536e 100755 --- a/library_generation/test/generate_library_unit_tests.sh +++ b/library_generation/test/generate_library_unit_tests.sh @@ -22,15 +22,6 @@ extract_folder_name_test() { assertEquals "google-cloud-aiplatform-v1-java" "${folder_name}" } -get_grpc_version_fails_with_no_env_var_test() { - # the absence of DOCKER_GRPC_VERSION will make this function to fail - exit_code=0 - ( - get_grpc_version - ) || exit_code=$? - assertEquals 1 "${exit_code}" -} - get_grpc_version_succeed_docker_env_var_test() { local version_with_docker local version_without_docker @@ -51,15 +42,6 @@ get_protoc_version_succeed_docker_env_var_test() { unset DOCKER_PROTOC_VERSION } -get_protoc_version_fails_with_no_env_var_test() { - # the absence of DOCKER_PROTOC_VERSION will make this function to fail - exit_code=0 - ( - get_protoc_version - ) || exit_code=$? - assertEquals 1 "${exit_code}" -} - get_gapic_opts_with_rest_test() { local proto_path="${script_dir}/resources/gapic_options" local transport="grpc" @@ -183,40 +165,6 @@ download_tools_succeed_with_baked_grpc() { unset grpc_path } -download_tools_fails_without_baked_generator() { - # This test has the same structure as - # download_tools_succeed_with_baked_protoc, but meant for - # gapic-generator-java. - local test_dir=$(mktemp -d) - pushd "${test_dir}" - export output_folder=$(get_output_folder) - mkdir "${output_folder}" - - local test_protoc_version="1.64.0" - local test_grpc_version="1.64.0" - local jar_location="${SIMULATED_HOME}/.library_generation/gapic-generator-java.jar" - # we expect the function to fail because the generator jar is not found in - # its well-known location. To achieve this, we temporarily remove the fake - # generator jar - rm "${jar_location}" - log=$(mktemp) - ( - download_tools "${test_protoc_version}" "${test_grpc_version}" "linux-x86_64" > /dev/null 2>"${log}" - ) || echo '' # expected - - has_expected_log="false" - if grep -q "Please configure your environment" < "${log}"; then - has_expected_log="true" - fi - assertEquals "true" "${has_expected_log}" - - rm -rdf "${output_folder}" - touch "${jar_location}" - unset DOCKER_GRPC_LOCATION - unset output_folder - unset grpc_path -} - download_grpc_plugin_succeed_with_valid_version_linux_test() { download_grpc_plugin "1.55.1" "linux-x86_64" assertFileOrDirectoryExists "protoc-gen-grpc-java-1.55.1-linux-x86_64.exe" @@ -334,10 +282,8 @@ get_proto_path_from_preprocessed_sources_multiple_proto_dirs_fails() { # One line per test. test_list=( extract_folder_name_test - get_grpc_version_fails_with_no_env_var_test get_grpc_version_succeed_docker_env_var_test get_protoc_version_succeed_docker_env_var_test - get_protoc_version_fails_with_no_env_var_test get_gapic_opts_with_rest_test get_gapic_opts_without_rest_test get_gapic_opts_with_non_default_test @@ -348,7 +294,6 @@ test_list=( download_protoc_failed_with_invalid_arch_test download_tools_succeed_with_baked_protoc download_tools_succeed_with_baked_grpc - download_tools_fails_without_baked_generator download_grpc_plugin_succeed_with_valid_version_linux_test download_grpc_plugin_succeed_with_valid_version_macos_test download_grpc_plugin_failed_with_invalid_version_linux_test diff --git a/library_generation/utils/utilities.py b/library_generation/utils/utilities.py index 59f238aaea..1aa60d21c0 100755 --- a/library_generation/utils/utilities.py +++ b/library_generation/utils/utilities.py @@ -17,6 +17,8 @@ import os import shutil from pathlib import Path +from typing import Any + from library_generation.model.generation_config import GenerationConfig from library_generation.model.library_config import LibraryConfig from typing import List @@ -39,20 +41,43 @@ def create_argument(arg_key: str, arg_container: object) -> List[str]: return [] -def run_process_and_print_output(arguments: List[str], job_name: str = "Job"): +def run_process_and_print_output( + arguments: List[str] | str, job_name: str = "Job", exit_on_fail=True, **kwargs +) -> Any: """ Runs a process with the given "arguments" list and prints its output. If the process fails, then the whole program exits + :param arguments: list of strings or string containing the arguments + :param job_name: optional job name to be printed + :param exit_on_fail: True if the main program should exit when the + subprocess exits with non-zero. Used for testing. + :param kwargs: passed to the underlying subprocess.run() call """ - # check_output() raises an exception if it exited with a nonzero code - try: - output = subprocess.check_output(arguments, stderr=subprocess.STDOUT) - print(output.decode(), end="", flush=True) - print(f"{job_name} finished successfully") - except subprocess.CalledProcessError as ex: - print(ex.output.decode(), end="", flush=True) - print(f"{job_name} failed") + # split "arguments" if passed as a single string + if isinstance(arguments, str): + arguments = arguments.split(" ") + if "stderr" not in kwargs: + kwargs["stderr"] = subprocess.STDOUT + proc_info = subprocess.run(arguments, stdout=subprocess.PIPE, **kwargs) + print(proc_info.stdout.decode(), end="", flush=True) + print( + f"{job_name} {'finished successfully' if proc_info.returncode == 0 else 'failed'}" + ) + if exit_on_fail and proc_info.returncode != 0: sys.exit(1) + return proc_info + + +def run_process_and_get_output_string( + arguments: List[str] | str, job_name: str = "Job", exit_on_fail=True, **kwargs +) -> Any: + """ + Wrapper of run_process_and_print_output() that returns the merged + stdout and stderr in a single string without its trailing newline char. + """ + return run_process_and_print_output( + arguments, job_name, exit_on_fail, stderr=subprocess.PIPE, **kwargs + ).stdout.decode()[:-1] def sh_util(statement: str, **kwargs) -> str: diff --git a/library_generation/utils/utilities.sh b/library_generation/utils/utilities.sh index d10daccf11..ae285b1b04 100755 --- a/library_generation/utils/utilities.sh +++ b/library_generation/utils/utilities.sh @@ -124,7 +124,7 @@ get_protoc_version() { echo "${DOCKER_PROTOC_VERSION}" return else - >&2 echo "Cannot infer protoc version because DOCKER_GRPC_VERSION is not set" + >&2 echo "Cannot infer protoc version because DOCKER_PROTOC_VERSION is not set" exit 1 fi } From 6a4f446d0026fb90038ae0217b26bba5298771ef Mon Sep 17 00:00:00 2001 From: Diego Marquez Date: Tue, 3 Sep 2024 16:32:45 -0400 Subject: [PATCH 62/63] Remove uncertain prediction in DEVELOPMENT.md --- library_generation/DEVELOPMENT.md | 2 -- 1 file changed, 2 deletions(-) diff --git a/library_generation/DEVELOPMENT.md b/library_generation/DEVELOPMENT.md index 3226f2dc83..858b467797 100644 --- a/library_generation/DEVELOPMENT.md +++ b/library_generation/DEVELOPMENT.md @@ -54,8 +54,6 @@ Located in `${HOME}/.library_generation`, this folder is assumed by the scripts to contain the generator JAR. Please note that this is a recent feature and only this jar is expected to be there. -Soon enough, more binaries such as the gRPC and protoc plugins will also be -stored here. Developers must make sure this folder is properly configured before running the scripts locally. Note that this relies on the `HOME` en var which is always defined as per From b524768d41b72f9d65713b46f5829b7105b0d3a3 Mon Sep 17 00:00:00 2001 From: diegomarquezp Date: Tue, 3 Sep 2024 20:47:32 +0000 Subject: [PATCH 63/63] ignore jars in library_generation, from root blacklist --- .gitignore | 2 ++ library_generation/test/resources/integration/.gitignore | 2 -- 2 files changed, 2 insertions(+), 2 deletions(-) delete mode 100644 library_generation/test/resources/integration/.gitignore diff --git a/.gitignore b/.gitignore index 98c7b77b4a..6768e8cd69 100644 --- a/.gitignore +++ b/.gitignore @@ -24,3 +24,5 @@ target/ **/*egg-info/ **/build/ **/dist/ +library_generation/**/*.jar + diff --git a/library_generation/test/resources/integration/.gitignore b/library_generation/test/resources/integration/.gitignore deleted file mode 100644 index cb2fa3374a..0000000000 --- a/library_generation/test/resources/integration/.gitignore +++ /dev/null @@ -1,2 +0,0 @@ -# Ignore all downloaded jars. The integration test downloads a specific generator jar -*.jar