Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 1 commit into from
Feb 20, 2023

Conversation

Vampire
Copy link
Contributor

@Vampire Vampire commented Feb 20, 2023

Fixes #725

@ben-manes
Copy link
Owner

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
Copy link
Contributor Author

Vampire commented Feb 20, 2023

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
Copy link
Contributor Author

Vampire commented Feb 20, 2023

Can you maybe have a look?

@Vampire
Copy link
Contributor Author

Vampire commented Feb 20, 2023

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
Copy link
Contributor Author

Vampire commented Feb 20, 2023

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

@Vampire
Copy link
Contributor Author

Vampire commented Feb 20, 2023

Well, the tests passed with that change :-D

@ben-manes
Copy link
Owner

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.

@ben-manes ben-manes merged commit 91a63f2 into ben-manes:master Feb 20, 2023
@Vampire Vampire deleted the issue-725 branch February 20, 2023 20:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Only locally available included builds are shown as unresolved
2 participants