java icon indicating copy to clipboard operation
java copied to clipboard

`optional` tag in `dependencyManagement` is not inherited

Open hisener opened this issue 1 year ago • 2 comments

Describe the bug

The client-java-parent POM has a few "optional" dependencies; however, they are no-ops. For example, the dependency tree of a module that uses io.kubernetes:client-java:jar:19.0.1:

$ mvn dependency:tree
...
[INFO] +- io.kubernetes:client-java:jar:19.0.1:compile
[INFO] |  +- io.prometheus:simpleclient:jar:0.16.0:compile
[INFO] |  |  +- io.prometheus:simpleclient_tracer_otel:jar:0.16.0:compile
[INFO] |  |  |  \- io.prometheus:simpleclient_tracer_common:jar:0.16.0:compile
[INFO] |  |  \- io.prometheus:simpleclient_tracer_otel_agent:jar:0.16.0:compile
[INFO] |  +- io.prometheus:simpleclient_httpserver:jar:0.16.0:compile
[INFO] |  |  \- io.prometheus:simpleclient_common:jar:0.16.0:compile

This is because Maven doesn't take the tag into account. See also https://issues.apache.org/jira/browse/MNG-5227 and https://issues.apache.org/jira/browse/MNG-5632. We noticed this issue, because Coursier seems to take the tag into account unlike Maven.

Could we push optional tags to module POMs and drop them from dependencyManagement?

https://github.com/kubernetes-client/java/blob/d7f939125ab13cea9e1a64e150b3ff6781eeb173/pom.xml#L185-L190

https://github.com/kubernetes-client/java/blob/d7f939125ab13cea9e1a64e150b3ff6781eeb173/util/pom.xml#L14-L17

Client Version 19.0.1

Kubernetes Version 1.26.15

Java Version Java 21

To Reproduce

  1. Create a module that depends on client-java and run mvn dependency:tree. io.prometheus:simpleclient:jar:0.16.0:compile will be listed there.

Expected behavior Optional dependencies (e.g., io.prometheus:simpleclient) should not be pulled transitively in downstream modules.

KubeConfig N/A

Server (please complete the following information): N/A

hisener avatar Jul 03 '24 09:07 hisener

If you move the optional tag into the module pom but you use a central property (e.g. ${prometheus.client.optional}) that is set in a single location, then I think that I'm ok with it. Please also add a comment that references that bug, so that we can remove this when that bug is fixed.

Ultimately, this feels like a bug in maven, so I don't really want to decentralize our pom files just to make up for their bug.

brendandburns avatar Jul 03 '24 16:07 brendandburns

If you move the optional tag into the module pom but you use a central property (e.g. ${prometheus.client.optional}) that is set in a single location, then I think that I'm ok with it. Please also add a comment that references that bug, so that we can remove this when that bug is fixed.

This works for me.

Ultimately, this feels like a bug in maven, so I don't really want to decentralize our pom files just to make up for their bug.

The question is whether we want these dependencies to be pulled transitively.

  • If so, we should drop optional tags to include them downstream. It's already the case for Maven due to the issue linked above, but Coursier doesn't include them. I don't know the Gradle behavior. Dropping them would make them behave the same.
  • If not, we should apply your suggestion so that they are not included in downstream projects. This could be a breaking change for downstream users that don't depend on these dependencies.

FWIW, google-auth-library-oauth2-http has <optional>true</optional> in both dependencyManagement and dependencies sections. https://github.com/kubernetes-client/java/blob/b14c36d7c3ff5b30a3c6a6456c3156e92c5fbbe6/util/pom.xml#L84

hisener avatar Jul 04 '24 09:07 hisener

The optional is what we want, we've had complaints in the past about pulling in large dependency trees that people don't use.

We refactored the pom files a while ago to clean them up, we must have missed that one. If you wanted to send a PR to clean it up, that'd be great :)

brendandburns avatar Jul 08 '24 16:07 brendandburns