Clean up DependencyState#34927
Merged
Merged
Conversation
e3bd2dd to
5d9503a
Compare
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Collaborator
|
The following builds have passed: |
5b09b84 to
df66daa
Compare
837bbd7 to
36468e3
Compare
Base automatically changed from
jvandort/dm/prepare-to-deprecate-module-version-selector
to
master
September 23, 2025 02:39
The previous caching dependency substitution applicator failed to include the requested artifacts as part of the cache key. The requested artifacts were passed to the dependency subtitution details, making the cache unreliable. We combine the default and caching substitution applicators to increase locality, making it more difficult to make this cache unreliable in the future. We also remove the substitution cache from DependencyState. Instead, we only construct DependencyState instances directly from DependencyMetadata instances, after substitution. This saves us from having a map for every DependencyState instance, and saves us from instantiating one extra DependencyState for every substituted edge.
36468e3 to
eacae5d
Compare
Member
Author
|
@bot-gradle test prf apt |
This comment has been minimized.
This comment has been minimized.
Collaborator
|
The following builds have passed: |
Collaborator
|
The following builds have passed: |
jvandort
added a commit
that referenced
this pull request
Dec 6, 2025
jvandort
added a commit
that referenced
this pull request
Dec 24, 2025
jvandort
added a commit
that referenced
this pull request
Jan 26, 2026
This was originally introduced in #34927 , but there was no test to verify this behavior. Now, we add a test to verify that exclusions are applied to dependencies before and after substitution
jvandort
added a commit
that referenced
this pull request
Jan 26, 2026
This was originally introduced in #34927 , but there was no test to verify this behavior. Now, we add a test to verify that exclusions are applied to dependencies before and after substitution
Merged
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
2 participants
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.
The previous caching dependency substitution applicator failed to include the requested artifacts as part of the cache key. The requested artifacts were passed to the dependency substitution details, making the cache unreliable. We combine the default and caching substitution applicators to increase locality, making it more difficult to make this cache unreliable in the future.
We also remove the substitution cache from DependencyState. Instead, we only construct DependencyState instances directly from DependencyMetadata instances, after substitution. This saves us from having a map for every DependencyState instance, and saves us from instantiating one extra DependencyState for every substituted edge.
Reviewing cheatsheet
Before merging the PR, comments starting with