Add Jackson 3 support as org.msgpack.jackson3:jackson-dataformat-msgpack - #1012
Add Jackson 3 support as org.msgpack.jackson3:jackson-dataformat-msgpack#1012xerial wants to merge 10 commits into
Conversation
Address PR #987 feedback asking for a clear artifact-name mapping between jackson-dataformat-msgpack (Jackson 2) and jackson-dataformat-msgpack-jackson3 (Jackson 3) to reduce upgrade confusion.
# Conflicts: # .github/workflows/CI.yml
main migrated the build to sbt 2 (feb87ef) without sbt-jmh, since sbt-jmh 0.4.7 has no sbt-2-compatible artifact. This branch's msgpack-jackson3 JMH benchmarks depend on sbt-jmh, so after merging main the plugin fails to resolve. 0.4.8 publishes an sbt_2_3 build.
…3 to msgpack-jackson Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014ZZUaHusAjVx6SNu4sA42s
…format-msgpack - msgpack-jackson now hosts the Jackson 3.x integration (artifact jackson-dataformat-msgpack, package org.msgpack.jackson3.dataformat, Java 17+) - msgpack-jackson2 hosts the Jackson 2.x integration in maintenance mode (artifact jackson2-dataformat-msgpack, package org.msgpack.jackson.dataformat unchanged, Java 8+) - Distinct packages and artifactIds let both integrations coexist on one classpath for incremental migration - Add sbt-jupiter-interface so the Jackson 2 module's JUnit 5 tests actually run; they had been silently skipped (0 tests detected) on main - Fix MessagePackDataformatTestBase lifecycle annotations (JUnit 4 @Before/ @after on JUnit 5 tests meant setup never ran) - Restore byte-offset-as-columnNr in Jackson 2 MessagePackParser locations, regressed unnoticed in #903 when the deprecated JsonLocation constructor was replaced Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014ZZUaHusAjVx6SNu4sA42s
0.17+ upgrades to JUnit 6, which requires Java 17; the JDK 8 CI lane failed compiling msgpack-jackson2 tests against junit-jupiter-api 6.0.3 (class file version 61). 0.15.2 stays on JUnit 5.14, and the module's explicit junit-jupiter 5.14.4 dependency wins resolution. Verified by forking the tests onto a real JDK 8. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014ZZUaHusAjVx6SNu4sA42s
|
@xerial Thanks for building on the PR. I understand the goal of giving Jackson 3 the unqualified
Could we instead give Jackson 3 a new group ID?
This is JLBP-6 Case 4: the Maven ID and Java package are renamed together. Existing Jackson 2 users do not need to change coordinates, both modules keep the Jackson used a similar structure for its XML module:
Would this structure work here? |
|
Good catch. We need to add a new group ID to fully isolate modules. In that sense, a 1.0.0 version bump is unnecessary. A tricky part is that users need to specify a different group ID for msgpack-core and jackson3, but jackson3 is a new module, so this group naming should be acceptable. |
Restructure the module layout per PR review feedback (JLBP-6): instead of renaming the Jackson 2 artifact to jackson2-dataformat-msgpack and giving Jackson 3 the unqualified jackson-dataformat-msgpack artifactId, rename the Maven coordinates and the Java package together: - Jackson 2 (msgpack-jackson): keeps org.msgpack:jackson-dataformat-msgpack and the org.msgpack.jackson.dataformat package, exactly as 0.9.x. Existing users need no changes. Maintenance mode: Jackson 2.x bumps and bug fixes only. - Jackson 3 (msgpack-jackson3): published as org.msgpack.jackson3:jackson-dataformat-msgpack with the org.msgpack.jackson3.dataformat package, mirroring Jackson's own 2->3 groupId rename (com.fasterxml.jackson.dataformat -> tools.jackson.dataformat). Because the two artifacts share the same fully-qualified class names in neither direction, they can coexist on one classpath, and dependency resolution can never silently replace the Jackson 2 classes with Jackson 3 ones (or vice versa). The 1.0.0 version bump is no longer needed. Both sbt subprojects now share the module name jackson-dataformat-msgpack, so the Jackson 3 project overrides outputPath to keep sbt 2 build output directories from overlapping. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01C4Sp7Q2urgnYfuNXK2fVFm
|
Restructured the PR as discussed (98e90f5):
The Jackson 2 module is back at |
| description := "Jackson 3.x extension that adds support for MessagePack", | ||
| OsgiKeys.bundleSymbolicName := "org.msgpack.jackson3.jackson-dataformat-msgpack", | ||
| OsgiKeys.exportPackage := Seq("org.msgpack.jackson3", "org.msgpack.jackson3.dataformat"), | ||
| OsgiKeys.importPackage := Seq("!android.os", "!sun.*"), |
There was a problem hiding this comment.
bnd drops any referenced package that matches nothing in this list, so with only exclusions the bundle gets no Import-Package header at all and can't resolve tools.jackson.* in OSGi. msgpack-jackson doesn't set this key and gets a correct header.
msgpack-jackson3 references neither sun.misc nor android.os, so the line can just go. My bug from #987.
| OsgiKeys.importPackage := Seq("!android.os", "!sun.*"), |
| val junitInterface = "com.github.sbt" % "junit-interface" % "0.13.3" % "test" | ||
|
|
||
| // Project settings | ||
| val isJava17Plus: Boolean = { |
There was a problem hiding this comment.
This is meant to skip msgpack-jackson3, which compiles with --release 17, when the target JDK is older. But java.specification.version is the JDK running sbt, not the one compiling. Since #998 those are two different JDKs whenever TEST_JAVA_HOME is set, and sbt 2 requires its own to be 17+, so this is always true and the module is never skipped.
Before #998 there was no TEST_JAVA_HOME and one JDK did both, so it worked.
TEST_JAVA_HOME=<jdk8> ./sbt clean compile fails with javac: invalid flag: --release. The clean matters, stale classes hide it.
Minor: the comment says "the Jackson 3 module (msgpack-jackson)", which is the Jackson 2 module now.
) JmhPlugin adds jmh-core, jmh-generator-bytecode and jmh-generator-reflection to libraryDependencies without a configuration, so enabling it on msgpack-jackson3 published them as compile dependencies of jackson-dataformat-msgpack, inherited by every consumer along with jopt-simple, commons-math3 and ASM. The released org.msgpack:jackson-dataformat-msgpack POM has no such entries. Move src/jmh into msgpack-jackson3-benchmark, which depends on msgpack-jackson3 and is not published. This is also how sbt-jmh recommends laying out benchmarks, since the code generator needs a project of its own.
Summary
This PR builds on #987 (thanks @komamitsu — all the Jackson 3 port code here is from that PR) and adds Jackson 3 support as a new artifact, keeping the existing Jackson 2 artifact untouched.
Following the review discussion (JLBP-6), the Maven coordinates and the Java package are renamed together, mirroring Jackson's own 2→3 groupId change (
com.fasterxml.jackson.dataformat→tools.jackson.dataformat):msgpack-jacksonorg.msgpackjackson-dataformat-msgpackorg.msgpack.jackson.dataformatmsgpack-jackson3org.msgpack.jackson3jackson-dataformat-msgpackorg.msgpack.jackson3.dataformatorg.msgpack.jackson3groupId with a new Java package (JLBP-6 Case 4: Maven ID and Java package renamed together). No two artifacts ever share fully-qualified class names, so both can coexist on one classpath for incremental Jackson 2→3 migration, and dependency resolution can never silently swap one for the other.msgpack-core+msgpack-jackson(Jackson 2); JDK 17+ lanes test everything. Release publishes core/jackson (Jackson 2) with JDK 8 andmsgpack-jackson3with JDK 17.jackson-dataformat-msgpack, so the Jackson 3 project overridesoutputPathto keep sbt 2 build output directories from overlapping.Pre-existing test issues found and fixed along the way
./sbt msgpack-jackson/testdetects 0 tests and CI passes vacuously. Addedsbt-jupiter-interfaceso they actually run (77 tests).MessagePackDataformatTestBaseused JUnit 4@Before/@Afteron JUnit 5 tests, so setup never ran → converted to@BeforeEach/@AfterEach.JsonLocationconstructor inMessagePackParserand silently dropped the byte offset reported viagetColumnNr(). Restored using the non-deprecated 5-arg constructor, matching the Jackson 3 port's behavior (byte offset ascolumnNr).Test results
Closes #933. Supersedes #987 (includes all of its commits).
🤖 Generated with Claude Code
https://claude.ai/code/session_014ZZUaHusAjVx6SNu4sA42s