search

cashapp / tempest

Typesafe DynamoDB for Kotlin and Java.
https://cashapp.github.io/tempest/
Apache License 2.0
83 stars 33 forks source link

Set -Xjvm-default=all-compatibility compiler arg #178

Closed szabado-faire closed 6 months ago

szabado-faire commented 6 months ago

I think I finally cracked https://github.com/cashapp/tempest/issues/171. It turns out the issue is kotlin related.

It seems the root of the problem was that our project has the -Xjvm-default=all compiler flag set. Commenting it out eliminates this issue.

Seems like there's some subtle backwards incompatibility around annotations on pre-Java 8 interfaces with default implementations. Before Java 8, interfaces didn't support default implementations and would have a DefaultImpls static class declared to emulate that behaviour. Kotlin defaults to that pre-Java 8 behaviour (docs), and generates DefaultImpl objects. It seems like tempest using the pre-java-8 style here causes in projects that enable the post-java 8 behaviour.

all-compatibility seems like the correct setting to apply to solve this, based on these docs:

In addition to the all mode, generate compatibility stubs in the DefaultImpls classes. Compatibility stubs could be useful for library and runtime authors to keep backward binary compatibility for existing clients compiled against previous library versions. all and all-compatibility modes are changing the library ABI surface that clients will use after the recompilation of the library. In that sense, clients might be incompatible with previous library versions. This usually means that you need a proper library versioning, for example, major version increase in SemVer.

Hopefully this will fix my side without breaking other users of tempest.

szabado-faire commented 6 months ago

An alternative is just updating to all. That'll only break any users of tempest that are on Java 7; aws has dropped support for Java 6

Java 7 has entered EOL and no longer gets security updates, so it's probably fine to drop support for

kyeotic commented 6 months ago

Increasing compatibility on the library is probably the right move, but as the note says

This usually means that you need a proper library versioning, for example, major version increase in SemVer.

Unfortunately tempest does not use SemVer, so the only way to release a major version is to create a tempest3 version. This would also require that we introduce separate build steps for each version.

Or we could release as-is and be ready to rollback... I hate our versioning strategy. I would much rather be on SemVer.

szabado-faire commented 6 months ago

Or we could release as-is and be ready to rollback...

I think this is probably the easiest path forwards here. If it breaks something we can roll back