chore: update hermetic library generation workflow #10693

JoeWang1127 · 2024-04-12T21:14:35Z

In this PR:

Add a workflow, when generation_config.yaml is changed in a pull request, to generate changed libraries and push commits to the corresponding pull request.
Update workflow only to update googleapis commit.
Update workflow only to add library configuration to generation_config.yaml.
Change image tag to 2.39.0

Follow up of googleapis/sdk-platform-java#2616

For the goal of series of PRs, please refer to improvement proposal.

JoeWang1127 · 2024-04-17T18:24:44Z

The workflows are tests in a forked repo:
https://github.com/JoeWang1127/google-cloud-java/pull/19 (update a library)
https://github.com/JoeWang1127/google-cloud-java/pull/17 (update googleapis commit)

blakeli0 · 2024-04-17T20:12:57Z

.github/workflows/hermetic_library_generation.yaml

      shell: bash
      run: |
-        diff generation_config.yaml baseline/generation_config.yaml
+        git show ${{ github.base_ref }}:generation_config.yaml > baseline_generation_config.yaml
+    - name: show configuration diff


Can we extract most of the logics here to a shell script so that we can call the script directly here? That way we can easily call it in local and possibly in a non-Github environment in the future.
Ideally, we should try to put this logic into Docker container as well, so that this logic could be reused by handwritten libraries, otherwise we would need to duplicate this in every handwritten repos. Bake it into Docker can be a future enhancement, but I think we should at least extract it to a separate script.

Moved command lines to a script.

blakeli0 · 2024-04-17T20:27:44Z

.github/workflows/update-googleapis-commit.yaml

+      base_branch: main
+    steps:
+    - uses: actions/checkout@v4
+    - name: Setup branch for pull request


Same comment as above. Let's extract it to a script for now, and think about ways to make it more reusable.

Moved command lines to a script.

JoeWang1127 · 2024-04-19T15:14:20Z

Should we remove "Verify GAPIC client library generation" or rewrite it using hermetic build script?

diegomarquezp

LGTM, just a couple nits and a question

diegomarquezp · 2024-04-19T15:18:43Z

.github/workflows/generate_new_client_hermetic_build.yaml

@@ -1,7 +1,7 @@
 name: Generate new GAPIC client library (Hermetic Build)
 on:
  workflow_dispatch:
-      # some inputs are ommited due to limit of 10 input arguments
+      # some inputs are emitted due to limit of 10 input arguments


nit: ommited should be omitted

diegomarquezp · 2024-04-19T15:21:08Z

.github/workflows/generated_files_sync.yaml

nit (maybe not in scope): What's the purpose of this script? Maybe a one-liner comment can help.

There's a one-liner comment in each job in the workflow.

diegomarquezp · 2024-04-19T15:43:32Z

generation/hermetic_library_generation.sh

+message="chore: generate libraries at $(date)"
+
+git checkout "${target_branch}"
+git checkout "${current_branch}"


qq: Has this double checkout statement a special effect?

In the action, the fetch-depth is 0 so it needs git checkout "${target_branch}" to load target_branch.

We want to copy generation_config.yaml from target branch to current branch.

blakeli0 · 2024-04-19T18:11:56Z

.github/workflows/hermetic_library_generation.yaml

 jobs:
  library_generation:
    runs-on: ubuntu-latest
    env:
-      library_generation_image_tag: latest
-      repo_volumes: "-v repo-google-cloud-java:/workspace/google-cloud-java"
+      library_generation_image_tag: 2.39.0


Is this configured to be updated renovate bot?

I plan to update renovate config in separate PR.

generation/new_client/new-client.py

blakeli0 · 2024-04-19T18:14:49Z

.github/workflows/generated_files_sync.yaml

@@ -20,7 +20,7 @@ on:
  pull_request:
 name: generation diff
 env:
-  library_generation_image_tag: latest
+  library_generation_image_tag: 2.39.0


Is this configured to be updated renovate bot?

I plan to update renovate config in separate PR.

blakeli0 · 2024-04-19T18:23:25Z

generation/hermetic_library_generation.sh

+# copy generation configuration from target branch to current branch.
+git show "${target_branch}":"${generation_config}" > "${baseline_generation_config}"
+diff "${generation_config}" "${baseline_generation_config}" || echo "config diff"
+# bind docker volume to include google-cloud-java in docker running environment.


This script is not specific to google-cloud-java anymore, can we mention something more generic?

blakeli0 · 2024-04-19T18:25:41Z

.github/workflows/hermetic_library_generation.yaml

    paths:
-      - generation_config.yaml
+    - "generation_config.yaml"


Is the trigger only for main branch? If yes, can this be configured to be triggered for other branches, like a future LTS branch?

This can be triggered when a pull request changes generation_config.yaml.

Therefore, it can be used in any branch since the pull request is not necessarily based on main.

I don't see any configuration for a target branch, does it mean if I create a PR with generation_config.yaml changes against a release/2.x branch, this workflow would be triggered too?

blakeli0 · 2024-04-19T18:26:26Z

generation/hermetic_library_generation.sh

Really like this script! This script is generic enough to be reused by any repo. We need to come up with a way to reused this script, but worst case we can just copy this script to every handwritten repos.

generation/hermetic_library_generation.sh

blakeli0 · 2024-04-19T18:50:18Z

.github/workflows/generated_files_sync.yaml

Can you remind me what this file is used for?

This workflow is used to verify generated files, e.g., owlbot.py, are not modified by pull request. Each job verifies one file.

What if we do want to update owlbot.py for a certain library? Also does it mean we run the generation for the whole repo on every PR?

What if we do want to update owlbot.py for a certain library?

