brettwooldridge / HikariCP

光 HikariCP・A solid, high-performance, JDBC connection pool at last.
Apache License 2.0
19.95k stars 2.93k forks source link

module-info.java contains wrong module names #1608

Open zalavaari opened 4 years ago

zalavaari commented 4 years ago

The module-info.java contains module names metrics.core and metrics.healthchecks however the modul names of io.dropwizard.metrics artifacts are com.codahale.metrics and com.codahale.metrics.health

So these lines should be:

   requires com.codahale.metrics;
   requires com.codahale.metrics.health;

OR because these dependencies are optional, the following lines might be right as well:

   requires static com.codahale.metrics;
   requires static com.codahale.metrics.health;

The related source of the metrics-core: https://github.com/dropwizard/metrics/blob/v4.1.9/metrics-core/pom.xml

The output of java --describe-module:

>jar --file=metrics-core-4.1.9.jar --describe-module --release 11
No module descriptor found. Derived automatic module.

com.codahale.metrics@4.1.9 automatic
requires java.base mandated
contains com.codahale.metrics
zalavaari commented 4 years ago

Another one is slf4j.api which should be org.slf4j:

org.slf4j@1.7.30 automatic
requires java.base mandated
contains org.slf4j
contains org.slf4j.event
contains org.slf4j.helpers
contains org.slf4j.spi

From slf4j 2.x, the module-info will contain the org.slf4j module name explicitly: https://github.com/qos-ch/slf4j/blob/master/slf4j-api/src/main/java9/module-info.java

zalavaari commented 4 years ago

Sorry, my mistake. It seems I have newer dependencies than Hikari was built for. I using it with Spring, and Spring's bom overwrites several dependencies Hikari using. So this is not a bug for HikariCP itself. The problem is that HikariCP uses too old module names for using it with the latest Spring releases in a modular configuration.

FBibonne commented 4 years ago

Hello zalavaari,

It seems I encountered the same issue as you : the problems and different workarounds are discribed here : https://github.com/spring-projects/spring-boot/issues/22111

zalavaari commented 4 years ago

Hi @FBibonne

My workaround was that I've changed these module definitions in HikariCP's module-info.java to static requirements, so it doesn't expect the presence of these modules runtime, and I can use the latest Spring Boot without problems. For now, it seems working, but I'm not sure about the possible side effects, I'm still new in the modular world.

Here is my fork: https://github.com/zalavaari/HikariCP But be aware, it was just a rush work (I wont create a PR for it), I've just commented out whole segments from the poms to be able to run the project on jitpack.

You can give it a try if you like:

<repositories>
    <repository>
    <id>jitpack.io</id>
    <url>https://jitpack.io</url>
    </repository>
</repositories>
<dependencyManagement>
    <dependency>
        <groupId>com.github.zalavaari</groupId>
        <artifactId>HikariCP</artifactId>
        <version>14b303c</version>
    </dependency>
    <dependency>
        <groupId>org.springframework.boot</groupId>
        <artifactId>spring-boot-starter-jdbc</artifactId>
        <version>${version.spring-boot}</version>
        <exclusions>
            <exclusion>
                <groupId>com.zaxxer</groupId>
                <artifactId>HikariCP</artifactId>
            </exclusion>
        </exclusions>
    </dependency>
</dependencyManagement>
jaapcoomans commented 4 years ago

I ran into the same issue as @zalavaari. I solved it by overwriting the version of HakariCP in the Spring BOM.

I disagree however that this is solely a bug for Spring. The core problem here is that HakariCP requires a filename-based automatic module. This is fine for a local project, but not for a library published in Maven central. Such module names aare subject to change and can break the library (as demonstrated with this issue) .Maven gives a clear warning when like below (example with feign) when building a project with required filename-based automatic modules:

[WARNING] ****************************************************************************************************************************************************************
[WARNING] * Required filename-based automodules detected: [feign-core-11.0.jar, feign-jaxb-11.0.jar]. Please don't publish this project to a public artifact repository! *
[WARNING] ****************************************************************************************************************************************************************

Maybe the construction that is used with the module descriptor in a multi-release jar circumvents this detection (not sure), but IMO it remains that a public library should not depend on automatic modules.

So however I appreciate the fact that HakiriCP tries to support modules, I think it is too early to do so, at least as long as there are downstream dependencies that are not modular or have a t least an automatic module name claimed in the manifest file.

testphreak commented 4 years ago

Hi @zalavaari, @FBibonne and @jaapcoomans, I've run into the same issue you are discussing here when I tried upgrading my module path project from v2.2.6 to v2.3.4. With Andy Wilkinson's suggestion to downgrade HikariCP to v3.4.3, I was able to upgrade my project to v2.3.3, but not to the latest version. For me my service starts up normally, but fails to connect to it after initialization. Debug logs don't shed any light into this strange issue either.

Have any of you tried to upgrade your projects to v2.3.4 and experience the issue I described above? Thanks in advance for your inputs.

jaapcoomans commented 4 years ago

No sorry, can't help you there @testphreak. I migrated my project to Spring Boot 2.3.1 and still on that version with a manually downgraded HikariCP 3.4.3.