There is no module-info.java

[rackaracka]

The latest version as of the time of writing does not contain a module-info module descriptor, only an automatic module name listed in the manifest of the jar file.
This makes it impossible to link and pack a platform dependent artefact, because the jlink utility does not accept automatic modules.

A basic module-info.java can be generated using jdeps.

[davecramer]

Hi,

Did previous versions contain a module-info descriptor ?

[rackaracka]

I don't know but I guess not. I only referred to the latest version, because that was the one I downloaded today.

[davecramer]

Thanks for the report. Care to provide a PR ?

[rackaracka]

what is a PR?

[davecramer]

Pull Request with changes to change the code so that it does what you want.
Keep in mind that we build on JAVA 8

[rackaracka]

It would be a two step job:
-adding the module-info
-changing the build scripts to create a multi-release jar

I tried to do a build following the instructions on https://jdbc.postgresql.org/development/

Running ./gradlew build -DskipTests
runs all tests regardless and more than 100 tests fail.

The module-info.java should probably look like this:

` module org.postgresql.jdbc {

requires java.management;

requires transitive java.desktop;
requires transitive java.logging;
requires transitive java.naming;
requires transitive java.security.jgss;
requires transitive java.security.sasl;
requires transitive java.sql;
requires transitive java.transaction.xa;
requires transitive java.xml;

provides java.sql.Driver with
    org.postgresql.Driver;

}`

I'm not very familiar with gradle, so you guys may be better at step 2.

[desruisseaux]

