ben-manes / gradle-versions-plugin

Gradle plugin to discover dependency updates
Apache License 2.0
3.86k stars 199 forks source link

Do not use textual 'null' for dependencies without group (#725) #731

Closed Vampire closed 1 year ago

Vampire commented 1 year ago

Fixes #725

ben-manes commented 1 year ago

Thanks! Can you fix the test failures? It might just be some minor updates, e.g.

DependencyUpdatesSpec > Single project with flatDir repository FAILED
    Condition not satisfied:

    unresolved.collect { it.selector }.collectEntries { dependency -> [['group': dependency.group, 'name': dependency.name]: dependency.version] } == [['group': 'null', 'name': 'guava-18.0']: NONE_VERSION]
    |          |                       |                                                                                                           |                                            |
    |          [:guava-18.0:none]      [[group:, name:guava-18.0]:none]                                                                            false                                        none
    [:guava-18.0:none]
        at com.github.benmanes.gradle.versions.DependencyUpdatesSpec.Single project with flatDir repository_closure21(DependencyUpdatesSpec.groovy:359)
        at groovy.lang.Closure.call(Closure.java:412)
        at spock.lang.Specification.with(Specification.java:191)
        at com.github.benmanes.gradle.versions.DependencyUpdatesSpec.Single project with flatDir repository(DependencyUpdatesSpec.groovy:358)
Vampire commented 1 year ago

Hm, I'm not sure about the undeclared failure, whether my changes just surfaced that there is some other bug in Resolver.kt or whether my changes are problematic. With the current state the guice dependency is also treated as hidden.

Vampire commented 1 year ago

Can you maybe have a look?

Vampire commented 1 year ago

Actually if I comment out the if (originalCoordinate == null && resolvedCoordinate.groupId != "none") { and just keep the else, the tests in DependencyUpdatesSpec all pass. Maybe it is not necessary and not appropriate actually as there is already Ignore undeclared (hidden) dependencies at an earlier place in that file?

Vampire commented 1 year ago

I pushed a version with that change, let's see whether something else breaks or you say this actually is necessary. :-)

Vampire commented 1 year ago

Well, the tests passed with that change :-D

ben-manes commented 1 year ago

It looks good to me. Running it against Caffeine and I don't see any of the hidden dependencies in the report (bundled ant, jacoco) and the undeclared category from v39 has unit tests.