Migration of javax.json to jakarta.json

[jbescos]

jsonp-tck.zip

This PR contains the required changes to run the tests of jsonp with the jakarta json libraries. The tests can run with JDK8 and JDK11.

I attach one zip file that contains:
-The logs of the tests when you run it with jdk8 and jdk11.
-The script I created to execute the tests.
-The ts.jte file I was using for the tests.

I had to make some local changes in sigtest project to generate the jar for JDK11 because currently that is not supported.

[jansupol]

Note that for the signature tests to work properly with jdk 11, a sigtest built from the repository https://hg.openjdk.java.net/code-tools/sigtest has been used.

[scottmarlow]

Should we update the sigtest.jar + sigtestdev.jar in the jakartaee-tck repo?

Also, is there a sigtest repo pull request with the needed changes?

[scottmarlow]

I may be pointing out the obvious but if we merge this change into the master branch, the nightly TCK Jenkins CI jobs will start showing failures, I think the failures will continue until we have a GlassFish release that uses the jakarta namespace.

[jansupol]

Also, is there a sigtest repo pull request with the needed changes?

I do not think this PR required any changes in the sigtest repo.

[jbescos]

sigtest.zip

I'm attaching the sigtest here but I don't put it in version because it was created by me locally (without pull requests, reviews, etc).

We need to wait for a new glassfish release, also because I made some assumptions about some libraries that should be there (for example jaxb is no longer in JDK11).

[bshannon]

We should verify what version of sigtest.jar was checked into the TCK repo. I thought it was supposed to be the latest version. What exactly were the problems you ran into @jbescos that required you to build your own version of sigtest.jar?

[jbescos]

@bshannon I downloaded the zip file from this link (is it the last version?):
https://hg.openjdk.java.net/code-tools/sigtest/archive/tip.zip

I added a jdk11.home in build.properties and some ant targets to compile with jdk11, in the same way it was done before for jdk9.

[bshannon]

That explains what you did. It doesn't explain why you did it. What didn't work with the sigtest that's included in the TCK repo?

[jbescos]

@bshannon find attached the log with the errors when you try to execute the signature tests with a jar file that was not compiled with jdk11
failedSignature.log

In summary, it complains about some added methods and annotations.

Additionally the ts.jte needs to have another dependency in jsop.classes: ${pathsep}${java.home}/lib/modules. Otherwise sigtest will not find classes like Object, Map, etc (and it will complain about that).

Note that new sigtest jars were not included in my pull request. The ts.jte was not included also, because I found the key of jsop.classes is empty by default. If you want to take a look to the jte it is attached inside jsonp-tck.zip.

But this pull request contains a file that is jdk11 related. The file: ts.common.props.xml. In jdk11 we need to add more modules for javac, for example jaxb. This depends on Glassfish libraries (which has not been released yet). So there is a potential risk that one library will not be there. It makes sense to wait till a new GlassFish is released + new sigtests are added in the lib to support jdk11.

[bshannon]

It's interesting that the problem was caused by the new jakarta.json.jar file.
I've used that same version of sigtest with my new jakarta.mail.jar file and
it worked fine. I tried to reproduce the problem you were seeing with a isolated
jakarta.json.jar file, but I couldn't. I don't quite understand what's going wrong
here but if we need a new version of sigtest we need to get a version compiled
and released by the jtharness project.

[jbescos]

@bshannon I'm taking a quick look into the first error of the log:
jakarta.json.JsonArray: method public <%0 extends java.lang.Object> {%%0}[] java.util.Collection.toArray(java.util.function.IntFunction<{%%0}[]>)

That JsonArray extends List, and that one extends Collection. A new method was added in Collection if you compare JDK11 with JDK8, and thats why is complaining that JsonArray is having an unexpected new method.
default T[] toArray(IntFunction<T[]> generator)

That function exist in the signatures file 'jakarta.json.sig_2.0.0_se11':
intf java.lang.Iterable<{java.util.Collection%0}>
meth public <%0 extends java.lang.Object> {%%0}[] toArray(java.util.function.IntFunction<{%%0}[]>)

I think your test is passing because one or two of these reasons:

• jakarta.mail is not extending any class that was modified between jdks.
• You are not loading the correct signature file, so it is not checking if new methods were added. In the other hand if a method was removed you would get an error.

I know, this doesn't explain why do we need to compile sigtest with JDK 11. Running the test with JDK11 should be enough. But if you look at the sigtest project you can see that it is relevant to compile with different JDK versions. I'm still wondering why, but to be honest, I didn't investigate this.

Let me sort out the 2 blockers we have "glassfish" and "sigtest" and I'll comeback.

[bshannon]

If it's failing because of a method that was added in JDK 11, and the method is in
the JDK 11 signature file, then perhaps it's not picking up the correct signature file?
I don't know how it's done in this TCK but I changed the Mail TCK to pick up the
correct signature file based on ${java.version}. Before that, the JDK version that it
would use was hard coded in the SignatureTest.html file. Perhaps the JSONP TCK
was assuming it would only ever be run on JDK 8 so even though the JDK 11 signature
file was present it wasn't picking it up?

