skyscreamer / JSONassert

Write JSON unit tests in less code. Great for testing REST interfaces.
http://jsonassert.skyscreamer.org
Apache License 2.0
990 stars 198 forks source link

[1.5.2] New patch version requires Java 21 #190

Closed DreierF closed 3 months ago

DreierF commented 3 months ago

Hi,

Our build breaks after updating from 1.5.1 to 1.5.2 because the library seems to no longer be compiled against Java 8, but Java 21. As this was not mentioned in the changelog and also would warrant a new major version I assume this was by accident?

java: cannot access org.skyscreamer.jsonassert.JSONAssert
  bad class file: .../.gradle/caches/modules-2/files-2.1/org.skyscreamer/jsonassert/1.5.2/917a0be22be2cd752408c49cf99655d0c6275b9/jsonassert-1.5.2.jar!/org/skyscreamer/jsonassert/JSONAssert.class
    class file has wrong version 65.0, should be 61.0
    Please remove or make sure it appears in the correct subdirectory of the classpath.
philib commented 3 months ago

+1

marcindabrowski commented 3 months ago

If it was intended it should be at least minor not a patch version release.

JaredHatfield commented 3 months ago

@marcindabrowski agreed.

Updating to Java 21 as the new required minimum is one thing, but doing it on an incremental release is another all together. I would have expected this to at lest be a minor release but ideally a major release.

I have multiple projects that use Java 17 and the upgrade to Java 21 breaks the use of this library.

itineric commented 3 months ago

If it was intended it should be at least minor not a patch version release.

It should even be a major release. Automated dependency updaters (like dependabot or renovate) work on minor + patch versions. Such a breaking change should be in a major release.

That said, what's the point of such a tool to move to the last version of java? It should stick to the oldest supported version as long as possible to maximize usability

anasoid commented 3 months ago

+1

TWiStErRob commented 3 months ago

This is the breaking change: https://github.com/skyscreamer/JSONassert/commit/eebcfe8e50b8681f8a1422106b72e25b48d7be12#diff-9c5fb3d1b7e3b0f54bc5c4182965c4fe1f9023d449017cece3005d3f90e8e4d8L73

and I received a runtime error!

java.lang.UnsupportedClassVersionError: org/skyscreamer/jsonassert/JSONCompareMode has been compiled by a more recent version of the Java Runtime (class file version 65.0), this version of the Java Runtime only recognizes class file versions up to 61.0

The project can use any Java they want as long as the binary produced is compatible with a wide range of Java versions.

Java 8 has extended support until 2030! Java 17 is still actively supported for 2 more years. https://endoflife.date/oracle-jdk

I think the above change was made because Java 21 removed Java 7 support and deprecated targeting Java 8. That doesn't mean the library should jump from 8 to 21, but rather to increase to 9 or 11 at most. This change would still be a major version bump, because it's trivially and strictly not binary compatible.

carterpage commented 3 months ago

This was in error.

Since the last release a few years ago, updates to the release process with Sonatype required modernizing the pom.xml file. This should not have involved a compiler version change, particularly going from 1.5.1 to 1.5.2.

A patch is in the works.

davsclaus commented 3 months ago

Thanks we were also hit by this at Apache Camel. We will pickup 1.5.3 when its fixed thanks

TWiStErRob commented 3 months ago

@carterpage actually I just noticed that compared to 1.5.1, 1.5.3 will still update from Java 6 to Java 8: https://github.com/skyscreamer/JSONassert/pull/187, i.e. is major. Should that be also reverted?

carterpage commented 3 months ago

1.5.3 is released: https://github.com/skyscreamer/JSONassert/releases/tag/jsonassert-1.5.3 using Java 8.

I chose 8 because 6 is no longer supported. But @TWiStErRob is probably right that this should technically be a minor bump not a patch.

What do we think? 1.5.3.1 using Java 6? I could do that this weekend, and then start catching up on other patches in 1.6.0 using Java 8 going forward.

chadlwilson commented 3 months ago

Perhaps this is sacrilegious to some, but IMHO, I think it's fine to drop Java 6 and Java 7 in a patch release at this point, given even Oracle extended support ended 2 years ago for Java 7.

TWiStErRob commented 3 months ago

I agree with Chad, technically it's still a major breaking change, but practically hopefully not many are affected.

@carterpage please update the release notes:

(Normally versioning would do this for us, but you know... 😅)

I don't think there's a point for confusing the situation even more with another patch release or a patch of a patch 4-component release. Unless there's a need for it.

carterpage commented 3 months ago

Ok, I'm going to leave 1.5.3 as is. Apologies for any trouble this caused anyone. Dangers inherent with trying to do the first release in a few years.

@TWiStErRob I added release notes as you suggested to versions 1.5.1-1.5.3.

If it turns out the lack of Java6/7 is a major painpoint, open an issue, and if there is enough consensus, I'll look at one more patch release.