Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Migrate to Jakarta EE 9 #124

Closed
wants to merge 10 commits into from
Closed

Conversation

tamersaadeh
Copy link

Jakarta EE 9 platform renames all the "old" namespaces from javax.* to jakarta.*. This pull request performs this migration so that when Jacakrta EE 9 is released in June 2020 it will be compatible.

I ran mvn test and travis builds with no failures*.

Note 1: this is completely incompatible with the old namespace, so probably the major version of the library should be bumped.
Note 2: currently depends on pre-released versions of the JAXB specification (RC3). This can be updated when the final version is released.
Note 3: this PR is similar to my PR for FasterXML/jackson-modules-base#95 and FasterXML/jackson-dataformat-xml#407

*: I needed to modify 3 tests (7ff95e7) but I'm not exactly sure way this change was needed on my development PC.

pom.xml Outdated
@@ -34,20 +34,20 @@
<project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>

<!-- Need Jersey+Jetty for testing; deps from DropWizard 1.2.9 -->
<version.jersey>2.25.1</version.jersey>
<version.jersey>3.0.0-M1</version.jersey>
<!-- wrt CVE-2019-10247 -->
<version.jetty>9.4.17.v20190418</version.jetty>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

9.4.29.v20200521

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure I can update this, I just didn't think my commits should touch other dependency not strictly relevant to the pull request.

@cowtowncoder cowtowncoder added the 3.x For Jackson 3.x features, fixes label Jun 1, 2020
@@ -13,12 +13,12 @@

requires com.fasterxml.jackson.jaxrs.base;

requires static javax.ws.rs.api;
requires static jakarta.ws.rs.api;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this line and all similar in other module-info.java files should not change. See #127 and #121

Copy link
Contributor

@GedMarc GedMarc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the static clauses in the moditect file are for backwards compatibility,

don't change them, add new ones instead - this makes sure that when modules are used, it can be done easily

also jakarta artifacts should be a shade on the javax. ones, with the package replaced, it shouldn't be a source code change

@cowtowncoder
Copy link
Member

Just a quick comment: I have no plans to move ahead with this change at this point in time.
I am not against migration in principle but it does not sound like this could be done without significant breakage for existing users, for now.

@joschi
Copy link

joschi commented Jan 1, 2021

@tamersaadeh @cowtowncoder With the jakarta.* and javax.* namespaces existing in parallel, I think it would make sense to cater both ecosystems with jackson-jaxrs-providers because javax.* won't be vanishing over night.

What do you think?

@cowtowncoder
Copy link
Member

@joschi Agreed, and for 2.12 there is something like that (see #127) -- "jakarta" classifier for new variants.

@joschi
Copy link

joschi commented Jan 4, 2021

Agreed, and for 2.12 there is something like that (see #127) -- "jakarta" classifier for new variants.

As far as I understand #127 only addresses the JPMS module information but doesn't update the package imports in the actual source files, does it?

So we would need a second set of sources (src/main/jakarta?) with the appropriate imports from jakarta.*.

@GedMarc
Copy link
Contributor

GedMarc commented Jan 4, 2021

no - it get's shaded in with all the imports correctly applied

@joschi
Copy link

joschi commented Jan 4, 2021

no - it get's shaded in with all the imports correctly applied

@GedMarc Could you elaborate? I've checked out the master branch of this repository and ran mvn install but there are no artifacts using the Jakarta EE namespace implemented by Jersey 3.0.0 (only the default jar and the no-metainf-services artifacts).

I also only see Jersey 2.25.1 referenced in the POM files, not Jersey 3.0.0 (as of fcc3e95):

<version.jersey>2.25.1</version.jersey>

@GedMarc
Copy link
Contributor

GedMarc commented Jan 4, 2021

No the code itself does not reference it,. it get's shaded in and all the relevant imports get modified into a classified artefact on pom, called "-jakarta",

Look at https://maven.apache.org/plugins/maven-shade-plugin/shade-mojo.html

This keeps a single code base, while enabling full jakarta usage.
The implementation is correct and complete, I forgot about the OSGi things will see if this has been pulled in or if I must do it quick

You need to follow the readme and reference the jakarta classified artefacts.

@joschi
Copy link

joschi commented Jan 4, 2021

No the code itself does not reference it,. it get's shaded in and all the relevant imports get modified into a classified artefact on pom, called "-jakarta",

Turns out that this is missing on the master branch and it's only available on the 2.12 branch.

@GedMarc
Copy link
Contributor

GedMarc commented Jan 4, 2021

:D

Ok i think it might be waiting for the latest merge,
Although you make a good point...

@cowtowncoder are you targeting javax. for 3.0 release? Should I update the references?

@cowtowncoder
Copy link
Member

Ok, so the omission from master is an artifact of one pom.xml merge conflicts being ignored -- since that is a royal PITA when rolling forward changes after releasing new versions (since version numbers across branches will "conflict" when releases are made from maintenance branches).
Basically all changes to pom.xmls need to be manually merged.

As to 3.0 itself: I don't yet know, so probably the best thing to do would be to just merge functionality from 2.12, and figure it out in future, based on success of Jakarta variant.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3.x For Jackson 3.x features, fixes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants