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

Ban com.sun.xml.bind:jaxb-impl, replace it with jaxb-runtime where needed #15298

Merged
merged 1 commit into from
Aug 27, 2024

Conversation

ppalaga
Copy link
Contributor

@ppalaga ppalaga commented Aug 23, 2024

Description

org.glassfish.jaxb:jaxb-runtime is the new Jakarta implementation of JAXB. It should be preferred to the old com.sun.xml.bind:jaxb-impl

Target

  • I checked that the commit is targeting the correct branch (note that Camel 3 uses camel-3.x, whereas Camel 4 uses the main branch)

Tracking

No Jira, because this is just housekeeping. But I can create one, if needed.

Apache Camel coding standards and style

  • I checked that each commit in the pull request has a meaningful subject line and body.
  • I have run mvn clean install -DskipTests locally from root folder and I have committed all auto-generated changes.

Copy link
Contributor

🌟 Thank you for your contribution to the Apache Camel project! 🌟

🤖 CI automation will test this PR automatically.

🐫 Apache Camel Committers, please review the following items:

  • First-time contributors require MANUAL approval for the GitHub Actions to run

  • You can use the command /component-test (camel-)component-name1 (camel-)component-name2.. to request a test from the test bot.

  • You can label PRs using build-all, build-dependents, skip-tests and test-dependents to fine-tune the checks executed by this PR.

  • Build and test logs are available in the Summary page. Only Apache Camel committers have access to the summary.

  • ⚠️ Be careful when sharing logs. Review their contents before sharing them publicly.

@davsclaus
Copy link
Contributor

Where is this information. Do we have a page that states this.

At this page the front page says
https://eclipse-ee4j.github.io/jaxb-ri/

        <dependencies>
            <dependency>
                <groupId>jakarta.xml.bind</groupId>
                <artifactId>jakarta.xml.bind-api</artifactId>
                <version>4.0.2</version>
            </dependency>
        </dependencies>

        <dependencies>
            <dependency>
                <groupId>com.sun.xml.bind</groupId>
                <artifactId>jaxb-impl</artifactId>
                <version>4.0.5</version>
                <scope>runtime</scope>
            </dependency>

@ppalaga
Copy link
Contributor Author

ppalaga commented Aug 26, 2024

Thanks for the pointer, @davsclaus. Well, the authoritative resources about JAXB implementations are rather sparse and confusing.
This SO answer seems to describe the situation in the most complete way:

  • com.sun.xml.bind:jaxb-impl is marked as "old" in its own pom description: https://repo1.maven.org/maven2/com/sun/xml/bind/jaxb-impl/4.0.5/jaxb-impl-4.0.5.pom
  • org.glassfish.jaxb:jaxb-runtime and com.sun.xml.bind:jaxb-impl classes overlap and differ only in whether they include or depend on some transitives (istack/txw2)
  • The SO answer proposes to use newest versions of org.glassfish.jaxb:jaxb-runtime for projects after Jakarta migration and com.sun.xml.bind:jaxb-impl (versions before v3) for projects that still require javax.xml.bind. They can even co-exist in that way.

Mainly the last point makes great sense to me.

What are other's thoughts? Maybe I am missing something?

@davsclaus
Copy link
Contributor

Yeah I can understand that sun is regarded as old - just wanted to make sure that glashfish is really the replacement also for jakarta ee.

We need to make sure this kind of change does not cause pain - and should be ready to fix or revert before the 4.8 LTS release.

So this would need to check that camel and CSB and CEQ still works aftwards.

@davsclaus
Copy link
Contributor

And it would be good to add a note in the 4.8 upgrade docs about this change (so users would be aware) but hopefully they dont need to worried.

@ppalaga
Copy link
Contributor Author

ppalaga commented Aug 26, 2024

So this would need to check that camel and CSB and CEQ still works aftwards.

It is fine for Camel Quarkus, where we already merged a similar change apache/camel-quarkus#6382

And it would be good to add a note in the 4.8 upgrade docs

Where can I do that?

@davsclaus
Copy link
Contributor

@davsclaus
Copy link
Contributor

I wonder if CSB need some kind of update as well since CEQ had that ?

@ppalaga ppalaga force-pushed the 240823-ban-com.com.sun.xml.bind branch from 491de28 to 57f07d3 Compare August 27, 2024 14:21
@ppalaga
Copy link
Contributor Author

ppalaga commented Aug 27, 2024

I wonder if CSB need some kind of update as well since CEQ had that ?

Yes, having an enforcer check might be a good idea to make sure that com.sun.xml.bind:jaxb-impl is not smuggled back by SB deps.

@ppalaga
Copy link
Contributor Author

ppalaga commented Aug 27, 2024

@ppalaga ppalaga merged commit 651957d into apache:main Aug 27, 2024
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants