chore(hermetic-build): include .github template updates as part of generation#3723
chore(hermetic-build): include .github template updates as part of generation#3723diegomarquezp merged 14 commits intomainfrom
Conversation
The templates come from https://github.com/googleapis/synthtool/tree/bf182cd41d9a7de56092cafcc7befe6b398332f6 The fix follows from https://setuptools.pypa.io/en/latest/userguide/datafiles.html: ``` Glob patterns do not automatically match dotfiles, i.e., directory or file names starting with a dot (.). To include such files, you must explicitly start the pattern with a dot, e.g. .* to match .gitignore. ```
This reverts commit c25a14a.
| #!/usr/bin/env bash | ||
|
|
||
| set -eo pipefail | ||
| set -exo pipefail |
There was a problem hiding this comment.
xtrace was the only way I could debug my way out in local development. This decision may have taken longer for someone less familiar with the codebase.
There was a problem hiding this comment.
I didn't add -x because I think the extra logs may substantial and this level of logs is not recommended for production.
Do you think it's necessary to add these logs?
There was a problem hiding this comment.
I reverted this and explained how this would be useful in the dev guide.
hermetic_build/DEVELOPMENT.md
Outdated
|
|
||
| ```shell | ||
| mv /path/to/jar "${HOME}/.library_generation/gapic-generator-java.jar" | ||
| mv ~/.m2/repository/com/google/api/gapic-generator-java/{version}/gapic-generator-java-{version}.jar "${HOME}/.library_generation/gapic-generator-java.jar" |
There was a problem hiding this comment.
Is this too much detail? What if a user change the default maven dir?
There was a problem hiding this comment.
I thought it may be more convenient for copying the whole line and limit to making a replacement in {version}. I think a non-default maven setup is not a common use case, but still checks out, so I reverted this line.
| shell session. | ||
|
|
||
| If you get a permission denied error when running the command `owl-bot`, try | ||
| relinking owl-bot by running `npm unlink -g` and re-running the steps above. |
There was a problem hiding this comment.
Why would rerun the same command can solve the permission issue?
There was a problem hiding this comment.
npm link has effect in the current shell session. My guess is that what's in an old session may get its permissions overriden.
| "owlbot/templates/poms/*.j2", | ||
| "owlbot/templates/java_library/.github/**/*", | ||
| "owlbot/templates/java_library/.kokoro/**/*", | ||
| "owlbot/templates/java_library/**/*", |
There was a problem hiding this comment.
Do we still need this line?
There was a problem hiding this comment.
yes, there are files outside .github and .kokoro that this line would include.
| #!/usr/bin/env bash | ||
|
|
||
| set -eo pipefail | ||
| set -exo pipefail |
There was a problem hiding this comment.
Again, should we include extra logs in production?
There was a problem hiding this comment.
I reverted this and explained how this would be useful in the dev guide.
diegomarquezp
left a comment
There was a problem hiding this comment.
Thanks for the review!
| shell session. | ||
|
|
||
| If you get a permission denied error when running the command `owl-bot`, try | ||
| relinking owl-bot by running `npm unlink -g` and re-running the steps above. |
There was a problem hiding this comment.
npm link has effect in the current shell session. My guess is that what's in an old session may get its permissions overriden.
| "owlbot/templates/poms/*.j2", | ||
| "owlbot/templates/java_library/.github/**/*", | ||
| "owlbot/templates/java_library/.kokoro/**/*", | ||
| "owlbot/templates/java_library/**/*", |
There was a problem hiding this comment.
yes, there are files outside .github and .kokoro that this line would include.
hermetic_build/DEVELOPMENT.md
Outdated
|
|
||
| ```shell | ||
| mv /path/to/jar "${HOME}/.library_generation/gapic-generator-java.jar" | ||
| mv ~/.m2/repository/com/google/api/gapic-generator-java/{version}/gapic-generator-java-{version}.jar "${HOME}/.library_generation/gapic-generator-java.jar" |
There was a problem hiding this comment.
I thought it may be more convenient for copying the whole line and limit to making a replacement in {version}. I think a non-default maven setup is not a common use case, but still checks out, so I reverted this line.
| #!/usr/bin/env bash | ||
|
|
||
| set -eo pipefail | ||
| set -exo pipefail |
There was a problem hiding this comment.
I reverted this and explained how this would be useful in the dev guide.
| #!/usr/bin/env bash | ||
|
|
||
| set -eo pipefail | ||
| set -exo pipefail |
There was a problem hiding this comment.
I reverted this and explained how this would be useful in the dev guide.
|
I don't think CODEOWNERS can be templated any more. |
|
Can you evaluate the templated, generated files are really needed? Seeing kokoro/nightly/java7.cfg, it seems you haven't checked that. |
hermetic_build/library_generation/owlbot/templates/java_library/.github/generated-files-bot.yml
Show resolved
Hide resolved
|
|
…neration (#3723) Part of the fix for #3701 ☕ ### Approach The templates come from https://github.com/googleapis/synthtool/tree/bf182cd41d9a7de56092cafcc7befe6b398332f6. The only update was to include `generated-files-bot`, which is already up to date in all the HW repos. The `.kokoro` folder will be a separate follow up task. We will now focus on solving the update of `update_generation_config` `yaml` and `sh` files. The fix to include the `.github` folder follows from https://setuptools.pypa.io/en/latest/userguide/datafiles.html: ``` Glob patterns do not automatically match dotfiles, i.e., directory or file names starting with a dot (.). To include such files, you must explicitly start the pattern with a dot, e.g. .* to match .gitignore. ``` Interestingly, this is also the reason `cp synthool/gcp/templates/java_library/* ...` did not bring folders starting with dot (such as .kokoro) into #2884 ### Confirming effects in downstream repos Demos show the results as of b66af92 in - googleapis/java-storage#3012 - googleapis/java-logging#1787 - googleapis/java-pubsub#2384 - googleapis/java-bigtable#2546 - googleapis/java-spanner#3711 - googleapis/java-firestore#2065 - googleapis/java-datastore#1810 - googleapis/java-bigquerystorage#2929 - googleapis/java-pubsublite#1837 There were no regressions on templated files that were manually modified.
…neration (#3723) Part of the fix for #3701 ☕ ### Approach The templates come from https://github.com/googleapis/synthtool/tree/bf182cd41d9a7de56092cafcc7befe6b398332f6. The only update was to include `generated-files-bot`, which is already up to date in all the HW repos. The `.kokoro` folder will be a separate follow up task. We will now focus on solving the update of `update_generation_config` `yaml` and `sh` files. The fix to include the `.github` folder follows from https://setuptools.pypa.io/en/latest/userguide/datafiles.html: ``` Glob patterns do not automatically match dotfiles, i.e., directory or file names starting with a dot (.). To include such files, you must explicitly start the pattern with a dot, e.g. .* to match .gitignore. ``` Interestingly, this is also the reason `cp synthool/gcp/templates/java_library/* ...` did not bring folders starting with dot (such as .kokoro) into #2884 ### Confirming effects in downstream repos Demos show the results as of b66af92 in - googleapis/java-storage#3012 - googleapis/java-logging#1787 - googleapis/java-pubsub#2384 - googleapis/java-bigtable#2546 - googleapis/java-spanner#3711 - googleapis/java-firestore#2065 - googleapis/java-datastore#1810 - googleapis/java-bigquerystorage#2929 - googleapis/java-pubsublite#1837 There were no regressions on templated files that were manually modified.



Part of the fix for #3701 ☕
Approach
The templates come from https://github.com/googleapis/synthtool/tree/bf182cd41d9a7de56092cafcc7befe6b398332f6. The only update was to include
generated-files-bot, which is already up to date in all the HW repos.The
.kokorofolder will be a separate follow up task. We will now focus on solving the update ofupdate_generation_configyamlandshfiles.The fix to include the
.githubfolder follows from https://setuptools.pypa.io/en/latest/userguide/datafiles.html:Interestingly, this is also the reason
cp synthool/gcp/templates/java_library/* ...did not bring folders starting with dot (such as .kokoro) into #2884Confirming effects in downstream repos
Demos show the results as of b66af92 in
There were no regressions on templated files that were manually modified.