fix(hermetic-build): prevent overwrite of Version.java in new libraries'#12742
fix(hermetic-build): prevent overwrite of Version.java in new libraries'#12742diegomarquezp merged 3 commits intomainfrom
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request generalizes the path for preserving Version.java in OwlBot configurations by replacing specific module names with a wildcard regex. However, this introduces an inconsistency with the preservation pattern for integration tests, which still uses specific module names. It is recommended to update the integration test patterns to match the new generalized approach to ensure consistency and prevent potential build issues where integration tests might be incorrectly overwritten.
| deep-preserve-regex: | ||
| - "/{{ module_name }}/google-.*/src/test/java/com/google/cloud/.*/v.*/it/IT.*Test.java" | ||
| - "/{{ module_name }}/google-.*/src/main/java/com/google/cloud/.*/v.*/stub/Version.java" | ||
| - "/.*google-.*/src/main/java/com/google/cloud/.*/v.*/stub/Version.java" |
There was a problem hiding this comment.
The generalization of the path to use .* instead of {{ module_name }} for Version.java introduces an inconsistency with the integration tests pattern on line 23, which still uses the specific {{ module_name }}. If the goal is to support libraries where the directory structure might not strictly match {{ module_name }}, or to provide a more generic template for the monorepo, line 23 should also be updated to use .*. Otherwise, integration tests for these libraries may be incorrectly overwritten during the build process.
There was a problem hiding this comment.
This is due to new files being introduced to match the owl-bot-staging/v.* prefix.
There was a problem hiding this comment.
the integration tests do not need this pattern because ITs are not generated, so only a pattern that prevents removal is necessary.
On the other hand Version.java is both preserved and generated, where the existing Version.java needs a different pattern than the one about to be transferred (i.e. owl-bot-staging/...), so we need a more relaxed pattern.
| deep-preserve-regex: | ||
| - "/java-bare-metal-solution/google-.*/src/test/java/com/google/cloud/.*/v.*/it/IT.*Test.java" | ||
| - "/java-bare-metal-solution/google-.*/src/main/java/com/google/cloud/.*/v.*/stub/Version.java" | ||
| - "/.*google-.*/src/main/java/com/google/cloud/.*/v.*/stub/Version.java" |
There was a problem hiding this comment.
This golden file now contains an inconsistent preservation policy: Version.java is preserved using a generic regex, while integration tests (line 23) are still tied to the specific java-bare-metal-solution path. This should be updated to match the generalized pattern if the intention is for the configuration to be library-agnostic.
There was a problem hiding this comment.
This is due to new files being introduced to match the owl-bot-staging/v.* prefix.
There was a problem hiding this comment.
the integration tests do not need this pattern because ITs are not generated, so only a pattern that prevents removal is necessary.
On the other hand Version.java is both preserved and generated, where the existing Version.java needs a different pattern than the one about to be transferred (i.e. owl-bot-staging/...), so we need a more relaxed pattern.
2 participants
Applies changes found to work in 59581ef