Java 17 Support with Sigtest 2.2 from JakartaEE TCK Tools

[lkishalmi]

I've tried to upgrade Sigtest to the latest available version 1.7, and that still fails on Java 17.
@mbien pionted out that there is a fork in JakartaEE TCK that could work. It did the job.

This is needed when a module is on Java 17 and exposes some API.

[lkishalmi]

Well, this version seems to fail on plarfom/libs.osgi which shall generate an empty report.
The new tool has some specific handling for Java 17.

The old tool does something somehow not match any package with the following patterns info.dmtree.**, org.osgi.**

The new version does. Unfortunately osgi.cmpn-7.0.0.jar has references some javax.servlet classes which are not on the classpath.

Adding sigtest.public.packages=- to the project.properties helped, but there are other classloader issues after that.

I'm out of ideas here.

@jtulach ?

[matthiasblaesing]

Not sure if it helps, but I think this causes the "error" in libs.osgi:

eclipse-ee4j/jakartaee-tck-tools@56e3e33

If I read the summary of the commit correctly, before this commit recursive definitions like info.dmtree.**, org.osgi.** were not handled correctly. If I'm not mistaken, only the first level was considered.

The commit was added between 1.5 and 1.6 and indeed using 1.5 I can run the sigtest generator in libs.osgi and 1.6 fails. And for some unknown reason that also matches the fact, that NetBeans itself stayed with sigtest 1.4.

Channeling my bash deamons I came up with this list of modules that might be affected:

find . -name project.xml | xargs grep -l subpackages | cut --delimiter "/" -f 2-3
extide/libs.gradle
webcommon/libs.nashorn
java/j2ee.eclipselink
java/maven.embedder
enterprise/web.jsf12
enterprise/web.jsf20
enterprise/j2eeapis
ide/servletapi
ide/libs.lucene
ide/libs.jcodings
ide/o.apache.xml.resolver
ide/libs.snakeyaml_engine
ide/libs.xerces
ide/libs.flexmark
platform/libs.asm
platform/libs.osgi

[ebarboni]

I think 1.4 was needed to build on jdk 11. As it was "working" did'nt think on upgrading.

I'm always puzzled with sigtest as on jenkins we have issue
In one there is 24 test failing: https://ci-builds.apache.org/job/Netbeans/job/netbeans-apisigcheck/lastCompletedBuild/testReport/
with ga checker we swallow them.

In both case there are a bunch of java.lang.ClassFormatError: Index out of the constant pool bounds during sig phase.

[lkishalmi]

Well the last commit forces to use sigtest to generate signatures without recursion.

That behavior was fixed in the commit mentioned above, so that commit is just keeping the same wrong way of doing the things as before.

[mbien]

thanks for updating this dependency! Its interesting that it went full circle, I believe the jakarta lib is a fork of the netbeans fork - and we would use it now again ;)

tests are green, couldn't find any regressions while browsing through the log (the familiar java.lang.ClassFormatError: Index out of the constant pool bounds traces are still there)

+1 from me, but someone who has more experience with the sig tests and public API requirements should take a look too.

[mbien]

this restricts the sig test to only the listed package prefix?

[lkishalmi]

Yes. That's the idea, as the recursive behavior was not working prior 1.6. They checked only the listed part of the package hierarchy. So adding these mimics the former behavior.

[ebarboni]

LGTM with my limited understanding

[lkishalmi]

Anyone, a word? I'm about to merge this one today or tomorrow, as #7024 is depending on this one.

[matthiasblaesing]

I would not go down the sigtest.public.packages property route. It is doomed to fail because in the future nobody will remember why on earth these broken declarations were made. In most cases I saw the packages would need to be declared as package.** (i.e. cover all subpackages) and then the original errors reappear.

I wrote it inline already and would suggest to recreate the sigfile where it works and where not disable sigtest with a comment explaining why it is deactivated.

[matthiasblaesing]

I think you are looking for sigtest.public.packages=javax.faces.**,com.sun.faces.** (the API is declared base on the subpackages element).

This needs a deeper look. For example Groovy is being looked for because there is com.sun.faces.scripting.groovy.GroovyHelperImpl. The API declaration of this module is ugly/lazy/wrong.

[lkishalmi]

Strangely, this did not occur to me.

Though I think these definitions web.jsf* are there for providing libraries to Ant based projects. So technically they should not provide any public api to any NB modules. Also JSF 1.2 is soon 18 years old. Probably it would be better not to include the jars in NB.

web.jsf20 hide JSF 2.3 we seem to be built our JSF support on that.

I'm going to file separate PR-s to remove the JSF 1.2 libraries.

[matthiasblaesing]

Same reasoning as libs.gradle. The approach might work for OSGI if all packages would be listed here and only packages with problematic references are excluded and commented acordingly.

[matthiasblaesing]

Strange error here when generating signature file.

[lkishalmi]

@matthiasblaesing thanks for your feedback!

[mbien]

would it make sense to open a meta issue afterwards which has a list of all mods where sig tests are turned off so that we can look that up when needed?

[matthiasblaesing]

In general looks good. I don't agree with all comments, but not to problematic, I left a few inline suggestions.

[matthiasblaesing]

Disabling is fine, the comment though does not match the reason (from my POV). The reported error is:

/home/matthias/src/netbeans/nbbuild/templates/projectized.xml:765: 
java.lang.AssertionError: org.apache.lucene.search.CachingWrapperFilter$FilterCache<org.apache.lucene.search.DocIdSet>
	at com.sun.tdk.signaturetest.loaders.BinaryClassDescrLoader.load(BinaryClassDescrLoader.java:225)
	at com.sun.tdk.signaturetest.core.ClassHierarchyImpl.load(ClassHierarchyImpl.java:180)
	at com.sun.tdk.signaturetest.core.ClassHierarchyImpl.getClassInfo(ClassHierarchyImpl.java:292)
	at com.sun.tdk.signaturetest.core.ClassHierarchyImpl.isInterface(ClassHierarchyImpl.java:272)
	at com.sun.tdk.signaturetest.core.ClassCorrector.fixType(ClassCorrector.java:315)
	at com.sun.tdk.signaturetest.core.ClassCorrector.fixMethods(ClassCorrector.java:298)

So I would say this is the same as in other cases: sigtest fails to parse the API classes.