Disable JAR downloading in JavaSourceSetUpdater#7459
Merged
Conversation
Short-circuit changeDependency and addDependency to return the source set unchanged and make downloadAndScanTypes return an empty list. This prevents MavenArtifactDownloader from being invoked during dependency mutation recipes, eliminating the per-visitor artifact fetches that multiplied outbound traffic during Spring Boot migrations. Two tests in ChangeDependencyGroupIdAndArtifactIdTest are @disabled because they asserted on classpath mutation performed by the download path.
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.
Short-circuits
JavaSourceSetUpdater.changeDependencyandaddDependencyto return the source set unchanged, and makesdownloadAndScanTypesreturn an empty list. This preventsMavenArtifactDownloaderfrom being invoked during dependency-mutation recipes (AddDependency,UpgradeDependencyVersion,ChangeDependencyGroupIdAndArtifactIdon Maven;AddDependency,UpgradeDependencyVersion,ChangeDependencyon Gradle).Motivation
The JAR-download path introduced in Expand star imports in ChangePackage and related recipes when they would create ambiguity #7202 (and refined by Prevent marker-only changes from producing phantom diffs #7358/Fix ChangeDependency routing Gradle build files to updateJavaSourceSet instead of gradleVisitor #7373/Fix JavaSourceSet phantom diffs and use cached updates in all wrapping visitors #7376/Share artifact cache across JavaSourceSetUpdater visitors #7454) fetches the newly-added/changed dependency's JAR on every visitor invocation so that
JavaSourceSetcan be updated in place. In high-volume migrations (e.g. Spring Boot 4, where a single run performs ~30–90 dependency mutations per repo), this produces a large volume of outbound artifact requests — even with the shared cache added in Share artifact cache across JavaSourceSetUpdater visitors #7454, each distinct GAV still has to be fetched once.This change disables that download path entirely while we reconsider the approach.
JavaSourceSetreverts to the pre-Expand star imports in ChangePackage and related recipes when they would create ambiguity #7202 behavior of reflecting only the parse-time classpath.Behavior change
JavaSourceSetwith types from new/changed dependencies.JavaSourceSet(e.g.ChangePackagestar-import disambiguation) observe the pre-mutation classpath only.JavaSourceSetUpdater.Tests
Two tests in
ChangeDependencyGroupIdAndArtifactIdTestare@Disabledbecause they asserted on the classpath mutation performed by the download path:updatesJavaSourceSetMarkerOnJavaFilescomposedWithChangePackageUpdatesImportsAll other tests in
:rewrite-mavenpass unchanged, including:JavaSourceSetUpdaterTest.typeCacheIsSharedAcrossUpdaters(checks cache sharing, not downloads)javaSourceSetUnchangedWhenModuleDoesNotHaveDependency(negative assertion)No Gradle tests reference
JavaSourceSet/gavToTypeson the affected recipes, so nothing to disable there.