Re: [log4j] What is JPMS support and its state

[Piotr P. Karwasz]

Hi Volkan,

On Mon, 6 Nov 2023 at 22:02, Volkan Yazıcı  wrote:
> OTOH, JPMS support in the `main` branch was first established by
> hand-written `module-info.java` files and then later on reimplemented (work
> in progress!) by Piotr via porting stuff from `2.x`. Nobody has tested it
> yet, AFAIK. `jlink` is known to not work there.

As a disclaimer, `jlink` fails on `log4j-core` version 3.0.0-alpha1 as
well as 3.0.0-SNAPSHOT.
The problems are deps, which are automatic modules (with or without an
`Automatic-Module-Name`).

In the case of version 3.x, there are three runtime dependencies that
are neither used nor declared as optional.
This shows once again that integration tests are necessary.

> Though `3.x` is good at two things regarding modularization:
>
>1. It breaks down `log4j-core` into more fine-grained modules (e.g.,
>`log4j-csv`)
>2. It has slightly improved package encapsulation. That is, some
>internal classes are indeed moved to internal packages.
>
> `2.x` can never implement #2, since it is a breaking change. Thought #1 can
> very well be implemented for `2.x` – users will just need to add a new JAR
> to their classpath.

Removing any class from an artifact is a **breaking** change, even if
we put it in another artifact without changing its name.
Besides, most of the "additional" features are not in a package of
their own and multiple modules can not share the same package.

I agree with Ralph: we don't need to backport everything from `main`
to `2.x` by bending the rules. If a single protected/public class or
method must disappear, let's do the right thing and don't backport the
change.
The tools like BND Baseline plugin use the most strict interpretation
of breaking change possible. We don't need to follow each alert
literally, but if we don't need to break the rules, let's choose the
safest path.

Piotr

Re: [log4j] What is JPMS support and its state

[Volkan Yazıcı]

On Tue, Nov 7, 2023 at 5:04 AM Ralph Goers 
wrote:

> You know, I almost didn’t want to answer this email because after reading
> the text it was quite obvious to me that the question you are really trying
> to ask is:
> “What more needs to be done to kill off 3.x?”  That being said I will
> still answer your question.
>

