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/

Reply via email to