Fix transitive dep resolution when exclusion is present in other POM #919
Conversation
| }, | ||
| "org.openjfx:javafx-base": { | ||
| "shasums": { | ||
| "jar": "c5084a74417a89c69a0c122fae96a4b70bf619fc3d6218ea102a4047ec85ad04", | ||
| "linux": "2ebf6fa2cbbe1c8b4f7780e06e97beb038f644d5ecf9f15a41c5e88ee0ef9cf1" | ||
| "mac": "2d8052a08fd2e5d98e1d5a16d724ea5dd02102879de20a193225f57199803983" |
There was a problem hiding this comment.
This is unfortunate, but I don't have a Linux machine/image with all required Bazel deps to regenerate this for Linux and have matching hashes
| @@ -257,6 +257,10 @@ maven_install( | |||
| # Any two platforms to ensure that it doesn't work _only_ under the host operating system | |||
| "com.google.protobuf:protoc:exe:linux-x86_64:3.21.12", | |||
| "com.google.protobuf:protoc:exe:osx-aarch_64:3.21.12", | |||
| # https://github.com/bazelbuild/rules_jvm_external/issues/917 | |||
| # androidx core-testing POM has "exclusion" for "byte-buddy" but it should be downloaded as mockito-core dependency. | |||
| "org.mockito:mockito-core:jar:3.3.3", | |||
There was a problem hiding this comment.
Since jar is the default, it's usually omitted.
There was a problem hiding this comment.
Yep yep, it's just a convention in our large Android monorepo to see what deps are JVM and what are Android AARs I think it's fine for purpose of this regression test
| "org.mockito:mockito-core:jar:3.3.3", | ||
| "androidx.arch.core:core-testing:aar:2.1.0", |
There was a problem hiding this comment.
I gave a try with these 2 artifacts exactly as written above and indeed faced the issue that net.bytebuddy:byte-buddy and net.bytebuddy:byte-buddy-agent were missing from mockito-core's transitive dependencies.
It turns out that, despite the theoretical equivalence, stripping out the jar packaging type leads to the intended behavior.
- "org.mockito:mockito-core:jar:3.3.3",
+ "org.mockito:mockito-core:3.3.3",
"androidx.arch.core:core-testing:aar:2.1.0",(just FYI, not to be merged)
There was a problem hiding this comment.
@artem-zinnatullin I just filed artem-zinnatullin#1 you may merge to your branch or not, at your convenience.
It's still unclear why Coursier behaves differently whether being passed `some:artifact:version,type=jar` or just `some:artifact:version` since they're theoratically equivalent, but this anyway removes a redundancy.
|
Closes #917. |
Do not append `,type=jar` to `coursier fetch`ed artifacts
artem-zinnatullin
changed the title
|
Merged your fix and updated PR title and description for the fix and test to be ready for merge! |
| @@ -258,7 +258,7 @@ json = struct( | |||
| # | |||
| def _artifact_to_coord(artifact): | |||
| classifier = (",classifier=" + artifact["classifier"]) if artifact.get("classifier") != None else "" | |||
| type = (",type=" + artifact["packaging"]) if artifact.get("packaging") != None else "" | |||
| type = (",type=" + artifact["packaging"]) if artifact.get("packaging") not in (None, "jar") else "" | |||
#908 uncovered a bug in
coursierwhich leads to transitive dep not being resolved if anexclusionrule for it is present in POM of a non-related dependency.This PR adds a regression test and a fix by @rdesgroppes.
TL;TR:
mockito-coredepends onbyte-buddyandroidx.arch.core:core-testingPOM hasexclusionfor<groupId>net.bytebuddy</groupId>maven_installthenmockito-coredoesn't get itsbyte-buddydependencyandroidx.arch.core:core-testingis removed frommaven_installthenmockito-coregetsbyte-buddydependencybyte-buddymanually tomaven_installdoesn't helpmockito-corewas gettingbyte-buddydependency withandroidx.arch.core:core-testinginmaven_installprior to change Add preliminary support forexepackaging type #908exepackaging type #908 passes classifier and type for dependency resolution