Disable build cache for release artifacts#11733
Conversation
🟡 Java Benchmark SLOs — Performance SLO warning (near threshold)
PR vs. master results
Commit: Load and DaCapo benchmarks can be triggered manually in the GitLab pipeline. Results will appear in the Benchmarking Platform UI after completion. |
| fi | ||
| - export GRADLE_OPTS="-Dorg.gradle.jvmargs='-Xms$GRADLE_MEMORY_MIN -Xmx$GRADLE_MEMORY_MAX -XX:ErrorFile=/tmp/hs_err_pid%p.log -XX:+HeapDumpOnOutOfMemoryError -XX:HeapDumpPath=/tmp'" | ||
| - export GRADLE_ARGS=" --build-cache --stacktrace --no-daemon --parallel --max-workers=$GRADLE_WORKERS" | ||
| - export RELEASE_GRADLE_ARGS=" --no-build-cache --rerun-tasks --stacktrace --no-daemon --parallel --max-workers=$GRADLE_WORKERS" |
There was a problem hiding this comment.
suggestion: Looks good, but instead of duplicating all args, I was suggest having a variable only for the different args between snapshots and release, which currently only set or disable the build cache.
There was a problem hiding this comment.
What do you think of the current state? I wasn't able to extract only the release variables because we need to remove --build-cache from the original GRADLE_ARGS
7481157 to
a5e1ee0
Compare
| fi | ||
| - export GRADLE_OPTS="-Dorg.gradle.jvmargs='-Xms$GRADLE_MEMORY_MIN -Xmx$GRADLE_MEMORY_MAX -XX:ErrorFile=/tmp/hs_err_pid%p.log -XX:+HeapDumpOnOutOfMemoryError -XX:HeapDumpPath=/tmp'" | ||
| - export GRADLE_ARGS=" --build-cache --stacktrace --no-daemon --parallel --max-workers=$GRADLE_WORKERS" | ||
| - "export RELEASE_GRADLE_ARGS=${CI_COMMIT_TAG:+${GRADLE_ARGS/--build-cache/--no-build-cache --rerun-tasks}}" |
There was a problem hiding this comment.
thought: I'd rather not forcibly rerun tasks during release. For re-run tasks I'd rather validate it in another job.
Especially for the publish job.
There was a problem hiding this comment.
thought: I'd rather not forcibly rerun tasks during release. For re-run tasks I'd rather validate it in another job. Especially for the publish job.
Makes sense! IIUC the tasks run from scratch anyway after --no-build-cache, so I think the re-run can be removed entirely 🤔 ->Removed in 04cab87
Also, why double quote?
The quotes are required for the ${...} parsing. I tested valid gitlab file configurations here: https://gitlab.ddbuild.io/DataDog/apm-reliability/dd-trace-java/-/ci/editor
| - ./gradlew --version | ||
| - ./gradlew clean :dd-java-agent:shadowJar :dd-java-agent:check :dd-trace-api:jar :dd-trace-ot:shadowJar -PskipTests -x spotlessCheck $GRADLE_ARGS | ||
| - 'if [ -n "$CI_COMMIT_TAG" ]; then echo "=== RELEASE BUILD: assembling agent jar with build cache disabled ==="; fi' | ||
| - './gradlew clean :dd-java-agent:shadowJar :dd-java-agent:check :dd-trace-api:jar :dd-trace-ot:shadowJar -PskipTests -x spotlessCheck ${RELEASE_GRADLE_ARGS:-$GRADLE_ARGS}' |
There was a problem hiding this comment.
The single quotes here are just to match the single quotes in the line above (needed because the : wasn't parsed correctly otherwise) and for the ${...} parsing.
Actually though, I will remove the problematic : from the logging and stick to double quotes here. ->Done in 04cab87
| - export GPG_PASSWORD=$(aws ssm get-parameter --region us-east-1 --name ci.dd-trace-java.signing.gpg_passphrase --with-decryption --query "Parameter.Value" --out text) | ||
| - ./gradlew -PbuildInfo.build.number=$CI_JOB_ID publishToSonatype closeSonatypeStagingRepository -PskipTests $GRADLE_ARGS | ||
| - 'if [ -n "$CI_COMMIT_TAG" ]; then echo "=== RELEASE BUILD: publishing artifacts with build cache disabled ==="; fi' | ||
| - './gradlew -PbuildInfo.build.number=$CI_JOB_ID publishToSonatype closeSonatypeStagingRepository -PskipTests ${RELEASE_GRADLE_ARGS:-$GRADLE_ARGS}' |
There was a problem hiding this comment.
I think for the publication having the build cache is necessary, otherwise it might rebuild the jar!?
There was a problem hiding this comment.
Yes duh you're right! It should use build's same rebuilt jar! Reverted this in 04cab87, thanks
There was a problem hiding this comment.
Nevermind.... Let me think about this some more.
7e0e5bd to
6440971
Compare
6440971 to
dee36d3
Compare
What Does This Do
Disable gradle's build cache for release artifacts
Motivation
Ensure release artifacts are assembled from a totally fresh task execution, rather than relying on potentially stale or incorrect build-cache states
Additional Notes
The
buildstep was tested by manually running a GitLab pipeline withCI_COMMIT_TAG=test-release-cache-disabledas a variable (seeEFFECTIVE_GRADLE_ARGSprint lines in build job here). However, thedeploy_to_maven_centraljob is dependent on a version commit tag that is protected by repo settings, i.e. it can only be tested during an actual release.Contributor Checklist
type:and (comp:orinst:) labels in addition to any other useful labelsclose,fix, or any linking keywords when referencing an issueUse
solvesinstead, and assign the PR milestone to the issue/merge. You can also:/merge --commit-message "..."/merge -c/merge -f --reason "reason"; please use this judiciously, as some checks do not run at the PR-level (note: the PR still needs to be mergeable, this will only skip the pre-merge build)Jira ticket: APMLP-1377