Re: Spring 3 and Log4j 3

[Volkan Yazıcı]

Thanks for chasing this Ralph, it is indeed a nice step forwards.

The aforementioned Log4j issue is fixed in #2062
:

   1. You submitted the PR on Dec 5 at 04:15 (GMT+1) and merged it at
   05:58. Next time, would you mind giving us a couple of days for the review,
   please?
   2. Piotr and I both reviewed the changes and we had remarks. None of
   them have been addressed so far. Would you mind responding to them, please?
   (See the PR for the comments.)
   3. Could you backport the fix to `2.x` branch too, please?


On Tue, Jan 9, 2024 at 10:45 PM Ralph Goers 
wrote:

> FYI - in
> https://github.com/spring-projects/spring-boot/issues/33450#issuecomment-1883014368
> has confirmed that Log4j 3.0.0-beta1 now works correctly with Spring 3.x.
>
> Ralph

Re: Spring 3 and Log4j 3

[Piotr P. Karwasz]

Hi Ralph,

On Tue, 9 Jan 2024 at 22:45, Ralph Goers  wrote:
>
> FYI - in 
> https://github.com/spring-projects/spring-boot/issues/33450#issuecomment-1883014368
>  has confirmed that Log4j 3.0.0-beta1 now works correctly with Spring 3.x.

Are you planning to backport that patch or is it specific to 3.x?

Piotr

Re: Spring 3 and Log4j 3

[Matt Sicker]

This might affect 2.x, though it’s largely in the Spring environment property 
source. Given the application lifecycle there, Spring doesn’t seem to remove 
its own closed property sources at shutdown, hence the exception. The issue was 
only reported against Spring 3, though.

> On Jan 10, 2024, at 4:04 AM, Piotr P. Karwasz  wrote:
> 
> Hi Ralph,
> 
> On Tue, 9 Jan 2024 at 22:45, Ralph Goers  wrote:
>> 
>> FYI - in 
>> https://github.com/spring-projects/spring-boot/issues/33450#issuecomment-1883014368
>>  has confirmed that Log4j 3.0.0-beta1 now works correctly with Spring 3.x.
> 
> Are you planning to backport that patch or is it specific to 3.x?
> 
> Piotr

Re: Spring 3 and Log4j 3

[Piotr P. Karwasz]

Hi Matt,

On Wed, 10 Jan 2024 at 18:45, Matt Sicker  wrote:
>
> This might affect 2.x, though it’s largely in the Spring environment property 
> source. Given the application lifecycle there, Spring doesn’t seem to remove 
> its own closed property sources at shutdown, hence the exception. The issue 
> was only reported against Spring 3, though.

A lot of 2.x and 3.x here. ;-)
What I meant is the patch should be ported to Log4j 2.x, so it can
work with Spring 3.x. After all Spring is currently using the 2.x
branch of Log4j.

Piotr

Re: Spring 3 and Log4j 3

[Matt Sicker]

Oh, in that case, assuming it’s a bug in log4j 2.x as well, then yes, seems 
worth fixing.

> On Jan 10, 2024, at 12:29 PM, Piotr P. Karwasz  
> wrote:
> 
> Hi Matt,
> 
> On Wed, 10 Jan 2024 at 18:45, Matt Sicker  wrote:
>> 
>> This might affect 2.x, though it’s largely in the Spring environment 
>> property source. Given the application lifecycle there, Spring doesn’t seem 
>> to remove its own closed property sources at shutdown, hence the exception. 
>> The issue was only reported against Spring 3, though.
> 
> A lot of 2.x and 3.x here. ;-)
> What I meant is the patch should be ported to Log4j 2.x, so it can
> work with Spring 3.x. After all Spring is currently using the 2.x
> branch of Log4j.
> 
> Piotr

Removal of `log4j-jcl` in `3.x`

[Piotr P. Karwasz]

Hi all,

Since `commons-logging` 1.3.0 was published last month and it supports
Log4j API out-of-the-box, does it still make sense to keep `log4j-jcl`
around in 3.x?

