feat: hermetic build scripts to use a single output/generation folder #1987
Conversation
diegomarquezp
changed the title
| # compare with gapic library | ||
| (diff -ru "${SHOWCASE_DIR}/${GAPIC_PROJECT_DIR}/src/main/java" "${SCRIPT_DIR}/output/showcase-output/gapic-showcase-output/src/main/java") | ||
| } || { | ||
| failure="true" |
There was a problem hiding this comment.
Is this logic correct? I see that showcase integration tests always exit with 1.
There was a problem hiding this comment.
It is correct, I found out why they were failing. It was because there was a pushd cd (fixed in 0b54240)
Now mvn verify -P enable-golden-tests behaves correctly
Co-authored-by: Joe Wang <[email protected]>
| @@ -45,11 +45,14 @@ done | |||
| script_dir=$(dirname "$(readlink -f "$0")") | |||
| source "${script_dir}/../utilities.sh" | |||
| library_generation_dir="${script_dir}"/.. | |||
| cd "${library_generation_dir}" | |||
| output_folder="$(pwd)/output" | |||
| mkdir -p "${output_folder}" | |||
There was a problem hiding this comment.
I think output_folder will be created by generate_library.sh, does it need to be created here?
There was a problem hiding this comment.
we need external repositories to be located in there as well, which is handled by generate_showcase.sh and generate_library_integration_test.sh. I guess it would make more sense to rely on a function to prevent duplication
There was a problem hiding this comment.
Fixed in 0ebc098
| @@ -181,15 +181,14 @@ generate_library_failed_with_invalid_generator_version() { | |||
| local destination="google-cloud-alloydb-v1-java" | |||
| local res=0 | |||
| cd "${script_dir}/resources" | |||
| $("${script_dir}"/../generate_library.sh \ | |||
| "${script_dir}"/../generate_library.sh \ | |||
There was a problem hiding this comment.
The reason I wrap this command into $() is because generate_library.sh will exit if the version is incorrect and use $() to force back to the current shell and execute command after \\.
I'm not sure whether this is the best approach though, maybe worth an investigation later.
There was a problem hiding this comment.
Not sure if this approach is good either but I prepended bash $script in 7fc632a to prevent the whole test suite from exiting if that's what you mean by force back to the current shell. I was having issues when running the unit tests after the adding the OS detection (or a few commits later) function which happened to be fixed by removing $().
…/single-folder-output' into feat/hermetic-build/single-folder-output
Co-authored-by: Joe Wang <[email protected]>
Co-authored-by: Joe Wang <[email protected]>
…/single-folder-output' into feat/hermetic-build/single-folder-output
library_generation/utilities.sh
Outdated
| @@ -1,6 +1,8 @@ | |||
| #!/usr/bin/env bash | |||
|
|
|||
| set -xeo pipefail | |||
| utilities_script_dir=$(echo "${BASH_SOURCE}" | rev | cut -d/ -f2- | rev) | |||
There was a problem hiding this comment.
Could you add a comment to explain what this line does?
There was a problem hiding this comment.
@JoeWang1127 I forgot we are calling this script from wherever, leaving our output folder at mercy of the user's CWD instead of forcing it to be in library_generation. I reverted this change but still left the utility function now relying on $(pwd)
|
[gapic-generator-java-root] Kudos, SonarCloud Quality Gate passed!
|
|
[java_showcase_integration_tests] Kudos, SonarCloud Quality Gate passed!
|
|
[java_showcase_unit_tests] Kudos, SonarCloud Quality Gate passed!
|
This enables the hermetic build scripts to generate and download everything into a single
outputfolder, to ease the cleanup in case it failsShowcase scripts were adjusted accordingly
README was adjusted to explain the new structure