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

jaxrs jakarta versions have javax.ws references in OSGi manifest #132

Closed
scottslewis opened this issue Dec 14, 2020 · 40 comments
Closed

jaxrs jakarta versions have javax.ws references in OSGi manifest #132

scottslewis opened this issue Dec 14, 2020 · 40 comments

Comments

@scottslewis
Copy link

In the maven-released version 2.12.0, the jackson-jaxrs-base-2.12.0-jakarta.jar has this in Import-Packages:

javax.ws.rs;v ersion="[2.0,2.2)",javax.ws.rs.core;version="[2.0,2.2)",javax.ws.rs.e xt;version="[2.0,2.2)"

It's therefore looking for 2.0 version of jaxrs spec in javax. namespace rather than the 3.0 jakarta version in jakarta.* namespace as it should for the jakarta version.

The jackson-jaxrs-json-provider-2.12.0-jakarta.jar seems to have the same problem (still importing javax.* namespace/2.0 version of jaxrs api classes)

@cowtowncoder
Copy link
Member

@scottslewis this is wrt OSGi metadata? What would be the best way to test this? I do not use OSGi anywhere directly myself and most aspects are handled by Felix plug-in. Further complications arise from the way jax-ws/jakarta versioning, module information has been changing.

@GedMarc I assume this is related to the lovely challenges of new module structure for j2ee thingies?

@scottslewis
Copy link
Author

scottslewis commented Dec 14, 2020

@scottslewis this is wrt OSGi metadata?

Yes. Specifically the Import-Package statement is what I quote above. As you can see it has javax.* vs jakarta.* for package name, and the version range is incorrect (I believe).

What would be the best way to test this? I do not use OSGi anywhere directly myself and most aspects are handled by Felix plug-in. Further complications arise from the way jax-ws/jakarta versioning, module information has been changing.

scottslewis: From the manifest you seem to be using this maven plugin:

Created-By: Apache Maven Bundle Plugin

And here's the maven central for the 'base' plugin:

https://search.maven.org/artifact/com.fasterxml.jackson.jaxrs/jackson-jaxrs-base/2.12.0/bundle

In the pom is this:

  <properties>
    <osgi.export>
${project.groupId}.annotation.*;version=${project.version}
,${project.groupId}.base.*;version=${project.version}
,${project.groupId}.cfg.*;version=${project.version}
,${project.groupId}.util.*;version=${project.version}
</osgi.export>
    <osgi.import>javax.ws.rs;version="${javax.ws.rs.version}"
,javax.ws.rs.core;version="${javax.ws.rs.version}"
,javax.ws.rs.ext;version="${javax.ws.rs.version}",
*
</osgi.import>
  </properties>

For the jakarta jar, this should read something like this:

  <properties>
    <osgi.export>
${project.groupId}.annotation.*;version=${project.version}
,${project.groupId}.base.*;version=${project.version}
,${project.groupId}.cfg.*;version=${project.version}
,${project.groupId}.util.*;version=${project.version}
</osgi.export>
    <osgi.import>jakarta.ws.rs;version="${jakarta.ws.rs.version}"
,jakarta.ws.rs.core;version="${jakarta.ws.rs.version}"
,jakarta.ws.rs.ext;version="${jakarta.ws.rs.version}",
*
</osgi.import>
  </properties>

where jakarta.ws.rs.version == [3.0,4)

I'm happy to help test if you like. I can easily test in Eclipse...to see if the package names/versions in the packaged jar are correct (given the references needed in the java classes)

@GedMarc I assume this is related to the lovely challenges of new module structure for j2ee thingies?

@cowtowncoder
Copy link
Member

@scottslewis I am actually not quite sure I still understand: none of the dependencies from either pom.xml or classes is to "jakarta.ws." anything; they are all to "java.ws.". Wouldn't changing metadata be wrong in this case?

If changes make sense, they would be needed in almost all sub-projects (6, it looks like).

@GedMarc
Copy link
Contributor

GedMarc commented Dec 22, 2020

Yeah, we can add a manifest transformer to the shade to alter the MF files as well, this also came up in the OSGi things as well in PFaces,
I just reformatted - will have the PR in a little bit

@cowtowncoder
Copy link
Member

