fix: join the build lifecycle with dependsOn, not finalizedBy - #89
Merged
Conversation
Since 0.1.2 a consumer that declares its own
`build.dependsOn(assembleRequirements)` fails at configuration time:
Circular dependency between the following tasks:
:assembleRequirements
\--- :build
\--- :assembleRequirements (*)
Nothing compiles -- the build dies before any task runs.
0.1.2 added `build.finalizedBy(assembleRequirements)`, which says the task
runs AFTER build. The consumer's line says before. Gradle rejects the
contradiction. Consumers had every reason to have written that line:
0.1.0 and 0.1.1 shipped no lifecycle wiring at all, while the 0.1.0
README documented the task as depending on `check` and running as part of
`build`. They wired by hand to get the documented behaviour, and 0.1.2
added the opposing edge with no migration note.
Wiring `build dependsOn assembleRequirements dependsOn check` instead
keeps the same execution order -- the task still runs after the tests
whose XML it reads -- while composing with those consumers rather than
contradicting them. dependsOn is idempotent, so their line is now
redundant and harmless, and no consumer has to edit its build file.
It also makes `gradle assembleRequirements` on its own meaningful: it now
runs `check` first, so the zip contains test results instead of silently
omitting them.
Covered by BuildLifecycleWiringTest, which drives a real consumer project
through TestKit with `--dry-run` -- a circular dependency is raised while
the task graph is built, so the regression is reproduced without
compiling or resolving anything. Verified to fail against the old wiring
before being made to pass. tests/fixtures/test_project cannot cover this:
it declares no wiring of its own, so it only ever exercises the case
where the plugin is the sole author of these edges.
Closes #88
Signed-off-by: Jimisola Laursen <jimisola@jimisola.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #88.
What & Why
Since 0.1.2, a consumer that declares its own
build.dependsOn(assembleRequirements)fails at configuration time — nothing compiles:0.1.2 added
build.finalizedBy(assembleRequirements), which says the task runs afterbuild. The consumer's line says before. Gradle rejects the contradiction.Consumers had every reason to have written that line. 0.1.0 and 0.1.1 shipped no lifecycle wiring at all, while the 0.1.0 README documented
assembleRequirementsas "Depends onchecktask (ensures tests have run)" and "Runs automatically as part ofbuildtask". The plugin implemented neither, so consumers wired it by hand to get the documented behaviour — and 0.1.2 added the opposing edge with no migration note. Affected: 0.1.2, 1.0.0 andmain; 0.1.1 is the last clean version.The fix
Wire
build dependsOn assembleRequirements dependsOn checkinstead ofbuild finalizedBy assembleRequirements.Same execution order — the task still runs after the tests whose XML it reads — but
dependsOncomposes wherefinalizedBycontradicts.dependsOnis idempotent, so an existing consumer's line becomes redundant rather than fatal, and no consumer has to edit its build file.It also makes
gradle assembleRequirementson its own meaningful: it now runscheckfirst, so the zip contains test results instead of silently omitting them. Verified — running the task standalone produces bothtest_results/*.xml.Verification
New
BuildLifecycleWiringTestdrives a real consumer project through TestKit.--dry-runis deliberate: a circular dependency is raised while Gradle builds the task graph, so the regression reproduces without compiling or resolving anything — the suite stays fast and offline.I confirmed the test fails against the old wiring before making it pass:
tests/fixtures/test_projectstructurally cannot catch this — it declares no wiring of its own, so it only ever exercises the case where the plugin is the sole author of these edges. That is why CI stayed green through the regression.End-to-end, with the fixed plugin published to
mavenLocaland the fixture given the wiring that used to break:Zip contents unchanged and correct —
requirements.yml,software_verification_cases.yml,annotations.ymlcarrying bothSVC_001andSVC_002, both test-result XMLs,reqstool_config.yml.Docs
README.md,docs/(usage, configuration, index) andCLAUDE.mdall describedbuildas finalized by the task. Updated, including a note that an existingbuild.dependsOn(assembleRequirements)is now redundant but harmless.Author checklist
./gradlew buildgreen, formatting applied via./gradlew formattests/fixtures/test_projectTest Plan
./gradlew build— all tests pass, including the three new lifecycle tests.consumerDeclaringBuildDependsOnAssembleRequirementsStillConfiguresmust fail withCircular dependency.tests/fixtures/test_project, addtasks.named('build') { dependsOn(tasks.named('assembleRequirements')) }and rungradle buildagainst a locally published plugin — configures and builds;checkprecedesassembleRequirements.Note for release
This wants a patch release so affected consumers can move forward — 1.0.0 carries the regression, so the fix version is 1.0.1. Until then the workaround is to delete the manual wiring, which #88 records.