I would keep the 2.x version, so that users don't need to modify their
stacks. However we might assume that 3.x adopters are already running
the latest versions of the remaining dependencies, specifically they
upgraded `commons-logging`. This is why I would propose to remove
`log4j-jcl` from 3.x.

Remark that Spring users have been using `spring-jcl` for a very long
time (which also supports the Log4j API), so they didn't need
`log4j-jcl` either.

Piotr

Re: Spring 3 and Log4j 3

[Ralph Goers]

> On Jan 10, 2024, at 11:29 AM, Piotr P. Karwasz  
> wrote:
> 
> Hi Matt,
> 
> On Wed, 10 Jan 2024 at 18:45, Matt Sicker  wrote:
>> 
>> This might affect 2.x, though it’s largely in the Spring environment 
>> property source. Given the application lifecycle there, Spring doesn’t seem 
>> to remove its own closed property sources at shutdown, hence the exception. 
>> The issue was only reported against Spring 3, though.
> 
> A lot of 2.x and 3.x here. ;-)
> What I meant is the patch should be ported to Log4j 2.x, so it can
> work with Spring 3.x. After all Spring is currently using the 2.x
> branch of Log4j.
> 

I fixed two bugs. One was reported against 2.x ages ago. The second was first 
reported in the Spring issue. 

Yes, they can be ported to 2.x but if we release 3.0.0 in the next couple of 
months I would prefer to tell users just to upgrade since 3.x specifically 
targets the environment Spring 3.x uses.

Ralph

Re: Removal of `log4j-jcl` in `3.x`

[Ralph Goers]

Fine by me.

Ralph

> On Jan 10, 2024, at 3:11 PM, Piotr P. Karwasz  wrote:
> 
> Hi all,
> 
> Since `commons-logging` 1.3.0 was published last month and it supports
> Log4j API out-of-the-box, does it still make sense to keep `log4j-jcl`
> around in 3.x?
> 
> I would keep the 2.x version, so that users don't need to modify their
> stacks. However we might assume that 3.x adopters are already running
> the latest versions of the remaining dependencies, specifically they
> upgraded `commons-logging`. This is why I would propose to remove
> `log4j-jcl` from 3.x.
> 
> Remark that Spring users have been using `spring-jcl` for a very long
> time (which also supports the Log4j API), so they didn't need
> `log4j-jcl` either.
> 
> Piotr

Re: Spring 3 and Log4j 3

[Piotr P. Karwasz]

Hi Ralph,

On Thu, 11 Jan 2024 at 01:38, Ralph Goers  wrote:
> I fixed two bugs. One was reported against 2.x ages ago. The second was first 
> reported in the Spring issue.
>
> Yes, they can be ported to 2.x but if we release 3.0.0 in the next couple of 
> months I would prefer to tell users just to upgrade since 3.x specifically 
> targets the environment Spring 3.x uses.

Both solutions are fine with me.

I looked at the current Spring Boot code and the new methods you
introduced with a comment "Used in Spring Boot" are not used there
yet. Is it something you are currently working on?

If you introduce them, we'll need to make a choice:

 * either Spring Boot 3.3.x will support only Log4j 3.x,
 * or we backport all changes and only use Java types and methods that
are present in both releases.

The custom Spring Boot configuration file detection logic[2] already
fails with Log4j Core 3.x (except JSON and XML, the remaining config
formats require an additional Log4j dependency), so if they decide to
switch to Log4j 3.x in Spring Boot 3.3.x, we can double-check that the
Log4j types they use ([1]) are really something we consider a public
API.

Piotr

[1] 
https://github.com/spring-projects/spring-boot/blob/5b6e3fab2cb7355d1053c71d0b4411687f463aaa/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/logging/log4j2/Log4J2LoggingSystem.java#L32-L53
[2] 
https://github.com/spring-projects/spring-boot/blob/5b6e3fab2cb7355d1053c71d0b4411687f463aaa/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/logging/log4j2/Log4J2LoggingSystem.java#L137-L162