@GedMarc ah, of course, that makes sense. Should have remembered transformation phase.

@scottslewis
Copy link
Author

@cowtowncoder Sounds like you may have worked it out with @GedMarc , but for all the references to jakarta (code and manifest.mf/meta-data) it's my understanding that they have to have package: jakarta.ws.* rather than javax.ws.. I haven't looked at the class references but what I was seeing was that the manifest.mf for the jakarta jar had javax.ws. for import-package and my expectation is that it should be jakarta.ws.*.

@cowtowncoder
Copy link
Member

@scottslewis right, I think as @GedMarc's explanation and earlier work build actually produces two sets of artifacts, one for older JAX-WS API, and another for Jakarta variants; latter having separate classifier of "jakarta". So what is missing is just transformation of OSGi dependence metadata for jakarta artifacts.
Users will then have to add proper dependency wrt which API to use (based on implementation to use it with).

@scottslewis
Copy link
Author

@cowtowncoder right. If you would like me to examine/test a new/improved jakarta artifact (i.e. with jakarta package info in manifest.mf) please just let know here.

@cowtowncoder
Copy link
Member

Hi @GedMarc! Just checking to see if you might have time to help here; was thinking of releasing 2.12.2 relatively soon.

@cowtowncoder
Copy link
Member

@scottslewis With the PR merged, it'd be good to have some sanity checking before I release 2.12.2, if you have time? I'll push snapshot to Sonatype OSS repo, or you can build locally.

@cowtowncoder cowtowncoder changed the title jaxrs jakarta versions have javax.ws references in manifest jaxrs jakarta versions have javax.ws references in OSGi manifest Feb 21, 2021
@scottslewis
Copy link
Author

@scottslewis With the PR merged, it'd be good to have some sanity checking before I release 2.12.2, if you have time? I'll push snapshot to Sonatype OSS repo, or you can build locally.

I haven't been keeping up on the changes, so if you can push a snapshot to OSS repo I'll take a look/review/try out.

Thanks!

@cowtowncoder
Copy link
Member

@scottslewis yes, I did push 2.12.2-SNAPSHOT of jax-rs providers.

@scottslewis
Copy link
Author

I've looked over the two jaxrs jakarta bundles and tried them out in my osgi project.

They now seem to be referring to the new jakarta.* package names...which is correct.

However, the version range specified for import of these packages is still wrong/old as the jakarta.* packages have version 3+, where the old version is 2.x. Here's what's in the 2.12.2 snapshot right now:

jakarta.ws.rs ;version="[2.0,2.2)",jakarta.ws.rs.core;version="[2.0,2.2)",jakarta.w s.rs.ext;version="[2.0,2.2)"

The range "[2.0,2.2)" is incorrect for the jakarta package (version 3) and should be something like this:

jakarta.ws.rs;version="[3.0,3.2)",jakarta.ws.rs.core;version="[3.0,3.2)",jakarta.ws.rs.ext;version="[3.0,3.2)"

in both the jackson-jaxrs-base-2.12.2-jakarta.jar
and the jackson-jaxrs-json-provider-2.12.2-jakarta.jar
artifacts

@cowtowncoder
Copy link
Member

Hmmmh. Yes, that is bit gnarly... not sure how transformer could handle this case, since in addition to renaming, version range needs to be translated as well.
Not sure if extending version range (just short term) to [2.0, 3.2) would allow override just to get 2.12.2 released, unless full solution is found.

@scottslewis
Copy link
Author

Hmmmh. Yes, that is bit gnarly... not sure how transformer could handle this case, since in addition to renaming, version range needs to be translated as well.
Not sure if extending version range (just short term) to [2.0, 3.2) would allow override just to get 2.12.2 released, unless full solution is found.

Although having such a version range [2.0, 3.2) would likely work...it's not great as jakarta.* namespace only exists 3.0+

@GedMarc
Copy link
Contributor

GedMarc commented Feb 22, 2021

@scottslewis Hmm the Jakarta namespace is from 2.10.4 strictly speaking... wondering how to solve for this...
What would the bare minimum be as a resolution -

I know a lot of frameworks and libraries are looking at Jetty and Guice for solving for Jakarta and modularization and already implement and use the artefacts from 2.10.4 and up

Obviously using modularization, OSGi plays no role whatsoever, so I'm not really testing it at all

@scottslewis
Copy link
Author

@scottslewis Hmm the Jakarta namespace is from 2.10.4 strictly speaking... wondering how to solve for this...
What would the bare minimum be as a resolution -

The jakarta.ws.rs.* packages start at version 3.0.0. You can see this by examining the manifest in the jar here:

https://search.maven.org/search?q=g:jakarta.ws.rs

where all the packages are exported as version 3.0.0 i.e.

jakarta.ws.rs;version="3.0.0"

so consumers of these packages/classes (like these two bundles) should import like this:

Import-Package: jakarta.ws.rs;version="[3.0,4)"

which says: import any version of this package with lower bound 3.0 (inclusive...i.e. 3.0 is fine) to 4 (exclusive...i.e. 4.0 is not allowed).

A version range like this will allow the osgi resolver to import version 3.0.0 of the jakarta.ws.rs version because it fits between [3.0,4).

Note that for the old/non-jakarta fasterxml jar, it should be the version range you were using...i.e.:

javax.ws.rs;version="[2.0,2.2)"

So...I don't know how your build system does this, but couldn't it put:

Import-Package: javax.ws.rs;version="[2.0,2.2)", etc

for the old/non-jakarta jar
and

Import-Package: jakarta.ws.rs;version="[3.0,4)", etc

for the new/jakarta jars?

I assumed that if you could substitute jakarta for javax in package name, that you could substitute "[3.0,4)" for "[2.0,2.2)"

If you cannot do this, however, then you could have a version range for both that's very wide...i.e.:

javax.ws.rs;version="[2.0,4)",
and
jakarta.ws.rs;version="[2.0,4)",

but this is less satisfactory, as the javax package will never go above 2.1, and the jakarta package will never go below 3.0. And keeping the version range as small as possible is a much better practice.

@GedMarc
Copy link
Contributor

GedMarc commented Feb 22, 2021

Understood - Yes this can be done during the build process (y)

@GedMarc
Copy link
Contributor

GedMarc commented Feb 22, 2021

jackson-jaxrs-base-2.12.2-SNAPSHOT-jakarta.zip

Please can we verify the built manifests :) @scottslewis @cowtowncoder

@scottslewis
Copy link
Author

jackson-jaxrs-base-2.12.2-SNAPSHOT-jakarta.zip

Please can we verify the built manifests :) @scottslewis @cowtowncoder

These manifests look good to me. I assume that you are going to do same thing for jackson-jaxrs-json-provider.

In a few hours I will be able to try them out.

@GedMarc
Copy link
Contributor

GedMarc commented Feb 22, 2021

@scottslewis The PR targets all affected clients, and has each jar packaged - you can test all of them

#136

@scottslewis
Copy link
Author

@GedMarc I've tested both of the jaxrs jakarta jars, and they resolve properly (in osgi) against the jakarta 3.0.0 version. I'm +1 on this change. Thanks for fixing this.

cowtowncoder pushed a commit that referenced this issue Feb 23, 2021
… shade in the new manifest for Jakarta separately before install (#136)

* Transform Service Loaders, and Manifests for Jakarta Artifacts

* Create manifest files separately and reference in bundle plugin, THEN shade in the new manifest for jakarta separately before install

#132
@cowtowncoder
Copy link
Member

Assuming fixed now. Found one more potential issue (aside from question of of cbor/smile/yaml modules) wrt lack of module-info from "no-metainf" variants -- that's unrelated and already seemed to be missing.

@GedMarc
Copy link
Contributor

GedMarc commented Feb 23, 2021

Hmm not going to be easy - can't exclude provision clauses to the same effect without creating multiple module-info's hmmm

@cowtowncoder
Copy link
Member

Oh. Well, it's not critical for now, I'll file and see if something can be done or not.
There's also side-question of jakarta-vs-none anyway. I wish JDK SPI approach was not such a fail.

@cowtowncoder
Copy link
Member

cowtowncoder commented Feb 23, 2021

Also possible that by the time 3.0.0 is ready, it might be sufficient to drop META-INF service provider part anyway, and leave the module-info one -- I'd hope JAX-xx specs and Java platform have evolved enough to work better. Auto-registration seems like an idea that looks great at first for simple systems but turns into R-PITA quite often.

Anyway -- I could also try asking on mailing lists whether intermediate solution where module-info does have registration but older SPI does not might be sufficient. It might be.

@cowtowncoder
Copy link
Member

Ok more question: @scottslewis (and @GedMarc): is the same change needed for jackson-module-jaxb-annotations (in https://github.com/FasterXML/jackson-modules-base/) as well?
JAX-RS providers do have dependency to it as well.

I just thought I'd ask, after adding Jakarta-variants in jackson-bom (see FasterXML/jackson-bom#40), as well as adding variants for cbor, smile and yaml providers.

@scottslewis
Copy link
Author

Ok more question: @scottslewis (and @GedMarc): is the same change needed for jackson-module-jaxb-annotations (in https://github.com/FasterXML/jackson-modules-base/) as well?

It looks from the current manifest (2.12.1) that it has references to the xml binding spec...i.e. javax.xml.bind:

Import-Package: ...
javax.xml.bind;version="[2.3,3)",javax.xml.bind.annotation;
version="[2.3,3)",javax.xml.bind.annotation.adapters;version="[2.3,3)
",javax.xml.parsers,org.w3c.dom

And in Export-Package: ... (it's kind of strange to me that these javax.xml.bind packages are exported by this bundle)
javax.xml
.bind.annotation,javax.xml.bind.annotation.adapters";version="2.12.1"

In any case, it's my understanding that the javax.xml.bind.* packages are also part of the EE(javax) -> Jakarta transition. Might make sense to look anywhere where javax.xml.bind is being used.

@cowtowncoder
Copy link
Member

These are the only places within Jackson/fasterxml modules that I know of; jackson-dataformat-xml does depend on JAXB annotations module but not on JAXB types (nor api or implementations).

I plan to remove JAXB annotation module dependency from JAX-RS providers in Jackson 3.0 (see #123), to reduce dependency slightly (users can use JAXB Annotation introspector, just not brought in as hard dependency).
And depending on timing, perhaps it'd be possible to make jakarta-APIs the default (and either use classifier for legacy ones or just not release them at all; TBD).

On package exporting: I think these are calculated by maven bundle plug-in, not explicitly specified (as far as I know?). Could be that some types are exposed via configuration methods?

@scottslewis
Copy link
Author

> > On package exporting: I think these are calculated by maven bundle plug-in, not explicitly specified (as far as I know?). Could be that some types are exposed via configuration methods?

I suspect it's something having to do with how you've configured the builder...or rather the maven plugin that generates the manifest.mf for this jar.

@cowtowncoder
Copy link
Member

Default settings come all the way from FasterXML/oss-parent's pom.xml:

https://github.com/FasterXML/oss-parent/blob/master/pom.xml

but look straight-forward, with

 <osgi.export>${project.groupId}.*;version=${project.version}</osgi.export>
...
        <plugin>
          <groupId>org.apache.felix</groupId>
          <artifactId>maven-bundle-plugin</artifactId>
          <version>${version.plugin.bundle}</version>
          <configuration>
            <instructions>
...
              <Export-Package>${osgi.export}</Export-Package>

but I am not very familiar with workings of that plugin (aside from the fact that it generates necessary bundle metadata for OSGi).

@cowtowncoder cowtowncoder removed this from the 2.12.2 milestone Mar 12, 2021
@cowtowncoder cowtowncoder reopened this Mar 12, 2021
@cowtowncoder
Copy link
Member

Unfortunately patch that would have fixed this problem had some (as of yet) poorly understood issue which lead to the whole 2.12.2 release being broken wrt JAX-RS providers.
So I had to revert that, republish.

Need to figure out something else...

@scottslewis
Copy link
Author

Unfortunately patch that would have fixed this problem had some (as of yet) poorly understood issue which lead to the whole 2.12.2 release being broken wrt JAX-RS providers.
So I had to revert that, republish.

Need to figure out something else...

What was happening with the patch that broke the 2.12.2 release? Seems like that's the thing to fix, as without the fixes for this bug the jakarta jackson jars are unusable.

@cowtowncoder
Copy link
Member

cowtowncoder commented Mar 13, 2021

@scottslewis really? Wish I had thought about that :-D

Snark aside if I had any idea how to fix it, I'd do that but as things are I can neither reproduce it without attempting release, nor have idea of how to avoid the problem.
Author of patch, @GedMarc wasn't sure of details, unfortunately, although he may have a reproduction as he has used plugins for other projects.
Symptom itself is that .pom files are missing from release that is staged. Client does not complain, nor Nexus, but without pom release itself is broken.
Local publish works just fine; not sure about snapshots (presumably those would exhibit the same problem).

But even without this problem, it seems like having just one pom for 2 variants might be problematic wrt dependencies.

At higher level I feel quite irritated about the mess Oracle/Jakarta folks have created here with their reckless forking of existing APIs.

As to 2.12.2 -- since non-OSGi users were heavily affected (but OSGi usage was not working before), it made much more sense to try to get 2.12.2 working to the degree 2.12.1 was, instead of waiting for possible but not guaranteed fix, blocking everyone -- and getting daily bug reports on broken release.

@GedMarc
Copy link
Contributor

GedMarc commented Mar 13, 2021

@scottslewis The felix bundle plugin does not support multiple attachments and is removing the default pom as a release artifact for the jakarta classified release, I'm struggling to find a plugin replacement, or find assistance in doing it

This has nothing to do with the strategy for becoming Jakarta compatible, and is purely OSGi related, in my opinion the manifest entries should actually be removed going forward and be left to the implementer to do it's auto thing...

I think it is important to remember that modules + JRT replace OSGi and virtual file systems, and that manifests are going to become much much thinner going forward, as mentioned, the core of this is that the Jakarta 9 goal of becoming fully JDK 11 compatible was missed

It's a big question now to be sure though, at which point does Jakarta become compatible, and at what stage do libraries support it, even now servlet 5 actually hasn't got a module-info and the others are invalid for usage in a compiled space, so even they aren't sticking to it making it hellish to plan for and support (notice the 4 module names for jax/rs they came up with over the last 3 years in the module definition?)

This change caused a mess because of how the bundle plugin handles additional executions and classifiers, being unchanged for so long, it is unlikely for it be changed now

@GedMarc
Copy link
Contributor

GedMarc commented Mar 13, 2021

I set up an antifactory that I am using to replicate the release issue, it looks like felix cannot create a bundle pom and jar classified pom in the same build,

I mean it can, it just won't release the bundle pom at the final step

@cowtowncoder
Copy link
Member

@scottslewis @GedMarc Created #141 to suggest that for 2.13, a hard fork be made of JSON provider (and base as necessary), to produce separate artifact altogether.

Funnily enough I am more worried about naming -- jackson-jakrs-json-provider or what? -- than other aspects. Yes, this is code duplication, but providers have not changed rapidly.

WDYT?

@scottslewis
Copy link
Author

My primary hope is that external (e.g. my...or Jersey's) references to your packages and classes doesn't have to change. I personally don't mind too much if the maven identifiers change for compile-time access, but of course if you do that then all your consumers (e.g. Jersey) will have to update their compile-time dependencies (i.e. poms).

As a library maintainer myself it seems to me like a fork might create a lot of extra work...particularly since although it's going to be a long while before everyone has moved to jakarta.* from javax......as jakarta is apparently the future for the javaee classes. I know, for example that the servlet engines...commercial and otherwise...are doing runtime package renaming (javax. -> jakarta.) so as to support this without having clients change to jakarta. right away.

So if forking is the best approach for Jackson then I'm +1. I guess I can't really tell how much this will be a burden for you folks.

@cowtowncoder
Copy link
Member

Aside from possible change to register providers (which may or may not be done handled by Jersey), I think that would be achieved by whatever change Jackson chooses. App code does need to change set of annotations for JAX-RS (JAK-RS? :) ) in use, which likely is just changing import statements. But that would be separate from Jackson provider question.
And Jackson annotations themselves would not change (... then again, reference to "JAXB" annotation types would...).

@cowtowncoder
Copy link
Member

Ok, so, the real solution for 2.13 is #146: creation of essentially a fork that just implements Jakarta API variant. Repo is here:

https://github.com/FasterXML/jackson-jakarta-rs-providers

and I hope we'll get to Release Candidate phase soon. Help with testing would be much appreciated.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants