Repository navigation
fix: address v3.2.0 review findings - #11
Merged
Merged
Conversation
…erified empirically with a real Gradle 9.7.1 build: under PREFER_SETTINGS, project.repositories.mavenCentral() does not throw InvalidUserCodeException — it silently succeeds and triggers a warning that Gradle now flags as deprecated (incompatible with Gradle 10). The old hasMavenCentral + catch-exception guard was blind to this. Fixed by detecting the resolved RepositoriesMode up front via Gradle's internal GradleInternal/SettingsInternal API (no public API exposes this to a project plugin; guarded with a try/catch(Throwable) fallback in case that internal API ever changes). Added a TestKit test (preferSettingsRepositoriesAreNotMutatedByTheProject) proving project.repositories.size() stays 0 and the deprecation warning never fires. Also fixed the sibling settingsManagedRepositoriesDoNotBreakTheBuild test, which was failing on this branch due to an unrelated typo (gmdVersion = 'not-a-real-gmd-version'). Finding 2 — Maven plugin forced-destroy not awaited (confirmed). destroyForcibly() is asynchronous; the Mojo could return with the forked JVM still alive. Added destroyForciblyAndAwaitTermination, which blocks on waitFor() after the forced kill and retries if that wait is itself interrupted, restoring the interrupt flag once termination is confirmed. Added two unit tests covering both the "actually waits" and "second interrupt preserved" behavior. Also updated gmd-gradle-plugin/release.md and readme.md to reflect the corrected repository-detection behavior (the old readme's guidance was itself misleading — declaring a private mirror without setting repositoriesMode wouldn't have stopped the plugin from adding Central).
…rated TestKit build script (GmdGradlePluginTest.groovy:339), so the invalidation check doesn't rely
on the convention default.
• The invalidation step swaps log4j instead of gmd-core (GmdGradlePluginTest.groovy:373): log4jVersion = '2.26.1' → '2.25.1'. Both are @input properties on the task, so
this exercises the identical invalidation path while leaving the runnable gmd-core:3.1.0 in place — avoiding the JavaFX dependency graph that every pre-3.1.0 gmd-core
release carries. The assertion "Changing a runtime dependency version must rerun processGmd" is unchanged and now passes (processGmd reruns with SUCCESS after the
swap).
…ovy:36-46 now sets conventions for all four version @inputs in the constructor, mirroring the plugin's defaults. I chose conventions over @optional per the reviewer's tradeoff note: a direct registrant's classpath is then still tracked by some version values rather than nothing. To prevent the two sites diverging, the literals now live as constants (DEFAULT_GROOVY_VERSION, DEFAULT_LOG4J_VERSION, DEFAULT_IVY_VERSION) shared with the extension conventions in GmdGradlePlugin.groovy:28-32, and defaultGmdVersion() is now package-visible for the task's lazy convention. Regression test: directlyRegisteredTaskHasVersionInputDefaults. Fix 2 — thematic break in a list item drops list context — the isLeafBlock branch in GmdTemplateEngine.groovy now applies the same dedent rule as the else branch (relativeIndent < listContentColumn), so --- aligned with the list content column keeps the list alive while * * * at column 0 still resets it. Tests: aThematicBreakInsideAListItemKeepsTheListContext, aThematicBreakDedentPastTheListContentColumnEndsTheList. Fix 3 — blank block-quote line classified as a paragraph — a quote line whose stripped content is blank (>, > ) is now classified as a blank line: it clears previousLineWasParagraph, sets previousLineWasBlockQuote, and leaves inIndentedCode/listContentColumn alone. Test: aBlankBlockQuoteLineDoesNotStartAParagraph. Fix 4 — misleading timeout diagnostic — destroyForciblyAndAwaitTermination now returns a ForcedTerminationAwait enum (TERMINATED/TIMED_OUT/INTERRUPTED), a single warnOnIncompleteForcedTermination helper replaces the two duplicated warn blocks, and the timeout is a FORCED_TERMINATION_TIMEOUT_SECONDS constant also used to build the message. The three reflection-based tests were updated to assert the enum names; the bounded-wait and interrupt-restoration behavior they pin is unchanged. Fix 6 — coupled-test comment — testPlugin now carries a comment stating that runtimeVersionsAreTrackedAsTaskInputs is load-bearing alongside the log4jVersion invalidation proof, plus a note that the pinned '2.26.1' duplicates GmdGradlePlugin.DEFAULT_LOG4J_VERSION.
…iagnostics - gradle plugin: add a required classpathIdentity @input to ProcessGmdTask so a directly registered task's runtime classpath invalidates up-to-date checks; GmdGradlePlugin derives it from the four version inputs, and direct registrants must wire it from their own configuration or fail validation loudly instead of going silently stale (the version conventions alone never reflected a real classpath, and classpath itself is @internal) - move the DEFAULT_* version constants and defaultGmdVersion() from GmdGradlePlugin onto ProcessGmdTask, package-scoped via @PackageScope, so no new public static API lands on the published plugin class and the dependency stays plugin -> task - maven plugin: switch explicitly on TERMINATED instead of default, move FORCED_TERMINATION_TIMEOUT_SECONDS into the field block, and read it reflectively in tests instead of hard-coding 30 seconds - add TestKit proofs: an unwired classpathIdentity fails loudly, and a wired identity invalidates a directly registered task when the classpath swaps
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.
Summary
Validation