init.gradle: Ignore BOM dependencies when comparing dependency trees #6092
Conversation
This file contains 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
mnonnenmacher
previously approved these changes
Nov 18, 2022
|
@mpbecker and @pirvudoru, could help confirming whether this fixes your issues described here and here, respectively? |
sschuberth
reviewed
Nov 18, 2022
| @@ -251,7 +251,7 @@ class AbstractDependencyTreePlugin<T> implements Plugin<T> { | |||
| /** | |||
| * Return true if the provided DependencyResult represents an imported Maven BOM. | |||
| */ | |||
| private boolean isBom(DependencyResult dependencyResult) { | |||
| private static boolean isBom(DependencyResult dependencyResult) { | |||
There was a problem hiding this comment.
Ah, sorry, expanding the diffs shows that it's not unrelated as isBom is called from the static dependencyTreeEquals now.
sschuberth
reviewed
Nov 18, 2022
| @@ -394,7 +394,8 @@ class AbstractDependencyTreePlugin<T> implements Plugin<T> { | |||
|
|
|||
| def resultDependencies = dependencyResult instanceof ResolvedDependencyResult ? | |||
| dependencyResult.selected.dependencies : [] as Set<DependencyResult> | |||
| if (dependency.dependencies.size() != resultDependencies.size()) { | |||
| // Ignore the BOM dependencies for calculating the count since they are skipped by `fetchDependency`. | |||
| if (dependency.dependencies.size() != resultDependencies.findAll { !isBom(it) }.size()) { | |||
There was a problem hiding this comment.
Nit: Could we use count { !isBom(it) } here?
When comparing dependency trees, the count of sub dependencies should not include BOM dependencies, since they are skipped by the `fetchDependency` function. This solves a bug where an Android project shipping 'androidx.appcompat:appcompat:1.5.1' would hang indefinitely. This is because, for instance, 'kotlinx-coroutines-android:1.6.1' depends on 'kotlinx-coroutines-core:1.6.1, kotlin-stdlib-jdk8:1.6.0 and kotlinx-coroutines-bom:1.6.1'. Since no ORT dependency is created for 'kotlinx-coroutines-bom' because it's a BOM file, it should also not be taken in account when comparing dependency trees. Otherwise, comparison would always fail, leading to a cache miss and overgrowth of the latter. Signed-off-by: Nicolas Nobelis <[email protected]>
nnobelis
force-pushed
the
nnobelis/fix_grade_android_dependencies
branch
from
ecef86a
to
4f570f9
Compare
nnobelis
requested review from
sschuberth and
mnonnenmacher
and removed request for
sschuberth and
mnonnenmacher
sschuberth
approved these changes
Nov 18, 2022
sschuberth
added
the
release notes
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.
When comparing dependency trees, the count of sub dependencies should not include BOM dependencies, since they are skipped by the
fetchDependencyfunction.This solves a bug where an Android project shipping 'androidx.appcompat:appcompat:1.5.1' would hang indefinitely. This is because, for instance, 'kotlinx-coroutines-android:1.6.1' depends on 'kotlinx-coroutines-core:1.6.1, kotlin-stdlib-jdk8:1.6.0 and kotlinx-coroutines-bom:1.6.1'. Since no ORT dependency is created for 'kotlinx-coroutines-bom' because it's a BOM file, it should also not be taken in account when comparing dependency trees. Otherwise, comparison would always fail, leading to a cache miss and overgrowth of the latter.
Signed-off-by: Nicolas Nobelis [email protected]