Fix JavaSourceSet phantom diffs and use cached updates in all wrapping visitors#7376
Merged
steve-aom-elliott merged 4 commits intomainfrom Apr 14, 2026
Merged
Conversation
When multiple dependency recipes update JavaSourceSet markers, JAR downloads may succeed for some recipes in cycle 1 but fail for others. In cycle 2, the previously-failed downloads retry and succeed, creating marker-only diffs that trigger the 2-cycle assertion in downstream tests like Boot3UpgradeTest.xmlBindMissing in rewrite-spring. The fix: in changeDependency(), when gavToTypes is non-empty but doesn't contain the old dependency key, skip the update. This dependency was never part of the tracked classpath, so there's nothing to change. The existing empty-gavToTypes path is preserved for initial population.
Four wrapping visitors (Maven/Gradle UpgradeDependencyVersion and Maven/Gradle AddDependency) were not using the cached overload of JavaSourceSet.updateOnSourceFile(), causing redundant JAR downloads per file and missing cross-file identity preservation within a source set. Align them with the pattern already used by ChangeDependencyGroupIdAndArtifactId and Gradle ChangeDependency.
steve-aom-elliott
changed the title
Avoid unnecessary object allocation and findRemoteRepository calls for files that hit the updateOnSourceFile cache.
Defer construction of JavaSourceSetUpdater (which creates temp directories and HTTP downloaders) until the cache misses, avoiding unnecessary work when the cached JavaSourceSet is reused.
This was referenced Apr 22, 2026
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.
Summary
changeDependency()idempotency inJavaSourceSetUpdater— skip updates whengavToTypesis non-empty but doesn't contain the old dependency key (the dep was never part of the tracked classpath)updateOnSourceFileoverload in all six JavaSourceSet wrapping visitors for consistent cross-file deduplication within a source setJavaSourceSetUpdaterconstruction and object allocation (newGav/newDep) into the cached lambda so they only execute on cache missesContext
PR #7202 added JavaSourceSet marker updating to dependency-modifying recipes. Two issues emerged:
Phantom diffs from non-idempotent
changeDependency(): When multiple recipes in a composite likeUpgradeSpringBoot_3_0run, some JAR downloads succeed in cycle 1 while others fail. In cycle 2, retries succeed and add new types — creating marker-only diffs that trigger extra-cycle assertions.Inconsistent caching:
ChangeDependencyGroupIdAndArtifactId(Maven) andChangeDependency(Gradle) used the cachedupdateOnSourceFile(sf, cache, transform)overload, butUpgradeDependencyVersion(Maven/Gradle) andAddDependency(Maven/Gradle) did not — causing redundant JAR downloads per file and missing cross-file identity preservation.Changes
JavaSourceSetUpdater.changeDependency(): Return early whengavToTypesis non-empty but doesn't contain the old GAV key. Preserve the empty-gavToTypespath for initial population.UpgradeDependencyVersion(Maven): Replace manual marker extraction and identity check withJavaSourceSet.updateOnSourceFile(sf, cache, transform)UpgradeDependencyVersion(Gradle): Same refactor, moving the multi-dep loop inside the transform lambdaAddDependency(Maven/Gradle): Switch from non-cached to cachedupdateOnSourceFileoverloadJavaSourceSetUpdaterinitialization and object construction (newGav, newDep, remote repo lookup) inside the cached lambda to avoid unnecessary work on cache hitsTest plan
ChangeDependencyGroupIdAndArtifactIdTest.updatesJavaSourceSetMarkerOnJavaFilespassesChangeDependencyGroupIdAndArtifactIdTest.composedWithChangePackageUpdatesImportspassesrewrite-maventest suite passes (UpgradeDependencyVersionTest,AddDependencyTest,ChangeDependencyGroupIdAndArtifactIdTest)rewrite-gradletest suite passes (UpgradeDependencyVersionTest,AddDependencyTest,ChangeDependencyTest)rewrite-springBoot3UpgradeTest.xmlBindMissingpasses (was failing with "Expected recipe to complete in 1 cycle, but took 2 cycles")