matrei commented on code in PR #16726:
URL: https://github.com/apache/grails-core/pull/16726#discussion_r4349738140
##########
.github/workflows/gradle.yml:
##########
@@ -31,9 +31,15 @@ on:
# results would be for a superseded commit, and the run that replaces it
publishes the newer one. The publish
# jobs (publishGradle, publish, publishForge, verifyWrapper, docs) run on
always() with explicit result checks
# instead, so a publish that has started completes: an interrupted upload
would leave the snapshot repository
-# with modules from two commits. Nothing of the cancelled commit is published
when the cancel arrives before
-# publish starts, because a cancelled test job fails the explicit checks. The
replacing run is pending until
-# the cancelled run has finished every job, so two publishes of the same
branch never overlap.
+# with modules from two commits. publishGradle waits for every build job
(buildGradle, buildForge and
+# validateDependencies), so nothing of the cancelled commit is published when
the cancel arrives before the
+# builds have finished. A later cancel lets the publish jobs whose needs have
already succeeded or been skipped
+# run: on a run without tests that is the whole chain, and on a run whose
tests are still running only
+# publishGradle, because a cancelled test job fails the explicit checks of
publish and of everything after it.
Review Comment:
Good catch, thanks. I added `buildRerunTasks` to the `needs` and the checks
of `publish` in dbf938c01d7. I'd rather take the 24 minutes than publish a core
whose `--rerun-tasks` build failed: it is the only core test run whose results
can't come from the build cache.
The same commit adds a line to the publish comment and a rule to
`AGENTS.md`: every test job goes into the `needs` of `publish`, and every job
that builds or validates without tests goes into those of `publishGradle`. That
should catch the next `casSecurityConfig`.
##########
.github/workflows/gradle.yml:
##########
@@ -536,14 +542,18 @@ jobs:
-PonlyHibernate5Tests
-PskipCodeStyle
publishGradle:
- # always() with an explicit result check, see the concurrency comment at
the top.
+ # always() with explicit result checks, see the concurrency comment at the
top. It waits for every build job,
+ # not only buildGradle, so that a run cancelled before its builds have
finished publishes nothing, and a
+ # commit whose Forge build or dependency validation failed is not
published.
if: >-
${{ always() &&
github.repository_owner == 'apache' &&
(github.event_name == 'push' || github.event_name ==
'workflow_dispatch') &&
- needs.buildGradle.result == 'success'
+ needs.buildGradle.result == 'success' &&
+ needs.buildForge.result == 'success' &&
+ needs.validateDependencies.result == 'success'
}}
- needs: [ buildGradle ]
+ needs: [ buildGradle, buildForge, validateDependencies ]
Review Comment:
Agreed. With `buildRerunTasks` added, the merge-up now conflicts on the
`needs` line of `publish` on 8.0.x and above, because those branches also list
`hibernate7Functional` and `neo4jFunctional` there. So the merge-up needs a
hand anyway. I checked this resolution on 8.0.x, 8.1.x and 9.0.x:
```yaml
publishGradle:
needs: [ buildGradle, buildForge, validateDependencies,
validateBomProperties, validateGithubBranch, buildLogic ]
publish:
needs: [ publishGradle, build, buildRerunTasks, functional, security,
securityConfig, casSecurityConfig, redis, hibernate5Functional,
hibernate7Functional, mongodbFunctional, neo4jFunctional ]
```
It also needs the matching result checks (`success` for the new build jobs,
`success || skipped` for `casSecurityConfig`), and the three build jobs added
to the list in the comment at the top and in `AGENTS.md`. With that, the
job-graph check passes on all three branches, every job is upstream of
`publish`, and actionlint is clean. I can do the 8.0.x merge-up of this PR
myself, or open a PR against 8.0.x right after it, whichever suits whoever
merges up.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
