fix failing tests in java17 CI run

[pjfanning]

Relates to #186

See https://github.com/FasterXML/jackson-modules-base/actions/runs/4039573403

Unfortunately, this change stops the gradle-module-plugin working. It does appear to fix the tests. I tried reworking how that gradle-module-plugin is declared but nothing seems to work.

https://github.com/FasterXML/jackson-modules-base/actions/runs/4080247967/jobs/7032511522

[cowtowncoder]

Sounds good, thank you for this.

On Gradle module plugin... is that the gradle module metadata thing? It should only be needed with JDK 8 build (for releases).

[pjfanning]

Sounds good, thank you for this.

On Gradle module plugin... is that the gradle module metadata thing? It should only be needed with JDK 8 build (for releases).

Yes - that plugin.

I simply can't get that plugin to work - I either end up with it not working (the .module files it is supposed to produce do not get produced) or that plugin ends up causing the build to fail.

It may require us to choose - support that funky plugin or be able to run the tests in JDK 17.

In the end, the .module file is only a minor nicety - Gradle is well able to use poms, why we need to hack the pom and have this weird .module too - I struggle to see the real benefit.

[cowtowncoder]

Sounds good, thank you for this.
On Gradle module plugin... is that the gradle module metadata thing? It should only be needed with JDK 8 build (for releases).

Yes - that plugin.

I simply can't get that plugin to work - I either end up with it not working (the .module files it is supposed to produce do not get produced) or that plugin ends up causing the build to fail.

It may require us to choose - support that funky plugin or be able to run the tests in JDK 17.

In the end, the .module file is only a minor nicety - Gradle is well able to use poms, why we need to hack the pom and have this weird .module too - I struggle to see the real benefit.

Yeah I hear you. At the same time, dropping something that was working up until now (for couple of versions) does not sound good either. I do not know how valuable module info files are; I got the impression they might be more of an experimental feature earlier. But maybe its status has improved.

But let's see if we can get help: @jjohannes I think you know about this module, right?

[jjohannes]

Thanks for letting me know. I am certainly interested in fixing this in the plugin, if it wrongly throws this error in certain cases.

As a first step, can you upgrade the plugin to 0.3.0 (latest version). Maybe that helps already. Can we do that just on this project first to test? It's probably not that easy to update the parent POM where the version is in (?)

Not sure how the different Java version can be responsible for this, but I'll investigate.

[pjfanning]

@jjohannes thanks for the input - v0.3.0 still has same problem

[jjohannes]

So I don't know why this only fails in the Java 17 build on CI, but this is a valid problem with the build.

jackson-module-afterburner uses the maven shade plugin which generates a "new POM" that is then published. This POM no longer contains the comment. You can see the path in the error message:

/home/runner/work/jackson-modules-base/jackson-modules-base/afterburner/dependency-reduced-pom.xml

This is already broken. If you look at he published POM, you see that it also misses the comment:
https://repo1.maven.org/maven2/com/fasterxml/jackson/module/jackson-module-afterburner/2.14.2/jackson-module-afterburner-2.14.2.pom

The real fix would be to add the comment to the dependency-reduced-pom.xml as well. Do you know if and how that can be done in the maven shade plugin?

[pjfanning]

So I don't know why this only fails in the Java 17 build on CI, but this is a valid problem with the build.

jackson-module-afterburner uses the maven shade plugin which generates a "new POM" that is then published. This POM no longer contains the comment. You can see the path in the error message:

/home/runner/work/jackson-modules-base/jackson-modules-base/afterburner/dependency-reduced-pom.xml

This is already broken. If you look at he published POM, you see that it also misses the comment: https://repo1.maven.org/maven2/com/fasterxml/jackson/module/jackson-module-afterburner/2.14.2/jackson-module-afterburner-2.14.2.pom

The real fix would be to add the comment to the dependency-reduced-pom.xml as well. Do you know if and how that can be done in the maven shade plugin?

in fairness, XML comments are regarded as nice but non-essential - tools keep them or lose them but hardly anyone makes that behaviour controllable

[jjohannes]

in fairness, XML comments are regarded as nice but non-essential - tools keep them or lose them but hardly anyone makes that behaviour controllable

Sure. One can debate if it was the best decision to use a comment like this by Gradle.

But no matter if one likes it or not, that's how it works.

So if we want to fix it, we need to add that comment to the pom.

In any case I think this is completely unrelated to this PR. The build is already broken on main. And the problem is probably there for longer and the Maven CI build just did not catch it.

[jjohannes]

@pjfanning can you remove the plugin version upgrade commit again from this PR?

@cowtowncoder can you integrate it as it is, as the build failure is not new from this PR?

Then we can open a separate issue for this. If I find some time next week, I can check how this could be fixed.

[cowtowncoder]

Ok sounds good, I can merge this.

EDIT: Actually change would remove Gradle module metadata generation from all components, not just Afterburner/Blackbird.
I am not 100% sure but wouldn't it be possible to have override to disable module for AB/BB?
If not, we'd need to move plug-in enabling from main level to individual sub-modules other than AB/BB.

[cowtowncoder]

Okay looks good; will merge, need to manually push to 2.15 and master as well.

I wonder if we can have downstream tests to verify successful publishing of GMM (ditto for OSGi metadata). But first things first.

[cowtowncoder]

Merged to 2.15 and master as well.

[jjohannes]

I wonder if we can have downstream tests to verify successful publishing of GMM (ditto for OSGi metadata). But first things first.

The test I added here was supposed to inform about issues like this:
https://github.com/FasterXML/jackson-integration-tests/blob/master/src/test/java/com/fasterxml/jackson/integtest/gradle/GradleTest.java

But it's not "strict" enough. I will have a look. I should also upgrade the Gradle version there to make jackson-integration-tests work on Java 17.

[jjohannes]

FasterXML/jackson-integration-tests#16