Don't bump a shared version property to a version its siblings lack - #8460
Draft
timtebeek wants to merge 2 commits into
Draft
Don't bump a shared version property to a version its siblings lack#8460timtebeek wants to merge 2 commits into
timtebeek wants to merge 2 commits into
Conversation
`UpgradeDependencyVersion` wrote the new version into a `<properties>` entry without checking the other artifacts resolving through that property, leaving those siblings unresolvable. Now the dependencies consuming each property are collected during the scan, and when the new version is not published for all of them the targeted dependency is decoupled from the property with an explicit `<version>` instead.
Route the plugin dependency and annotation processor path upgrades through the same seam as regular dependencies, so those too decouple from a shared property rather than raising it to a version their siblings lack; the paths are now recorded as property consumers as well. Identify a property by the POM that declares it, so same-named properties in unrelated modules no longer block one another, and answer the "is this version published" probe once per coordinate.
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.
UpgradeDependencyVersionwrote the new version into a shared<properties>entry without checking that the version exists for the other artifacts resolving through that property, leaving those siblings unresolvable.The trigger observed in the wild is
UpgradeJackson_2_3, which upgradescom.fasterxml.jackson.core:jackson-annotationsto2.21. That version exists —jackson-annotationsuses bare minor versions — but the rest of Jackson 2.x uses three-part versions, so every sibling sharing${jackson.version}was rewritten to a version that does not exist. A Moderne flagship run on ALL/Open Source/FINOS (changeset 20260811034058-h2uII, "Spring Boot 4.0 best practices") produced 102 WARNING error markers of the form "Unable to download POM: com.fasterxml.jackson.core:jackson-core:2.21", acrossfinos/legend-sdlc,finos/legend-depotandfinos/legend-shared. This change should remove those markers; the recipe's chosen version stays as it is, so please verify against a later flagship run on that org.Behaviour
The scan phase now records, per property, the dependency coordinates whose version resolves through it (across every POM in the project, including
dependencyManagementand plugin dependencies). Before a property is written, every consumer is checked for the new version; a consumer that is a module of the build is skipped, and metadata that cannot be resolved is not treated as evidence that a version is missing, so a download failure never becomes a hard error or a silent skip.When the version is not published for all consumers, the targeted dependency is decoupled: an explicit
<version>is written on the dependency (or itsdependencyManagemententry) and the property is left alone. That was the preferred option in the report, and it is also what the Gradle recipe already does for a shared version variable —rewrite-gradle'sUpgradeDependencyVersionTest#doesNotCorruptSharedGradlePropertyasserts exactly this outcome on the same Jackson 2.21 case. The alternatives (skip with aMarkup.warn, or only update when all consumers agree) leave the requested upgrade undone, and would have left Maven and Gradle behaving differently on identical input.Single-consumer property updates are unchanged — that path still rewrites the property, now after confirming the version exists for the one artifact that uses it, which also keeps the decision stable across recipe cycles once a dependency has been decoupled.
Gradle
Already handled, no change needed:
safeUpdatedVersioninorg.openrewrite.gradle.UpgradeDependencyVersionrequires the agreed version to be published for every untargeted neighbour sharing the variable, coveringextblocks, Kotlinvals andgradle.properties.:rewrite-gradle:test --tests "*UpgradeDependencyVersionTest"is green.Every write of a
<version>tag now goes through one seam (changeVersionTagValue), so plugin<dependencies>and<annotationProcessorPaths>entries — which reach the sameChangePropertyValuepath — are covered too, and are recorded as property consumers.Tests
Four tests in
rewrite-maven'sUpgradeDependencyVersionTest, all failing before this change except the third:doNotUpgradeSharedPropertyWhenNewVersionMissingForOtherConsumer— the reported shape;jackson-coreused to end up at a nonexistent2.21.doNotUpgradeSharedManagedPropertyWhenNewVersionMissingForOtherConsumer— same throughdependencyManagement.upgradeSharedPropertyWhenNewVersionExistsForOtherConsumer— the property is still updated when the version resolves for both.doNotUpgradeSharedParentPropertyWhenNewVersionMissingForOtherModule— property in a local parent, consumers in two modules.doNotUpgradeSharedPluginDependencyPropertyWhenNewVersionMissingForOtherConsumer— a plugin dependency sharing the property with a regular dependency.upgradeSharedPropertyNameDeclaredSeparatelyPerModule— same property name declared independently in two unrelated modules must not block each other.Full
:rewrite-maven:test(1440 tests) is green. Rebased onto Ignore blank GAVs inretainVersionsandexcept#8455.