I am glad you took the effort to answer. We stand on the shoulders of your
(and others') work. Your insight is invaluable. Thanks for taking time to
share it.

I am trying to improve the JPMS support in `2.x`. If there are things we
can improve without breaking backward compatibility, I would like to know.

I don't want to kill `3.x` – I support a new major release that rid us of
the bloat.


> First, I will say that the work to create module-info files in 2.x
> wouldn’t have been possible without all the effort put in to figure out how
> to separate the tests from the code. You have absolutely no idea how many
> hours I put in trying various permutations and asking around.
>

I agree with the effort that you/others put into this. But that experience
is incorporated into both `3.x` and `2.x` too. Hence, I don't think this
counts as an argument.


> Second, providing a module-info.class does not, in and of itself, make a
> library JPMS compliant. That is a requirement for it to be a named module,
> but it is only one facet of JPMS.
>
> Ask yourself, why was JPMS created? To allow Oracle to keep its JDK
> developers busy? No. The JDK had become bloated and even a simple
> HelloWorld app required the whole thing. Likewise, for a library like Log4j
> to depend on anything besides java.base will force the application to be
> bloated.
> Consider this (decompiled by IntelliJ - I am not really sure why none of
> them are listed as optional).:
>
> requires com.fasterxml.jackson.annotation;
> requires com.fasterxml.jackson.core;
> ...
> requires org.osgi.core;
>
> With this:
> // Required Dependencies
> requires transitive org.apache.logging.log4j;
> requires transitive org.apache.logging.log4j.plugins;
> ...
> requires static org.fusesource.jansi;
>
> And even this list for 3.x is larger than we would like. I believe it has
> been identified how to eliminate all the java.* dependencies, but I would
> be surprised if even more of the non-java dependencies can be extracted.
> A lot of this was gained by splitting the modules in 3.x, which you
> admitted cannot be done in 2.x. Since that is one of the primary benefits
> of JPMS I am not sure how that makes 2.x better.
>

I am not able to follow you here. That is exactly what I said in my email:
`3.x` has better modularization *and* we can do the same for `2.x`. As a
matter of fact, we already had a tiny fake shot at it by moving
`log4j-core` test JAR to `log4j-core-test`, which was a success.

Exercising this "breaking down into more fine-grained modules" practice in
`2.x` will not only improve the JPMS quality we deliver in `2.x`, but also
immensely ease porting fixes between `2.x` and `3.x`, which is right now
impossible without a full merge conflict.


> Yes, we had to generate the module-info.java files by hand. Why? Because
> tooling didn’t exist. If it does now great1 But I am not sure why you are
> calling that out as if it is a negative thing. I mean, you still had to
> create the list of dependencies in the pom.xml by hand. What really is the
> difference?
>

Using `bnd-maven-plugin` to generate JPMS descriptors compared to writing
`module-info.java` by hand has following advantages:

   1. No `module-info.java` in the sources rendering IDEs unusable (I
   explained this in the first post.)
   2. We tweak `b-m-p` in `pom.xml` for *only* optional dependencies, the
   plugin figures out the rest itself. Hence, in an ideal `3.x` where there
   are no optional dependencies, no `b-m-p` tweaking should be necessary.
   3. Manually maintained `module-info.java` is prone to be broken when
   sources are moved between modules and classes. `b-m-p` captures this in
   `module-info.java` automatically.

Next, there is a good chance that users really using JPMS won’t be able to
> access any plugins outside of log4j-core. See
> https://docs.oracle.com/en/java/javase/11/docs/api/java.base/java/lang/ClassLoader.html#getResource(java.lang.String).
> This is precisely why 3.x uses ServiceLoader to locate all plugins.
>
> So, while you may have successfully create module-info.class files was
> that a good thing? As an automatic module the rules in getResource() don’t
> apply.
>
> Next, Java supports ModuleLayers. See
> https://stackoverflow.com/questions/61195909/what-is-the-relation-between-modulelayer-and-classloader.
> If you get nothing out of this other than “it is complicated” that is
> enough. The bottom line here is again that you can’t rely on
> ClassLoader.getResource() to find plugins.
>
> As you may or may not have noticed, annotation processors cannot be
> located on the module path and must be strictly declared. In 2.x the
> processor is always i

Re: [log4j] What is JPMS support and its state

[Volkan Yazıcı]

I need to do a correction to the following statement of mine:

> `3.x` has a better modularization and plugin system, both
> of which can be ported to `2.x` without breaking backward
> compatibility.

In his recent post Piotr is right: for backporting modularization, packages
need to change too and that is indeed unacceptable. This presses down my
request to only the backport of the plugin system.

On Tue, Nov 7, 2023 at 10:27 AM Volkan Yazıcı  wrote:

> On Tue, Nov 7, 2023 at 5:04 AM Ralph Goers 
> wrote:
>
>> You know, I almost didn’t want to answer this email because after reading
>> the text it was quite obvious to me that the question you are really trying
>> to ask is:
>> “What more needs to be done to kill off 3.x?”  That being said I will
>> still answer your question.
>>
>
> I am glad you took the effort to answer. We stand on the shoulders of your
> (and others') work. Your insight is invaluable. Thanks for taking time to
> share it.
>
> I am trying to improve the JPMS support in `2.x`. If there are things we
> can improve without breaking backward compatibility, I would like to know.
>
> I don't want to kill `3.x` – I support a new major release that rid us of
> the bloat.
>
>
>> First, I will say that the work to create module-info files in 2.x
>> wouldn’t have been possible without all the effort put in to figure out how
>> to separate the tests from the code. You have absolutely no idea how many
>> hours I put in trying various permutations and asking around.
>>
>
> I agree with the effort that you/others put into this. But that experience
> is incorporated into both `3.x` and `2.x` too. Hence, I don't think this
> counts as an argument.
>
>
>> Second, providing a module-info.class does not, in and of itself, make a
>> library JPMS compliant. That is a requirement for it to be a named module,
>> but it is only one facet of JPMS.
>>
>> Ask yourself, why was JPMS created? To allow Oracle to keep its JDK
>> developers busy? No. The JDK had become bloated and even a simple
>> HelloWorld app required the whole thing. Likewise, for a library like Log4j
>> to depend on anything besides java.base will force the application to be
>> bloated.
>> Consider this (decompiled by IntelliJ - I am not really sure why none of
>> them are listed as optional).:
>>
>> requires com.fasterxml.jackson.annotation;
>> requires com.fasterxml.jackson.core;
>> ...
>> requires org.osgi.core;
>>
>> With this:
>> // Required Dependencies
>> requires transitive org.apache.logging.log4j;
>> requires transitive org.apache.logging.log4j.plugins;
>> ...
>> requires static org.fusesource.jansi;
>>
>> And even this list for 3.x is larger than we would like. I believe it has
>> been identified how to eliminate all the java.* dependencies, but I would
>> be surprised if even more of the non-java dependencies can be extracted.
>> A lot of this was gained by splitting the modules in 3.x, which you
>> admitted cannot be done in 2.x. Since that is one of the primary benefits
>> of JPMS I am not sure how that makes 2.x better.
>>
>
> I am not able to follow you here. That is exactly what I said in my email:
> `3.x` has better modularization *and* we can do the same for `2.x`. As a
> matter of fact, we already had a tiny fake shot at it by moving
> `log4j-core` test JAR to `log4j-core-test`, which was a success.
>
> Exercising this "breaking down into more fine-grained modules" practice in
> `2.x` will not only improve the JPMS quality we deliver in `2.x`, but also
> immensely ease porting fixes between `2.x` and `3.x`, which is right now
> impossible without a full merge conflict.
>
>
>> Yes, we had to generate the module-info.java files by hand. Why? Because
>> tooling didn’t exist. If it does now great1 But I am not sure why you are
>> calling that out as if it is a negative thing. I mean, you still had to
>> create the list of dependencies in the pom.xml by hand. What really is the
>> difference?
>>
>
> Using `bnd-maven-plugin` to generate JPMS descriptors compared to writing
> `module-info.java` by hand has following advantages:
>
>1. No `module-info.java` in the sources rendering IDEs unusable (I
>explained this in the first post.)
>2. We tweak `b-m-p` in `pom.xml` for *only* optional dependencies, the
>plugin figures out the rest itself. Hence, in an ideal `3.x` where there
>are no optional dependencies, no `b-m-p` tweaking should be necessary.
>3. Manually maintained `module-info.java` is prone to be broken when
>sources are moved between modules and classes. `b-m-p` captures this in
>`module-info.java` automatically.
>
> Next, there is a good chance that users really using JPMS won’t be able to
>> access any plugins outside of log4j-core. See
>> https://docs.oracle.com/en/java/javase/11/docs/api/java.base/java/lang/ClassLoader.html#getResource(java.lang.String).
>> This is precisely why 3.x uses ServiceLoader to locate all plugins.
>>
>> So, while you may have successfully create module-info.class f

Re: [log4j] What is JPMS support and its state

[Piotr P. Karwasz]

Hi Ralph,

On Tue, 7 Nov 2023 at 05:05, Ralph Goers  wrote:
> Next, there is a good chance that users really using JPMS won’t be able to 
> access any plugins outside of log4j-core. See 
> https://docs.oracle.com/en/java/javase/11/docs/api/java.base/java/lang/ClassLoader.html#getResource(java.lang.String).
>  This is precisely why 3.x uses ServiceLoader to locate all plugins.

The `META-INF` directory is not encapsulated, so `getResource` should
work fine. Nevertheless using `ServiceLoader` is a much cleaner
solution and would allow us to greatly simplify our OSGi-specific
code.

Piotr

Re: [log4j] What is JPMS support and its state

[Matt Sicker]

I’m not going to backport the DI system. It relies on Java 11 in all sorts of 
random places, first of all. I had to update numerous plugins and tests along 
the way, especially things that set up or manipulate static state, much of 
which has been cleaned up in main. It’s hard to describe all the little things 
that went into that.
—
Matt Sicker

> On Nov 7, 2023, at 05:31, Piotr P. Karwasz  wrote:
> 
> Hi Ralph,
> 
>> On Tue, 7 Nov 2023 at 05:05, Ralph Goers  wrote:
>> Next, there is a good chance that users really using JPMS won’t be able to 
>> access any plugins outside of log4j-core. See 
>> https://docs.oracle.com/en/java/javase/11/docs/api/java.base/java/lang/ClassLoader.html#getResource(java.lang.String).
>>  This is precisely why 3.x uses ServiceLoader to locate all plugins.
> 
> The `META-INF` directory is not encapsulated, so `getResource` should
> work fine. Nevertheless using `ServiceLoader` is a much cleaner
> solution and would allow us to greatly simplify our OSGi-specific
> code.
> 
> Piotr

Re: Deterministic formatter

[Matt Sicker]

In the worst case scenario, we can still format from maven before committing 
(which is what I used to do before finding that there were IntelliJ plugins for 
this). In fact, I have to do that all the time lately anyways by running `mvn 
spotless:apply`.

> On Nov 6, 2023, at 9:00 AM, Carter Kozak  wrote:
> 
> I'd be happy to review+release changes to get the eclipse plugin in that repo 
> into a good place as long as it doesn't make the build process a great deal 
> more complicated. We don't have many folks internally using eclipse so 
> support hasn't been a priority, but the easier it is to use across common 
> toolchains, the better!
> 
> -ck
> 
> On Mon, Nov 6, 2023, at 06:55, Piotr P. Karwasz wrote:
>> Hi Gary,
>> 
>> On Mon, 6 Nov 2023 at 11:45, Gary Gregory  wrote:
>>> 
>>> Well, I use Eclipse, so... I won't be using whatever this does or when it
>>> does it.
>> 
>> What version of Eclipse do you use? The Eclipse plugin is one class, I
>> can probably fix it, compile it and release it.
>> 
>> Piotr
>> 

Re: [log4j] Security page refactoring

[Matt Sicker]

Not that it’s relevant to adopt right away, but do note that there is a CVSS 
4.0 now (that was finished quite recently) which is supposed to produce more 
useful severity scores.

> On Nov 6, 2023, at 2:17 PM, Volkan Yazıcı  wrote:
> 
> I have created #1948 
> refactoring the Log4j security page. See the PR description for details.
> Reviews are welcome.

Re: [log4j] Security page refactoring

[Matt Sicker]

Also note that I’m at KubeCon right now, so I won’t have a chance to review 
this PR until, well, sometime during the conference. Possibly today, but likely 
within the next day or two.

> On Nov 6, 2023, at 2:17 PM, Volkan Yazıcı  wrote:
> 
> I have created #1948 
> refactoring the Log4j security page. See the PR description for details.
> Reviews are welcome.

Re: [log4j] Security page refactoring

[Matt Sicker]

Actually, I was able to review this just now during the lunch break.

> On Nov 7, 2023, at 1:24 PM, Matt Sicker  wrote:
> 
> Also note that I’m at KubeCon right now, so I won’t have a chance to review 
> this PR until, well, sometime during the conference. Possibly today, but 
> likely within the next day or two.
> 
>> On Nov 6, 2023, at 2:17 PM, Volkan Yazıcı  wrote:
>> 
>> I have created #1948 
>> refactoring the Log4j security page. See the PR description for details.
>> Reviews are welcome.
>