Are you sure that all above-cited dependencies need to be declared transitive? Generally a dependency should be transitive only if it appears in public API. For example if no public method from org.postgresql has in its method signature a class provided by java.transaction.xa (I didn't checked if it was the case), then that dependency does not need to be transitive.

[rackaracka]

Probably not, I used jdeps to generate it and this is the result.

[bowbahdoe]

If you compile with -Xlint:all and -Werror it should catch any situations where a dependency should be transitive.

[desruisseaux]

Did a test on a clone. This test requires an upgrade to Java 11 because I do not know how to create a multi-JAR file with Gradle. The following module-info.java file seems sufficient, except for unnamed dependencies:

module org.postgresql.jdbc {
    requires transitive java.sql;
    requires transitive java.desktop;           // Used for java.awt.Point only
    requires transitive java.naming;

    requires java.management;
    requires java.security.jgss;                // TODO: make transitive if org.postgresql.gss is exported
    requires org.checkerframework.checker.qual;

    exports org.postgresql;
    exports org.postgresql.ds;
    exports org.postgresql.core;
    exports org.postgresql.util;
    // TODO: what else to export?

    provides java.sql.Driver with org.postgresql.Driver;
}

The project depends on unnamed modules. So it requires the following option to be passed to javac:

--add-reads org.postgresql.jdbc=ALL-UNNAMED

For avoiding above option, we should check if the dependencies providing the following packages have more recent releases as named modules:

• com.ongres.scram.client
• org.osgi.framework
• org.osgi.service.jdbc
• com.sun.jna.*
• waffle.windows.auth.*

Note also that the project has a dependency to the huge java.desktop module, but uses only the java.awt.Point class at one trivial place, the PGpoint.setLocation(Point) method. I suggest to deprecate that method for removal.

[davecramer]

Deprecating setLocation along with an implementation of Point of our own. We really only have it there to set x and y

[bowbahdoe]

Would making the dependency on org.checkerframework.checker.qual static make sense? IDK if that is currently brought into dependent projects.

[davecramer]

@bowbahdoe I'm unsure what you are asking here ?

[bowbahdoe]

Well, just looking at the module-info @desruisseaux posted, they have org.checkerframework.checker.qual as a required module

requires org.checkerframework.checker.qual;

I'm questioning whether that is actually required - i.e. if someone depends on the maven artifact are they always going to get it - or if it is optional.

The way you denote optional dependencies in module-infos is with requires static

requires static org.checkerframework.checker.qual;

The only reason i question that one is I have a sense memory of that being an automatic module and its "just annotations"

[davecramer]

The project requires it to build, but it is not shipped in the jar. So users do not require it.

[bowbahdoe]

Then it would be requires static. Not really a point to quibble over at this moment

[desruisseaux]

Verified that the removal of the setLocation(Point) method removes the java.desktop dependency, at least at compile time. Created pull request #2967 for deprecating this method now in anticipation for future modularisation.

Regarding the org.checkerframework.checker.qual dependency, PgJDBC uses annotations such as Nullable which are declared with @Retention(RUNTIME). This dependency would be truly optional if the annotations were declared with @Retention(SOURCE). If the JAR file is omitted, I would have expected ClassNotFoundError at runtime. Maybe it happens only when Class.getAnnotations() is invoked? It may be worth to verify, and if it is the case, either document this behaviour or avoid using org.checkerframework.checker.qual.

[vlsi]

Created pull request #2967 for deprecating this method now in anticipation for future modularisation

+1

This dependency would be truly optional if the annotations were declared with @retention(SOURCE)

The annotations need to be transitive since the users of pgjdbc.jar need to know which types are nullable and which are not.
If the annotation is marked with retention=source, then pgjdbc users won't.

See #2053 (comment)

[desruisseaux]

Thanks for the link. So my understanding is that requires org.checkerframework.checker.qual cannot be static at this time, but may become static (or completely removed?) in the future if #2262 is merged.

[vlsi]

I think so.
An alternative option could be migrating to https://github.com/jspecify/jspecify once it is ready.

See

• Is it safe to have jspecify be an optional dependency? jspecify/jspecify#389
• Is it safe to have jspecify be an optional dependency? jspecify/jspecify#389 (comment)
• Add module descriptor ben-manes/caffeine#535

[bowbahdoe]

Annotations with a retention of runtime are still safe to not include at runtime. getAnnotation and similar methods should just omit them.

There are some asterisks, they just don't apply here best I can tell.

[vlsi]

@bowbahdoe , see #2053
If you convince Guava switch, we follow.

[bowbahdoe]

@vlsi Guava is, best i can tell, planning to try.

[bowbahdoe]

Bumping this, as I am writing a tiny app and got to the point where i figured "sqlite is less convenient than postgres" and I have to rewrite my build+deployment mechanism to use this.

Which sucks.

[bowbahdoe]

I no longer care about where checker qual is a static import since it has a proper module info typetools/checker-framework#6326 or at least will in an upcoming release.

[davecramer]

So can this be closed ?

[bowbahdoe]

@davecramer No, you misunderstand.

This artifact still needs a module-info.class.

There was a separate conversation above this where what we were discussing was whether the dependency on the checker framework annotation jar should be requires or requires static. requires static is useful if a jar is optional at runtime (which annotation only jars generally are) and you do not want to mandate people have it on their module path.

However, the only reason to really care was that the checker framework annotation jar also didn't have a module-info. Now that it does or will, that concern is gone.

I'll try to make a PR. Gradle is always a nightmare for me so hopefully I figure it out

[davecramer]

Look forward to your PR

[bowbahdoe]

Going through the work now.

This seems as if it will be dependent on

• resolving the issues with fix(deps): update dependency com.github.waffle:waffle-jna to v3 #2796 so that we can have a stable module name for waffle
  • Here are some reports of people running in to issues when they are put in a position where they need to upgrade waffle. waffle 2.1.0 - java.lang.NoSuchMethodError com.sun.jna.platform.win32.Sspi$SecBufferDesc.<init>(I[B)V Waffle/waffle#838
  • My understanding is that right now pgjdbc uses reflection to handle both situations. This won't work for module names because without Automatic-Module-Name s in their manifests, waffle (and the JNA dependencies it wants) have module names based on their file names. Which for waffle itself actually works out, but JNA chose com.sun.jna and com.sun.jna.platform for those later versions but would have jna and jna.platform from the inferred names.
    • I'm sorry I care about this stuff. It must be a pain in the ass for everyone.
• this issue being resolved + upgrading to the version where it is Add Java 9+ modules ongres/scram#21

I'm gonna chip away at the waffle changes, see if thats easy enough to fix.