DidierLoiseau commented on PR #1788: URL: https://github.com/apache/maven/pull/1788#issuecomment-2403653986
With a fresh mind, the answer is quite simple: :sweat_smile: * [`palantir-java-format:2.38.0`](https://repo1.maven.org/maven2/com/palantir/javaformat/palantir-java-format/2.38.0/palantir-java-format-2.38.0.pom) has `<dependencyManagement>` for `error_prone_annotations:2.19.1` and `checker-qual:3.35.0` – previously ignored * it also depends on [`guava:32.1.2-jre`](https://mvnrepository.com/artifact/com.google.guava/guava/32.1.2-jre), which has a direct dependency on `error_prone_annotations:2.18.0` and `checker-qual:3.33.0` So this change is perfectly expected – and I think it makes sense that, if Spotless requests `palantir-java-format:2.38.0`, it also gets the managed dependency versions. I wanted to propose to add `error_prone_annotations:2.18.0` and `checker-qual:3.33.0` to `bootstrap.txt` but then… Spotless also loads [`google-java-format:1.17.0`](https://mvnrepository.com/artifact/com.google.googlejavaformat/google-java-format/1.17.0) (for `<removeUnusedImports />`), which depends on `error_prone_annotations`:**`2.16.0`** and `checker-qual`:**`3.21.2`**! Spotless loads it in a separate call to `DefaultRepositorySystem#resolveDependencies()`, so the resolution is done independently. With or without the change, `BfDependencyCollector#doRecurse()` will take `google-java-format:1.17.0`’s dependencies from the `descriptorResult` and put them as `context.managedDependencies`, except that the `ClassicDependencyManager` will ignore them whereas the `TransitiveDependencyManager` now keeps track of them. The trick is that `google-java-format` declares them both as **`<optional>`**, so they are not added to the `dependencyProcessingQueue`! Back in the `while` loop of `BfDependencyCollector#doCollectDependencies()`, the `dependencyProcessingQueue` now contains just 1 entry: the [`guava:31.1-jre`](https://mvnrepository.com/artifact/com.google.guava/guava/31.1-jre), also a dependency of `google-java-format:1.17.0`! It depends on `error_prone_annotations`:**`2.11.0`** and `checker-qual`:**`3.12.0`**, so `ClassicDependencyManager` will happily use those. However, `TransitiveDependencyManager` remembers the versions specified by `google-java-format:1.17.0`, and will use them instead! Basically, GJF says “those dependencies are optional” (this is kinda incorrect, actually, since it has a strong dependency on Guava, so they are actually mandatory), but if you include them (which will thus always be the case), I need these versions”. Here again, in my opinion, `TransitiveDependencyManager` seems right (although I hadn’t really thought about how `<optional>` dependencies influence the version selection until now). Neither dependency manager takes the same versions of the 2 offending dependencies when doing resolution for Palantir and GJF, so I don’t think we should try to fix that now – it might actually be a bug in Spotless if the classpath of the two is merged. The nasty trick is that Spotless 2.40 runs as part of Core ITs build, before any integration test runs. This causes the Spotless dependencies decided by `ClassicDependencyManager` to be cached without needing to add them in the `bootstrap.txt`. I suppose it works as long as Spotless 2.40 isn’t upgraded in the Core ITs, at which point you’ll need to align the version for the `pom.hocon`. # TL;DR To make it work with `TransitiveDependencyManager`, I suppose you you could either disable Spotless in this test, disable Palantir and GJF for Spotless in this test, or add the following lines in `bootstrap.txt`: ``` com.google.errorprone:error_prone_annotations:2.16 com.google.errorprone:error_prone_annotations:2.19.1 org.checkerframework:checker-qual:3.21.2 org.checkerframework:checker-qual:3.35.0 ``` As a side note, the error you get when a plugin dependency fails to download such dynamic dependencies is particularly unclear, as it never says which `DependencyRequest` it was actually trying to resolve (here Palantir and GJF) despite the very long stracktraces – but I guess that’s an issue for Maven Resolver. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
