Hi Gary, On Sat, 19 Oct 2024 at 05:48, Gary D. Gregory <ggreg...@apache.org> wrote: > I do not see calls to japicmp or revapi. > > How do we check that we are not breaking binary compatibility?
We are using BND Baseline[1], which performs two tasks: 1. It checks that we adhere to strict semantic versioning on a per-package level. This means that we can still have packages in Log4j Core 3.x that maintain full binary compatibility with Log4j Core 2.x, even if the library as a whole constitutes a breaking change. 2. It detects MICRO(adding or removing an annotation)/MINOR(adding a public/protected method/class)/MAJOR(removing a public/protected method/class) changes in classes. Personally I have a lot of confidence in this tool and its author, since it can even handle subtle cases of type erasure and synthetic default methods[2]. The tool is continuously evolving and covers a lot of edge cases (e.g. adding `final` to a class without public constructor is not a breaking change[3]). Regarding 1, I should probably document somewhere an unwritten policy I have been using to align the version numbers of packages, with the version number of artifacts. If we have a package with version `2.20.4` it means: * the last MINOR update (e.g. a new public method) occurred in Log4j 2.20.0, * the artifact had 4 MICRO updates since then (in any Log4j version, e.g. `2.21.0`, `2.22.1`, `2.23.0` and `2.24.1`. So, whenever BND asks you to upgrade the version of a package that currently has version `2.20.4`: * if it is a MICRO update, go with BND's recommendation (2.20.5), * if it is a MINOR update, BND will propose 2.21.0. Please ignore that recommendation and use the next Log4j version instead. Of course that version **must** have a patch number of 0, since a minor update occurred. * if it is a MAJOR update and you are **sure** that this does not classify as breaking change (e.g. you just removed a public class, that has always been documented as `private`), you can use the `@BaselineIgnore` annotation on the appropriate element (the one that changed or its parent if the element was removed). Piotr PS: Regarding annotations, can you take a look at issue #3110[4]. Users are regularly reporting issues for annotations with a `provided` scope. This is due to the `classfile` option to `-Xlint`, which basically only covers missing annotations (of both `CLASS` and `RUNTIME` retention) that occur at **compile time**. I would like to take a per-annotation approach here: some annotations like JSpecify or Error Prone's `@InlineMe` can be really useful to users, so we can: * add 3819 bytes of JSpecify dependency to the `compile` scope of all artifacts. In the near future nullability annotations will be all over the place and JSpecify (compared to the other 13 kinds of annotations there) have some strong supporters[5]. * keep the current _status quo_ for some annotations (e.g. `@InlineMe` and those OSGi versioning annotations that are not mirrored in the Manifest), * write a tool that removes the other annotations from the class files. [1] https://github.com/bndtools/bnd/tree/master/maven-plugins/bnd-baseline-maven-plugin [2] https://github.com/apache/logging-log4j2/issues/2449#issuecomment-2045801954 [3] https://github.com/bndtools/bnd/issues/5808 [4] https://github.com/apache/logging-log4j2/issues/3110 [5] https://jspecify.dev/about/
