chore: bake protoc into the hermetic build docker image #2707

diegomarquezp · 2024-04-30T19:49:26Z

Does the config work with renovate bot?

Yes. After a local run using LOG_LEVEL=debug npx --yes --package renovate -- renovate --platform=local &> renovate.out, we obtained a dependency update in the logs:

           {
             "deps": [
               {     
                 "depName": "protocolbuffers/protobuf",
                 "currentValue": "25.2",
                 "datasource": "github-releases",
                 "extractVersion": "^v(?<version>.*)$",
                 "replaceString": "ARG PROTOC_VERSION=25.2\n",
                 "updates": [
                   {     
                     "bucket": "non-major",
                     "newVersion": "25.3",
                     "newValue": "25.3",
                     "releaseTimestamp": "2024-02-15T23:20:43.000Z",
                     "newMajor": 25,
                     "newMinor": 3,
                     "updateType": "minor",
                     "branchName": "renovate/protocolbuffers-protobuf-25.x"
                   },    
                   {     
                     "bucket": "major",
                     "newVersion": "26.1",
                     "newValue": "26.1",
                     "releaseTimestamp": "2024-03-27T20:28:47.000Z",
                     "newMajor": 26,
                     "newMinor": 1,
                     "updateType": "major",
                     "branchName": "renovate/protocolbuffers-protobuf-26.x"
                   }
                 ],
                 "packageName": "protocolbuffers/protobuf",
                 "versioning": "semver-coerced",
                 "warnings": [],
                 "sourceUrl": "https://github.com/protocolbuffers/protobuf",
                 "registryUrl": "https://github.com",
                 "currentVersion": "25.2",
                 "currentVersionTimestamp": "2024-01-09T23:52:55.000Z",
                 "isSingleVersion": true, 
                 "fixedVersion": "25.2"
               }
             ],

However, we disabled the setting for now. This log entry confirms it.
DEBUG: Dependency: protocolbuffers/protobuf, is disabled (repository=local)

JoeWang1127 · 2024-05-15T15:29:25Z

library_generation/test/resources/output/gapic-generator-java-pom-parent-2.24.0.pom

@@ -0,0 +1,197 @@
+<?xml version="1.0" encoding="UTF-8"?>


Since this is a test resource, could you remove all unused part?

My bad, it slipped into my commits!

JoeWang1127 · 2024-05-15T15:31:03Z

library_generation/utils/utilities.sh

@@ -157,7 +161,22 @@ download_generator_artifact() {
 download_protoc() {
  local protoc_version=$1
  local os_architecture=$2
-  if [ ! -d "protoc-${protoc_version}" ]; then
+
+  protoc_dirname="protoc-${protoc_version}"


Could you add a comment specifying the the protoc version priority?

.cloudbuild/library_generation/library_generation.Dockerfile

blakeli0 · 2024-05-15T21:28:53Z

renovate.json

+    {
+      "customType": "regex",
+      "fileMatch": [
+		"^\\.cloudbuild/library_generation/library_generation\\.Dockerfile$"


Actually we probably don't want to let renovate bot to automatically update protoc. Not before we can update the whole repo(java-comon-protos, java-iam, test files in gapic-generator-java) along side the protoc update. Because the new protoc version may not work with the current protos or the newly generated code may not work with the generator.

blakeli0 · 2024-05-15T21:36:28Z

.cloudbuild/library_generation/library_generation.Dockerfile

+
+# install protobuf
+WORKDIR /protoc
+RUN ls /src


Is this just for troubleshooting purposes?

Indeed. I removed it

blakeli0 · 2024-05-15T22:00:53Z

library_generation/utils/utilities.sh

+    # if the specified protoc_version matches the one baked in the docker
+    # container, we just copy it into the output folder
+    mkdir -p "${output_folder}/${protoc_dirname}"
+    cp -r "${DOCKER_PROTOC_LOCATION}/${protoc_dirname}" \


Can we use the protoc location specified in Dockerfile?

Yes. We now explicitly export the protoc_path variable to simply point to this location instead of copying it.

blakeli0 · 2024-05-15T22:05:09Z

library_generation/utils/utilities.sh

@@ -157,7 +161,22 @@ download_generator_artifact() {
 download_protoc() {


In general, I feel like we may not need any changes in download_protoc. Ideally this function can stay as it is and we can add logics before calling it, for example

if [[ "${protoc_version}" != "${DOCKER_PROTOC_VERSION}" ]]; then download_protoc "${protoc_version}" "${os_architecture}" fi

But I think this requires us to reuse the proto location specified in Dockerfile.

I left download_protoc almost untouched and left the DOCKER_PROTOC_VERSION handling in download_tools (one level up in the call hierarchy).

blakeli0 · 2024-05-15T22:07:24Z

library_generation/utils/utilities.sh

+  if [[ -n "${DOCKER_PROTOC_VERSION}" ]]; then
+    >&2 echo "Using protoc version baked into the container: ${DOCKER_PROTOC_VERSION}"
+    echo "${DOCKER_PROTOC_VERSION}"
+  fi


I guess we can return this function early?

~~This became obsolete as we moved this if statement outside of download_protoc~~

Removed.

Co-authored-by: Joe Wang <106995533+JoeWang1127@users.noreply.github.com>

…build

diegomarquezp · 2024-05-22T01:59:16Z

library_generation/test/integration_tests.py

@@ -308,6 +304,3 @@ def __recursive_diff_files(
                sub_dcmp, diff_files, left_only, right_only, dirname + sub_dirname + "/"
            )

-    @classmethod
-    def __remove_docker_image(cls):


Since the GitHub runner cleans itself up after running the ITs and we usually want to troubleshoot locally via this integration test, it may be more convenient to not have to build the image from scratch every time when debugging.

blakeli0 · 2024-05-22T22:26:36Z

.cloudbuild/library_generation/library_generation.Dockerfile

@@ -15,24 +15,35 @@
 # build from the root of this repo:
 FROM gcr.io/cloud-devrel-public-resources/python

-ARG SYNTHTOOL_COMMITTISH=a2c9b4a5da2d7f583c8a1869fd2843c206145834
+SHELL [ "/bin/bash", "-c" ]


Anything requires us to change the default shell to bash?

The nvm installation script assumes the environment is bash based. Using sh makes the following line to have no effects.

sdk-platform-java/.cloudbuild/library_generation/library_generation.Dockerfile

Line 63 in 5e71efa

RUN curl -o- https://raw.githubusercontent.com/nvm-sh/nvm/v0.35.3/install.sh | bash

sonarqubecloud · 2024-05-23T14:17:29Z

Please retry analysis of this Pull-Request directly on SonarCloud

sonarqubecloud · 2024-05-23T14:20:42Z

Quality Gate passed for 'java_showcase_integration_tests'

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
No data about Duplication

See analysis details on SonarCloud

### Does the config work with renovate bot? Yes. After a local run using `LOG_LEVEL=debug npx --yes --package renovate -- renovate --platform=local &> renovate.out`, we obtained a dependency update in the logs: ``` { "deps": [ { "depName": "protocolbuffers/protobuf", "currentValue": "25.2", "datasource": "github-releases", "extractVersion": "^v(?<version>.*)$", "replaceString": "ARG PROTOC_VERSION=25.2\n", "updates": [ { "bucket": "non-major", "newVersion": "25.3", "newValue": "25.3", "releaseTimestamp": "2024-02-15T23:20:43.000Z", "newMajor": 25, "newMinor": 3, "updateType": "minor", "branchName": "renovate/protocolbuffers-protobuf-25.x" }, { "bucket": "major", "newVersion": "26.1", "newValue": "26.1", "releaseTimestamp": "2024-03-27T20:28:47.000Z", "newMajor": 26, "newMinor": 1, "updateType": "major", "branchName": "renovate/protocolbuffers-protobuf-26.x" } ], "packageName": "protocolbuffers/protobuf", "versioning": "semver-coerced", "warnings": [], "sourceUrl": "https://github.com/protocolbuffers/protobuf", "registryUrl": "https://github.com", "currentVersion": "25.2", "currentVersionTimestamp": "2024-01-09T23:52:55.000Z", "isSingleVersion": true, "fixedVersion": "25.2" } ], ``` However, we disabled the setting for now. This log entry confirms it. `DEBUG: Dependency: protocolbuffers/protobuf, is disabled (repository=local)` --------- Co-authored-by: Joe Wang <106995533+JoeWang1127@users.noreply.github.com>

…3045) This PR bakes the gRPC plugin in the hermetic library generation image. The approach is very similar to #2707 * The docker image will have a configurable `ARG` for the gRPC plugin * The docker image will download the gRPC plugin at the version specified in the previous point * The docker image will set the variables DOCKER_GRPC_VERSION and DOCKER_GRPC_LOCATION * `generate_library.sh` will now be able to avoid downloading the gRPC plugin if the computed plugin version is the same as DOCKER_GRPC_VERSION (all this via `utilities.sh`). --------- Co-authored-by: Blake Li <blakeli@google.com>

diegomarquezp added 30 commits April 2, 2024 18:24

chore: bake synthtool into library_generation docker image

0b65f15

remove synthtool usage

a6bb064

do not use config file

d4f27e2

bake synthtool in dockerfile

19a0b04

bake copy-code into the image

bc693b5

remove unnecessary SHELL statement

f335cbb

add info for installing synthtool

aab50ef

modify owl-bot usage in postprocess_library

f97cdca

Merge remote-tracking branch 'origin/main' into transfer-synthtool-java

d68c14a

fix synthtool installation

eca0d92

change permissions to script folder

6aa8533

assume location of generation_config.yaml

53c5dea

fix permissions

cc4309b

fix path to config yaml

7a4b6b8

use entrypoint for docker image

4838d5b

format

1b01713

finish development guide

c149461

remove olwbot_cli usage

4ee1692

add entrypoint for docker image

e77bacc

manually set HOME var in owlbot

9120c94

Merge remote-tracking branch 'origin/main' into transfer-synthtool-java

3d18426

post-merge cleanup

427c564

fix parameters

7b79763

fix synthtool sha

240742a

fix entrypoint

8498f2c

Merge remote-tracking branch 'origin/main' into transfer-synthtool-java

f893e5c

lint

2faf45d

remove unused entrypoint file

7ce9548

remove owlbot CLI from readmne

8474452

restore xtrace

b58e990

product-auto-label bot added the size: l Pull request size is large. label May 14, 2024

JoeWang1127 reviewed May 15, 2024

View reviewed changes

.cloudbuild/library_generation/library_generation.Dockerfile Outdated Show resolved Hide resolved

blakeli0 reviewed May 15, 2024

View reviewed changes

diegomarquezp and others added 6 commits May 16, 2024 11:33

Update .cloudbuild/library_generation/library_generation.Dockerfile

eb48f45

Co-authored-by: Joe Wang <106995533+JoeWang1127@users.noreply.github.com>

remove debug dockerfile directive

706d095

move docker embedded protoc logic outside of download_protoc

868a1bf

Merge remote-tracking branch 'origin/main' into bake-protoc-hermetic-…

5d9beb5

…build

return from function early

6d76843

remove test file from commit history

8a57737

product-auto-label bot added size: m Pull request size is medium. and removed size: l Pull request size is large. labels May 22, 2024

diegomarquezp added 3 commits May 22, 2024 01:11

fix unit tests

550f162

fix python units

340622c

fix IT

5e71efa

diegomarquezp commented May 22, 2024

View reviewed changes

blakeli0 mentioned this pull request May 22, 2024

chore: add generation config #2792

Merged

JoeWang1127 approved these changes May 22, 2024

View reviewed changes

blakeli0 approved these changes May 22, 2024

View reviewed changes

diegomarquezp added 2 commits May 23, 2024 14:09

lint

c4b7f72

Merge branch 'main' into bake-protoc-hermetic-build

df0d23c

diegomarquezp enabled auto-merge (squash) May 23, 2024 14:10

diegomarquezp merged commit cf44b07 into main May 23, 2024
49 checks passed

diegomarquezp deleted the bake-protoc-hermetic-build branch May 23, 2024 14:23

diegomarquezp mentioned this pull request Jul 11, 2024

chore: bake the grpc plugin in the hermetic library generation image #3045

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

chore: bake protoc into the hermetic build docker image #2707

chore: bake protoc into the hermetic build docker image #2707

diegomarquezp commented Apr 30, 2024 •

edited

Loading

JoeWang1127 May 15, 2024

diegomarquezp May 16, 2024

diegomarquezp May 22, 2024

JoeWang1127 May 15, 2024

blakeli0 May 15, 2024

blakeli0 May 15, 2024

diegomarquezp May 22, 2024

blakeli0 May 15, 2024

diegomarquezp May 22, 2024

blakeli0 May 15, 2024

diegomarquezp May 22, 2024

blakeli0 May 15, 2024

diegomarquezp May 22, 2024 •

edited

Loading

diegomarquezp May 22, 2024

blakeli0 May 22, 2024

diegomarquezp May 23, 2024

sonarqubecloud bot commented May 23, 2024

sonarqubecloud bot commented May 23, 2024

		@@ -157,7 +161,22 @@ download_generator_artifact() {
		download_protoc() {

chore: bake protoc into the hermetic build docker image #2707

chore: bake protoc into the hermetic build docker image #2707

Conversation

diegomarquezp commented Apr 30, 2024 • edited Loading

Does the config work with renovate bot?

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

diegomarquezp May 22, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sonarqubecloud bot commented May 23, 2024

sonarqubecloud bot commented May 23, 2024

Quality Gate passed for 'java_showcase_integration_tests'

diegomarquezp commented Apr 30, 2024 •

edited

Loading

diegomarquezp May 22, 2024 •

edited

Loading