The check will fail. This is a not required check only to remain the author the change is indeed intentional.

Also does it mean we run the generation for the whole repo on every PR?

root pom and gapic-libraries-bom are generated through hermetic build cli script (just like repo-level post processing). Other checks are done through shell script in this repo, which are not generating any java file, so they are relatively fast.

blakeli0 · 2024-04-19T19:42:07Z

.github/workflows/update_googleapis_commit.yaml

+    runs-on: ubuntu-22.04
+    env:
+      # the branch into which the pull request is merged
+      base_branch: lts-11


Is this lts-11 intentional?

No, this is a test value. I changed it to main.

blakeli0 · 2024-04-19T21:56:47Z

Should we remove "Verify GAPIC client library generation" or rewrite it using hermetic build script?

I think we can remove it as the logic of "new client generation" is part of the hermetic build scripts now.

JoeWang1127 · 2024-04-19T22:34:58Z

I think we can remove it as the logic of "new client generation" is part of the hermetic build scripts now.

I'll remove this check in another PR.

blakeli0

LGTM. Please update the PR title to make it more meaningful before merging, and update relevant docs after merging.

JoeWang1127 added 5 commits April 12, 2024 17:14

chore: update generation workflow

ae8e207

update workflow

b20b0c5

update new client creation workflow

8cef63a

add base branch

018be46

Merge branch 'main' into chore/update-gen-workflow

003a3e2

JoeWang1127 marked this pull request as ready for review April 17, 2024 18:21

JoeWang1127 requested a review from a team as a code owner April 17, 2024 18:21

JoeWang1127 requested review from blakeli0 and diegomarquezp April 17, 2024 18:22

blakeli0 reviewed Apr 17, 2024

View reviewed changes

JoeWang1127 added 6 commits April 17, 2024 20:20

use shell script

932d1a1

generalize repo name

8a4293e

add script to update googleapis commit

064fff2

update comments

9ffd66d

update workflow file

19c7912

update image tag

cca96ea

JoeWang1127 requested review from blakeli0 and suztomo April 19, 2024 14:46

try to fix workflow error

11e2255

diegomarquezp approved these changes Apr 19, 2024

View reviewed changes

JoeWang1127 added 2 commits April 19, 2024 11:50

change comment

314d136

update comment

fc85560

blakeli0 reviewed Apr 19, 2024

View reviewed changes

JoeWang1127 added 2 commits April 19, 2024 15:02

refactor according to code review

9b612c8

restore change

3acdfde

blakeli0 reviewed Apr 19, 2024

View reviewed changes

update branch

102b46c

blakeli0 approved these changes Apr 20, 2024

View reviewed changes

JoeWang1127 changed the title ~~chore: update generation workflow~~ chore: update hermetic library generation workflow Apr 20, 2024

JoeWang1127 merged commit 91fa7be into main Apr 20, 2024
31 of 32 checks passed

JoeWang1127 deleted the chore/update-gen-workflow branch April 20, 2024 00:36

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

chore: update hermetic library generation workflow #10693

chore: update hermetic library generation workflow #10693

JoeWang1127 commented Apr 12, 2024 •

edited

Loading

JoeWang1127 commented Apr 17, 2024 •

edited

Loading

blakeli0 Apr 17, 2024

JoeWang1127 Apr 19, 2024 •

edited

Loading

blakeli0 Apr 17, 2024

JoeWang1127 Apr 19, 2024

JoeWang1127 commented Apr 19, 2024

diegomarquezp left a comment

diegomarquezp Apr 19, 2024

JoeWang1127 Apr 19, 2024

diegomarquezp Apr 19, 2024

JoeWang1127 Apr 19, 2024

diegomarquezp Apr 19, 2024

JoeWang1127 Apr 19, 2024 •

edited

Loading

blakeli0 Apr 19, 2024

JoeWang1127 Apr 19, 2024

blakeli0 Apr 19, 2024

JoeWang1127 Apr 19, 2024

blakeli0 Apr 19, 2024

JoeWang1127 Apr 19, 2024

blakeli0 Apr 19, 2024

JoeWang1127 Apr 19, 2024 •

edited

Loading

blakeli0 Apr 19, 2024

JoeWang1127 Apr 19, 2024

blakeli0 Apr 19, 2024

blakeli0 Apr 19, 2024

JoeWang1127 Apr 19, 2024

blakeli0 Apr 19, 2024

JoeWang1127 Apr 19, 2024

blakeli0 Apr 19, 2024

JoeWang1127 Apr 19, 2024

blakeli0 commented Apr 19, 2024

JoeWang1127 commented Apr 19, 2024

blakeli0 left a comment

chore: update hermetic library generation workflow #10693

chore: update hermetic library generation workflow #10693

Conversation

JoeWang1127 commented Apr 12, 2024 • edited Loading

JoeWang1127 commented Apr 17, 2024 • edited Loading

Choose a reason for hiding this comment

JoeWang1127 Apr 19, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

JoeWang1127 commented Apr 19, 2024

diegomarquezp left a 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

JoeWang1127 Apr 19, 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

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

JoeWang1127 Apr 19, 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

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

blakeli0 commented Apr 19, 2024

JoeWang1127 commented Apr 19, 2024

blakeli0 left a comment

Choose a reason for hiding this comment

JoeWang1127 commented Apr 12, 2024 •

edited

Loading

JoeWang1127 commented Apr 17, 2024 •

edited

Loading

JoeWang1127 Apr 19, 2024 •

edited

Loading

JoeWang1127 Apr 19, 2024 •

edited

Loading

JoeWang1127 Apr 19, 2024 •

edited

Loading