Fall back to the wrapper's distributionUrl when recipes find no BuildTool - #8433
Merged
Conversation
…dTool` `UpdateGradleWrapper` and `UpdateMavenWrapper` gated on a `BuildTool` marker of the matching type being present on the wrapper properties file. Only the OpenRewrite build-tool plugins attach that marker, so any other parser pipeline produced a silent no-op with no way to tell "up to date" apart from "provenance missing". It has broken three times in the Moderne CLI alone (moderneinc/moderne-cli#1005, #4414, #4472). The marker is also a worse source of truth than the file itself: the plugins set it from the tool that happened to run the build, while `distributionUrl` is the wrapper's version by definition, and both recipes already parse it. Both scanners now prefer the marker and fall back to the version in `distributionUrl`, bailing out as before when neither yields one. The marker is only updated when one was present to begin with. The `getVisitor()` guard that skipped any file carrying a non-matching `BuildTool` now only rejects non-wrapper paths, so a `gradlew`/`mvnw` stamped with another build tool is still updated once its properties file was accepted.
…tionUrl` Both `visitCompilationUnit` overloads bailed when the script they visited carried no `BuildTool` marker of type `Gradle`, the same invisible provenance contract the wrapper recipes just shed. The marker feeds a single decision — whether the build is Gradle 6 or newer, and so whether the plugin belongs in the settings script or the root build script — which `gradle-wrapper.properties` answers by definition. The recipe is now a `ScanningRecipe`. The scanner records each wrapper's version keyed by the directory that wrapper governs, and scripts resolve against the nearest enclosing wrapper, so a nested build with its own wrapper is never judged by the root's version. The marker still wins when present; with neither the recipe bails as before. The duplicated guard is now one helper so the Groovy and Kotlin paths cannot drift. `versionFromDistributionUrl` moves out of `UpdateGradleWrapper` into `org.openrewrite.gradle.internal.GradleWrapperProperties` so both recipes share one copy. This does not relax the `GradleSettings`/`GradleProject` requirement that follows, so it helps LSTs carrying Gradle project provenance but no `BuildTool`, not ones missing both.
timtebeek
changed the title
distributionUrl when the wrapper recipes find no BuildTooldistributionUrl when recipes find no BuildTool
The `wrapperUri` option's version was read with `GRADLE_VERSION_PATTERN`, whose `gradle-([0-9.]+)` stops at the first hyphen, so a pre-release distribution such as `gradle-8.5-rc-1-bin.zip` yielded `8.5`. That version is both written into the `gradleVersion` DSL, where it disagreed with the `distributionUrl` beside it, and used to pick the `gradlew` script template. Try `GradleWrapperProperties.versionFromDistributionUrl` first and fall back to the loose pattern only when it finds nothing, so URLs it does not recognize behave exactly as before. `versions.csv` carries script templates for pre-release versions, so `findNearest` now resolves an RC to that RC's own scripts rather than the release's.
- Move `versionFromDistributionUrl` from the new `internal.GradleWrapperProperties` onto `util.GradleWrapper`, beside the `GRADLE_VERSION_PATTERN` it complements, and drop the internal class. This makes the method public API rather than internal, so the two distribution URL parsers now sit together where the next reader will find them. - Revert the `getVisitor()` guards in both wrapper recipes to their original form and delete `isWrapperPath`. The guard only ever relaxed a case where the properties file carries a matching `BuildTool` while a sibling wrapper file carries a different one, an LST shape that does not occur, so the tests covering it go too. - Express `AddDevelocityGradlePlugin`'s version resolution as `filter`/`map`/`orElseGet`, matching the scanners in the wrapper recipes.
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
1 participant
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.
UpdateGradleWrapper,UpdateMavenWrapperandAddDevelocityGradlePluginall gated on aBuildToolmarker of the matching type, which only the OpenRewrite build-tool plugins attach — so on an LST from any other parser they silently did nothing, indistinguishable from "already up to date" (this has broken three times in the Moderne CLI alone: moderneinc/moderne-cli#1005, moderneinc/moderne-cli#4414, moderneinc/moderne-cli#4472). The marker is also a worse source of truth than the file itself: the plugins set it from whichever tool ran the build, whiledistributionUrlis the wrapper's version by definition. All three scanners now prefer the marker and otherwise derive the version fromdistributionUrl, still bailing out when neither yields one, and only bumping the marker when one was present to begin with. The shared extraction isGradleWrapper.versionFromDistributionUrl, whichGradleWrapper.create(URI)also now uses so a pre-releasewrapperUriyields8.5-rc-1rather than8.5— that value picks thegradlewscript template, andversions.csvcarries templates for pre-releases.Only the scanners changed; which files the visitors rewrite is untouched.
AddDevelocityGradlePluginbecomes aScanningRecipethat keys each wrapper's version by the directory it governs and resolves each script against the nearest enclosing wrapper, so a nested build with its own wrapper is never judged by the root's. ItsGradleSettings/GradleProjectrequirement is deliberately unchanged, so it helps LSTs that carry Gradle project provenance but noBuildTool, not ones missing both.New tests across all three recipes cover no marker, a uniform
BuildTool(ModerneCli, …)LST, wrappers in subdirectories, marker-wins-on-disagreement, the downgrade guard, and adistributionUrlwith no extractable version;AddDevelocityGradlePluginTestalso covers a nested build resolving against its own wrapper. The pre-existingupdateMultipleWrapperstests were fixed to stop sharingSourceSpecinstances acrossdir(...)calls, whichDir.iterator()mutates in place. Verified against the moderne-cli#4472 reproduction: rewrite-github-actions' wrapper on 9.5.1 with noBuildToolanywhere now updates all four files to 9.7.0.