[jbescos]

@bshannon I think it is picking the correct signature file. In this TCK project the mapping is done in this class: src/com/sun/ts/tests/signaturetest/SignatureTestDriver.java. You will find in this PR that I had to add support for JDK11.

In the failedSignature.log is written that is taking the correct signature file (se11):

[javatest.batch] 01-28-2020 09:29:42: Using the following as the sig-Test map file: sig-test_se11.map
[javatest.batch] 01-28-2020 09:29:42: Using the following as the SigTest Package file: sig-test-pkg-list_se11.txt

[bshannon]

@jansupol can you evaluate this PR and decide what needs to be done? Thanks.

[jbescos]

@bshannon I already spoke with @jansupol about this.
For the 2 blockers we have:

1. Sigtest: @alwin-joseph will create a PR containing the new sigtest jar files.
2. Jsonp updated libraries: to not break glassfish, we need that the TCK takes the jsonp libraries from other source. jsonp.classes of this folder jakartaee-tck/install/jsonp/bin/ts.jte need to use latest org.glassfish:jakarta.json ( https://mvnrepository.com/artifact/org.glassfish/jakarta.json/2.0.0-RC2 )

[alwin-joseph]

I am hoping to make the sigtest jar updates in a separate branch master-jdk11(created from current master) along with the new signature files for JDK11. Ideally this branch will contain the jdk11 specific changes for the tcks. This will correspond to the jdk11 branch created for glassfish as 5.1.0-run-with-JDK11.

If the sigtest jars are updated in the current master branch the tests are bound to fail in JDK8, as the sig tests need to be run with the same jars with which they were generated.

Please correct me if wrong, the javax -> jakarta changes are independent of jdk11 related changes. If so, can we please keep the javax -> jakarta changes in the master branch and jdk11 related ones in master-jdk11. We can later merge the required changes from the master to master-jdk11 if applicable.

Please share your thoughts on this.

[markt-asf]

I disagree. The TCK frame is set up to have different signature files for different JDK versions. Please see #150 and #157 for TCK updates for EL and WebSocket respectively that enable the same TCK build to be run with both Java 8 and Java 11 including a successful signature test.
The java 11 updates should happen in master and projects should be expected to provide PRs that include signature files for both Java 11 and Java 8.

[bshannon]

@bshannon I already spoke with @jansupol about this.

Good, but I still want @jansupol to review this PR.

For the 2 blockers we have:

1. Sigtest: @alwin-joseph will create a PR containing the new sigtest jar files.

It sounds like the plan is to update the sigtest file "centrally", all at once", for all the updated APIs.

2. Jsonp updated libraries: to not break glassfish, we need that the TCK takes the jsonp libraries from other source. jsonp.classes of this folder jakartaee-tck/install/jsonp/bin/ts.jte need to use latest org.glassfish:jakarta.json ( https://mvnrepository.com/artifact/org.glassfish/jakarta.json/2.0.0-RC2 )

If this PR updates the TCK tests, then some other configuration, presumably in the Jenkins job that runs the TCK, needs to be updated to use the new API+implementation. @alwin-joseph can
probably help with that.

[bshannon]

I am hoping to make the sigtest jar updates in a separate branch master-jdk11(created from current master) along with the new signature files for JDK11. Ideally this branch will contain the jdk11 specific changes for the tcks. This will correspond to the jdk11 branch created for glassfish as 5.1.0-run-with-JDK11.

THe 5.1.0-run-with-JDK1 branch doesn't have any of the jakarta.* API changes, as far as I know.

If the sigtest jars are updated in the current master branch the tests are bound to fail in JDK8, as the sig tests need to be run with the same jars with which they were generated.

Normally we would create two sigtest files, named for the version of the Jakarta EE API
being tested, and for the version of the JDK used for the test. You can see this in the
mail-tck repo FYI.

Please correct me if wrong, the javax -> jakarta changes are independent of jdk11 related changes. If so, can we please keep the javax -> jakarta changes in the master branch and jdk11 related ones in master-jdk11. We can later merge the required changes from the master to master-jdk11 if applicable.

Until we get to the point where the implementation must run on JDK 11, what you suggest
will work. Right now I assume most people are doing the package name update but are running
on JDK 8. Still, since the final result must run on JDK 11, people are going to want to convert to that soon.

I think you're going to be stuck maintaining two signature tests and update both of them when
one of the APIs is updated.

[alwin-joseph]

Hi @jbescos - I am making more signature file changes in #169. I will sync up my changes with respect to this this PR and other PRs with signature test changes . Thanks.

[jansupol]

This looks ok. There is no GF with jakarta.activation yet, so it's hard to make the classpath correct, otherwise, it looks good.