LOG4J2-2624 and LOG4J2-1606

[Tim Perry]

Hello,

I'd like to help fix LOG4J2-2624 and LOG4J2-1606. How can I help?

To me, the challenge is to ensure log4j is initialized the very first time
the ServletContext is provided to any object during application loading and
startup and to stop log4j during the very last event or execution hook a
servlet 3.0 container exposes. Right now using the servlet 3.0
auto-configuration stops log4j too soon in some cases and using the servlet
2.5 configuration starts log4j too late in some cases.

FWIW, I have posted a proposed fix in
https://issues.apache.org/jira/browse/LOG4J2-1606. I'm not sure if it is
the correct way to go. For one thing, it puts the Log4jWebLifeCycle
initializer into the ServletContext so that another object can grab it and
use it during log4j shutdown. Somewhere in the log4j dev archives I saw a
note about moving data out of the ServletContext so that it can't be
overwritten. I'm not sure if my solution would need to be modified or
abandoned in light of this.

The code changes I posted are based on a custom log4j-web artifact I
created for a client. It works for them on their Tomcat 8.x servers.
However, I'm not sure if I'm relying on any idiosyncratic behaviour of
Tomcat or if there are earlier or later servlet container events / hooks
that can be used to trigger configuration to happen earlier on startup or
stop log4j later when an application is stopped.

If I can be of any help fixing these issues, I'd like to help.

I've gotten a lot of good use out of log4j over the years. Thank you for
maintaining it.

Tim

Re: LOG4J2-2624 and LOG4J2-1606

[Tim Perry]

Matt, et al.,

I agree the deployment patterns you mention are more common and I wouldn't
start a new project embedding log4j in each WAR. However, I'm trying to
upgrade some old spring apps and my hands are tied on the deployment
pattern.

As mentioned in my comment on LOG4J2- 2624, the changes I proposed don't
fundamentally change the lifecycle hooks for web modules and each class
loader will still have its own independent log4j config. The changes just
provide the ability to stop log4j a little later. To me, this is a low risk
change since the default behaviour is unchanged. If my approach of passing
the Log4jWebLifeCycle around in the ServletContext is unacceptable, I'm
happy to revisit the code and come up with another solution. Here are the
proposed changes:
https://github.com/perry2of5/logging-log4j2/commit/56455af53920d69ff7a49a63c5bbf38773069e8d

I'd really like to fix these bugs. If you are telling me there are more
important things for the log4j team to work on and that there is no
interest from the log4j committers to make these changes, I can accept
that. However, I think these changes would be welcomed by some log4j users
and I hope one of the log4j committers will work with me on solving these
issues.

Thanks,
Tim




On Sun, Jan 24, 2021 at 6:29 PM Matt Sicker  wrote:

> I'm not sure how much any of the devs here use the log4j-web module
> anymore (seems more common to use fat jars or one app per servlet
> container instance at least), so it's hard to say about any
> idiosyncrasies. The main purpose of the lifecycle hooks for web
> modules is to allow each class loader to have its own independent
> log4j config, though I'm not sure how common that deployment pattern
> is anymore. There are alternative strategies such as hooking into the
> server code itself so that logging can shutdown with the server rather
> than the individual applications, but that's a different use case.
>
> As for design ideas, I think I had initially wanted to refactor the
> web context API to mimic how Spring Framework registers itself in the
> ServletContext, though I never got around to doing that, and now I
> typically use JVM-global logging configurations instead, so I never
> revisited that.
>
> On Fri, 22 Jan 2021 at 11:53, Tim Perry  wrote:
> >
> > Hello,
> >
> > I'd like to help fix LOG4J2-2624 and LOG4J2-1606. How can I help?
> >
> > To me, the challenge is to ensure log4j is initialized the very first
> time
> > the ServletContext is provided to any object during application loading
> and
> > startup and to stop log4j during the very last event or execution hook a
> > servlet 3.0 container exposes. Right now using the servlet 3.0
> > auto-configuration stops log4j too soon in some cases and using the
> servlet
> > 2.5 configuration starts log4j too late in some cases.
> >
> > FWIW, I have posted a proposed fix in
> > https://issues.apache.org/jira/browse/LOG4J2-1606. I'm not sure if it is
> > the correct way to go. For one thing, it puts the Log4jWebLifeCycle
> > initializer into the ServletContext so that another object can grab it
> and
> > use it during log4j shutdown. Somewhere in the log4j dev archives I saw a
> > note about moving data out of the ServletContext so that it can't be
> > overwritten. I'm not sure if my solution would need to be modified or
> > abandoned in light of this.
> >
> > The code changes I posted are based on a custom log4j-web artifact I
> > created for a client. It works for them on their Tomcat 8.x servers.
> > However, I'm not sure if I'm relying on any idiosyncratic behaviour of
> > Tomcat or if there are earlier or later servlet container events / hooks
> > that can be used to trigger configuration to happen earlier on startup or
> > stop log4j later when an application is stopped.
> >
> > If I can be of any help fixing these issues, I'd like to help.
> >
> > I've gotten a lot of good use out of log4j over the years. Thank you for
> > maintaining it.
> >
> > Tim
>

Re: LOG4J2-2624 and LOG4J2-1606

[Tim Perry]

Matt,

Thanks for clarifying.

I'd be happy to write some tests and submit a PR. How should I submit a
pull request? I don't think I can do it from the github repo I linked to.

Thanks,
Tim


On Mon, Jan 25, 2021 at 9:59 AM Matt Sicker  wrote:

> I'm just saying that I don't think any of the developers here would
> object to functional changes you'd like to introduce here, especially
> if you think this change makes sense for users other than yourself.
>
> If you submit your changes as a PR (and preferably add automated tests
> if possible), we'd be happy to merge!
>
> On Mon, 25 Jan 2021 at 11:51, Tim Perry  wrote:
> >
> > Matt, et al.,
> >
> > I agree the deployment patterns you mention are more common and I
> wouldn't
> > start a new project embedding log4j in each WAR. However, I'm trying to
> > upgrade some old spring apps and my hands are tied on the deployment
> > pattern.
> >
> > As mentioned in my comment on LOG4J2- 2624, the changes I proposed don't
> > fundamentally change the lifecycle hooks for web modules and each class
> > loader will still have its own independent log4j config. The changes just
> > provide the ability to stop log4j a little later. To me, this is a low
> risk
> > change since the default behaviour is unchanged. If my approach of
> passing
> > the Log4jWebLifeCycle around in the ServletContext is unacceptable, I'm
> > happy to revisit the code and come up with another solution. Here are the
> > proposed changes:
> >
> https://github.com/perry2of5/logging-log4j2/commit/56455af53920d69ff7a49a63c5bbf38773069e8d
> >
> > I'd really like to fix these bugs. If you are telling me there are more
> > important things for the log4j team to work on and that there is no
> > interest from the log4j committers to make these changes, I can accept
> > that. However, I think these changes would be welcomed by some log4j
> users
> > and I hope one of the log4j committers will work with me on solving these
> > issues.
> >
> > Thanks,
> > Tim
> >
> >
> >
> >
> > On Sun, Jan 24, 2021 at 6:29 PM Matt Sicker  wrote:
> >
> > > I'm not sure how much any of the devs here use the log4j-web module
> > > anymore (seems more common to use fat jars or one app per servlet
> > > container instance at least), so it's hard to say about any
> > > idiosyncrasies. The main purpose of the lifecycle hooks for web
> > > modules is to allow each class loader to have its own independent
> > > log4j config, though I'm not sure how common that deployment pattern
> > > is anymore. There are alternative strategies such as hooking into the
> > > server code itself so that logging can shutdown with the server rather
> > > than the individual applications, but that's a different use case.
> > >
> > > As for design ideas, I think I had initially wanted to refactor the
> > > web context API to mimic how Spring Framework registers itself in the
> > > ServletContext, though I never got around to doing that, and now I
> > > typically use JVM-global logging configurations instead, so I never
> > > revisited that.
> > >
> > > On Fri, 22 Jan 2021 at 11:53, Tim Perry  wrote:
> > > >
> > > > Hello,
> > > >
> > > > I'd like to help fix LOG4J2-2624 and LOG4J2-1606. How can I help?
> > > >
> > > > To me, the challenge is to ensure log4j is initialized the very first
> > > time
> > > > the ServletContext is provided to any object during application
> loading
> > > and
> > > > startup and to stop log4j during the very last event or execution
> hook a
> > > > servlet 3.0 container exposes. Right now using the servlet 3.0
> > > > auto-configuration stops log4j too soon in some cases and using the
> > > servlet
> > > > 2.5 configuration starts log4j too late in some cases.
> > > >
> > > > FWIW, I have posted a proposed fix in
> > > > https://issues.apache.org/jira/browse/LOG4J2-1606. I'm not sure if
> it is
> > > > the correct way to go. For one thing, it puts the Log4jWebLifeCycle
> > > > initializer into the ServletContext so that another object can grab
> it
> > > and
> > > > use it during log4j shutdown. Somewhere in the log4j dev archives I
> saw a
> > > > note about moving data out of the ServletContext so that it can't be
> > > > overwritten. I'm not sure if my solution would need to be modified or
> > > > abandoned in light of this.
> > > >
> > > > The code changes I posted are based on a custom log4j-web artifact I
> > > > created for a client. It works for them on their Tomcat 8.x servers.
> > > > However, I'm not sure if I'm relying on any idiosyncratic behaviour
> of
> > > > Tomcat or if there are earlier or later servlet container events /
> hooks
> > > > that can be used to trigger configuration to happen earlier on
> startup or
> > > > stop log4j later when an application is stopped.
> > > >
> > > > If I can be of any help fixing these issues, I'd like to help.
> > > >
> > > > I've gotten a lot of good use out of log4j over the years. Thank you
> for
> > > > maintaining it.
> > > >
> > > > Tim
> > >
>

Re: LOG4J2-2624 and LOG4J2-1606

[Tim Perry]

Thank you, Matt.

I'll get back to you after I've written some unit tests.

Tim

On Mon, Jan 25, 2021 at 10:37 AM Matt Sicker  wrote:

> Go to https://github.com/apache/logging-log4j2/compare and click the
> "compare across forks" link at the top to make a PR from your forked
> repo.
>
> On Mon, 25 Jan 2021 at 12:15, Tim Perry  wrote:
> >
> > Matt,
> >
> > Thanks for clarifying.
> >
> > I'd be happy to write some tests and submit a PR. How should I submit a
> > pull request? I don't think I can do it from the github repo I linked to.
> >
> > Thanks,
> > Tim
> >
> >
> > On Mon, Jan 25, 2021 at 9:59 AM Matt Sicker  wrote:
> >
> > > I'm just saying that I don't think any of the developers here would
> > > object to functional changes you'd like to introduce here, especially
> > > if you think this change makes sense for users other than yourself.
> > >
> > > If you submit your changes as a PR (and preferably add automated tests
> > > if possible), we'd be happy to merge!
> > >
> > > On Mon, 25 Jan 2021 at 11:51, Tim Perry  wrote:
> > > >
> > > > Matt, et al.,
> > > >
> > > > I agree the deployment patterns you mention are more common and I
> > > wouldn't
> > > > start a new project embedding log4j in each WAR. However, I'm trying
> to
> > > > upgrade some old spring apps and my hands are tied on the deployment
> > > > pattern.
> > > >
> > > > As mentioned in my comment on LOG4J2- 2624, the changes I proposed
> don't
> > > > fundamentally change the lifecycle hooks for web modules and each
> class
> > > > loader will still have its own independent log4j config. The changes
> just
> > > > provide the ability to stop log4j a little later. To me, this is a
> low
> > > risk
> > > > change since the default behaviour is unchanged. If my approach of
> > > passing
> > > > the Log4jWebLifeCycle around in the ServletContext is unacceptable,
> I'm
> > > > happy to revisit the code and come up with another solution. Here
> are the
> > > > proposed changes:
> > > >
> > >
> https://github.com/perry2of5/logging-log4j2/commit/56455af53920d69ff7a49a63c5bbf38773069e8d
> > > >
> > > > I'd really like to fix these bugs. If you are telling me there are
> more
> > > > important things for the log4j team to work on and that there is no
> > > > interest from the log4j committers to make these changes, I can
> accept
> > > > that. However, I think these changes would be welcomed by some log4j
> > > users
> > > > and I hope one of the log4j committers will work with me on solving
> these
> > > > issues.
> > > >
> > > > Thanks,
> > > > Tim
> > > >
> > > >
> > > >
> > > >
> > > > On Sun, Jan 24, 2021 at 6:29 PM Matt Sicker 
> wrote:
> > > >
> > > > > I'm not sure how much any of the devs here use the log4j-web module
> > > > > anymore (seems more common to use fat jars or one app per servlet
> > > > > container instance at least), so it's hard to say about any
> > > > > idiosyncrasies. The main purpose of the lifecycle hooks for web
> > > > > modules is to allow each class loader to have its own independent
> > > > > log4j config, though I'm not sure how common that deployment
> pattern
> > > > > is anymore. There are alternative strategies such as hooking into
> the
> > > > > server code itself so that logging can shutdown with the server
> rather
> > > > > than the individual applications, but that's a different use case.
> > > > >
> > > > > As for design ideas, I think I had initially wanted to refactor the
> > > > > web context API to mimic how Spring Framework registers itself in
> the
> > > > > ServletContext, though I never got around to doing that, and now I
> > > > > typically use JVM-global logging configurations instead, so I never
> > > > > revisited that.
> > > > >
> > > > > On Fri, 22 Jan 2021 at 11:53, Tim Perry 
> wrote:
> > > > > >
> > > > > > Hello,
> > > > > >
> > > > > > I'd like to help fix LOG4J2-2624 and LOG4J2-1606. How can I help?
> >

Re: LOG4J2-2624 and LOG4J2-1606

[Tim Perry]

Is it okay to import hamcrest to use in the tests?


org.hamcrest
hamcrest
2.2
test



On Mon, Jan 25, 2021 at 10:44 AM Tim Perry  wrote:

> Thank you, Matt.
>
> I'll get back to you after I've written some unit tests.
>
> Tim
>
> On Mon, Jan 25, 2021 at 10:37 AM Matt Sicker  wrote:
>
>> Go to https://github.com/apache/logging-log4j2/compare and click the
>> "compare across forks" link at the top to make a PR from your forked
>> repo.
>>
>> On Mon, 25 Jan 2021 at 12:15, Tim Perry  wrote:
>> >
>> > Matt,
>> >
>> > Thanks for clarifying.
>> >
>> > I'd be happy to write some tests and submit a PR. How should I submit a
>> > pull request? I don't think I can do it from the github repo I linked
>> to.
>> >
>> > Thanks,
>> > Tim
>> >
>> >
>> > On Mon, Jan 25, 2021 at 9:59 AM Matt Sicker  wrote:
>> >
>> > > I'm just saying that I don't think any of the developers here would
>> > > object to functional changes you'd like to introduce here, especially
>> > > if you think this change makes sense for users other than yourself.
>> > >
>> > > If you submit your changes as a PR (and preferably add automated tests
>> > > if possible), we'd be happy to merge!
>> > >
>> > > On Mon, 25 Jan 2021 at 11:51, Tim Perry  wrote:
>> > > >
>> > > > Matt, et al.,
>> > > >
>> > > > I agree the deployment patterns you mention are more common and I
>> > > wouldn't
>> > > > start a new project embedding log4j in each WAR. However, I'm
>> trying to
>> > > > upgrade some old spring apps and my hands are tied on the deployment
>> > > > pattern.
>> > > >
>> > > > As mentioned in my comment on LOG4J2- 2624, the changes I proposed
>> don't
>> > > > fundamentally change the lifecycle hooks for web modules and each
>> class
>> > > > loader will still have its own independent log4j config. The
>> changes just
>> > > > provide the ability to stop log4j a little later. To me, this is a
>> low
>> > > risk
>> > > > change since the default behaviour is unchanged. If my approach of
>> > > passing
>> > > > the Log4jWebLifeCycle around in the ServletContext is unacceptable,
>> I'm
>> > > > happy to revisit the code and come up with another solution. Here
>> are the
>> > > > proposed changes:
>> > > >
>> > >
>> https://github.com/perry2of5/logging-log4j2/commit/56455af53920d69ff7a49a63c5bbf38773069e8d
>> > > >
>> > > > I'd really like to fix these bugs. If you are telling me there are
>> more
>> > > > important things for the log4j team to work on and that there is no
>> > > > interest from the log4j committers to make these changes, I can
>> accept
>> > > > that. However, I think these changes would be welcomed by some log4j
>> > > users
>> > > > and I hope one of the log4j committers will work with me on solving
>> these
>> > > > issues.
>> > > >
>> > > > Thanks,
>> > > > Tim
>> > > >
>> > > >
>> > > >
>> > > >
>> > > > On Sun, Jan 24, 2021 at 6:29 PM Matt Sicker 
>> wrote:
>> > > >
>> > > > > I'm not sure how much any of the devs here use the log4j-web
>> module
>> > > > > anymore (seems more common to use fat jars or one app per servlet
>> > > > > container instance at least), so it's hard to say about any
>> > > > > idiosyncrasies. The main purpose of the lifecycle hooks for web
>> > > > > modules is to allow each class loader to have its own independent
>> > > > > log4j config, though I'm not sure how common that deployment
>> pattern
>> > > > > is anymore. There are alternative strategies such as hooking into
>> the
>> > > > > server code itself so that logging can shutdown with the server
>> rather
>> > > > > than the individual applications, but that's a different use case.
>> > > > >
>> > > > > As for design ideas, I think I had initially wanted to refactor
>> the
>> > > > > web context API to 

Re: LOG4J2-2624 and LOG4J2-1606

[Tim Perry]

Yes, junit pulls in an old version of hamcrest. There are some nice
goodies in the newer hamcrest, but I just stuck with the existing
dependencies. I did upgrade everything to JUnit 5.

What is the best branch to base my pull request on? I'm going to
re-apply the changes to get a clean history: one commit for the
testing upgrades and a second commit for substantive changes.

Thanks,
Tim

On Mon, Jan 25, 2021 at 1:15 PM Matt Sicker  wrote:

> I believe that's already included. There's also AssertJ which might be
> a dependency. No strong preferences from me, though; just try to use
> the JUnit 5 API preferably as not everything has been migrated yet! :)
>
> On Mon, 25 Jan 2021 at 15:06, Tim Perry  wrote:
> >
> > Is it okay to import hamcrest to use in the tests?
> >
> > 
> > org.hamcrest
> > hamcrest
> >     2.2
> > test
> > 
> >
> >
> > On Mon, Jan 25, 2021 at 10:44 AM Tim Perry  wrote:
> >
> > > Thank you, Matt.
> > >
> > > I'll get back to you after I've written some unit tests.
> > >
> > > Tim
> > >
> > > On Mon, Jan 25, 2021 at 10:37 AM Matt Sicker  wrote:
> > >
> > >> Go to https://github.com/apache/logging-log4j2/compare and click the
> > >> "compare across forks" link at the top to make a PR from your forked
> > >> repo.
> > >>
> > >> On Mon, 25 Jan 2021 at 12:15, Tim Perry  wrote:
> > >> >
> > >> > Matt,
> > >> >
> > >> > Thanks for clarifying.
> > >> >
> > >> > I'd be happy to write some tests and submit a PR. How should I
> submit a
> > >> > pull request? I don't think I can do it from the github repo I
> linked
> > >> to.
> > >> >
> > >> > Thanks,
> > >> > Tim
> > >> >
> > >> >
> > >> > On Mon, Jan 25, 2021 at 9:59 AM Matt Sicker 
> wrote:
> > >> >
> > >> > > I'm just saying that I don't think any of the developers here
> would
> > >> > > object to functional changes you'd like to introduce here,
> especially
> > >> > > if you think this change makes sense for users other than
> yourself.
> > >> > >
> > >> > > If you submit your changes as a PR (and preferably add automated
> tests
> > >> > > if possible), we'd be happy to merge!
> > >> > >
> > >> > > On Mon, 25 Jan 2021 at 11:51, Tim Perry 
> wrote:
> > >> > > >
> > >> > > > Matt, et al.,
> > >> > > >
> > >> > > > I agree the deployment patterns you mention are more common and
> I
> > >> > > wouldn't
> > >> > > > start a new project embedding log4j in each WAR. However, I'm
> > >> trying to
> > >> > > > upgrade some old spring apps and my hands are tied on the
> deployment
> > >> > > > pattern.
> > >> > > >
> > >> > > > As mentioned in my comment on LOG4J2- 2624, the changes I
> proposed
> > >> don't
> > >> > > > fundamentally change the lifecycle hooks for web modules and
> each
> > >> class
> > >> > > > loader will still have its own independent log4j config. The
> > >> changes just
> > >> > > > provide the ability to stop log4j a little later. To me, this
> is a
> > >> low
> > >> > > risk
> > >> > > > change since the default behaviour is unchanged. If my approach
> of
> > >> > > passing
> > >> > > > the Log4jWebLifeCycle around in the ServletContext is
> unacceptable,
> > >> I'm
> > >> > > > happy to revisit the code and come up with another solution.
> Here
> > >> are the
> > >> > > > proposed changes:
> > >> > > >
> > >> > >
> > >>
> https://github.com/perry2of5/logging-log4j2/commit/56455af53920d69ff7a49a63c5bbf38773069e8d
> > >> > > >
> > >> > > > I'd really like to fix these bugs. If you are telling me there
> are
> > >> more
> > >> > > > important things for the log4j team to work on and that there
> is no
> > >> > > > interest from the log4j committers to make these changes, I can
>

Re: LOG4J2-2624 and LOG4J2-1606

[Tim Perry]

Okay, I've fully updated the code to use JUnit 5.

I'm thinking of naming the ServletContextListener that should be registered
as a listener in web.xml "Log4jShutdownOnContextDestroyedListener". In
my initial code it was named "Log4jServletDestroyedListenerTest" which
isn't very useful. The general pattern in this log4j-web artifact is to
name
things after the servlet interface they implement. However, now we have two
classes implementing ServletContextListener so this class needs a name
that communicates its meaning better. Does the name
"Log4jShutdownOnContextDestroyedListener" make sense? It is the class
to register to do the log4j shutdown when the listener's
contextDestroyed method is called.

I'm open to other suggestions. If no one has a strong preference I'll check
in the code with "Log4jShutdownOnContextDestroyedListener" as the name
and create the pull request tomorrow morning PST.

I'm working on a branch based off release-2.x here:
https://github.com/perry2of5/logging-log4j2/tree/release-2.x-configurableShutdown

Thanks,
Tim


On Tue, Jan 26, 2021 at 12:07 PM Matt Sicker  wrote:

> The release-2.x branch is the current stable branch, while master is
> the 3.x branch right now. I'd suggest release-2.x since we're still
> likely to make at least one (if not more) more 2.x release before 3.0,
> though if you make a PR to both branches, that helps :)
>
> On Tue, 26 Jan 2021 at 13:47, Tim Perry  wrote:
> >
> > Yes, junit pulls in an old version of hamcrest. There are some nice
> > goodies in the newer hamcrest, but I just stuck with the existing
> > dependencies. I did upgrade everything to JUnit 5.
> >
> > What is the best branch to base my pull request on? I'm going to
> > re-apply the changes to get a clean history: one commit for the
> > testing upgrades and a second commit for substantive changes.
> >
> > Thanks,
> > Tim
> >
> > On Mon, Jan 25, 2021 at 1:15 PM Matt Sicker  wrote:
> >
> > > I believe that's already included. There's also AssertJ which might be
> > > a dependency. No strong preferences from me, though; just try to use
> > > the JUnit 5 API preferably as not everything has been migrated yet! :)
> > >
> > > On Mon, 25 Jan 2021 at 15:06, Tim Perry  wrote:
> > > >
> > > > Is it okay to import hamcrest to use in the tests?
> > > >
> > > > 
> > > > org.hamcrest
> > > > hamcrest
> > > > 2.2
> > > > test
> > > > 
> > > >
> > > >
> > > > On Mon, Jan 25, 2021 at 10:44 AM Tim Perry 
> wrote:
> > > >
> > > > > Thank you, Matt.
> > > > >
> > > > > I'll get back to you after I've written some unit tests.
> > > > >
> > > > > Tim
> > > > >
> > > > > On Mon, Jan 25, 2021 at 10:37 AM Matt Sicker 
> wrote:
> > > > >
> > > > >> Go to https://github.com/apache/logging-log4j2/compare and click
> the
> > > > >> "compare across forks" link at the top to make a PR from your
> forked
> > > > >> repo.
> > > > >>
> > > > >> On Mon, 25 Jan 2021 at 12:15, Tim Perry 
> wrote:
> > > > >> >
> > > > >> > Matt,
> > > > >> >
> > > > >> > Thanks for clarifying.
> > > > >> >
> > > > >> > I'd be happy to write some tests and submit a PR. How should I
> > > submit a
> > > > >> > pull request? I don't think I can do it from the github repo I
> > > linked
> > > > >> to.
> > > > >> >
> > > > >> > Thanks,
> > > > >> > Tim
> > > > >> >
> > > > >> >
> > > > >> > On Mon, Jan 25, 2021 at 9:59 AM Matt Sicker 
> > > wrote:
> > > > >> >
> > > > >> > > I'm just saying that I don't think any of the developers here
> > > would
> > > > >> > > object to functional changes you'd like to introduce here,
> > > especially
> > > > >> > > if you think this change makes sense for users other than
> > > yourself.
> > > > >> > >
> > > > >> > > If you submit your changes as a PR (and preferably add
> automated
> > > tests
> > > > >> > > if pos

Re: LOG4J2-2624 and LOG4J2-1606

[Tim Perry]

I submitted pull request 463 for this work.
https://github.com/apache/logging-log4j2/pull/463

Please let me know if there are any issues you would like me to address.

Thank you,
Tim


On Tue, Jan 26, 2021 at 3:27 PM Tim Perry  wrote:

> Okay, I've fully updated the code to use JUnit 5.
>
> I'm thinking of naming the ServletContextListener that should be
> registered
> as a listener in web.xml "Log4jShutdownOnContextDestroyedListener". In
> my initial code it was named "Log4jServletDestroyedListenerTest" which
> isn't very useful. The general pattern in this log4j-web artifact is to
> name
> things after the servlet interface they implement. However, now we have
> two
> classes implementing ServletContextListener so this class needs a name
> that communicates its meaning better. Does the name
> "Log4jShutdownOnContextDestroyedListener" make sense? It is the class
> to register to do the log4j shutdown when the listener's
> contextDestroyed method is called.
>
> I'm open to other suggestions. If no one has a strong preference I'll check
> in the code with "Log4jShutdownOnContextDestroyedListener" as the name
> and create the pull request tomorrow morning PST.
>
> I'm working on a branch based off release-2.x here:
>
> https://github.com/perry2of5/logging-log4j2/tree/release-2.x-configurableShutdown
>
> Thanks,
> Tim
>
>
> On Tue, Jan 26, 2021 at 12:07 PM Matt Sicker  wrote:
>
>> The release-2.x branch is the current stable branch, while master is
>> the 3.x branch right now. I'd suggest release-2.x since we're still
>> likely to make at least one (if not more) more 2.x release before 3.0,
>> though if you make a PR to both branches, that helps :)
>>
>> On Tue, 26 Jan 2021 at 13:47, Tim Perry  wrote:
>> >
>> > Yes, junit pulls in an old version of hamcrest. There are some nice
>> > goodies in the newer hamcrest, but I just stuck with the existing
>> > dependencies. I did upgrade everything to JUnit 5.
>> >
>> > What is the best branch to base my pull request on? I'm going to
>> > re-apply the changes to get a clean history: one commit for the
>> > testing upgrades and a second commit for substantive changes.
>> >
>> > Thanks,
>> > Tim
>> >
>> > On Mon, Jan 25, 2021 at 1:15 PM Matt Sicker  wrote:
>> >
>> > > I believe that's already included. There's also AssertJ which might be
>> > > a dependency. No strong preferences from me, though; just try to use
>> > > the JUnit 5 API preferably as not everything has been migrated yet! :)
>> > >
>> > > On Mon, 25 Jan 2021 at 15:06, Tim Perry  wrote:
>> > > >
>> > > > Is it okay to import hamcrest to use in the tests?
>> > > >
>> > > > 
>> > > > org.hamcrest
>> > > > hamcrest
>> > > > 2.2
>> > > > test
>> > > > 
>> > > >
>> > > >
>> > > > On Mon, Jan 25, 2021 at 10:44 AM Tim Perry 
>> wrote:
>> > > >
>> > > > > Thank you, Matt.
>> > > > >
>> > > > > I'll get back to you after I've written some unit tests.
>> > > > >
>> > > > > Tim
>> > > > >
>> > > > > On Mon, Jan 25, 2021 at 10:37 AM Matt Sicker 
>> wrote:
>> > > > >
>> > > > >> Go to https://github.com/apache/logging-log4j2/compare and
>> click the
>> > > > >> "compare across forks" link at the top to make a PR from your
>> forked
>> > > > >> repo.
>> > > > >>
>> > > > >> On Mon, 25 Jan 2021 at 12:15, Tim Perry 
>> wrote:
>> > > > >> >
>> > > > >> > Matt,
>> > > > >> >
>> > > > >> > Thanks for clarifying.
>> > > > >> >
>> > > > >> > I'd be happy to write some tests and submit a PR. How should I
>> > > submit a
>> > > > >> > pull request? I don't think I can do it from the github repo I
>> > > linked
>> > > > >> to.
>> > > > >> >
>> > > > >> > Thanks,
>> > > > >> > Tim
>> > > > >> >
>> > > > >> >
>> > > > >> > On Mon, Jan 25, 2021 at 9:59 AM Matt Sicke

Re: LOG4J2-2624 and LOG4J2-1606

[Tim Perry]

Matt,

Thank you. If all is well, I’ll make the equivalent PR for the master branch. 

Tim


Tim Perry
(916) 505-3634

> On Jan 28, 2021, at 6:49 PM, Matt Sicker  wrote:
> 
> Great, thanks for the PR! I'll make sure to review this over the weekend.
> 
>> On Thu, 28 Jan 2021 at 13:47, Tim Perry  wrote:
>> 
>> I submitted pull request 463 for this work.
>> https://github.com/apache/logging-log4j2/pull/463
>> 
>> Please let me know if there are any issues you would like me to address.
>> 
>> Thank you,
>> Tim
>> 
>> 
>>> On Tue, Jan 26, 2021 at 3:27 PM Tim Perry  wrote:
>>> 
>>> Okay, I've fully updated the code to use JUnit 5.
>>> 
>>> I'm thinking of naming the ServletContextListener that should be
>>> registered
>>> as a listener in web.xml "Log4jShutdownOnContextDestroyedListener". In
>>> my initial code it was named "Log4jServletDestroyedListenerTest" which
>>> isn't very useful. The general pattern in this log4j-web artifact is to
>>> name
>>> things after the servlet interface they implement. However, now we have
>>> two
>>> classes implementing ServletContextListener so this class needs a name
>>> that communicates its meaning better. Does the name
>>> "Log4jShutdownOnContextDestroyedListener" make sense? It is the class
>>> to register to do the log4j shutdown when the listener's
>>> contextDestroyed method is called.
>>> 
>>> I'm open to other suggestions. If no one has a strong preference I'll check
>>> in the code with "Log4jShutdownOnContextDestroyedListener" as the name
>>> and create the pull request tomorrow morning PST.
>>> 
>>> I'm working on a branch based off release-2.x here:
>>> 
>>> https://github.com/perry2of5/logging-log4j2/tree/release-2.x-configurableShutdown
>>> 
>>> Thanks,
>>> Tim
>>> 
>>> 
>>>> On Tue, Jan 26, 2021 at 12:07 PM Matt Sicker  wrote:
>>> 
>>>> The release-2.x branch is the current stable branch, while master is
>>>> the 3.x branch right now. I'd suggest release-2.x since we're still
>>>> likely to make at least one (if not more) more 2.x release before 3.0,
>>>> though if you make a PR to both branches, that helps :)
>>>> 
>>>> On Tue, 26 Jan 2021 at 13:47, Tim Perry  wrote:
>>>>> 
>>>>> Yes, junit pulls in an old version of hamcrest. There are some nice
>>>>> goodies in the newer hamcrest, but I just stuck with the existing
>>>>> dependencies. I did upgrade everything to JUnit 5.
>>>>> 
>>>>> What is the best branch to base my pull request on? I'm going to
>>>>> re-apply the changes to get a clean history: one commit for the
>>>>> testing upgrades and a second commit for substantive changes.
>>>>> 
>>>>> Thanks,
>>>>> Tim
>>>>> 
>>>>> On Mon, Jan 25, 2021 at 1:15 PM Matt Sicker  wrote:
>>>>> 
>>>>>> I believe that's already included. There's also AssertJ which might be
>>>>>> a dependency. No strong preferences from me, though; just try to use
>>>>>> the JUnit 5 API preferably as not everything has been migrated yet! :)
>>>>>> 
>>>>>> On Mon, 25 Jan 2021 at 15:06, Tim Perry  wrote:
>>>>>>> 
>>>>>>> Is it okay to import hamcrest to use in the tests?
>>>>>>> 
>>>>>>> 
>>>>>>>org.hamcrest
>>>>>>>hamcrest
>>>>>>>2.2
>>>>>>>test
>>>>>>> 
>>>>>>> 
>>>>>>> 
>>>>>>> On Mon, Jan 25, 2021 at 10:44 AM Tim Perry 
>>>> wrote:
>>>>>>> 
>>>>>>>> Thank you, Matt.
>>>>>>>> 
>>>>>>>> I'll get back to you after I've written some unit tests.
>>>>>>>> 
>>>>>>>> Tim
>>>>>>>> 
>>>>>>>> On Mon, Jan 25, 2021 at 10:37 AM Matt Sicker 
>>>> wrote:
>>>>>>>> 
>>>>>>>>> Go to https://github.com/apache/logging-log4j2/compare and
>>>> click the
>>>>>>>>> "compare across forks" link at

Re: LOG4J2-2624 and LOG4J2-1606

[Tim Perry]

How do I link the commits / pull request to JIRA LOG4J2-2624 and
LOG4J2-1606?


>> On Thu, 28 Jan 2021 at 13:47, Tim Perry  wrote:
> >>
> >> I submitted pull request 463 for this work.
> >> https://github.com/apache/logging-log4j2/pull/463
> >>
> >> Please let me know if there are any issues you would like me to address.
> >>
> >> Thank you,
> >> Tim
>
>

Re: LOG4J2-2624 and LOG4J2-1606

[Tim Perry]

Jochen,

Hamcrest 1.3 used to be pulled in by JUnit, but it isn't pulled in by
JUnit5. When I converted all the tests to JUnit5 syntax and dropped the
JUnit4 bridge then I needed to pull in hamcrest explicitly. I went with 1.3
to match the rest of the code base.

Tim


On Mon, Feb 1, 2021 at 12:59 AM Jochen Wiedmann 
wrote:

> On Mon, Jan 25, 2021 at 10:06 PM Tim Perry  wrote:
> >
> > Is it okay to import hamcrest to use in the tests?
> >
> > 
> > org.hamcrest
> > hamcrest
> > 2.2
> > test
> > 
>
> Isn't that pulled in automatically via junit? See, for example, the
> Pom in https://search.maven.org/artifact/junit/junit/4.13.1/jar.
>
> Jochen
>
>
>
> --
>
> Look, that's why there's rules, understand? So that you think before
> you break 'em.
>
> -- (Terry Pratchett, Thief of Time)
>

Re: Putting a ribbon around 2.14.1?

[Tim Perry]

Is it possible to merge PR 463 for LOG4J-2624 and LOG4J-1606 for 2.14.1?

On Wed, Feb 3, 2021 at 2:09 AM Volkan Yazıcı 
wrote:

> The LOG4J2-2973 fix is there for both master and release-2.x branches –
> thanks to Fabio Ricchiuti!
> I will really appreciate it if somebody can get the 2.14.1 out of the door.
>
> On Thu, Jan 28, 2021 at 12:02 PM Volkan Yazıcı 
> wrote:
>
> > For 2.14.1, would you wait for the resolution of LOG4J2-2973
> > , please?
> > I am helping Fabio Ricchiuti to put the fix
> >  into its last shape.
> > Once it lands on master, I will cherry-pick it to release-2.x too.
> >
> > On Sat, Jan 9, 2021 at 5:23 PM Volkan Yazıcı 
> > wrote:
> >
> >> Hello,
> >>
> >> Shall we release 2.14.1? I really want to get the next release out of
> the
> >> door in particular for the following changes:
> >>
> >> LOG4J2-2972 Refactor AsyncAppender and AppenderControl for handling of
> >> Throwables.
> >> LOG4J2-2985 Add eventTemplateRootObjectKey parameter to
> >> JsonTemplateLayout.
> >> LOG4J2-2962 Enrich "map" resolver by unifying its backend with "mdc"
> >> resolver.
> >> LOG4J2-2961 Fix reading of JsonTemplateLayout event additional fields
> >> from config.
> >> LOG4J2-2916 Avoid redundant Kafka producer instantiation causing thread
> >> leaks.
> >> LOG4J2-2967 Fix JsonTemplateLayout index based parameter resolution when
> >> messages contain too few parameters.
> >> LOG4J2-2976 JdbcAppender composes an incorrect INSERT statement without
> a
> >> ColumnMapping element.
> >>
> >> @Ralph, given you are the (unofficial?) release manager, what do you
> >> think? Can we have an ETA for the release?
> >>
> >> Kind regards.
> >>
> >
>

Re: Putting a ribbon around 2.14.1?

[Tim Perry]

Great! Thank you.

On Wed, Feb 3, 2021 at 8:44 AM Matt Sicker  wrote:

> Yes, I'll make sure that it is merged in time for that.
>
> On Wed, 3 Feb 2021 at 10:43, Tim Perry  wrote:
> >
> > Is it possible to merge PR 463 for LOG4J-2624 and LOG4J-1606 for 2.14.1?
> >
> > On Wed, Feb 3, 2021 at 2:09 AM Volkan Yazıcı 
> > wrote:
> >
> > > The LOG4J2-2973 fix is there for both master and release-2.x branches –
> > > thanks to Fabio Ricchiuti!
> > > I will really appreciate it if somebody can get the 2.14.1 out of the
> door.
> > >
> > > On Thu, Jan 28, 2021 at 12:02 PM Volkan Yazıcı <
> volkan.yaz...@gmail.com>
> > > wrote:
> > >
> > > > For 2.14.1, would you wait for the resolution of LOG4J2-2973
> > > > <https://issues.apache.org/jira/browse/LOG4J2-2973>, please?
> > > > I am helping Fabio Ricchiuti to put the fix
> > > > <https://github.com/apache/logging-log4j2/pull/462> into its last
> shape.
> > > > Once it lands on master, I will cherry-pick it to release-2.x too.
> > > >
> > > > On Sat, Jan 9, 2021 at 5:23 PM Volkan Yazıcı <
> volkan.yaz...@gmail.com>
> > > > wrote:
> > > >
> > > >> Hello,
> > > >>
> > > >> Shall we release 2.14.1? I really want to get the next release out
> of
> > > the
> > > >> door in particular for the following changes:
> > > >>
> > > >> LOG4J2-2972 Refactor AsyncAppender and AppenderControl for handling
> of
> > > >> Throwables.
> > > >> LOG4J2-2985 Add eventTemplateRootObjectKey parameter to
> > > >> JsonTemplateLayout.
> > > >> LOG4J2-2962 Enrich "map" resolver by unifying its backend with "mdc"
> > > >> resolver.
> > > >> LOG4J2-2961 Fix reading of JsonTemplateLayout event additional
> fields
> > > >> from config.
> > > >> LOG4J2-2916 Avoid redundant Kafka producer instantiation causing
> thread
> > > >> leaks.
> > > >> LOG4J2-2967 Fix JsonTemplateLayout index based parameter resolution
> when
> > > >> messages contain too few parameters.
> > > >> LOG4J2-2976 JdbcAppender composes an incorrect INSERT statement
> without
> > > a
> > > >> ColumnMapping element.
> > > >>
> > > >> @Ralph, given you are the (unofficial?) release manager, what do you
> > > >> think? Can we have an ETA for the release?
> > > >>
> > > >> Kind regards.
> > > >>
> > > >
> > >
>

Re: Putting a ribbon around 2.14.1?

[Tim Perry]

Ideally, yes, but I haven't done anything to test it on master yet. I was
expecting to send a separate pull request for that. If you can merge it,
please do. If you want me to submit a PR, please let me know.

On Wed, Feb 3, 2021 at 11:31 AM Volkan Yazıcı 
wrote:

> Resolved merge conflicts and rebased onto release-2.x branch. Is this also
> supposed to be applied to master too?
>
> On Wed, Feb 3, 2021 at 6:58 PM Matt Sicker  wrote:
>
> > Go ahead, Volkan. Thanks!
> >
> > On Wed, 3 Feb 2021 at 11:46, Volkan Yazıcı 
> > wrote:
> > >
> > > Matt, if you approve the changes, I can also merge them in a couple of
> > > hours.
> > >
> > > On Wed, 3 Feb 2021, 17:44 Matt Sicker  wrote:
> > >
> > > > Yes, I'll make sure that it is merged in time for that.
> > > >
> > > > On Wed, 3 Feb 2021 at 10:43, Tim Perry  wrote:
> > > > >
> > > > > Is it possible to merge PR 463 for LOG4J-2624 and LOG4J-1606 for
> > 2.14.1?
> > > > >
> > > > > On Wed, Feb 3, 2021 at 2:09 AM Volkan Yazıcı <
> > volkan.yaz...@gmail.com>
> > > > > wrote:
> > > > >
> > > > > > The LOG4J2-2973 fix is there for both master and release-2.x
> > branches –
> > > > > > thanks to Fabio Ricchiuti!
> > > > > > I will really appreciate it if somebody can get the 2.14.1 out of
> > the
> > > > door.
> > > > > >
> > > > > > On Thu, Jan 28, 2021 at 12:02 PM Volkan Yazıcı <
> > > > volkan.yaz...@gmail.com>
> > > > > > wrote:
> > > > > >
> > > > > > > For 2.14.1, would you wait for the resolution of LOG4J2-2973
> > > > > > > <https://issues.apache.org/jira/browse/LOG4J2-2973>, please?
> > > > > > > I am helping Fabio Ricchiuti to put the fix
> > > > > > > <https://github.com/apache/logging-log4j2/pull/462> into its
> > last
> > > > shape.
> > > > > > > Once it lands on master, I will cherry-pick it to release-2.x
> > too.
> > > > > > >
> > > > > > > On Sat, Jan 9, 2021 at 5:23 PM Volkan Yazıcı <
> > > > volkan.yaz...@gmail.com>
> > > > > > > wrote:
> > > > > > >
> > > > > > >> Hello,
> > > > > > >>
> > > > > > >> Shall we release 2.14.1? I really want to get the next release
> > out
> > > > of
> > > > > > the
> > > > > > >> door in particular for the following changes:
> > > > > > >>
> > > > > > >> LOG4J2-2972 Refactor AsyncAppender and AppenderControl for
> > handling
> > > > of
> > > > > > >> Throwables.
> > > > > > >> LOG4J2-2985 Add eventTemplateRootObjectKey parameter to
> > > > > > >> JsonTemplateLayout.
> > > > > > >> LOG4J2-2962 Enrich "map" resolver by unifying its backend with
> > "mdc"
> > > > > > >> resolver.
> > > > > > >> LOG4J2-2961 Fix reading of JsonTemplateLayout event additional
> > > > fields
> > > > > > >> from config.
> > > > > > >> LOG4J2-2916 Avoid redundant Kafka producer instantiation
> causing
> > > > thread
> > > > > > >> leaks.
> > > > > > >> LOG4J2-2967 Fix JsonTemplateLayout index based parameter
> > resolution
> > > > when
> > > > > > >> messages contain too few parameters.
> > > > > > >> LOG4J2-2976 JdbcAppender composes an incorrect INSERT
> statement
> > > > without
> > > > > > a
> > > > > > >> ColumnMapping element.
> > > > > > >>
> > > > > > >> @Ralph, given you are the (unofficial?) release manager, what
> > do you
> > > > > > >> think? Can we have an ETA for the release?
> > > > > > >>
> > > > > > >> Kind regards.
> > > > > > >>
> > > > > > >
> > > > > >
> > > >
> >
>

Re: Putting a ribbon around 2.14.1?

[Tim Perry]

Volkan,

Thank you very much!

Tim

On Wed, Feb 3, 2021 at 11:55 AM Volkan Yazıcı 
wrote:

> Took some time, but managed to get it merged (with succeeding tests) into
> master.
>
> On Wed, Feb 3, 2021 at 8:39 PM Tim Perry  wrote:
>
> > Ideally, yes, but I haven't done anything to test it on master yet. I was
> > expecting to send a separate pull request for that. If you can merge it,
> > please do. If you want me to submit a PR, please let me know.
> >
> > On Wed, Feb 3, 2021 at 11:31 AM Volkan Yazıcı 
> > wrote:
> >
> > > Resolved merge conflicts and rebased onto release-2.x branch. Is this
> > also
> > > supposed to be applied to master too?
> > >
> > > On Wed, Feb 3, 2021 at 6:58 PM Matt Sicker  wrote:
> > >
> > > > Go ahead, Volkan. Thanks!
> > > >
> > > > On Wed, 3 Feb 2021 at 11:46, Volkan Yazıcı 
> > > > wrote:
> > > > >
> > > > > Matt, if you approve the changes, I can also merge them in a couple
> > of
> > > > > hours.
> > > > >
> > > > > On Wed, 3 Feb 2021, 17:44 Matt Sicker  wrote:
> > > > >
> > > > > > Yes, I'll make sure that it is merged in time for that.
> > > > > >
> > > > > > On Wed, 3 Feb 2021 at 10:43, Tim Perry 
> wrote:
> > > > > > >
> > > > > > > Is it possible to merge PR 463 for LOG4J-2624 and LOG4J-1606
> for
> > > > 2.14.1?
> > > > > > >
> > > > > > > On Wed, Feb 3, 2021 at 2:09 AM Volkan Yazıcı <
> > > > volkan.yaz...@gmail.com>
> > > > > > > wrote:
> > > > > > >
> > > > > > > > The LOG4J2-2973 fix is there for both master and release-2.x
> > > > branches –
> > > > > > > > thanks to Fabio Ricchiuti!
> > > > > > > > I will really appreciate it if somebody can get the 2.14.1
> out
> > of
> > > > the
> > > > > > door.
> > > > > > > >
> > > > > > > > On Thu, Jan 28, 2021 at 12:02 PM Volkan Yazıcı <
> > > > > > volkan.yaz...@gmail.com>
> > > > > > > > wrote:
> > > > > > > >
> > > > > > > > > For 2.14.1, would you wait for the resolution of
> LOG4J2-2973
> > > > > > > > > <https://issues.apache.org/jira/browse/LOG4J2-2973>,
> please?
> > > > > > > > > I am helping Fabio Ricchiuti to put the fix
> > > > > > > > > <https://github.com/apache/logging-log4j2/pull/462> into
> its
> > > > last
> > > > > > shape.
> > > > > > > > > Once it lands on master, I will cherry-pick it to
> release-2.x
> > > > too.
> > > > > > > > >
> > > > > > > > > On Sat, Jan 9, 2021 at 5:23 PM Volkan Yazıcı <
> > > > > > volkan.yaz...@gmail.com>
> > > > > > > > > wrote:
> > > > > > > > >
> > > > > > > > >> Hello,
> > > > > > > > >>
> > > > > > > > >> Shall we release 2.14.1? I really want to get the next
> > release
> > > > out
> > > > > > of
> > > > > > > > the
> > > > > > > > >> door in particular for the following changes:
> > > > > > > > >>
> > > > > > > > >> LOG4J2-2972 Refactor AsyncAppender and AppenderControl for
> > > > handling
> > > > > > of
> > > > > > > > >> Throwables.
> > > > > > > > >> LOG4J2-2985 Add eventTemplateRootObjectKey parameter to
> > > > > > > > >> JsonTemplateLayout.
> > > > > > > > >> LOG4J2-2962 Enrich "map" resolver by unifying its backend
> > with
> > > > "mdc"
> > > > > > > > >> resolver.
> > > > > > > > >> LOG4J2-2961 Fix reading of JsonTemplateLayout event
> > additional
> > > > > > fields
> > > > > > > > >> from config.
> > > > > > > > >> LOG4J2-2916 Avoid redundant Kafka producer instantiation
> > > causing
> > > > > > thread
> > > > > > > > >> leaks.
> > > > > > > > >> LOG4J2-2967 Fix JsonTemplateLayout index based parameter
> > > > resolution
> > > > > > when
> > > > > > > > >> messages contain too few parameters.
> > > > > > > > >> LOG4J2-2976 JdbcAppender composes an incorrect INSERT
> > > statement
> > > > > > without
> > > > > > > > a
> > > > > > > > >> ColumnMapping element.
> > > > > > > > >>
> > > > > > > > >> @Ralph, given you are the (unofficial?) release manager,
> > what
> > > > do you
> > > > > > > > >> think? Can we have an ETA for the release?
> > > > > > > > >>
> > > > > > > > >> Kind regards.
> > > > > > > > >>
> > > > > > > > >
> > > > > > > >
> > > > > >
> > > >
> > >
> >
>

Re: What is going on with Master?

[Tim Perry]

On master, I noticed that when I build in my IDE that lots of tests in
log4j-core fail but when I build with maven they pass. In the past, when
I've seen this on other projects it was caused by race conditions. I
wonder: are there race conditions in the tests or the code that need to be
addressed?

On Sun, Feb 7, 2021 at 1:46 PM Ralph Goers 
wrote:

> Yes, that is why I am wondering why master is so flaky.
>
> Ralph
>
> > On Feb 7, 2021, at 2:34 PM, Gary Gregory  wrote:
> >
> > FWIW but not master, I've been building release-2.x lately over and over
> > without issues aside from the rare random failure.
> >
> > G
> >
> > On Sun, Feb 7, 2021 at 2:37 PM Matt Sicker  wrote:
> >
> >> I'm not getting errors in log4j-api, though it's possible some test is
> >> missing a JUnit annotation which is causing that ThreadContext data
> >> corruption.
> >>
> >> I am, however, getting the same log4j-kafka test error. I'll try
> >> running the log4j-api tests a few more times to see if I can reproduce
> >> any test errors there.
> >>
> >> On Sun, 7 Feb 2021 at 12:20, Ralph Goers 
> >> wrote:
> >>>
> >>> I am trying to apply patches to the release-2.x branch and make the
> >> corresponding changes in master. When I run a build in master I cannot
> get
> >> it to succeed. It randomly fails in different ways. I typically run a
> full
> >> build when I make significant changes and don’t commit unless it
> passes. So
> >> far I have spent well over an hour running the build at least 6 times
> >> trying to get a successful build with no luck.
> >>>
> >>> Run 1 fails in log4j-api
> >>> [ERROR] Failures:
> >>> [ERROR]   AbstractLoggerTest.testMessageThrowsAndNullFormat:912
> >>> Expected: a string containing
> >> "org.apache.logging.log4j.spi.AbstractLogger caught
> >> java.lang.IllegalStateException logging TestMessage: "
> >>> but: was "2021-02-07 10:00:31,277 ForkJoinPool-1-worker-30 WARN
> >> Ignoring java.io.NotSerializableException: java.lang.Object for key[2]
> >> ('unserializable')"
> >>> [ERROR]   CloseableThreadContextTest.canReuseCloseableThreadContext:169
> >> expected:  but was: 
> >>> [ERROR]   CloseableThreadContextTest.closeIsIdempotent:203 expected:
> >>  but was: 
> >>> [ERROR]
> >>
> CloseableThreadContextTest.ifTheSameKeyIsAddedTwiceTheOriginalShouldBeUsed:103
> >> expected:  but was: 
> >>> [ERROR]   CloseableThreadContextTest.pushAllWillPushAllValues:237
> >> expected:  but was: <>
> >>> [ERROR]   CloseableThreadContextTest.putAllWillPutAllValues:222
> >> expected:  but was: 
> >>> [ERROR]   CloseableThreadContextTest.shouldAddAnEntryToTheMap:53
> >> expected:  but was: 
> >>> [ERROR]
> >> CloseableThreadContextTest.shouldAddEntriesToBothStackAndMap:156
> expected:
> >>  but was: 
> >>> [ERROR]   CloseableThreadContextTest.shouldAddTwoEntriesToTheMap:63
> >> expected:  but was: 
> >>> [ERROR]   CloseableThreadContextTest.shouldNestEntries:75 expected:
> >>  but was: 
> >>> [ERROR]
> >>
> CloseableThreadContextTest.shouldPreserveOldEntriesFromTheMapWhenAutoClosed:91
> >> expected:  but was: 
> >>> [ERROR]
> >>
> CloseableThreadContextTest.shouldPushAndPopAParameterizedEntryToTheStack:137
> >> expected:  but was: <>
> >>> [ERROR]
> >> CloseableThreadContextTest.shouldPushAndPopAnEntryToTheStack:113
> expected:
> >>  but was: <>
> >>> [ERROR]
> >> CloseableThreadContextTest.shouldPushAndPopTwoEntriesToTheStack:124
> >> expected:  but was: <>
> >>> [ERROR]
> >>
> CloseableThreadContextTest.shouldRemoveAnEntryFromTheMapWhenAutoClosed:146
> >> expected:  but was: 
> >>> [INFO]
> >>>
> >>>
> >>> Run 2 fails in log4j-api
> >>> [ERROR] Failures:
> >>> [ERROR]   LoggerTest.mdc:556 Test Year is null ==> expected: not 
> >>>
> >>> Run 3 log4j-api passes but log4j-core fails
> >>> [INFO]
> >>> [ERROR] Failures:
> >>> [ERROR]   RollingDirectTimeNewDirectoryTest.streamClosedError:95 check
> >> failure
> >>>
> >>> Run 4 - log4j-api and log4j-core pass but fails in log4j-kafka
> >>> [ERROR] Failures:
> >>> [ERROR]   ConfigurationBuilderTest.testXmlConstructing:116
> >> expected:<
> >>> ] but was:< >> [encoding="UTF-8"?>]
> >>>
> >>> Run 5 - failure in log4j-core (I’m probably the last one to touch this
> >> so I’m the one who will most likely need to figure this one out)
> >>>
> >>> [ERROR] Failures:
> >>> [ERROR]   RollingAppenderDirectWrite1906Test.testAppender:95 2021-02-07
> >> 10:50:17,478 main INFO Log4j appears to be running in a Servlet
> >> environment, but there's no log4j-web module available. If you want
> better
> >> web container support, please add the log4j-web JAR to your web archive
> or
> >> server lib directory.
> >>> 2021-02-07 10:50:17,488 main INFO Log4j appears to be running in a
> >> Servlet environment, but there's no log4j-web module available. If you
> want
> >> better web container support, please add the log4j-web JAR to your web
> >> archive or server lib directory.
> >>> 2021-02-07 10:50:17,499 main DEBUG Took 0.003727 seconds to load 3
> >> plugins from sun.misc.Launcher$AppClassLoader@4a

Re: What is going on with Master?

[Tim Perry]

I mean is that:

* when I run "mvn install" on the command line then usually 1, but
sometimes 2 tests fail.
* when I right-click the src/tests/java folder in eclipse and do "run as
JUnit" I get ~100 failures.

I'm fairly sure the tests are *not* running in parallel.

On Wed, Feb 17, 2021 at 1:11 PM Matt Sicker  wrote:

> I think he's referring to the need to run mvn install once before the
> IDE will pick up snapshot jars for things like annotation processing
> or other fancy things. Simple way to reproduce: try running "mvn
> package" without the "install" target.
>
> On Wed, 17 Feb 2021 at 13:58, Ralph Goers 
> wrote:
> >
> > Yeah, there is a very good chance if you are trying to run tests in core
> in parallel on multiple threads that they will fail. Most of them expect
> their own LoggerContext.
> >
> > Ralph
> >
> > > On Feb 17, 2021, at 12:51 PM, Tim Perry  wrote:
> > >
> > > On master, I noticed that when I build in my IDE that lots of tests in
> > > log4j-core fail but when I build with maven they pass. In the past,
> when
> > > I've seen this on other projects it was caused by race conditions. I
> > > wonder: are there race conditions in the tests or the code that need
> to be
> > > addressed?
> > >
> > > On Sun, Feb 7, 2021 at 1:46 PM Ralph Goers  >
> > > wrote:
> > >
> > >> Yes, that is why I am wondering why master is so flaky.
> > >>
> > >> Ralph
> > >>
> > >>> On Feb 7, 2021, at 2:34 PM, Gary Gregory 
> wrote:
> > >>>
> > >>> FWIW but not master, I've been building release-2.x lately over and
> over
> > >>> without issues aside from the rare random failure.
> > >>>
> > >>> G
> > >>>
> > >>> On Sun, Feb 7, 2021 at 2:37 PM Matt Sicker  wrote:
> > >>>
> > >>>> I'm not getting errors in log4j-api, though it's possible some test
> is
> > >>>> missing a JUnit annotation which is causing that ThreadContext data
> > >>>> corruption.
> > >>>>
> > >>>> I am, however, getting the same log4j-kafka test error. I'll try
> > >>>> running the log4j-api tests a few more times to see if I can
> reproduce
> > >>>> any test errors there.
> > >>>>
> > >>>> On Sun, 7 Feb 2021 at 12:20, Ralph Goers <
> ralph.go...@dslextreme.com>
> > >>>> wrote:
> > >>>>>
> > >>>>> I am trying to apply patches to the release-2.x branch and make the
> > >>>> corresponding changes in master. When I run a build in master I
> cannot
> > >> get
> > >>>> it to succeed. It randomly fails in different ways. I typically run
> a
> > >> full
> > >>>> build when I make significant changes and don’t commit unless it
> > >> passes. So
> > >>>> far I have spent well over an hour running the build at least 6
> times
> > >>>> trying to get a successful build with no luck.
> > >>>>>
> > >>>>> Run 1 fails in log4j-api
> > >>>>> [ERROR] Failures:
> > >>>>> [ERROR]   AbstractLoggerTest.testMessageThrowsAndNullFormat:912
> > >>>>> Expected: a string containing
> > >>>> "org.apache.logging.log4j.spi.AbstractLogger caught
> > >>>> java.lang.IllegalStateException logging TestMessage: "
> > >>>>>but: was "2021-02-07 10:00:31,277 ForkJoinPool-1-worker-30 WARN
> > >>>> Ignoring java.io.NotSerializableException: java.lang.Object for
> key[2]
> > >>>> ('unserializable')"
> > >>>>> [ERROR]
>  CloseableThreadContextTest.canReuseCloseableThreadContext:169
> > >>>> expected:  but was: 
> > >>>>> [ERROR]   CloseableThreadContextTest.closeIsIdempotent:203
> expected:
> > >>>>  but was: 
> > >>>>> [ERROR]
> > >>>>
> > >>
> CloseableThreadContextTest.ifTheSameKeyIsAddedTwiceTheOriginalShouldBeUsed:103
> > >>>> expected:  but was: 
> > >>>>> [ERROR]   CloseableThreadContextTest.pushAllWillPushAllValues:237
> > >>>> expected:  but was: <>
> > >>>>> [ERROR]   CloseableThreadContextTest.putAllWillPutAllValues:222
> > >>>

Re: What is going on with Master?

[Tim Perry]

Yes, I am on windows 10. I'm using Zulu OpenJdk for both Java 8 and 11,
maven 3.6.3, and a fairly new Eclipse. I'd be happy to help you sort out
the problem. It will be fun: I haven't played around with those junit
features.

On Wed, Feb 17, 2021 at 1:45 PM Matt Sicker  wrote:

> Is that on Windows? If so, there seems to be a bug I've introduced in
> the JUnit 5 code that I tried to port from the JUnit 4 code correctly,
> but I don't have a Windows machine to test that out on iteratively.
> See
> https://github.com/apache/logging-log4j2/blob/master/log4j-core/src/test/java/org/apache/logging/log4j/junit/LoggerContextResolver.java
> for relevant code and the JUnit 4 version here in
>
> https://github.com/apache/logging-log4j2/blob/master/log4j-core/src/test/java/org/apache/logging/log4j/junit/LoggerContextRule.java
> which does reconfiguration a bit more heavily (but also in such a way
> that it works more consistently on Windows apparently?)
>
> On Wed, 17 Feb 2021 at 15:36, Tim Perry  wrote:
> >
> > I mean is that:
> >
> > * when I run "mvn install" on the command line then usually 1, but
> > sometimes 2 tests fail.
> > * when I right-click the src/tests/java folder in eclipse and do "run as
> > JUnit" I get ~100 failures.
> >
> > I'm fairly sure the tests are *not* running in parallel.
> >
> > On Wed, Feb 17, 2021 at 1:11 PM Matt Sicker  wrote:
> >
> > > I think he's referring to the need to run mvn install once before the
> > > IDE will pick up snapshot jars for things like annotation processing
> > > or other fancy things. Simple way to reproduce: try running "mvn
> > > package" without the "install" target.
> > >
> > > On Wed, 17 Feb 2021 at 13:58, Ralph Goers 
> > > wrote:
> > > >
> > > > Yeah, there is a very good chance if you are trying to run tests in
> core
> > > in parallel on multiple threads that they will fail. Most of them
> expect
> > > their own LoggerContext.
> > > >
> > > > Ralph
> > > >
> > > > > On Feb 17, 2021, at 12:51 PM, Tim Perry 
> wrote:
> > > > >
> > > > > On master, I noticed that when I build in my IDE that lots of
> tests in
> > > > > log4j-core fail but when I build with maven they pass. In the past,
> > > when
> > > > > I've seen this on other projects it was caused by race conditions.
> I
> > > > > wonder: are there race conditions in the tests or the code that
> need
> > > to be
> > > > > addressed?
> > > > >
> > > > > On Sun, Feb 7, 2021 at 1:46 PM Ralph Goers <
> ralph.go...@dslextreme.com
> > > >
> > > > > wrote:
> > > > >
> > > > >> Yes, that is why I am wondering why master is so flaky.
> > > > >>
> > > > >> Ralph
> > > > >>
> > > > >>> On Feb 7, 2021, at 2:34 PM, Gary Gregory  >
> > > wrote:
> > > > >>>
> > > > >>> FWIW but not master, I've been building release-2.x lately over
> and
> > > over
> > > > >>> without issues aside from the rare random failure.
> > > > >>>
> > > > >>> G
> > > > >>>
> > > > >>> On Sun, Feb 7, 2021 at 2:37 PM Matt Sicker 
> wrote:
> > > > >>>
> > > > >>>> I'm not getting errors in log4j-api, though it's possible some
> test
> > > is
> > > > >>>> missing a JUnit annotation which is causing that ThreadContext
> data
> > > > >>>> corruption.
> > > > >>>>
> > > > >>>> I am, however, getting the same log4j-kafka test error. I'll try
> > > > >>>> running the log4j-api tests a few more times to see if I can
> > > reproduce
> > > > >>>> any test errors there.
> > > > >>>>
> > > > >>>> On Sun, 7 Feb 2021 at 12:20, Ralph Goers <
> > > ralph.go...@dslextreme.com>
> > > > >>>> wrote:
> > > > >>>>>
> > > > >>>>> I am trying to apply patches to the release-2.x branch and
> make the
> > > > >>>> corresponding changes in master. When I run a build in master I
> > > cannot
> > > > >> get
> > > > >>&

Re: What is going on with Master?

[Tim Perry]

Matt,

The problem is:
target\status.log failed with java.nio.file.FileSystemException:
target\status.log: The process cannot access the file because it is being
used by another process.

I'm not familiar with how log4j should be releasing the log file to know
where to look. I did notice the afterAll(context) method is never called on
the LoggerContextResolver. Would that explain why the file stays locked?

Tim

On Wed, Feb 17, 2021 at 1:49 PM Tim Perry  wrote:

> Yes, I am on windows 10. I'm using Zulu OpenJdk for both Java 8 and 11,
> maven 3.6.3, and a fairly new Eclipse. I'd be happy to help you sort out
> the problem. It will be fun: I haven't played around with those junit
> features.
>
> On Wed, Feb 17, 2021 at 1:45 PM Matt Sicker  wrote:
>
>> Is that on Windows? If so, there seems to be a bug I've introduced in
>> the JUnit 5 code that I tried to port from the JUnit 4 code correctly,
>> but I don't have a Windows machine to test that out on iteratively.
>> See
>> https://github.com/apache/logging-log4j2/blob/master/log4j-core/src/test/java/org/apache/logging/log4j/junit/LoggerContextResolver.java
>> for relevant code and the JUnit 4 version here in
>>
>> https://github.com/apache/logging-log4j2/blob/master/log4j-core/src/test/java/org/apache/logging/log4j/junit/LoggerContextRule.java
>> which does reconfiguration a bit more heavily (but also in such a way
>> that it works more consistently on Windows apparently?)
>>
>> On Wed, 17 Feb 2021 at 15:36, Tim Perry  wrote:
>> >
>> > I mean is that:
>> >
>> > * when I run "mvn install" on the command line then usually 1, but
>> > sometimes 2 tests fail.
>> > * when I right-click the src/tests/java folder in eclipse and do "run as
>> > JUnit" I get ~100 failures.
>> >
>> > I'm fairly sure the tests are *not* running in parallel.
>> >
>> > On Wed, Feb 17, 2021 at 1:11 PM Matt Sicker  wrote:
>> >
>> > > I think he's referring to the need to run mvn install once before the
>> > > IDE will pick up snapshot jars for things like annotation processing
>> > > or other fancy things. Simple way to reproduce: try running "mvn
>> > > package" without the "install" target.
>> > >
>> > > On Wed, 17 Feb 2021 at 13:58, Ralph Goers > >
>> > > wrote:
>> > > >
>> > > > Yeah, there is a very good chance if you are trying to run tests in
>> core
>> > > in parallel on multiple threads that they will fail. Most of them
>> expect
>> > > their own LoggerContext.
>> > > >
>> > > > Ralph
>> > > >
>> > > > > On Feb 17, 2021, at 12:51 PM, Tim Perry 
>> wrote:
>> > > > >
>> > > > > On master, I noticed that when I build in my IDE that lots of
>> tests in
>> > > > > log4j-core fail but when I build with maven they pass. In the
>> past,
>> > > when
>> > > > > I've seen this on other projects it was caused by race
>> conditions. I
>> > > > > wonder: are there race conditions in the tests or the code that
>> need
>> > > to be
>> > > > > addressed?
>> > > > >
>> > > > > On Sun, Feb 7, 2021 at 1:46 PM Ralph Goers <
>> ralph.go...@dslextreme.com
>> > > >
>> > > > > wrote:
>> > > > >
>> > > > >> Yes, that is why I am wondering why master is so flaky.
>> > > > >>
>> > > > >> Ralph
>> > > > >>
>> > > > >>> On Feb 7, 2021, at 2:34 PM, Gary Gregory <
>> garydgreg...@gmail.com>
>> > > wrote:
>> > > > >>>
>> > > > >>> FWIW but not master, I've been building release-2.x lately over
>> and
>> > > over
>> > > > >>> without issues aside from the rare random failure.
>> > > > >>>
>> > > > >>> G
>> > > > >>>
>> > > > >>> On Sun, Feb 7, 2021 at 2:37 PM Matt Sicker 
>> wrote:
>> > > > >>>
>> > > > >>>> I'm not getting errors in log4j-api, though it's possible some
>> test
>> > > is
>> > > > >>>> missing a JUnit annotation which is causing that ThreadContext
>> data
>> > > > >&g

Re: What is going on with Master?

[Tim Perry]

It looks to me like LoggerContext.stop(...) closes the file 'test.log'.
However, I can't find anything closing 'status.log'. They are configured in
the same XML file: log4j-filetest.xml


  
target/test.log
...


I modified the @Test from FileOutputTest to see if it could delete the log
files in the test. On windows, neither file gets deleted and in both delete
attempts I get an IOException with the message "The process cannot access
the file because it is being used by another process.". On Ubuntu, no
exception is thrown even though the file is obviously open. See the code
below.

>From this I'm guessing that nothing closes status.log before the
FileCleaner goes to delete it and that it triggers an exception on Windows,
but not Linux. My guess is that the default file locking level is different
between JVM's or OS's. What should be releasing the file handle for
status.log? LoggerContext?


@Test
@LoggerContextSource("classpath:log4j-filetest.xml")
public void testConfig() throws IOException {
final Path logFile = Paths.get("target", "status.log");
assertTrue(Files.exists(logFile), "Status output file does not
exist");
assertTrue(Files.size(logFile) > 0, "File is empty");

try {
Files.deleteIfExists(logFile);
fail("Should not have been able to delete " + logFile); // this
is called on Ubuntu, but not Windows
} catch (IOException ioe) {
// this is called for Windows,but not Ubuntu
System.err.println("THIS SHOULD FAIL: Failed to delete " +
logFile + ": " + ioe.getMessage());
}
final Path testLog = Paths.get("target", "test.log");
try {
Files.deleteIfExists(testLog);
fail("Should not have been able to delete " + testLog); // this
presumably would be called for Ubuntu, but we failed above.
} catch (IOException ioe) {
// this is called for Windows,but not Ubuntu
System.err.println("THIS SHOULD FAIL: Failed to delete " +
testLog + ": " + ioe.getMessage());
}
}

On Thu, Feb 18, 2021 at 8:48 AM Matt Sicker  wrote:

> The gist of what I recall the problem being was that sometimes, a file
> might still be in use asynchronously for various appenders (e.g.,
> during rolling, compression, etc.), so the shutdown and removal of
> temporary files needs to signal that the configuration needs to shut
> down. It looks like you may have found the missing link as to why this
> wasn't working properly. In that particular error message, that is
> some race condition where the temporary file is attempted to be
> deleted before the configuration stops.
>
> On Wed, 17 Feb 2021 at 18:19, Tim Perry  wrote:
> >
> > Matt,
> >
> > The problem is:
> > target\status.log failed with java.nio.file.FileSystemException:
> > target\status.log: The process cannot access the file because it is being
> > used by another process.
> >
> > I'm not familiar with how log4j should be releasing the log file to know
> > where to look. I did notice the afterAll(context) method is never called
> on
> > the LoggerContextResolver. Would that explain why the file stays locked?
> >
> > Tim
> >
> > On Wed, Feb 17, 2021 at 1:49 PM Tim Perry  wrote:
> >
> > > Yes, I am on windows 10. I'm using Zulu OpenJdk for both Java 8 and 11,
> > > maven 3.6.3, and a fairly new Eclipse. I'd be happy to help you sort
> out
> > > the problem. It will be fun: I haven't played around with those junit
> > > features.
> > >
> > > On Wed, Feb 17, 2021 at 1:45 PM Matt Sicker  wrote:
> > >
> > >> Is that on Windows? If so, there seems to be a bug I've introduced in
> > >> the JUnit 5 code that I tried to port from the JUnit 4 code correctly,
> > >> but I don't have a Windows machine to test that out on iteratively.
> > >> See
> > >>
> https://github.com/apache/logging-log4j2/blob/master/log4j-core/src/test/java/org/apache/logging/log4j/junit/LoggerContextResolver.java
> > >> for relevant code and the JUnit 4 version here in
> > >>
> > >>
> https://github.com/apache/logging-log4j2/blob/master/log4j-core/src/test/java/org/apache/logging/log4j/junit/LoggerContextRule.java
> > >> which does reconfiguration a bit more heavily (but also in such a way
> > >> that it works more consistently on Windows apparently?)
> > >>
> > >> On Wed, 17 Feb 2021 at 15:36, Tim Perry  wrote:
> > >

Re: What is going on with Master?

[Tim Perry]

The simplest fix is to add "StatusLogger.getLogger().reset();" to the very
end of LoggerContext.stop(timeout, timeUnit).

However, I'm not sure that is a great idea -- all status messages after
that point would be lost.

Perhaps we want to have some logic that updates the LoggerContext to have
one StatusConsoleListener writing to stdout or stderr and remove the rest.
If there is a StatusConsoleListener, which there always is with the current
code, check if it is writing to a file and is so mutate it's destination to
stdout or stderr and close the file stream. Then remove any other
StatusListeners and it should cleanly shut down. The problem here is that
now some of the status messages went to a log file and some went to
standard out. I'm not sure that is avoidable.




On Thu, Feb 18, 2021 at 7:57 PM Ralph Goers 
wrote:

> This is a very good observation. When the status logger was implemented it
> targeted System.out, which you obviously never want to close. Shortly there
> after the ability to route it to a file instead was added. I guess we just
> never thought to add logic to close it at shutdown.
>
> Ralph
>
> > On Feb 18, 2021, at 7:01 PM, Tim Perry  wrote:
> >
> > It looks to me like LoggerContext.stop(...) closes the file 'test.log'.
> > However, I can't find anything closing 'status.log'. They are configured
> in
> > the same XML file: log4j-filetest.xml
> >
> > > name="XMLConfigTest">
> >  
> >target/test.log
> >...
> >
> >
> > I modified the @Test from FileOutputTest to see if it could delete the
> log
> > files in the test. On windows, neither file gets deleted and in both
> delete
> > attempts I get an IOException with the message "The process cannot access
> > the file because it is being used by another process.". On Ubuntu, no
> > exception is thrown even though the file is obviously open. See the code
> > below.
> >
> > From this I'm guessing that nothing closes status.log before the
> > FileCleaner goes to delete it and that it triggers an exception on
> Windows,
> > but not Linux. My guess is that the default file locking level is
> different
> > between JVM's or OS's. What should be releasing the file handle for
> > status.log? LoggerContext?
> >
> >
> >@Test
> >@LoggerContextSource("classpath:log4j-filetest.xml")
> >public void testConfig() throws IOException {
> >final Path logFile = Paths.get("target", "status.log");
> >assertTrue(Files.exists(logFile), "Status output file does not
> > exist");
> >assertTrue(Files.size(logFile) > 0, "File is empty");
> >
> >try {
> >Files.deleteIfExists(logFile);
> >fail("Should not have been able to delete " + logFile); //
> this
> > is called on Ubuntu, but not Windows
> >} catch (IOException ioe) {
> >// this is called for Windows,but not Ubuntu
> >System.err.println("THIS SHOULD FAIL: Failed to delete " +
> > logFile + ": " + ioe.getMessage());
> >}
> >final Path testLog = Paths.get("target", "test.log");
> >try {
> >Files.deleteIfExists(testLog);
> >fail("Should not have been able to delete " + testLog); //
> this
> > presumably would be called for Ubuntu, but we failed above.
> >} catch (IOException ioe) {
> >// this is called for Windows,but not Ubuntu
> >System.err.println("THIS SHOULD FAIL: Failed to delete " +
> > testLog + ": " + ioe.getMessage());
> >}
> >}
> >
> > On Thu, Feb 18, 2021 at 8:48 AM Matt Sicker  wrote:
> >
> >> The gist of what I recall the problem being was that sometimes, a file
> >> might still be in use asynchronously for various appenders (e.g.,
> >> during rolling, compression, etc.), so the shutdown and removal of
> >> temporary files needs to signal that the configuration needs to shut
> >> down. It looks like you may have found the missing link as to why this
> >> wasn't working properly. In that particular error message, that is
> >> some race condition where the temporary file is attempted to be
> >> deleted before the configuration stops.
> >>
> >> On Wed, 17 Feb 2021 at 18:19, Tim Perry  wrote:
> >>>
> >>> Matt,
> >>>
> >>> The problem is:

Re: What is going on with Master?

[Tim Perry]

I have a fork of log4j2 master that builds on windows here:
https://github.com/perry2of5/logging-log4j2

For log4j-core, I close the StatusLogger using
StatusLogger.getLogger().reset() to LoggerContext.stop(...). Now
log4j2-core tests pass. (Usually). I doubt this is the best solution, but
it shows that closing the logger does in fact allow the test to complete
correctly.
https://github.com/perry2of5/logging-log4j2/commit/9f5b083956ca7e5de8e7acc3b2e7397cb770b186

For log4j-layout-template-json, I had to change the hard-coded port in
JsonTemplateLayoutNullEventDelimiterTest in log4j-layout-template-json to
get the tests to run. I don't know why, but changing the port to 35514
worked.
https://github.com/perry2of5/logging-log4j2/commit/5ca4fba8aaa1879d48619d1f23631ae81c37ec6d

I had to update the expected XML in the Kafka ConfigurationBuilderTest to
match what windows outputs. Obviously we need to make this conditional
based on the OS it is running under...
https://github.com/perry2of5/logging-log4j2/commit/dec236a4d358b9847ad7710f668749241a1b50e5

On Fri, Feb 19, 2021 at 10:26 AM Tim Perry  wrote:

> The simplest fix is to add "StatusLogger.getLogger().reset();" to the very
> end of LoggerContext.stop(timeout, timeUnit).
>
> However, I'm not sure that is a great idea -- all status messages after
> that point would be lost.
>
> Perhaps we want to have some logic that updates the LoggerContext to have
> one StatusConsoleListener writing to stdout or stderr and remove the rest.
> If there is a StatusConsoleListener, which there always is with the current
> code, check if it is writing to a file and is so mutate it's destination to
> stdout or stderr and close the file stream. Then remove any other
> StatusListeners and it should cleanly shut down. The problem here is that
> now some of the status messages went to a log file and some went to
> standard out. I'm not sure that is avoidable.
>
>
>
>
> On Thu, Feb 18, 2021 at 7:57 PM Ralph Goers 
> wrote:
>
>> This is a very good observation. When the status logger was implemented
>> it targeted System.out, which you obviously never want to close. Shortly
>> there after the ability to route it to a file instead was added. I guess we
>> just never thought to add logic to close it at shutdown.
>>
>> Ralph
>>
>> > On Feb 18, 2021, at 7:01 PM, Tim Perry  wrote:
>> >
>> > It looks to me like LoggerContext.stop(...) closes the file 'test.log'.
>> > However, I can't find anything closing 'status.log'. They are
>> configured in
>> > the same XML file: log4j-filetest.xml
>> >
>> >> > name="XMLConfigTest">
>> >  
>> >target/test.log
>> >...
>> >
>> >
>> > I modified the @Test from FileOutputTest to see if it could delete the
>> log
>> > files in the test. On windows, neither file gets deleted and in both
>> delete
>> > attempts I get an IOException with the message "The process cannot
>> access
>> > the file because it is being used by another process.". On Ubuntu, no
>> > exception is thrown even though the file is obviously open. See the code
>> > below.
>> >
>> > From this I'm guessing that nothing closes status.log before the
>> > FileCleaner goes to delete it and that it triggers an exception on
>> Windows,
>> > but not Linux. My guess is that the default file locking level is
>> different
>> > between JVM's or OS's. What should be releasing the file handle for
>> > status.log? LoggerContext?
>> >
>> >
>> >@Test
>> >@LoggerContextSource("classpath:log4j-filetest.xml")
>> >public void testConfig() throws IOException {
>> >final Path logFile = Paths.get("target", "status.log");
>> >assertTrue(Files.exists(logFile), "Status output file does not
>> > exist");
>> >assertTrue(Files.size(logFile) > 0, "File is empty");
>> >
>> >try {
>> >Files.deleteIfExists(logFile);
>> >fail("Should not have been able to delete " + logFile); //
>> this
>> > is called on Ubuntu, but not Windows
>> >} catch (IOException ioe) {
>> >// this is called for Windows,but not Ubuntu
>> >System.err.println("THIS SHOULD FAIL: Failed to delete " +
>> > logFile + ": " + ioe.getMessage());
>> >}
>> >final Path testLog = Paths.get(&

Re: What is going on with Master?

[Tim Perry]

Thank you, Volkan.

I'm not quite ready to submit a PR. I was hoping some of you with more
knowledge of log4j-core would weigh in on what we should do about shutting
down the StatusLogger.

My thought is we choose one of two options:

Option A:
1) check if any StatusLogger is writing to standard out or standard error.
If not, add one.
2) stop any loggers that don't write to standard out or standard error.

Option B:
1) stop any loggers that don't write to standard out or standard error.

Option A could cause the log messages to be split across two destinations,
but they all get sent somewhere. Option B could lose shutdown messages when
writing to a file, but by that point it may not matter.

If any of you have a better idea, I'm happy to implement it. If nobody
weighs in on the best option, I'll probably submit Option A as a pull
request on Friday or Saturday.

Tim



On Tue, Feb 23, 2021 at 1:00 PM Volkan Yazıcı 
wrote:

> Great work Tim! I can't be happier finally somebody stepping up to fix
> sporadic build failures on Windows due to open file related issues. Would
> you mind creating a PR, please? (I haven't missed one, right?) This will
> make it easier to review your changes and merge them.
>
> On Mon, Feb 22, 2021 at 8:19 PM Tim Perry  wrote:
>
> > I have a fork of log4j2 master that builds on windows here:
> > https://github.com/perry2of5/logging-log4j2
> >
> > For log4j-core, I close the StatusLogger using
> > StatusLogger.getLogger().reset() to LoggerContext.stop(...). Now
> > log4j2-core tests pass. (Usually). I doubt this is the best solution, but
> > it shows that closing the logger does in fact allow the test to complete
> > correctly.
> >
> >
> https://github.com/perry2of5/logging-log4j2/commit/9f5b083956ca7e5de8e7acc3b2e7397cb770b186
> >
> > For log4j-layout-template-json, I had to change the hard-coded port in
> > JsonTemplateLayoutNullEventDelimiterTest in log4j-layout-template-json to
> > get the tests to run. I don't know why, but changing the port to 35514
> > worked.
> >
> >
> https://github.com/perry2of5/logging-log4j2/commit/5ca4fba8aaa1879d48619d1f23631ae81c37ec6d
> >
> > I had to update the expected XML in the Kafka ConfigurationBuilderTest to
> > match what windows outputs. Obviously we need to make this conditional
> > based on the OS it is running under...
> >
> >
> https://github.com/perry2of5/logging-log4j2/commit/dec236a4d358b9847ad7710f668749241a1b50e5
> >
> > On Fri, Feb 19, 2021 at 10:26 AM Tim Perry  wrote:
> >
> > > The simplest fix is to add "StatusLogger.getLogger().reset();" to the
> > very
> > > end of LoggerContext.stop(timeout, timeUnit).
> > >
> > > However, I'm not sure that is a great idea -- all status messages after
> > > that point would be lost.
> > >
> > > Perhaps we want to have some logic that updates the LoggerContext to
> have
> > > one StatusConsoleListener writing to stdout or stderr and remove the
> > rest.
> > > If there is a StatusConsoleListener, which there always is with the
> > current
> > > code, check if it is writing to a file and is so mutate it's
> destination
> > to
> > > stdout or stderr and close the file stream. Then remove any other
> > > StatusListeners and it should cleanly shut down. The problem here is
> that
> > > now some of the status messages went to a log file and some went to
> > > standard out. I'm not sure that is avoidable.
> > >
> > >
> > >
> > >
> > > On Thu, Feb 18, 2021 at 7:57 PM Ralph Goers <
> ralph.go...@dslextreme.com>
> > > wrote:
> > >
> > >> This is a very good observation. When the status logger was
> implemented
> > >> it targeted System.out, which you obviously never want to close.
> Shortly
> > >> there after the ability to route it to a file instead was added. I
> > guess we
> > >> just never thought to add logic to close it at shutdown.
> > >>
> > >> Ralph
> > >>
> > >> > On Feb 18, 2021, at 7:01 PM, Tim Perry  wrote:
> > >> >
> > >> > It looks to me like LoggerContext.stop(...) closes the file
> > 'test.log'.
> > >> > However, I can't find anything closing 'status.log'. They are
> > >> configured in
> > >> > the same XML file: log4j-filetest.xml
> > >> >
> > >> > > >> > name="XMLConfigTest">
> > >> >  
> > >> >

Re: What is going on with Master?

[Tim Perry]

Thank you for the feedback. I'll put in a pull request.

On Tue, Feb 23, 2021 at 1:29 PM Volkan Yazıcı 
wrote:

> I still would favor a PR – it greatly narrows down the scope one needs to
> wrap his/her head around. Not to mention the convenience of having the
> discussion "on the code".
>
> Ralph appears to be the best candidate to answer your questions. But again,
> a PR would incentivize others to chime in too.
>
> One final remark, don't forget to update changes.xml too.
>
> On Tue, Feb 23, 2021 at 10:22 PM Tim Perry  wrote:
>
> > Thank you, Volkan.
> >
> > I'm not quite ready to submit a PR. I was hoping some of you with more
> > knowledge of log4j-core would weigh in on what we should do about
> shutting
> > down the StatusLogger.
> >
> > My thought is we choose one of two options:
> >
> > Option A:
> > 1) check if any StatusLogger is writing to standard out or standard
> error.
> > If not, add one.
> > 2) stop any loggers that don't write to standard out or standard error.
> >
> > Option B:
> > 1) stop any loggers that don't write to standard out or standard error.
> >
> > Option A could cause the log messages to be split across two
> destinations,
> > but they all get sent somewhere. Option B could lose shutdown messages
> when
> > writing to a file, but by that point it may not matter.
> >
> > If any of you have a better idea, I'm happy to implement it. If nobody
> > weighs in on the best option, I'll probably submit Option A as a pull
> > request on Friday or Saturday.
> >
> > Tim
> >
> >
> >
> > On Tue, Feb 23, 2021 at 1:00 PM Volkan Yazıcı 
> > wrote:
> >
> > > Great work Tim! I can't be happier finally somebody stepping up to fix
> > > sporadic build failures on Windows due to open file related issues.
> Would
> > > you mind creating a PR, please? (I haven't missed one, right?) This
> will
> > > make it easier to review your changes and merge them.
> > >
> > > On Mon, Feb 22, 2021 at 8:19 PM Tim Perry  wrote:
> > >
> > > > I have a fork of log4j2 master that builds on windows here:
> > > > https://github.com/perry2of5/logging-log4j2
> > > >
> > > > For log4j-core, I close the StatusLogger using
> > > > StatusLogger.getLogger().reset() to LoggerContext.stop(...). Now
> > > > log4j2-core tests pass. (Usually). I doubt this is the best solution,
> > but
> > > > it shows that closing the logger does in fact allow the test to
> > complete
> > > > correctly.
> > > >
> > > >
> > >
> >
> https://github.com/perry2of5/logging-log4j2/commit/9f5b083956ca7e5de8e7acc3b2e7397cb770b186
> > > >
> > > > For log4j-layout-template-json, I had to change the hard-coded port
> in
> > > > JsonTemplateLayoutNullEventDelimiterTest in
> log4j-layout-template-json
> > to
> > > > get the tests to run. I don't know why, but changing the port to
> 35514
> > > > worked.
> > > >
> > > >
> > >
> >
> https://github.com/perry2of5/logging-log4j2/commit/5ca4fba8aaa1879d48619d1f23631ae81c37ec6d
> > > >
> > > > I had to update the expected XML in the Kafka
> ConfigurationBuilderTest
> > to
> > > > match what windows outputs. Obviously we need to make this
> conditional
> > > > based on the OS it is running under...
> > > >
> > > >
> > >
> >
> https://github.com/perry2of5/logging-log4j2/commit/dec236a4d358b9847ad7710f668749241a1b50e5
> > > >
> > > > On Fri, Feb 19, 2021 at 10:26 AM Tim Perry 
> wrote:
> > > >
> > > > > The simplest fix is to add "StatusLogger.getLogger().reset();" to
> the
> > > > very
> > > > > end of LoggerContext.stop(timeout, timeUnit).
> > > > >
> > > > > However, I'm not sure that is a great idea -- all status messages
> > after
> > > > > that point would be lost.
> > > > >
> > > > > Perhaps we want to have some logic that updates the LoggerContext
> to
> > > have
> > > > > one StatusConsoleListener writing to stdout or stderr and remove
> the
> > > > rest.
> > > > > If there is a StatusConsoleListener, which there always is with the
> > > > current
> > > > > code, check if it is writing to a file and is so mutate it's
> > &

Re: What is going on with Master?

[Tim Perry]

Ralph,

I implemented what you suggested. Feel free to suggest improvements.
https://github.com/apache/logging-log4j2/pull/469

Tim

On Tue, Feb 23, 2021 at 2:14 PM Ralph Goers 
wrote:

> I would suggest that if it is writing to something other than System.out
> that it be redirected back there and then the OutputStream be closed.
> However, I’ve not looked at the code recently so I am not sure what it
> takes to do that.
>
> Ralph
>
> > On Feb 23, 2021, at 2:22 PM, Tim Perry  wrote:
> >
> > Thank you, Volkan.
> >
> > I'm not quite ready to submit a PR. I was hoping some of you with more
> > knowledge of log4j-core would weigh in on what we should do about
> shutting
> > down the StatusLogger.
> >
> > My thought is we choose one of two options:
> >
> > Option A:
> > 1) check if any StatusLogger is writing to standard out or standard
> error.
> > If not, add one.
> > 2) stop any loggers that don't write to standard out or standard error.
> >
> > Option B:
> > 1) stop any loggers that don't write to standard out or standard error.
> >
> > Option A could cause the log messages to be split across two
> destinations,
> > but they all get sent somewhere. Option B could lose shutdown messages
> when
> > writing to a file, but by that point it may not matter.
> >
> > If any of you have a better idea, I'm happy to implement it. If nobody
> > weighs in on the best option, I'll probably submit Option A as a pull
> > request on Friday or Saturday.
> >
> > Tim
> >
>
>
>

Re: What is going on with Master?

[Tim Perry]

Ralph,

Thanks for the review. Yep, that *is* a problem...I knew it was a singleton
but didn't think through the use case you describe. This is ironic since a
few months ago I recommended that one of my clients bundle log4j in each
war rather than on Tomcat's classpath so there would be less chance of
instances walking on each other. Sigh.


What is the correct behaviour if:

   - log4j is on Tomcat's classpath
   - App A has status_A.log
   - App B has status_B.log

Now assume both apps are started. At this point I assume we should be
writing to both status_A.log and status_B.log. Now we stop App B. I assume
we should stop writing to status_B.log but not status_A.log. Further, I
assume that if both apps are unloaded from Tomcat, but Tomcat is left
running, then the status logger should send its messages to standard out.
If my assumptions are correct, then maybe we need to keep track of what
file, if any, each web app requested messages to be written to. On top of
that, I think we need a Callback in Log4j's shutdown registry and we need
to run it last.


In some ways this seems like an XY problem. Is the correct question how do
we reconfigure the logging when a web app shuts down? Or should it be:
should the StatusLogger be shared across multiple LoggerContexts?


This will be more interesting than I first realized!

Tim


On Tue, Feb 23, 2021 at 10:38 PM Ralph Goers 
wrote:

> Yeah, I started a review but then I thought it probably would be better to
> respond here.
>
> You are on the right track but there is a problem. StatusLogger is a
> singleton - there is one instance anchored in a static. You are invoking
> the shutdown logic from the shutdown of the LoggerContext which is not a
> singleton. Log4j supports multiple LoggerContexts in an application. For
> example, if you are old school and running multiple web applications in
> Tomcat and have Log4j on Tomcat’s class path then you will have multiple
> LoggerContexts with a single StatusLogger. So if one web app gets
> redeployed then its LoggerContext will be shutdown and a new one created
> all while another app is continuing to run.
>
> If you’ll notice the StatusConfiguration class in log4j-core tries to
> accommodate for this during startup, but it doesn’t do anything at
> shutdown. StatusLogger currently isn’t smart enough to handle one app
> writing to one destination and a different on writing to a different one.
> Since StatusLogger is a singleton it can’t really know which app a status
> log event is for.
>
> There are a couple of ways I can think of to handle this but none of them
> is perfect.
> Modify StatusConfiguration to keep track of what each StatusConfiguration
> set up and reset to whatever the prior StatusConfiguration had. The problem
> with this is that applications might shutdown in a different order than
> they were started, so figuring out what the prior configuration was could
> be difficult.
> Add the call to prepareToStop() as a new Callback to Log4j’s shutdown
> registry. However, this callback would need to run last. The shutdown
> registry currently doesn’t support a way to specify the order of callbacks.
> Support for that would need to be added for this to work.
>
> Ralph
>
> > On Feb 23, 2021, at 10:48 PM, Tim Perry  wrote:
> >
> > Ralph,
> >
> > I implemented what you suggested. Feel free to suggest improvements.
> > https://github.com/apache/logging-log4j2/pull/469
> >
> > Tim
> >
> > On Tue, Feb 23, 2021 at 2:14 PM Ralph Goers 
> > wrote:
> >
> >> I would suggest that if it is writing to something other than System.out
> >> that it be redirected back there and then the OutputStream be closed.
> >> However, I’ve not looked at the code recently so I am not sure what it
> >> takes to do that.
> >>
> >> Ralph
> >>
> >>> On Feb 23, 2021, at 2:22 PM, Tim Perry  wrote:
> >>>
> >>> Thank you, Volkan.
> >>>
> >>> I'm not quite ready to submit a PR. I was hoping some of you with more
> >>> knowledge of log4j-core would weigh in on what we should do about
> >> shutting
> >>> down the StatusLogger.
> >>>
> >>> My thought is we choose one of two options:
> >>>
> >>> Option A:
> >>> 1) check if any StatusLogger is writing to standard out or standard
> >> error.
> >>> If not, add one.
> >>> 2) stop any loggers that don't write to standard out or standard error.
> >>>
> >>> Option B:
> >>> 1) stop any loggers that don't write to standard out or standard error.
> >>>
> >>> Option A could cause the log messages to be split across two
> >> destinations,
> >>> but they all get sent somewhere. Option B could lose shutdown messages
> >> when
> >>> writing to a file, but by that point it may not matter.
> >>>
> >>> If any of you have a better idea, I'm happy to implement it. If nobody
> >>> weighs in on the best option, I'll probably submit Option A as a pull
> >>> request on Friday or Saturday.
> >>>
> >>> Tim
> >>>
> >>
> >>
> >>
>
>

Re: [apache/logging-log4j2] MapMessage put methods should not mandate String values (#477)

[Tim Perry]

Would we also have
   public String getObject(String) {...}

What will happen when someone calls get and the object that key is mapped
to is not a String? Does it return null? Throw an exception?

Instead of allowing non-string values maybe change the type of data to
Map instead of  and avoid the mess?


On Thu, Mar 25, 2021 at 8:02 AM Gary Gregory  wrote:

> The 2 postfix gives me flashbacks of Microsoft COM interfaces! :-p
> putObject is less shudder inducing than put2... for me at least.
>
> Gary
>
>
> On Thu, Mar 25, 2021, 01:08 Remko Popma  wrote:
>
> > Haha!
> > putObject?
> >
> >
> >
> > > On Mar 25, 2021, at 11:39, Ralph Goers 
> > wrote:
> > >
> > > I’m sure that will drive Gary nuts.  Let’s call the new method
> “put2()”.
> > >
> > > Ralph
> > >
> > >> On Mar 24, 2021, at 5:18 PM, Remko Popma 
> wrote:
> > >>
> > >> Instead of overloading the existing method(s), how about adding new
> > methods
> > >> with a slightly different name that takes Object parameters?
> > >>
> > >>> On Thu, Mar 25, 2021 at 8:46 AM Carter Kozak 
> > wrote:
> > >>>
> > >>> The method argument type changes are an ABI break, but I recall
> > extending
> > >>> MapMessage within a project in order to support more expressive types
> > --
> > >>> that
> > >>> may have relied on dangerously casting the result of
> > >>> getIndexedReadOnlyStringMap
> > >>> to an IndexedStringMap.
> > >>> Including a safer Object-valued MapMessage subclass (with overloaded
> > put
> > >>> methods)
> > >>> sounds reasonable to me given at least two of us have run into this!
> > >>>
> > >>> -Carter
> > >>>
> > >>> On Wed, Mar 24, 2021, at 19:29, Remko Popma wrote:
> >  I called it StringMap because the keys must be Strings. Admittedly
> > not a
> >  great name. :-)
> > 
> >  Not sure exactly, but there may be cases where this change could
> > cause an
> >  issue:
> >  putAll(final Map map) ->
> >  putAll(final Map map)
> > 
> >  On Thu, Mar 25, 2021 at 2:12 AM Ralph Goers <
> > ralph.go...@dslextreme.com
> > >>> >
> >  wrote:
> > 
> > > I looked the other day and wondered the same thing Volkan did.
> There
> > >>> are
> > > no unit tests and the contributor didn’t even indicate that he had
> > >>> tried
> > > it.
> > >
> > > I was initially concerned that the underlying Map wouldn’t support
> it
> > > since it has StringMap in its name. It turns out the values are
> > >>> objects.
> > >
> > > Technically I don’t think this would break compatibility. Any code
> > > referencing the put(String, String) would automatically map to
> > >>> put(String,
> > > Object). He didn’t modify the get method which would have broken
> > > compatibility.
> > >
> > > Ralph
> > >
> > >> On Mar 24, 2021, at 8:27 AM, Matt Sicker   > >>> boards%40gmail.com>> wrote:
> > >>
> > >> Pretty sure that would break binary compatibility since it removes
> > >>> the
> > >> String method. I think it might be addable but not removed like
> > that.
> > >>
> > >> On Wed, 24 Mar 2021 at 02:39, Volkan Yazıcı <
> > volkan.yaz...@gmail.com
> > >>> >
> > > wrote:
> > >>>
> > >>> Hello,
> > >>>
> > >>> Adding non-String-typed value support to MapMessage was also
> > >>> something
> > > on
> > >>> my radar too. But this PR replacing String with Object in two
> lines
> > > seems
> > >>> too good to be true to me. Does anybody mind taking a second look
> > at
> > > this,
> > >>> please?
> > >>>
> > >>> Kind regards.
> > >>>
> > >>> -- Forwarded message -
> > >>> From: Henry Widd  > >>> notifications%40github.com>>
> > >>> Date: Tue, Mar 23, 2021 at 4:58 PM
> > >>> Subject: [apache/logging-log4j2] MapMessage put methods should
> not
> > > mandate
> > >>> String values (#477)
> > >>> To: apache/logging-log4j2  > >>> >
> > >>> Cc: Subscribed  > >>> subscribed%40noreply.github.com>>
> > >>>
> > >>>
> > >>> the underlying Map is typed  so the put methods on
> > >>> MapMessage can also be.
> > >>> --
> > >>> You can view, comment on, or merge this pull request online at:
> > >>>
> > >>> https://github.com/apache/logging-log4j2/pull/477
> > >>> Commit Summary
> > >>>
> > >>> - MapMessage put methods should not mandate String values
> > >>>
> > >>> File Changes
> > >>>
> > >>> - *M*
> > >>>
> > >
> > >>>
> > log4j-api/src/main/java/org/apache/logging/log4j/message/MapMessage.java
> > >>> <
> > >
> > >>>
> >
> https://github.com/apache/logging-log4j2/pull/477/files#diff-f03ffe9ceefd37c87fd118ce591bd8ad288e43b08cd663dde14441f4e7c117ef
> > >>
> > >>> (6)
> > >>>
> > >>> Patch Links:
> > >>>
> > >>> - https://github.com/apache/logging-log4j

Re: [apache/logging-log4j2] MapMessage put methods should not mandate String values (#477)

[Tim Perry]

If we are only going to allow strings in the map, then we don't need a new
method.

If we are going to allow other values in the map, then we would need a new
method.

Maybe I lost the plot, but I don't see the point of allowing people to put
non-string objects into the map unless they can get non-string objects back.

On Thu, Mar 25, 2021 at 10:51 AM Ralph Goers 
wrote:

> The existing get method does a deep toString on the value object already.
> I’m not sure why we would need a new method if the return value is a String.
>
> Ralph
>
> > On Mar 25, 2021, at 10:28 AM, Tim Perry  wrote:
> >
> > Would we also have
> >   public String getObject(String) {...}
> >
> > What will happen when someone calls get and the object that key is mapped
> > to is not a String? Does it return null? Throw an exception?
> >
> > Instead of allowing non-string values maybe change the type of data to
> > Map instead of  and avoid the mess?
> >
> >
> > On Thu, Mar 25, 2021 at 8:02 AM Gary Gregory 
> wrote:
> >
> >> The 2 postfix gives me flashbacks of Microsoft COM interfaces! :-p
> >> putObject is less shudder inducing than put2... for me at least.
> >>
> >> Gary
> >>
> >>
> >> On Thu, Mar 25, 2021, 01:08 Remko Popma  wrote:
> >>
> >>> Haha!
> >>> putObject?
> >>>
> >>>
> >>>
> >>>> On Mar 25, 2021, at 11:39, Ralph Goers 
> >>> wrote:
> >>>>
> >>>> I’m sure that will drive Gary nuts.  Let’s call the new method
> >> “put2()”.
> >>>>
> >>>> Ralph
> >>>>
> >>>>> On Mar 24, 2021, at 5:18 PM, Remko Popma 
> >> wrote:
> >>>>>
> >>>>> Instead of overloading the existing method(s), how about adding new
> >>> methods
> >>>>> with a slightly different name that takes Object parameters?
> >>>>>
> >>>>>> On Thu, Mar 25, 2021 at 8:46 AM Carter Kozak 
> >>> wrote:
> >>>>>>
> >>>>>> The method argument type changes are an ABI break, but I recall
> >>> extending
> >>>>>> MapMessage within a project in order to support more expressive
> types
> >>> --
> >>>>>> that
> >>>>>> may have relied on dangerously casting the result of
> >>>>>> getIndexedReadOnlyStringMap
> >>>>>> to an IndexedStringMap.
> >>>>>> Including a safer Object-valued MapMessage subclass (with overloaded
> >>> put
> >>>>>> methods)
> >>>>>> sounds reasonable to me given at least two of us have run into this!
> >>>>>>
> >>>>>> -Carter
> >>>>>>
> >>>>>> On Wed, Mar 24, 2021, at 19:29, Remko Popma wrote:
> >>>>>>> I called it StringMap because the keys must be Strings. Admittedly
> >>> not a
> >>>>>>> great name. :-)
> >>>>>>>
> >>>>>>> Not sure exactly, but there may be cases where this change could
> >>> cause an
> >>>>>>> issue:
> >>>>>>> putAll(final Map map) ->
> >>>>>>> putAll(final Map map)
> >>>>>>>
> >>>>>>> On Thu, Mar 25, 2021 at 2:12 AM Ralph Goers <
> >>> ralph.go...@dslextreme.com
> >>>>>> <mailto:ralph.goers%40dslextreme.com>>
> >>>>>>> wrote:
> >>>>>>>
> >>>>>>>> I looked the other day and wondered the same thing Volkan did.
> >> There
> >>>>>> are
> >>>>>>>> no unit tests and the contributor didn’t even indicate that he had
> >>>>>> tried
> >>>>>>>> it.
> >>>>>>>>
> >>>>>>>> I was initially concerned that the underlying Map wouldn’t support
> >> it
> >>>>>>>> since it has StringMap in its name. It turns out the values are
> >>>>>> objects.
> >>>>>>>>
> >>>>>>>> Technically I don’t think this would break compatibility. Any code
> >>>>>>>> referencing the put(String, String) would automatically map to
> >>>>>> put(String,
> >>>>>>>> Object). He did

Re: [apache/logging-log4j2] MapMessage put methods should not mandate String values (#477)

[Tim Perry]

Oh, I see. I meant to have it return an object. I should have written:
Object getObject(String key)

I was meaning to ask: if we allow putting Object instead of just String
into the map, then shouldn't we provide a way to get the object out?

Sorry about my poor proofreading.


On Thu, Mar 25, 2021 at 12:17 PM Ralph Goers 
wrote:

> You getObject method below returns a String. That is why I said the method
> isn’t needed.
>
> Ralph
>
> > On Mar 25, 2021, at 11:31 AM, Tim Perry  wrote:
> >
> > If we are only going to allow strings in the map, then we don't need a
> new
> > method.
> >
> > If we are going to allow other values in the map, then we would need a
> new
> > method.
> >
> > Maybe I lost the plot, but I don't see the point of allowing people to
> put
> > non-string objects into the map unless they can get non-string objects
> back.
> >
> > On Thu, Mar 25, 2021 at 10:51 AM Ralph Goers  >
> > wrote:
> >
> >> The existing get method does a deep toString on the value object
> already.
> >> I’m not sure why we would need a new method if the return value is a
> String.
> >>
> >> Ralph
> >>
> >>> On Mar 25, 2021, at 10:28 AM, Tim Perry  wrote:
> >>>
> >>> Would we also have
> >>>  public String getObject(String) {...}
> >>>
> >>> What will happen when someone calls get and the object that key is
> mapped
> >>> to is not a String? Does it return null? Throw an exception?
> >>>
> >>> Instead of allowing non-string values maybe change the type of data to
> >>> Map instead of  and avoid the mess?
> >>>
> >>>
> >>> On Thu, Mar 25, 2021 at 8:02 AM Gary Gregory 
> >> wrote:
> >>>
> >>>> The 2 postfix gives me flashbacks of Microsoft COM interfaces! :-p
> >>>> putObject is less shudder inducing than put2... for me at least.
> >>>>
> >>>> Gary
> >>>>
> >>>>
> >>>> On Thu, Mar 25, 2021, 01:08 Remko Popma 
> wrote:
> >>>>
> >>>>> Haha!
> >>>>> putObject?
> >>>>>
> >>>>>
> >>>>>
> >>>>>> On Mar 25, 2021, at 11:39, Ralph Goers 
> >>>>> wrote:
> >>>>>>
> >>>>>> I’m sure that will drive Gary nuts.  Let’s call the new method
> >>>> “put2()”.
> >>>>>>
> >>>>>> Ralph
> >>>>>>
> >>>>>>> On Mar 24, 2021, at 5:18 PM, Remko Popma 
> >>>> wrote:
> >>>>>>>
> >>>>>>> Instead of overloading the existing method(s), how about adding new
> >>>>> methods
> >>>>>>> with a slightly different name that takes Object parameters?
> >>>>>>>
> >>>>>>>> On Thu, Mar 25, 2021 at 8:46 AM Carter Kozak 
> >>>>> wrote:
> >>>>>>>>
> >>>>>>>> The method argument type changes are an ABI break, but I recall
> >>>>> extending
> >>>>>>>> MapMessage within a project in order to support more expressive
> >> types
> >>>>> --
> >>>>>>>> that
> >>>>>>>> may have relied on dangerously casting the result of
> >>>>>>>> getIndexedReadOnlyStringMap
> >>>>>>>> to an IndexedStringMap.
> >>>>>>>> Including a safer Object-valued MapMessage subclass (with
> overloaded
> >>>>> put
> >>>>>>>> methods)
> >>>>>>>> sounds reasonable to me given at least two of us have run into
> this!
> >>>>>>>>
> >>>>>>>> -Carter
> >>>>>>>>
> >>>>>>>> On Wed, Mar 24, 2021, at 19:29, Remko Popma wrote:
> >>>>>>>>> I called it StringMap because the keys must be Strings.
> Admittedly
> >>>>> not a
> >>>>>>>>> great name. :-)
> >>>>>>>>>
> >>>>>>>>> Not sure exactly, but there may be cases where this change could
> >>>>> cause an
> >>>>>>>>> issue:
> >>>>>>>>> putAll(final Map map) ->
> >>>>>>&g

Re: Please do not commit to log4j2 master - but review this PR.

[Tim Perry]

Looks okay on Windows. I built on Windows 10 with Zulu Java 8 / 11 in my
toolchain and Maven 3.6.3 and it succeeded after I disabled the usual
culprits:
  * log4j-core fails due to the file locking issue with the status logger
  * log4j-layout-template-json failed with a port unavailable issue.

I encountered both before the changes in pull 480.

Also, as I commented on the pull request, there is an OSGI warning.

[WARNING] * Required filename-based automodules detected:
[org.osgi.core-6.0.0.jar]. Please don't publish this project to a public
artifact repository!

I hope that helps!

Tim


On Thu, Apr 1, 2021 at 12:59 AM Apache  wrote:

> Actually, they don’t disappear. They just move to closed status. I believe
> you can still comment but it is far less noticeable after it is merged.
> Really, I suggest you check out the branch and build it. I really didn’t
> change much code. I mostly moved it around and changed whatever was needed
> to make log4j-api and Log4J-plugins real JPMS modules in Java 11.
>
> Ralph
>
> > On Mar 31, 2021, at 11:57 PM, Volkan Yazıcı 
> wrote:
> >
> > I am still reviewing it.
> > As Matt noted, you can merge it.
> > This said, I'd appreciate an extra week.
> > It is easier to discuss changes over GitHub PR.
> > Once committed, PR will disappear.
> >
> >> On Thu, Apr 1, 2021 at 5:58 AM Ralph Goers 
> >> wrote:
> >>
> >> FYI - I plan on merging this code Friday morning MST unless my schedule
> >> changes between now and then.
> >>
> >> Ralph
> >>
> >>> On Mar 29, 2021, at 3:58 PM, Ralph Goers 
> >> wrote:
> >>>
> >>> I should have added that you may need a recent version of the JDK. I
> >> forget what error I was encountering but upgrading the JDK to a later
> >> version fixed it. But then I noticed that the Google java allocation
> >> instrumenter wasn’t working and it had to be upgraded too.
> >>>
> >>> Ralph
> >>>
>  On Mar 29, 2021, at 3:51 PM, Matt Sicker  wrote:
> 
>  I’ll make sure to look more closely at it this week. Nice work on
>  simplifying the modules a bit!
> 
>  On Sun, Mar 28, 2021 at 18:24 Ralph Goers  >
>  wrote:
> 
> > I have created https://github.com/apache/logging-log4j2/pull/480 for
> >> you
> > to review. It has many changes and merge conflicts will be painful to
> >> fix
> > so please do not commit to master until this PR is merged.
> >
> > Although I could merge this now I would prefer if you could checkout
> >> the
> > branch on your local machines, build, and test it. I haven’t tested
> it
> >> with
> > anything real yet but all the unit tests - except for the osgi
> module -
> > pass.
> >
> > If you open this in your IDE you might have some issues with some
> test
> > classes being flagged as having compile issues. This is because of
> the
> > weird extra directory I had to include in log4j-api and log4j-plugins
> >> to
> > create test jars.
> >
> > Please provide feedback so I can make any changes and get this
> merged.
> >
> > Ralph
> >
> >>>
> >>>
> >>>
> >>
> >>
> >>
>
>
>

Re: What is going on with Master?

[Tim Perry]

I rewrote this to shut down listeners based on the contextName. In testing,
I discovered that the StatusConsoleListener is created in
StatusConfiguration, but neither StatusConfiguration nor
StatusConsoleListener receive events to indicate when they should stop.

It appears that only one StatusConsoleListener object is ever created and
it is never shut down. Looking at the api XmlConfiguration, it calls
StatusConfiguration.initilize() which then either changes the log level to
match the config being parsed or creates a new StatusLogger directed to the
file indicated in the XML configuration. Unless I'm reading the code wrong,
this means that the status logger output location depends on if a previous
app was loaded. If so, then that location will continue to receive
StatusLogger messages but at the log level of the new application's config.
Am I reading this correctly? If I am, is this the intended behaviour?



On Wed, Feb 24, 2021 at 8:29 AM Matt Sicker  wrote:

> The StatusLogger has various listeners attached. I think adding and
> removing listeners on startup and shutdown of a LoggerContext might be
> a potential way to do this?
>
> On Wed, 24 Feb 2021 at 01:07, Tim Perry  wrote:
> >
> > Ralph,
> >
> > Thanks for the review. Yep, that *is* a problem...I knew it was a
> singleton
> > but didn't think through the use case you describe. This is ironic since
> a
> > few months ago I recommended that one of my clients bundle log4j in each
> > war rather than on Tomcat's classpath so there would be less chance of
> > instances walking on each other. Sigh.
> >
> >
> > What is the correct behaviour if:
> >
> >- log4j is on Tomcat's classpath
> >- App A has status_A.log
> >- App B has status_B.log
> >
> > Now assume both apps are started. At this point I assume we should be
> > writing to both status_A.log and status_B.log. Now we stop App B. I
> assume
> > we should stop writing to status_B.log but not status_A.log. Further, I
> > assume that if both apps are unloaded from Tomcat, but Tomcat is left
> > running, then the status logger should send its messages to standard out.
> > If my assumptions are correct, then maybe we need to keep track of what
> > file, if any, each web app requested messages to be written to. On top of
> > that, I think we need a Callback in Log4j's shutdown registry and we need
> > to run it last.
> >
> >
> > In some ways this seems like an XY problem. Is the correct question how
> do
> > we reconfigure the logging when a web app shuts down? Or should it be:
> > should the StatusLogger be shared across multiple LoggerContexts?
> >
> >
> > This will be more interesting than I first realized!
> >
> > Tim
> >
> >
> > On Tue, Feb 23, 2021 at 10:38 PM Ralph Goers  >
> > wrote:
> >
> > > Yeah, I started a review but then I thought it probably would be
> better to
> > > respond here.
> > >
> > > You are on the right track but there is a problem. StatusLogger is a
> > > singleton - there is one instance anchored in a static. You are
> invoking
> > > the shutdown logic from the shutdown of the LoggerContext which is not
> a
> > > singleton. Log4j supports multiple LoggerContexts in an application.
> For
> > > example, if you are old school and running multiple web applications in
> > > Tomcat and have Log4j on Tomcat’s class path then you will have
> multiple
> > > LoggerContexts with a single StatusLogger. So if one web app gets
> > > redeployed then its LoggerContext will be shutdown and a new one
> created
> > > all while another app is continuing to run.
> > >
> > > If you’ll notice the StatusConfiguration class in log4j-core tries to
> > > accommodate for this during startup, but it doesn’t do anything at
> > > shutdown. StatusLogger currently isn’t smart enough to handle one app
> > > writing to one destination and a different on writing to a different
> one.
> > > Since StatusLogger is a singleton it can’t really know which app a
> status
> > > log event is for.
> > >
> > > There are a couple of ways I can think of to handle this but none of
> them
> > > is perfect.
> > > Modify StatusConfiguration to keep track of what each
> StatusConfiguration
> > > set up and reset to whatever the prior StatusConfiguration had. The
> problem
> > > with this is that applications might shutdown in a different order than
> > > they were started, so figuring out what the prior configuration was
> could
> > > 

Re: What is going on with Master?

[Tim Perry]

After further thought, I am threading the context name into the location
where the StatusConfiguration creates the StatusConsoleListener and
registering the context name there.

In addition, if the new logger would write to a destination other than
standard out or standard error then I do not reconfigure the existing
logger in StatusConfiguration.configureExistingStatusConsoleListener(),
instead I have the

I am now correctly closing the status logger when the context is stopped.

I'll push the changes to github after I do a full build

On Mon, Apr 19, 2021 at 12:17 PM Tim Perry  wrote:

> I rewrote this to shut down listeners based on the contextName. In
> testing, I discovered that the StatusConsoleListener is created in
> StatusConfiguration, but neither StatusConfiguration nor
> StatusConsoleListener receive events to indicate when they should stop.
>
> It appears that only one StatusConsoleListener object is ever created and
> it is never shut down. Looking at the api XmlConfiguration, it calls
> StatusConfiguration.initilize() which then either changes the log level to
> match the config being parsed or creates a new StatusLogger directed to the
> file indicated in the XML configuration. Unless I'm reading the code wrong,
> this means that the status logger output location depends on if a previous
> app was loaded. If so, then that location will continue to receive
> StatusLogger messages but at the log level of the new application's config.
> Am I reading this correctly? If I am, is this the intended behaviour?
>
>
>
> On Wed, Feb 24, 2021 at 8:29 AM Matt Sicker  wrote:
>
>> The StatusLogger has various listeners attached. I think adding and
>> removing listeners on startup and shutdown of a LoggerContext might be
>> a potential way to do this?
>>
>> On Wed, 24 Feb 2021 at 01:07, Tim Perry  wrote:
>> >
>> > Ralph,
>> >
>> > Thanks for the review. Yep, that *is* a problem...I knew it was a
>> singleton
>> > but didn't think through the use case you describe. This is ironic
>> since a
>> > few months ago I recommended that one of my clients bundle log4j in each
>> > war rather than on Tomcat's classpath so there would be less chance of
>> > instances walking on each other. Sigh.
>> >
>> >
>> > What is the correct behaviour if:
>> >
>> >- log4j is on Tomcat's classpath
>> >- App A has status_A.log
>> >- App B has status_B.log
>> >
>> > Now assume both apps are started. At this point I assume we should be
>> > writing to both status_A.log and status_B.log. Now we stop App B. I
>> assume
>> > we should stop writing to status_B.log but not status_A.log. Further, I
>> > assume that if both apps are unloaded from Tomcat, but Tomcat is left
>> > running, then the status logger should send its messages to standard
>> out.
>> > If my assumptions are correct, then maybe we need to keep track of what
>> > file, if any, each web app requested messages to be written to. On top
>> of
>> > that, I think we need a Callback in Log4j's shutdown registry and we
>> need
>> > to run it last.
>> >
>> >
>> > In some ways this seems like an XY problem. Is the correct question how
>> do
>> > we reconfigure the logging when a web app shuts down? Or should it be:
>> > should the StatusLogger be shared across multiple LoggerContexts?
>> >
>> >
>> > This will be more interesting than I first realized!
>> >
>> > Tim
>> >
>> >
>> > On Tue, Feb 23, 2021 at 10:38 PM Ralph Goers <
>> ralph.go...@dslextreme.com>
>> > wrote:
>> >
>> > > Yeah, I started a review but then I thought it probably would be
>> better to
>> > > respond here.
>> > >
>> > > You are on the right track but there is a problem. StatusLogger is a
>> > > singleton - there is one instance anchored in a static. You are
>> invoking
>> > > the shutdown logic from the shutdown of the LoggerContext which is
>> not a
>> > > singleton. Log4j supports multiple LoggerContexts in an application.
>> For
>> > > example, if you are old school and running multiple web applications
>> in
>> > > Tomcat and have Log4j on Tomcat’s class path then you will have
>> multiple
>> > > LoggerContexts with a single StatusLogger. So if one web app gets
>> > > redeployed then its LoggerContext will be shutdown and a new one
>> created
>> > > all while another app is cont

Re: Running "master" tests in IntelliJ IDEA after Java 11 upgrade

[Tim Perry]

Can any of you recommend an idiots guide to setting up a log4j development
environment in IntelliJ? I have never used IntelliJ before. Eclipse won't
work with a maven project with multiple module-info.java files. There are
bugs into the maven-eclipse tools (m2e) for this.

I cannot see reverting to 1999 and using the command line debugger, so
IntelliJ seems like my best shot at getting a working development
environment. I just want autocomplete and the ability to connect a debugger
without too much hoop jumping.

Thanks,
Tim

On Thu, Apr 8, 2021 at 5:17 AM Volkan Yazıcı 
wrote:

> I have also tried that too, but no luck so far. @Matt, are you able to run
> any tests from IDEA using the most recent "master"?
>
> On Wed, Apr 7, 2021 at 5:57 PM Matt Sicker  wrote:
>
> > See also
> >
> https://lists.apache.org/thread.html/2ba2225043a6ca7d2c43e4293323309b041bd8d486516cc50fec61cd%40%3Cdev.logging.apache.org%3E
> >
> > On Wed, 7 Apr 2021 at 09:17, Ralph Goers 
> > wrote:
> > >
> > > mvn surefire:test -Dtest=TestClass -Dmaven.surefire.debug=“remote debug
> > options”
> > >
> > > Ralph
> > >
> > > > On Apr 7, 2021, at 7:04 AM, Volkan Yazıcı 
> > wrote:
> > > >
> > > > Okay, fair enough. Thanks so much for sharing these details, I
> really
> > > > appreciated it. One final question: If you want to debug a single
> > test, how
> > > > does your work flow look like? Do you create a new "Run
> Configuration"
> > > > invoking a certain Maven command triggering the code/test that you
> are
> > > > interested in and "Debug Run"ing that "Run Configuration"?
> > > >
> > > > For the records, I still would like to hear how I can make IDEA to
> > simply
> > > > run a test. If anybody has succeeded in doing that, I am all ears.
> > > >
> > > >> On Wed, Apr 7, 2021 at 3:38 PM Apache 
> > wrote:
> > > >>
> > > >> I will be honest. I have never tun any log4j tests in IntelliJ. I
> > rarely
> > > >> do it for any projects I work on. I use JVM remote debug all the
> > time. I
> > > >> don’t want to force others to have to do that, but I just never
> think
> > about
> > > >> it.
> > > >>
> > > >> I use various versions of Maven from time to time. 3.6.1 is the
> > default on
> > > >> my Mac but I just installed 3.8.1 to validate what I needed to
> change
> > in my
> > > >> setup to make it still work with my employers Nexus repository which
> > still
> > > >> uses http.
> > > >>
> > > >> When I am working on Log4J stuff I do a full mvn clean install
> several
> > > >> times a day. That takes a huge amount of time so I have learned to
> > > >> multitask and work on other stuff while builds are running.
> > > >>
> > > >> Ralph.
> > > >>
> > > >>> On Apr 7, 2021, at 12:15 AM, Volkan Yazıcı <
> volkan.yaz...@gmail.com>
> > > >> wrote:
> > > >>>
> > > >>> Ralph, when you delete the IDEA-specific configuration (i.e.,
> *.iml
> > > >> files
> > > >>> and .idea directory), compile the sources via Maven, and open the
> > folder
> > > >> in
> > > >>> IDEA, can you run *any* tests? If so, do you have any custom IDEA
> > > >>> configurations? Which IDEA version are you using? If you are not
> > using
> > > >> the
> > > >>> wrapper, which Maven version are you using?
> > > >>>
> > > >>> I use Maven Wrapper to make sure Maven behaves the same in all
> > > >>> environments, independent of my local setup. It also saves you
> from a
> > > >> local
> > > >>> Maven dependency.
> > > >>>
> > >  On Wed, Apr 7, 2021 at 1:17 AM Ralph Goers <
> > ralph.go...@dslextreme.com>
> > >  wrote:
> > > 
> > >  I deleted the files from my local repo and restarted the build. It
> > is
> > >  running along just fine - at least until it hits json template
> > layout.
> > > 
> > >  Is there a reason you use the maven wrapper instead of Maven
> > itself? I
> > >  have never used the wrapper. I am wondering if there is something
> > going
> > > >> on
> > >  there.
> > > 
> > >  Ralph
> > > 
> > > > On Apr 6, 2021, at 4:10 PM, Ralph Goers <
> > ralph.go...@dslextreme.com>
> > >  wrote:
> > > >
> > > > I’ve never seen that. What is
> > maven-annotations-production:log4j-api?
> > > >
> > > > Let me try removing the files from my maven local repo as you
> did.
> > > >
> > > > Ralph
> > > >
> > > >> On Apr 6, 2021, at 1:14 PM, Volkan Yazıcı <
> > volkan.yaz...@gmail.com>
> > >  wrote:
> > > >>
> > > >> As subject hints, I am not able to run tests in IDEA anymore
> after
> > > >> Java
> > >  11
> > > >> upgrade. I have deleted all IDEA related files and issued a
> clean
> > > >> Maven
> > > >> build:
> > > >>
> > > >> $ rm -rf ./.idea ./**/*.iml
> > > >> $ rm -rf ~/.m2/repository/org/apache/logging/log4j/*/3*-SNAPSHOT
> > > >> $ ./mvnw clean install -DskipTests=true
> > > >>
> > > >> Opened the directory using IDEA (2021.1 RC), but couldn't get it
> > to
> > > >> have
> > > >> successful build:
> > > >>
> > > >> W: Output path
> > >

Re: Running "master" tests in IntelliJ IDEA after Java 11 upgrade

[Tim Perry]

I deleted all log4j artifacts in my .m2 directory and I'm no longer getting
the "circular dependency" that Volkan reported.

I checked the box for "Delegate IDE build/run actions to Maven" like Matt
recommended and was able to build which went as far as maven on the command
line does.

Now I'm trying to run a unit test by clicking the arrow next to the test
and saying run. However, IntelliJ starts a maven build and runs all the
tests for log4j-core (which contains the test I am trying to run).

Ideas?

Thanks,
Tim


On Mon, Apr 19, 2021 at 3:34 PM Tim Perry  wrote:

> Can any of you recommend an idiots guide to setting up a log4j development
> environment in IntelliJ? I have never used IntelliJ before. Eclipse won't
> work with a maven project with multiple module-info.java files. There are
> bugs into the maven-eclipse tools (m2e) for this.
>
> I cannot see reverting to 1999 and using the command line debugger, so
> IntelliJ seems like my best shot at getting a working development
> environment. I just want autocomplete and the ability to connect a debugger
> without too much hoop jumping.
>
> Thanks,
> Tim
>
> On Thu, Apr 8, 2021 at 5:17 AM Volkan Yazıcı 
> wrote:
>
>> I have also tried that too, but no luck so far. @Matt, are you able to run
>> any tests from IDEA using the most recent "master"?
>>
>> On Wed, Apr 7, 2021 at 5:57 PM Matt Sicker  wrote:
>>
>> > See also
>> >
>> https://lists.apache.org/thread.html/2ba2225043a6ca7d2c43e4293323309b041bd8d486516cc50fec61cd%40%3Cdev.logging.apache.org%3E
>> >
>> > On Wed, 7 Apr 2021 at 09:17, Ralph Goers 
>> > wrote:
>> > >
>> > > mvn surefire:test -Dtest=TestClass -Dmaven.surefire.debug=“remote
>> debug
>> > options”
>> > >
>> > > Ralph
>> > >
>> > > > On Apr 7, 2021, at 7:04 AM, Volkan Yazıcı 
>> > wrote:
>> > > >
>> > > > Okay, fair enough. Thanks so much for sharing these details, I
>> really
>> > > > appreciated it. One final question: If you want to debug a single
>> > test, how
>> > > > does your work flow look like? Do you create a new "Run
>> Configuration"
>> > > > invoking a certain Maven command triggering the code/test that you
>> are
>> > > > interested in and "Debug Run"ing that "Run Configuration"?
>> > > >
>> > > > For the records, I still would like to hear how I can make IDEA to
>> > simply
>> > > > run a test. If anybody has succeeded in doing that, I am all ears.
>> > > >
>> > > >> On Wed, Apr 7, 2021 at 3:38 PM Apache 
>> > wrote:
>> > > >>
>> > > >> I will be honest. I have never tun any log4j tests in IntelliJ. I
>> > rarely
>> > > >> do it for any projects I work on. I use JVM remote debug all the
>> > time. I
>> > > >> don’t want to force others to have to do that, but I just never
>> think
>> > about
>> > > >> it.
>> > > >>
>> > > >> I use various versions of Maven from time to time. 3.6.1 is the
>> > default on
>> > > >> my Mac but I just installed 3.8.1 to validate what I needed to
>> change
>> > in my
>> > > >> setup to make it still work with my employers Nexus repository
>> which
>> > still
>> > > >> uses http.
>> > > >>
>> > > >> When I am working on Log4J stuff I do a full mvn clean install
>> several
>> > > >> times a day. That takes a huge amount of time so I have learned to
>> > > >> multitask and work on other stuff while builds are running.
>> > > >>
>> > > >> Ralph.
>> > > >>
>> > > >>> On Apr 7, 2021, at 12:15 AM, Volkan Yazıcı <
>> volkan.yaz...@gmail.com>
>> > > >> wrote:
>> > > >>>
>> > > >>> Ralph, when you delete the IDEA-specific configuration (i.e.,
>> *.iml
>> > > >> files
>> > > >>> and .idea directory), compile the sources via Maven, and open the
>> > folder
>> > > >> in
>> > > >>> IDEA, can you run *any* tests? If so, do you have any custom IDEA
>> > > >>> configurations? Which IDEA version are you using? If you are not
>> > using
>> > > >> the
>> > > >>> wrapper, wh

Re: Fix windows build on master (via Tim Perry)

[Tim Perry]

Volkan,

I should probably withdraw that pull request.

 It assumes that the status logger is not a singleton. However, status logger 
is a singleton and so this is a problem. 

I’ve implemented to changes which tracks the different status logger listeners 
by the context name. There are still a couple of kinks to work out on that 
though. If you want, I can push my changes to GitHub and you could take a look 
at those. There is some problem that’s causing some of the test to fail and I 
haven’t had time to figure out what the issue is.

Tim


Tim Perry
(916) 505-3634

> On Apr 28, 2021, at 6:39 AM, Volkan Yazıcı  wrote:
> 
> Hello,
> 
> Tim has submitted a PR to fix the Windows build on master
> <https://github.com/apache/logging-log4j2/pull/469> in February. In
> essence, it introduces prepareToStop() to StatusLogger. IIRC, there was a
> discussion regarding this subject in the mailing list. I think I have
> missed that. The PR looks good to me, though I'd appreciate some more
> context, which I've shared in my comments for the PR.
> 
> I will appreciate it if somebody else who has more knowledge on
> StatusLogger can also skim through the changes. It is not much and should
> be straightforward for somebody who has hacked on StatusLogger recently.
> 
> Cheers!

Re: Fix windows build on master (via Tim Perry)

[Tim Perry]

Should I just push the changes to pull request 469? That way there is a bit
of history.

On Wed, Apr 28, 2021 at 8:26 AM Ralph Goers 
wrote:

> Tim,
>
> Go ahead and create a PR and note that it still has unit test failures
> that need to be looked into. I can’t guarantee one of us will get to it
> before you but at least that way we might be able to.
>
> Ralph
>
> > On Apr 28, 2021, at 7:55 AM, Tim Perry  wrote:
> >
> > Volkan,
> >
> > I should probably withdraw that pull request.
> >
> > It assumes that the status logger is not a singleton. However, status
> logger is a singleton and so this is a problem.
> >
> > I’ve implemented to changes which tracks the different status logger
> listeners by the context name. There are still a couple of kinks to work
> out on that though. If you want, I can push my changes to GitHub and you
> could take a look at those. There is some problem that’s causing some of
> the test to fail and I haven’t had time to figure out what the issue is.
> >
> > Tim
> >
> >
> > Tim Perry
> > (916) 505-3634
> >
> >> On Apr 28, 2021, at 6:39 AM, Volkan Yazıcı 
> wrote:
> >>
> >> Hello,
> >>
> >> Tim has submitted a PR to fix the Windows build on master
> >> <https://github.com/apache/logging-log4j2/pull/469> in February. In
> >> essence, it introduces prepareToStop() to StatusLogger. IIRC, there was
> a
> >> discussion regarding this subject in the mailing list. I think I have
> >> missed that. The PR looks good to me, though I'd appreciate some more
> >> context, which I've shared in my comments for the PR.
> >>
> >> I will appreciate it if somebody else who has more knowledge on
> >> StatusLogger can also skim through the changes. It is not much and
> should
> >> be straightforward for somebody who has hacked on StatusLogger recently.
> >>
> >> Cheers!
> >
>
>
>

Re: Fix windows build on master (via Tim Perry)

[Tim Perry]

I pushed 46f7b08288e1f8843d293bb75950a8b466854ba6 to pull 469.

My recollection is that there was a test failing where I thought the
problem was the test, not the code. The test was checking to see if the log
file was gone inside the @Test and I don't think that is valid. I was
meaning to ask someone if the test was incorrect or if my code was the
problem.

I'll try to get back to this, but I'm slammed with work at the moment.


On Wed, Apr 28, 2021 at 8:42 AM Ralph Goers 
wrote:

> That would be up to you. If you have been working on the same branch there
> shouldn’t be a problem. But if it is going to cause merge conflicts then I
> wouldn’t bother as you can just link to the prior PR for the history.
>
> Ralph
>
> > On Apr 28, 2021, at 7:55 AM, Tim Perry  wrote:
> >
> >
> >
> > It assumes that the status logger is not a singleton. However, status
> logger is a singleton and so this is a problem.
> >
> > I’ve implemented to changes which tracks the different status logger
> listeners by the context name. There are still a couple of kinks to work
> out on that though. If you want, I can push my changes to GitHub and you
> could take a look at those. There is some problem that’s causing some of
> the test to fail and I haven’t had time to figure out what the issue is.
> >
> > Tim
> >
> >
> > Tim Perry
> > (916) 505-3634
>
>

Re: Fix windows build on master (via Tim Perry)

[Tim Perry]

After more carefully reading the code and unit tests, I have come to the
conclusion that the interaction between context-specific listeners and the
StatusLogger lifecycle need to be rethought. My approach, at best, would
have arrived at a kludge that worked but made the code difficult to
maintain.

As exhibit A, I point you to the method
configureExistingStatusConsoleListener in StatusConfiguration:

private boolean configureExistingStatusConsoleListener() {
boolean configured = false;
for (final StatusListener statusListener :
this.logger.getListeners()) {
if (statusListener instanceof StatusConsoleListener) {
final StatusConsoleListener listener =
(StatusConsoleListener) statusListener;
listener.setLevel(this.status);
this.logger.updateListenerLevel(this.status);
if (this.verbosity == Verbosity.QUIET) {
listener.setFilters(this.verboseClasses);
}
configured = true;
}
}
return configured;
}

If the code reconfigures existing StatusConsoleListeners when starting up,
then we can't very well shut them down when shutting down a context unless
all contexts that rely on that StatusLogger are shutting down. So my idea
that I would add the context name to each of the listeners and shut down
the correct one is flawed because the listener is likely to be shared
between contexts. Based on this, I have closed the pull request.

I need to take the time to wrap my head around possible lifecycles for the
StatusListener and StatusConsoleListener objects. I think the LoggerContext
needs to manage the context-specific StatusLoggers which means the
StatusConfiguration will need to pass any StatusLogger objects to the
LoggerContext so it can manage them. As I said before, it is a bit of an
X-Y problem: I'm trying to fix the file locking but really we need to
rethink the StatusLogger lifecycle.

I would like to leave my code accessible to you all. Will it remain in the
closed pull request if I change the underlying branch in GitHub? If not, I
can create an "bad_ideas" branch and put the code there.

I will carve out the changes I made to
log4j-kafka\src\test\java\org\apache\logging\log4j\kafka\builder\ConfigurationBuilderTest.java
to make it platform agnostic and create a pull request for that.

I could also create a pull request for the fixes I had to quell warnings if
you all are interested. They just deprecated imports and usage of
constructors for Integer, Long, and Short rather than using the getInteger,
getLong and getShort static methods.

Please give me your thoughts on:
1) managing the StatusLogger lifecycle
2) do I need to move my commits from pull 469 to a branch so they remain
accessible?
3) do you want a pull request for the warnings I fixed?

Thanks,
Tim


On Wed, Apr 28, 2021 at 9:06 AM Tim Perry  wrote:

> I pushed 46f7b08288e1f8843d293bb75950a8b466854ba6 to pull 469.
>
> My recollection is that there was a test failing where I thought the
> problem was the test, not the code. The test was checking to see if the log
> file was gone inside the @Test and I don't think that is valid. I was
> meaning to ask someone if the test was incorrect or if my code was the
> problem.
>
> I'll try to get back to this, but I'm slammed with work at the moment.
>
>
> On Wed, Apr 28, 2021 at 8:42 AM Ralph Goers 
> wrote:
>
>> That would be up to you. If you have been working on the same branch
>> there shouldn’t be a problem. But if it is going to cause merge conflicts
>> then I wouldn’t bother as you can just link to the prior PR for the history.
>>
>> Ralph
>>
>> > On Apr 28, 2021, at 7:55 AM, Tim Perry  wrote:
>> >
>> >
>> >
>> > It assumes that the status logger is not a singleton. However, status
>> logger is a singleton and so this is a problem.
>> >
>> > I’ve implemented to changes which tracks the different status logger
>> listeners by the context name. There are still a couple of kinks to work
>> out on that though. If you want, I can push my changes to GitHub and you
>> could take a look at those. There is some problem that’s causing some of
>> the test to fail and I haven’t had time to figure out what the issue is.
>> >
>> > Tim
>> >
>> >
>> > Tim Perry
>> > (916) 505-3634
>>
>>

Re: Fix windows build on master (via Tim Perry)

[Tim Perry]

I created a pull request which fixes the windows build. I have tested on
windows 10 using maven 3.6.3 and Zulu's OpenJDK 11 build and it works. I
noticed the automatic test framework that ran for the pull request failed.
https://github.com/apache/logging-log4j2/pull/491

This request adds an annotation @DisableOnWindows to prevent the
FileOutputTest test from running on Windows in log4j-core. This test cannot
pass because the listener to the StatusLogger is never shut down which
means the file handle to the log file is never released. On Windows, this
means the log file can't be deleted which fails the test.

The PR fixes the XML comparison in ConfigurationBuilderTest in log4j-kafka
to compare the XML as XML documents instead of as Strings which should
prevent the OS dependent quirks from causing the test to fail.

It also fixes an intermittent nuisance in
JsonTemplateLayoutNullEventDelimiterTest which fails when the port is
unavailable on windows. I changed the test to catch the exception and print
an error rather than fail the build. Ideally we'd find an available port
dynamically, write out the XML config to a temp file with the targeted
port, run the test, and then clean up the temporary config XML file. I took
the easy way out here.

I did not update changes.xml because nothing I changed affects the end
user. Is this the correct way to do this?

Please provide feedback on this pull request when it is convenient.

Thanks,
Tim

On Mon, May 3, 2021 at 11:10 PM Tim Perry  wrote:

> After more carefully reading the code and unit tests, I have come to the
> conclusion that the interaction between context-specific listeners and the
> StatusLogger lifecycle need to be rethought. My approach, at best, would
> have arrived at a kludge that worked but made the code difficult to
> maintain.
>
> As exhibit A, I point you to the method
> configureExistingStatusConsoleListener in StatusConfiguration:
>
> private boolean configureExistingStatusConsoleListener() {
> boolean configured = false;
> for (final StatusListener statusListener :
> this.logger.getListeners()) {
> if (statusListener instanceof StatusConsoleListener) {
> final StatusConsoleListener listener =
> (StatusConsoleListener) statusListener;
> listener.setLevel(this.status);
> this.logger.updateListenerLevel(this.status);
> if (this.verbosity == Verbosity.QUIET) {
> listener.setFilters(this.verboseClasses);
> }
> configured = true;
> }
> }
> return configured;
> }
>
> If the code reconfigures existing StatusConsoleListeners when starting up,
> then we can't very well shut them down when shutting down a context unless
> all contexts that rely on that StatusLogger are shutting down. So my idea
> that I would add the context name to each of the listeners and shut down
> the correct one is flawed because the listener is likely to be shared
> between contexts. Based on this, I have closed the pull request.
>
> I need to take the time to wrap my head around possible lifecycles for the
> StatusListener and StatusConsoleListener objects. I think the LoggerContext
> needs to manage the context-specific StatusLoggers which means the
> StatusConfiguration will need to pass any StatusLogger objects to the
> LoggerContext so it can manage them. As I said before, it is a bit of an
> X-Y problem: I'm trying to fix the file locking but really we need to
> rethink the StatusLogger lifecycle.
>
> I would like to leave my code accessible to you all. Will it remain in the
> closed pull request if I change the underlying branch in GitHub? If not, I
> can create an "bad_ideas" branch and put the code there.
>
> I will carve out the changes I made to
> log4j-kafka\src\test\java\org\apache\logging\log4j\kafka\builder\ConfigurationBuilderTest.java
> to make it platform agnostic and create a pull request for that.
>
> I could also create a pull request for the fixes I had to quell warnings
> if you all are interested. They just deprecated imports and usage of
> constructors for Integer, Long, and Short rather than using the getInteger,
> getLong and getShort static methods.
>
> Please give me your thoughts on:
> 1) managing the StatusLogger lifecycle
> 2) do I need to move my commits from pull 469 to a branch so they remain
> accessible?
> 3) do you want a pull request for the warnings I fixed?
>
> Thanks,
> Tim
>
>
> On Wed, Apr 28, 2021 at 9:06 AM Tim Perry  wrote:
>
>> I pushed 46f7b08288e1f8843d293bb75950a8b466854ba6 to pull 469.
>>
>> My recollection is that there was a test failing where I thought the
>> problem was the test, not the code. 

Pull 491 - fix windows build

[Tim Perry]

Volkan,

On pull 491 you requested I make variables final. I did so in commit
d70da9e7214d7baa3643363b849e5ec124b7b748.

Does that commit show up for you? Does it resolve the issue you flagged?

Thanks,
Tim

Re: Pull 491 - fix windows build

[Tim Perry]

Volkan,

Thanks for the update. I don’t mind waiting, I just didn’t know if I’d 
submitted the change in a way that would work. 

Tim


Tim Perry
(916) 505-3634

> On May 26, 2021, at 11:14 AM, Volkan Yazıcı  wrote:
> 
> Hey Tim,
> 
> You did a great job there. Unfortunately my agenda did not allow me to give
> it a second look and then merge it. I will make sure to take care of it
> this week. Thanks so much for the reminder.
> 
> Kind regards.
> 
>> On Wed, 26 May 2021, 18:51 Tim Perry  wrote:
>> 
>> Volkan,
>> 
>> On pull 491 you requested I make variables final. I did so in commit
>> d70da9e7214d7baa3643363b849e5ec124b7b748.
>> 
>> Does that commit show up for you? Does it resolve the issue you flagged?
>> 
>> Thanks,
>> Tim
>> 

Re: Cek's new log4j vs logback benchmark

[Tim Perry]

I’m fairly certain the JIT will optimize out the locking operations on the 
nested synchronized calls after a while. I’m not sure how soon into the 
performance tests that would happen. 

Tim


> On Aug 26, 2021, at 10:55 AM, Ralph Goers  wrote:
> 
> So are you continuing to look at this?  Frankly, the work on zero-gc stuff 
> made this 
> very complicated. I am not sure I can even follow the logic anymore. I also 
> noticed 
> that many of the methods dealing with byte buffers are synchronized. I am not 
> sure 
> why as only the method that moves data really needs to be.
> 
> Using multiple buffers would make sense if you are filling one from the 
> various patterns
> while it is unlocked and then only blocking when writing to the output stream 
> buffer.
> 
> For what its worth Ceki updated his performance page again yesterday to 
> change 
> the wording but I am pretty sure he didn’t change the log4j2 configuration to 
> use a 
> 256K buffer to match Logback.  And the page still makes it look like he is 
> testing 
> the performance of asynchronous processing when it is really testing the 
> performance 
> of the file appenders with a blocking queue in front of them.
> 
> Ralph
> 
>> On Aug 26, 2021, at 7:29 AM, Carter Kozak  wrote:
>> 
>> I also tried that with similar results, which leads me to believe we're 
>> spending
>> too much time copying between buffers.
>> 
>> We've proven that log4j can get LogEvents to the appender very quickly and
>> efficiently.
>> 
>> Once we hit PatternLayout we write data to a StringBuilder. 
>> AbstractStringBuilder used to store a char array in java 8, however with jep 
>> 254 compact strings it was updated (similarly to String) to use a byte array 
>> + byte representing the encoding.
>> We encode the StringBuilder value into a ByteBufferDestination 
>> (Heap)ByteBuffer in StringBuilderEncoder using the following steps:
>> 1. copy the StringBuilder value into a CharBuffer (this would have been a 
>> direct copy from char[] to char[] in java8, but requires conversion from 
>> byte[]+coder now)
>> 2. encode data from the CharBuffer to the StringBuilderEncoder ByteBuffer
>> 3. copy contents of the StringBuilderEncoder ByteBuffer to the 
>> ByteBufferDestination ByteBuffer (which is also always a heap buffer as far 
>> as I can tell)
>> 4. Write the ByteBufferDestination to the FileOutputStream/RandomAccessFile 
>> by extracting the heap byte-array.
>> 
>> We're copying bytes around a lot, and I think older JVMs would have 
>> optimized out a bit of the copying because the types exactly matched.
>> It's unclear why we need to copy through two separate ByteBuffers from 
>> StringBuilderEncoder into ByteBufferDestination
>> I've generally had better I/O performance using direct byte-buffers than 
>> heap buffers, although that experience largely comes from work on 
>> webservers, I haven't tested if it holds true for file I/O. If we operate 
>> directly upon a FileChannel, we can write direct buffers directly to files. 
>> The DirectByteBuffer memory is guaranteed to be contiguous, so it should 
>> have better performance around copying and manipulation as well.
>> 
>> -ck
>> 
>>> On Thu, Aug 26, 2021, at 10:00, Volkan Yazıcı wrote:
>>> Ralph, maybe a dumb idea but... Can't we simply write to /dev/null to avoid
>>> hardware's influence in the test results?
>>> 
>>> On Wed, Aug 25, 2021 at 8:05 PM Ralph Goers 
>>> wrote:
>>> 
 Did you add the no-op appender to Ceki’s project? Or are you using our
 NullAppender? I have
 my doubts about using that NullAppender as it does nothing - not even
 render the Layout, so
 it may get completely optimized away.
 
 I’d still like to know what kind of disks Remko did his original testing.
 The page says he got
 18.495 million messages per second. Each message would be about 130
 characters long
 from I can tell. That means the destination has to be able to support
 about at least 2.4 GB
 per second.
 
 In Ceki’s test the message is only 23 characters long (vs 100 in Remko’s).
 That means the
 message size is only going to be about 60 characters long. For me the
 async test is writing
 at about 1700 ops/ms which equates to 102 MBs, yet I am still seeing the
 queue backed up.
 So how we are writing must be terribly inefficient. Logback is getting
 about 2000 ops/ms,
 which is still only 120 MBs.
 
 Your test below would be generating about 197 MBs.
 
 One thing I did notice that made a big difference is that Ceki has Logback
 configured to
 use a buffer size of 256K while Log4j2 uses the default of 8K. Setting
 Log4j2 to 256K
 considerably improved the performance.
 
 
 Ralph
 
> On Aug 25, 2021, at 8:42 AM, Carter Kozak  wrote:
> 
>> If we would move the appender performance aside, am I right to conclude
>> that the entire complex async. framework built atop Disruptor with all
 i

Re: Cek's new log4j vs logback benchmark

[Tim Perry]

I see your point.

On Thu, Aug 26, 2021 at 11:21 AM Ralph Goers 
wrote:

> While that is true it does mean that we are locking higher up in the call
> stack than we need to be.
>
> Ralph
>
> > On Aug 26, 2021, at 11:13 AM, Tim Perry  wrote:
> >
> > I’m fairly certain the JIT will optimize out the locking operations on
> the nested synchronized calls after a while. I’m not sure how soon into the
> performance tests that would happen.
> >
> > Tim
> >
> >
> >> On Aug 26, 2021, at 10:55 AM, Ralph Goers 
> wrote:
> >>
> >> So are you continuing to look at this?  Frankly, the work on zero-gc
> stuff made this
> >> very complicated. I am not sure I can even follow the logic anymore. I
> also noticed
> >> that many of the methods dealing with byte buffers are synchronized. I
> am not sure
> >> why as only the method that moves data really needs to be.
> >>
> >> Using multiple buffers would make sense if you are filling one from the
> various patterns
> >> while it is unlocked and then only blocking when writing to the output
> stream buffer.
> >>
> >> For what its worth Ceki updated his performance page again yesterday to
> change
> >> the wording but I am pretty sure he didn’t change the log4j2
> configuration to use a
> >> 256K buffer to match Logback.  And the page still makes it look like he
> is testing
> >> the performance of asynchronous processing when it is really testing
> the performance
> >> of the file appenders with a blocking queue in front of them.
> >>
> >> Ralph
> >>
> >>> On Aug 26, 2021, at 7:29 AM, Carter Kozak  wrote:
> >>>
> >>> I also tried that with similar results, which leads me to believe
> we're spending
> >>> too much time copying between buffers.
> >>>
> >>> We've proven that log4j can get LogEvents to the appender very quickly
> and
> >>> efficiently.
> >>>
> >>> Once we hit PatternLayout we write data to a StringBuilder.
> AbstractStringBuilder used to store a char array in java 8, however with
> jep 254 compact strings it was updated (similarly to String) to use a byte
> array + byte representing the encoding.
> >>> We encode the StringBuilder value into a ByteBufferDestination
> (Heap)ByteBuffer in StringBuilderEncoder using the following steps:
> >>> 1. copy the StringBuilder value into a CharBuffer (this would have
> been a direct copy from char[] to char[] in java8, but requires conversion
> from byte[]+coder now)
> >>> 2. encode data from the CharBuffer to the StringBuilderEncoder
> ByteBuffer
> >>> 3. copy contents of the StringBuilderEncoder ByteBuffer to the
> ByteBufferDestination ByteBuffer (which is also always a heap buffer as far
> as I can tell)
> >>> 4. Write the ByteBufferDestination to the
> FileOutputStream/RandomAccessFile by extracting the heap byte-array.
> >>>
> >>> We're copying bytes around a lot, and I think older JVMs would have
> optimized out a bit of the copying because the types exactly matched.
> >>> It's unclear why we need to copy through two separate ByteBuffers from
> StringBuilderEncoder into ByteBufferDestination
> >>> I've generally had better I/O performance using direct byte-buffers
> than heap buffers, although that experience largely comes from work on
> webservers, I haven't tested if it holds true for file I/O. If we operate
> directly upon a FileChannel, we can write direct buffers directly to files.
> The DirectByteBuffer memory is guaranteed to be contiguous, so it should
> have better performance around copying and manipulation as well.
> >>>
> >>> -ck
> >>>
> >>>> On Thu, Aug 26, 2021, at 10:00, Volkan Yazıcı wrote:
> >>>> Ralph, maybe a dumb idea but... Can't we simply write to /dev/null to
> avoid
> >>>> hardware's influence in the test results?
> >>>>
> >>>> On Wed, Aug 25, 2021 at 8:05 PM Ralph Goers <
> ralph.go...@dslextreme.com>
> >>>> wrote:
> >>>>
> >>>>> Did you add the no-op appender to Ceki’s project? Or are you using
> our
> >>>>> NullAppender? I have
> >>>>> my doubts about using that NullAppender as it does nothing - not even
> >>>>> render the Layout, so
> >>>>> it may get completely optimized away.
> >>>>>
> >>>>> I’d still like to know what kind of disks Remko did his original

Re: Zero-copy rolling files

[Tim Perry]

I like this idea, but I think it would require non-default permissions for the 
account the application runs under on windows. However, it could be feature 
that can be switched on. 

https://docs.microsoft.com/en-us/windows/security/threat-protection/security-policy-settings/create-symbolic-links

Maybe I read the docs from MS incorrectly. 

Tim


> On Dec 18, 2021, at 7:07 AM, Gary Gregory  wrote:
> 
> Hi All:
> 
> And now for something completely different.
> 
> I wonder why we do not do file rollovers like below, and if we should:
> - Create the file with the target rolled over a name like applog-2021.txt
> - Create a symlink for the constant name like applog.txt to point to
> applog-2021.txt
> - When it's rollover time, start writing to the new file
> applog-2022.txt and change the symlink to point to it.
> 
> Zero copy.
> 
> Thoughts?
> 
> Gary

Re: Zero-copy rolling files

[Tim Perry]

Junctions are nice, but I think they are limited to pointing to directories on 
local file systems. Symlinks can point to remote files and directories on local 
or remote file systems (including using UNC paths). 

I didn’t bring up the windows permissions issue with symlinks because I think 
it is a stopper, I brought it up because we’ll need to include information 
about the minimal permissions needed to use the feature. It’s normal to request 
fine grained permissions for an application in a windows environment; I don’t 
think this would give anyone pause. 

Tim

> On Dec 19, 2021, at 2:16 PM, Matt Sicker  wrote:
> I think the NIO API for symlinks on Windows correspond to the ones that 
> require admin permissions to create. There are also file junctions that are a 
> feature of NTFS, though I’m not sure if there’s any way to create them 
> besides invoking cmd and running a mklink command from there (which, as it 
> might sound, requires a few layers of encoding to properly invoke). For more 
> info, take a look at 
> https://github.com/jenkinsci/jenkins/blob/master/core/src/main/java/hudson/os/WindowsUtil.java
>  
> <https://github.com/jenkinsci/jenkins/blob/master/core/src/main/java/hudson/os/WindowsUtil.java>
>  where I first encountered this filesystem madness due to an old security 
> vulnerability we fixed in the Jenkins project years ago and ended up writing 
> most of the code I’m talking about.
> --
> Matt Sicker
> 
>> On Dec 19, 2021, at 08:33, Gary Gregory  wrote:
>> 
>>> On Sun, Dec 19, 2021 at 9:03 AM Jochen Wiedmann
>>>  wrote:
>>> Having worked with symbolic links on Windows a lot, I find that
>>> privileges are present, in most cases. However, there is the technical
>>> question "How do I create them?"
>> 
>> java.nio.file.Files.createSymbolicLink(Path, Path, FileAttribute...)
>> 
>> The API is documented as an optional operation so we might need a set
>> of OS-specific calls to Runtime.exec(String).
>> 
>> Gary
>> 
>>> The best solution, that I have found so far is letting "cmd" do the
>>> job for me. (The mklink command is not a separate executable, but
>>> build into cmd.)
>>> https://github.com/jochenw/afw/blob/master/afw-core/src/main/java/com/github/jochenw/afw/core/components/WindowsCmdSymbolicLinksHandler.java
>>> Jochen
>>> On Sat, Dec 18, 2021 at 7:43 PM Tim Perry  wrote:
>>>> I like this idea, but I think it would require non-default permissions for 
>>>> the account the application runs under on windows. However, it could be 
>>>> feature that can be switched on.
>>>> https://docs.microsoft.com/en-us/windows/security/threat-protection/security-policy-settings/create-symbolic-links
>>>> Maybe I read the docs from MS incorrectly.
>>>> Tim
>>>>> On Dec 18, 2021, at 7:07 AM, Gary Gregory  wrote:
>>>>> Hi All:
>>>>> And now for something completely different.
>>>>> I wonder why we do not do file rollovers like below, and if we should:
>>>>> - Create the file with the target rolled over a name like applog-2021.txt
>>>>> - Create a symlink for the constant name like applog.txt to point to
>>>>> applog-2021.txt
>>>>> - When it's rollover time, start writing to the new file
>>>>> applog-2022.txt and change the symlink to point to it.
>>>>> Zero copy.
>>>>> Thoughts?
>>>>> Gary
>>> --
>>> Philosophy is useless, theology is worse. (Industrial Desease, Dire Straits)

Re: [VOTE] Release Apache Log4j 2.3.1-rc1 for Java 6

[Tim Perry]

I was able to build on Windows 10, with Zulu Java 6.

On Tue, Dec 21, 2021 at 1:29 PM Remko Popma  wrote:

> +1 I am changing my vote.
> My earlier pipecleaning program failed because the config had a JmsAppender
> configured in it... My bad.
> Signatures are good.
> Pipecleaning program works on Java 6 when I remove the JmsAppender from the
> config.
>
> On Wed, Dec 22, 2021 at 6:23 AM Ralph Goers 
> wrote:
>
> > My +1
> >
> > I tested it in an Ubuntu VM and verified it with Java 6.
> >
> > Ralph
> >
> > > On Dec 20, 2021, at 10:18 PM, Ralph Goers 
> > wrote:
> > >
> > > This is a vote to release Log4j 2.3.1, a security release for Java 6
> > users.
> > >
> > > Please download, test, and cast your votes on the log4j developers
> list.
> > > [] +1, release the artifacts
> > > [] -1, don't release because...
> > >
> > > The vote will remain open for as short amount as time as required to
> vet
> > the release. All votes are welcome and we encourage everyone to test the
> > release, but only Logging PMC votes are “officially” counted. As always,
> at
> > least 3 +1 votes and more positive than negative votes are required.
> > >
> > > Changes in this version include:
> > >
> > >
> > > New features:
> > > *  LOG4J2-3198:  Pattern layout no longer enables lookups within
> message
> > text.
> > >
> > > Fixed Bugs:
> > > *  LOG4J2-3242:  Limit JNDI to the java protocol only. JNDI will remain
> > disabled by default. Rename JNDI enablement property from
> > >'log4j2.enableJndi' to 'log4j2.enableJndiLookup',
> > 'log4j2.enableJndiJms', and 'log4j2.enableJndiContextSelector’.
> > > *  LOG4J2-3230:  Fix string substitution recursion.
> > >
> > > Tag:
> > > a)  for a new copy do "git clone
> > https://github.com/apache/logging-log4j2.git"; and then "git checkout
> > tags/log4j-2.3.1-rc1”  or just "git clone -b log4j-2.3.1-rc1
> > https://github.com/apache/logging-log4j2.git";
> > > b) for an existing working copy to “git pull” and then “git checkout
> > tags/log4j-2.12.3-rc1”
> > >
> > > Web Site:
> > https://logging.staged.apache.org/log4j/log4j-2.3.1/index.html
> > >
> > > Maven Artifacts:
> > https://repository.apache.org/content/repositories/orgapachelogging-1076
> > >
> > > Distribution archives:
> > https://dist.apache.org/repos/dist/dev/logging/log4j/
> > >
> > > You may download all the Maven artifacts by executing:
> > > wget -e robots=off --cut-dirs=7 -nH -r -p -np --no-check-certificate
> >
> https://repository.apache.org/content/repositories/orgapachelogging-1076/org/apache/logging/log4j/
> > .
> >
> >
>

Re: Broken CI

[Tim Perry]

Is it worth marching those tests with @Ignore and filing a Jira for each
one? That does seem to set a bad precedent though.

Last time I tried I couldn't get the mainline code to load in IntelliJ or
Eclipse because of the packing changes that were in progress. Did that get
fixed? If so, I might be able to carve out some time to look at a couple
tests if you point me in the right direction.

Tim

On Thu, Dec 23, 2021 at 7:24 AM Volkan Yazıcı  wrote:

> Since, there are some tests which occasionally constantly fail, we use
> `-Dmaven.test.failure.ignore=true` in `verify` goal. This causes a build
> with test failures to be marked green. The 3rd party component,
> `scacap/action-surefire-report@v1`, is used to feed these failures back
> into GitHub Actions status with some pretty printing. But since the PR is
> opened by a user who doesn't have commit rights, the 3rd party component
> fails to mark the failures due to insufficient privileges. In
> `.github/workflows/main.yml`, I had introduced the `if: github.repository
> == 'apache/logging-log4j2'` line to work around this, but apparently it is
> broken again.
>
> I totally share your frustration, same here. Though sparing time to fix
> this is pretty difficult nowadays.
>
> I also need to confess, in those brief moments of insanity, I contemplate
> nuking all those flaky tests. This will simplify the CI magic a lot and
> enhance our confidence in the tests.
>
> On Tue, Dec 21, 2021 at 3:10 AM Gary Gregory 
> wrote:
>
> > After getting https://github.com/apache/logging-log4j2/pull/646 in what
> I
> > think is a good state, I have no idea if it is safe or not to merge
> because
> > the 1st build GitHub shows is red:
> >
> >
> https://github.com/apache/logging-log4j2/runs/4589771553?check_suite_focus=true
> >
> > I don't use GH actions the way we do here and I'm not sure why we need
> the
> > publish test result set when anyone can just look at the build logs.
> >
> > Gary
> >
>

Re: Broken CI

[Tim Perry]

My issues were all on master. I've always been able to work on release-2.x
in IntelliJ or Eclipse without any issue.

I'll have to check out master and see if it works now. I have it on the
back burner to try to figure out an improved status logger life cycle.

On Thu, Dec 23, 2021 at 9:34 AM Carter Kozak  wrote:

> I haven't had a test flake locally in the last 6 months (at least), if we
> see flaky tests I'm in favor of fixing them rather than working around them.
>
> fwiw the non GitHub-actions tests work great on the release-2.x branch in
> my experience, when they aren't overwhelmed accessing build nodes anyhow.
> That said, I would prefer to get everything workin on GitHub actions as it
> seems to be the gold standard these days.
>
> > Last time I tried I couldn't get the mainline code to load in IntelliJ
> or Eclipse because of the packing changes that were in progress.
>
> I find release-2.x works nicely with IntelliJ idea when I set the
> project-level sdk to jdk8. the master branch may be another story. I have
> an INFRA ticket open to switch the default branch to reduce confusion:
> https://issues.apache.org/jira/browse/INFRA-22641
>
> -ck
>
> On Thu, Dec 23, 2021, at 12:25, Tim Perry wrote:
> > Is it worth marching those tests with @Ignore and filing a Jira for each
> > one? That does seem to set a bad precedent though.
> >
> > Last time I tried I couldn't get the mainline code to load in IntelliJ or
> > Eclipse because of the packing changes that were in progress. Did that
> get
> > fixed? If so, I might be able to carve out some time to look at a couple
> > tests if you point me in the right direction.
> >
> > Tim
> >
> > On Thu, Dec 23, 2021 at 7:24 AM Volkan Yazıcı  wrote:
> >
> > > Since, there are some tests which occasionally constantly fail, we use
> > > `-Dmaven.test.failure.ignore=true` in `verify` goal. This causes a
> build
> > > with test failures to be marked green. The 3rd party component,
> > > `scacap/action-surefire-report@v1`, is used to feed these failures
> back
> > > into GitHub Actions status with some pretty printing. But since the PR
> is
> > > opened by a user who doesn't have commit rights, the 3rd party
> component
> > > fails to mark the failures due to insufficient privileges. In
> > > `.github/workflows/main.yml`, I had introduced the `if:
> github.repository
> > > == 'apache/logging-log4j2'` line to work around this, but apparently
> it is
> > > broken again.
> > >
> > > I totally share your frustration, same here. Though sparing time to fix
> > > this is pretty difficult nowadays.
> > >
> > > I also need to confess, in those brief moments of insanity, I
> contemplate
> > > nuking all those flaky tests. This will simplify the CI magic a lot and
> > > enhance our confidence in the tests.
> > >
> > > On Tue, Dec 21, 2021 at 3:10 AM Gary Gregory 
> > > wrote:
> > >
> > > > After getting https://github.com/apache/logging-log4j2/pull/646 in
> what
> > > I
> > > > think is a good state, I have no idea if it is safe or not to merge
> > > because
> > > > the 1st build GitHub shows is red:
> > > >
> > > >
> > >
> https://github.com/apache/logging-log4j2/runs/4589771553?check_suite_focus=true
> > > >
> > > > I don't use GH actions the way we do here and I'm not sure why we
> need
> > > the
> > > > publish test result set when anyone can just look at the build logs.
> > > >
> > > > Gary
> > > >
> > >
> >
>

Re: [DISCUSS][VOTE] Future of Log4j 1.x

[Tim Perry]

I propose that this vote should stay open longer than 72 hours given that we 
are coming up on New Years and many people who would wish to weigh in might be 
on vacation right now. 

Tim

> On Dec 29, 2021, at 2:29 PM, Matt Sicker  wrote:
> 
> Consistent contributors are frequently invited to be committers and later 
> PMC members. Having at least three people maintaining anything is an Apache 
> standard for maintaining vendor neutrality, ensuring a minimum number of 
> people can verify release candidates to address security issues or any other 
> releases.
> 
> —
> Matt Sicker
> 
>> On Dec 29, 2021, at 14:41, Vladimir Sitnikov  
>> wrote:
>> 
>> 
>>> 
>>> Log4j is owned by the Logging Services PMC. You cannot incubate it without
>> this PMC’s approval.
>> 
>> Exactly. As far as I understand, Logging pmc should accept patches and
>> release fixes or they should approve reincubating.
>> Of course, you can try rejecting patches and disapprove reincubation,
>> however, that won't hold water.
>> 
>> Unfortunately, I have not seen the response from the logging pmc regarding
>> approve/disapprove re-incubating. There's a pending question to Ron still.
>> 
>> I do not consider forks outside of the ASF.
>> 
>>> But I notice the one topic you did not respond to was the lack of
>> interested people other than yourself. Why is that?
>> 
>> I find the question irrelevant, and I find it has nothing to do with
>> accepting patches and releasing 1.2
>> I belive there were even people on incubator thread, so it is strange why
>> do you demand that I provide you with a list of rock-star 1.x maintainers.
>> 
>> 1) I can't guarantee I will be alive in February. Can you guarantee all the
>> logging pmc members will be alive then? I doubt so. So I find that
>> questions like "how can we be sure you will send patches" too intimate.
>> 
>> 2) I have already filed a patch for buildscripts. Whould you review it and
>> merge?
>> 
>> 3) Suppose I find a team (e.g 4-5 ASF fellows) who are willing to support
>> 1.2. What do you do then? Would you add all of them to the logging pmc?
>> I don't really see the point why do you ask, and at the same time I can't
>> guarantee the people I gather will be alive tomorrow. I can't guarantee
>> they will always have interest in 1.2
>> 
>> Vladimir

Re: LOG4J2-3259 Limit max recursion depth when interpolating strings

[Tim Perry]

I think Ralph is right: there should (could) be a limit on how often the
user is informed but the user does need to be informed.

On Mon, Jan 3, 2022 at 8:32 AM Ralph Goers 
wrote:

> I am in favor of limiting the recursion depth to a configurable with a
> default number.
> I fully expect virtually all normal usage would fall within the limits of
> whatever we
> would pick as the default.
>
> But should that limit be hit we can’t just return crap silently. It almost
> certainly means
> something bad is going on that the user needs to be informed of. We can
> certainly use
> something like ErrorHandler to limit the frequency the user is informed.
>
> Ralph
>
>
> > On Jan 3, 2022, at 5:41 AM, Volkan Yazıcı  wrote:
> >
> > There is a PR  for
> this
> > issue, shall we bring this to a conclusion?
> >
> > I am in favor of this PR if the following changes were implemented:
> > - limit is read from a property
> > - limit is documented
> > - stay quiet (i.e., not StatusLogger exception logging) if the limit has
> > been exceeded
> >
> > My rationale for the "stay quiet" feature is that, if the runtime
> exhibits
> > a behavior not anticipated by the configuration, rather than nuking the
> > StatusLogger, simply practice the configuration: recurse no more.
> >
> > This said, I am struggling to not get drawn into Carter's following
> > remark: *"I'm
> > not sure I entirely understand what we're protecting against – I'd
> consider
> > any recursion beyond what the configuration author expects to be an
> > incredibly serious problem"*.
> >
> > Note that I am not strongly opinionated about the feature per se. I just
> > want to bring the discussion to a conclusion. If we decide to reject the
> PR
> > (preferably, with a good argument), that is fine by me too.
>
>

Re: [logging-log4j1] branch v1.2.8 created (now 0cde9dd)

[Tim Perry]

Maybe I'm missing something, but shouldn't it be 1.2.18? There was already
a log4j release 1.2.8 in 2005.

On Thu, Jan 6, 2022 at 2:54 PM Matt Sicker  wrote:

> Plus, the branch name sounds like a tag.
>
> On Thu, Jan 6, 2022 at 3:21 PM Gary Gregory 
> wrote:
> >
> > -1 This component reached End-of-Life in 2015.
> >
> > Gary
> >
> > On Thu, Jan 6, 2022 at 12:46 PM  wrote:
> >
> > > This is an automated email from the ASF dual-hosted git repository.
> > >
> > > ceki pushed a change to branch v1.2.8
> > > in repository https://gitbox.apache.org/repos/asf/logging-log4j1.git.
> > >
> > >
> > >   at 0cde9dd  Add EOL message
> > >
> > > No new revisions were added by this update.
> > >
>

Re: [apache/logging-log4j2] Bump tomcat-catalina from 8.5.20 to 10.0.14 (PR #662)

[Tim Perry]

Many libraries are producing one code line for the javax.* environment and
another code line for the jakarta.* environment. This is because when
Oracle gave the Eclipse Foundation the J2EE code they required name
changes. This affects code using Servlet API, JPA, Bean Validation,et
cetera.

Spring:
spring 5 uses javax.* and spring 6 will support jakarta.*

Hibernate:
Hibernate Validator 6.x will keep the javax.* packages while Hibernate
Validator 7.x moved to the jakarta.* packages.
https://in.relation.to/2021/01/06/hibernate-validator-700-62-final-released/

Tomcat:
When first released, Tomcat 9 and Tomcat 10 were functionally identical,
except Tomcat 10 supported jakarta.* and Tomcat 9 supported javax.*. They
have slowly diverged as more features have been added to Tomcat 10. The
difference isn't very big.


For log4j, I suspect we'll need to release two versions of log4j-web and
log4j-jpa: one for backwards compatibility with javax.* and another for
Jakarta EE. We might need to do this for other libs as well.

Looking through the source, I only see "import javax.servlet" in:
log4j-samples
log4j-taglib
log4j-web
src/site/asciidoc

I see "import javax.persistence in:
log4j-jpa
log4j-perf

If I expand my search to "import javax", I see this many more places. I
don't think all of these are affected by the Jakarta EE change. If I was at
a unix box I could slice and dice the imports, but here are the packages
that might be affected.
log4j-1.2-api
log4j-core
log4j-flume-ng
log4j-jdbc
log4j-jms
log4j-jmx-guil
log4j-jpa
log4j-kafka
log4j-layout-jakcons-xml
log4j-perf
log4j-plugins
log4j-samples
log4j-smtp
log4j-taglib
log4j-web
src/site/asciidoc

FWIW, I think I tabulated these on an old master branch.

On Fri, Jan 21, 2022 at 12:26 AM Volkan Yazıcı  wrote:

> This Tomcat upgrade looks legit to me.
> Nevertheless, I'd appreciate it if a Tomcat veteran could weigh in.
>
> -- Forwarded message -
> From: knoxyz 
> Date: Thu, Jan 20, 2022 at 3:28 PM
> Subject: Re: [apache/logging-log4j2] Bump tomcat-catalina from 8.5.20 to
> 10.0.14 (PR #662)
> To: apache/logging-log4j2 
> Cc: Subscribed 
>
>
> Pay attention!
> tomcat 8 and 9 are pretty good compatible, but with version 10 comes huge
> breaks (namespace javax -> jakarta)!
> Therefore still tomcat 9 is in use by the most production environments and
> not supported from the most API and frameworks.
>
> https://tomcat.apache.org/migration-10.html
>
> *There is a significant breaking change between Tomcat 9.0.x and Tomcat
> 10.0.x. The Java package used by the specification APIs has changed from
> javax... to jakarta It will be necessary to recompile web applications
> against the new APIs.*
>
> tomcat 8 and 9
>
> —
> Reply to this email directly, view it on GitHub
>  >,
> or unsubscribe
> <
> https://github.com/notifications/unsubscribe-auth/AAARTSPJ3TLBKEMT2FHBXW3UXALXHANCNFSM5KZH66WA
> >
> .
> You are receiving this because you are subscribed to this thread.Message
> ID: 
>

Re: [apache/logging-log4j2] Bump tomcat-catalina from 8.5.20 to 10.0.14 (PR #662)

[Tim Perry]

 log4j-server depends on log4j-jms do I don’t think log4j-server can be 
upgraded to tomcat 10 until log4j-jms-Jakarta (or whatever) is created. 

Tim

> On Jan 22, 2022, at 6:11 PM, Matt Sicker  wrote:
> 
> We’ve already got a log4j-jakarta-web module for the web change. Yes, I 
> agree we need an equivalent for JPA, JMS, JavaMail, and the tag library.
> 
> —
> Matt Sicker
> 
>> On Jan 22, 2022, at 18:14, Tim Perry  wrote:
>> 
>> Many libraries are producing one code line for the javax.* environment and
>> another code line for the jakarta.* environment. This is because when
>> Oracle gave the Eclipse Foundation the J2EE code they required name
>> changes. This affects code using Servlet API, JPA, Bean Validation,et
>> cetera.
>> 
>> Spring:
>> spring 5 uses javax.* and spring 6 will support jakarta.*
>> 
>> Hibernate:
>> Hibernate Validator 6.x will keep the javax.* packages while Hibernate
>> Validator 7.x moved to the jakarta.* packages.
>> https://in.relation.to/2021/01/06/hibernate-validator-700-62-final-released/
>> 
>> Tomcat:
>> When first released, Tomcat 9 and Tomcat 10 were functionally identical,
>> except Tomcat 10 supported jakarta.* and Tomcat 9 supported javax.*. They
>> have slowly diverged as more features have been added to Tomcat 10. The
>> difference isn't very big.
>> 
>> 
>> For log4j, I suspect we'll need to release two versions of log4j-web and
>> log4j-jpa: one for backwards compatibility with javax.* and another for
>> Jakarta EE. We might need to do this for other libs as well.
>> 
>> Looking through the source, I only see "import javax.servlet" in:
>> log4j-samples
>> log4j-taglib
>> log4j-web
>> src/site/asciidoc
>> 
>> I see "import javax.persistence in:
>> log4j-jpa
>> log4j-perf
>> 
>> If I expand my search to "import javax", I see this many more places. I
>> don't think all of these are affected by the Jakarta EE change. If I was at
>> a unix box I could slice and dice the imports, but here are the packages
>> that might be affected.
>> log4j-1.2-api
>> log4j-core
>> log4j-flume-ng
>> log4j-jdbc
>> log4j-jms
>> log4j-jmx-guil
>> log4j-jpa
>> log4j-kafka
>> log4j-layout-jakcons-xml
>> log4j-perf
>> log4j-plugins
>> log4j-samples
>> log4j-smtp
>> log4j-taglib
>> log4j-web
>> src/site/asciidoc
>> 
>> FWIW, I think I tabulated these on an old master branch.
>> 
>>>> On Fri, Jan 21, 2022 at 12:26 AM Volkan Yazıcı  wrote:
>>> 
>>> This Tomcat upgrade looks legit to me.
>>> Nevertheless, I'd appreciate it if a Tomcat veteran could weigh in.
>>> 
>>> -- Forwarded message -
>>> From: knoxyz 
>>> Date: Thu, Jan 20, 2022 at 3:28 PM
>>> Subject: Re: [apache/logging-log4j2] Bump tomcat-catalina from 8.5.20 to
>>> 10.0.14 (PR #662)
>>> To: apache/logging-log4j2 
>>> Cc: Subscribed 
>>> 
>>> 
>>> Pay attention!
>>> tomcat 8 and 9 are pretty good compatible, but with version 10 comes huge
>>> breaks (namespace javax -> jakarta)!
>>> Therefore still tomcat 9 is in use by the most production environments and
>>> not supported from the most API and frameworks.
>>> 
>>> https://tomcat.apache.org/migration-10.html
>>> 
>>> *There is a significant breaking change between Tomcat 9.0.x and Tomcat
>>> 10.0.x. The Java package used by the specification APIs has changed from
>>> javax... to jakarta It will be necessary to recompile web applications
>>> against the new APIs.*
>>> 
>>> tomcat 8 and 9
>>> 
>>> —
>>> Reply to this email directly, view it on GitHub
>>> <https://github.com/apache/logging-log4j2/pull/662#issuecomment-1017564083
>>>> ,
>>> or unsubscribe
>>> <
>>> https://github.com/notifications/unsubscribe-auth/AAARTSPJ3TLBKEMT2FHBXW3UXALXHANCNFSM5KZH66WA
>>>> 
>>> .
>>> You are receiving this because you are subscribed to this thread.Message
>>> ID: 
>>> 

Re: [apache/logging-log4j2] Bump tomcat-catalina from 8.5.20 to 10.0.14 (PR #662)

[Tim Perry]

Sorry Volkan, I think I somehow searched the wrong pom.xml. I was convinced
appserver was bringing in log4j-jms, but it isn't.

You will need to update the servlet version. This won't work with Tomcat 10:

javax.servlet
javax.servlet-api
3.0.1
provided


Sorry for the confusion.

On Mon, Jan 24, 2022 at 1:09 PM Volkan Yazıcı  wrote:

> This PR only addresses `log4j-appserver`, which doesn't have any `javax`
> package dependencies. Quoting from my comment to the PR:
>
> "AFAIK, Tomcat is only used by `log4j-appserver`. There I don't see any
> dependencies on the `javax` namespace, but just an implementation of
> `org.apache.juli.logging.Log` packaged by Tomcat. All CI checks also look
> green – note that there are no tests associated with `log4j-appserver`,
> though compilation succeeds. I don't see a reason not to upgrade. If the
> user wants to stick to a Tomcat version <10, they can still do so.
> `org.apache.juli.logging.Log` looks to be intact, hence I don't foresee any
> compatibility issues."
>
> Hence I still think this is a legit upgrade. Am I missing something?
>
> On Sun, Jan 23, 2022 at 1:14 AM Tim Perry  wrote:
>
> > Many libraries are producing one code line for the javax.* environment
> and
> > another code line for the jakarta.* environment. This is because when
> > Oracle gave the Eclipse Foundation the J2EE code they required name
> > changes. This affects code using Servlet API, JPA, Bean Validation,et
> > cetera.
> >
> > Spring:
> > spring 5 uses javax.* and spring 6 will support jakarta.*
> >
> > Hibernate:
> > Hibernate Validator 6.x will keep the javax.* packages while Hibernate
> > Validator 7.x moved to the jakarta.* packages.
> >
> >
> https://in.relation.to/2021/01/06/hibernate-validator-700-62-final-released/
> >
> > Tomcat:
> > When first released, Tomcat 9 and Tomcat 10 were functionally identical,
> > except Tomcat 10 supported jakarta.* and Tomcat 9 supported javax.*. They
> > have slowly diverged as more features have been added to Tomcat 10. The
> > difference isn't very big.
> >
> >
> > For log4j, I suspect we'll need to release two versions of log4j-web and
> > log4j-jpa: one for backwards compatibility with javax.* and another for
> > Jakarta EE. We might need to do this for other libs as well.
> >
> > Looking through the source, I only see "import javax.servlet" in:
> > log4j-samples
> > log4j-taglib
> > log4j-web
> > src/site/asciidoc
> >
> > I see "import javax.persistence in:
> > log4j-jpa
> > log4j-perf
> >
> > If I expand my search to "import javax", I see this many more places. I
> > don't think all of these are affected by the Jakarta EE change. If I was
> at
> > a unix box I could slice and dice the imports, but here are the packages
> > that might be affected.
> > log4j-1.2-api
> > log4j-core
> > log4j-flume-ng
> > log4j-jdbc
> > log4j-jms
> > log4j-jmx-guil
> > log4j-jpa
> > log4j-kafka
> > log4j-layout-jakcons-xml
> > log4j-perf
> > log4j-plugins
> > log4j-samples
> > log4j-smtp
> > log4j-taglib
> > log4j-web
> > src/site/asciidoc
> >
> > FWIW, I think I tabulated these on an old master branch.
> >
> > On Fri, Jan 21, 2022 at 12:26 AM Volkan Yazıcı  wrote:
> >
> > > This Tomcat upgrade looks legit to me.
> > > Nevertheless, I'd appreciate it if a Tomcat veteran could weigh in.
> > >
> > > -- Forwarded message -
> > > From: knoxyz 
> > > Date: Thu, Jan 20, 2022 at 3:28 PM
> > > Subject: Re: [apache/logging-log4j2] Bump tomcat-catalina from 8.5.20
> to
> > > 10.0.14 (PR #662)
> > > To: apache/logging-log4j2 
> > > Cc: Subscribed 
> > >
> > >
> > > Pay attention!
> > > tomcat 8 and 9 are pretty good compatible, but with version 10 comes
> huge
> > > breaks (namespace javax -> jakarta)!
> > > Therefore still tomcat 9 is in use by the most production environments
> > and
> > > not supported from the most API and frameworks.
> > >
> > > https://tomcat.apache.org/migration-10.html
> > >
> > > *There is a significant breaking change between Tomcat 9.0.x and Tomcat
> > > 10.0.x. The Java package used by the specification APIs has changed
> from
> > > javax... to jakarta It will be necessary to recompile web
> > applications
> > > against the new APIs.*
> > >
> > > tomcat 8 and 9
> > >
> > > —
> > > Reply to this email directly, view it on GitHub
> > > <
> >
> https://github.com/apache/logging-log4j2/pull/662#issuecomment-1017564083
> > > >,
> > > or unsubscribe
> > > <
> > >
> >
> https://github.com/notifications/unsubscribe-auth/AAARTSPJ3TLBKEMT2FHBXW3UXALXHANCNFSM5KZH66WA
> > > >
> > > .
> > > You are receiving this because you are subscribed to this
> thread.Message
> > > ID: 
> > >
> >
>

Re: [apache/logging-log4j2] Bump tomcat-catalina from 8.5.20 to 10.0.14 (PR #662)

[Tim Perry]

The pom.xml changes look reasonable to me. I haven't checked it out and
played with it. I was thinking I'd have time, but I just got buried with
work elsewhere.

I do wonder if log4j should have two modules:
 * log4j-appserver (Tomcat 9 or less)
 * log4j-appserver-jakarta (Tomcat 10 or greater)

Are there enough users of log4j-appserver for this to be useful?
Eventually, the world will need to adapt the the Jarkarta EE reality but I
imagine it will take years or decades. Thoughts?

Tim






On Wed, Jan 26, 2022 at 10:55 AM Volkan Yazıcı  wrote:

> Mind somebody reviewing the following PR, please?
> https://github.com/apache/logging-log4j2/pull/730
> Time, what do you think?
>
> On Mon, Jan 24, 2022 at 10:21 PM Tim Perry  wrote:
>
> > Sorry Volkan, I think I somehow searched the wrong pom.xml. I was
> convinced
> > appserver was bringing in log4j-jms, but it isn't.
> >
> > You will need to update the servlet version. This won't work with Tomcat
> > 10:
> > 
> > javax.servlet
> > javax.servlet-api
> > 3.0.1
> > provided
> > 
> >
> > Sorry for the confusion.
> >
> > On Mon, Jan 24, 2022 at 1:09 PM Volkan Yazıcı  wrote:
> >
> > > This PR only addresses `log4j-appserver`, which doesn't have any
> `javax`
> > > package dependencies. Quoting from my comment to the PR:
> > >
> > > "AFAIK, Tomcat is only used by `log4j-appserver`. There I don't see any
> > > dependencies on the `javax` namespace, but just an implementation of
> > > `org.apache.juli.logging.Log` packaged by Tomcat. All CI checks also
> look
> > > green – note that there are no tests associated with `log4j-appserver`,
> > > though compilation succeeds. I don't see a reason not to upgrade. If
> the
> > > user wants to stick to a Tomcat version <10, they can still do so.
> > > `org.apache.juli.logging.Log` looks to be intact, hence I don't foresee
> > any
> > > compatibility issues."
> > >
> > > Hence I still think this is a legit upgrade. Am I missing something?
> > >
> > > On Sun, Jan 23, 2022 at 1:14 AM Tim Perry  wrote:
> > >
> > > > Many libraries are producing one code line for the javax.*
> environment
> > > and
> > > > another code line for the jakarta.* environment. This is because when
> > > > Oracle gave the Eclipse Foundation the J2EE code they required name
> > > > changes. This affects code using Servlet API, JPA, Bean Validation,et
> > > > cetera.
> > > >
> > > > Spring:
> > > > spring 5 uses javax.* and spring 6 will support jakarta.*
> > > >
> > > > Hibernate:
> > > > Hibernate Validator 6.x will keep the javax.* packages while
> Hibernate
> > > > Validator 7.x moved to the jakarta.* packages.
> > > >
> > > >
> > >
> >
> https://in.relation.to/2021/01/06/hibernate-validator-700-62-final-released/
> > > >
> > > > Tomcat:
> > > > When first released, Tomcat 9 and Tomcat 10 were functionally
> > identical,
> > > > except Tomcat 10 supported jakarta.* and Tomcat 9 supported javax.*.
> > They
> > > > have slowly diverged as more features have been added to Tomcat 10.
> The
> > > > difference isn't very big.
> > > >
> > > >
> > > > For log4j, I suspect we'll need to release two versions of log4j-web
> > and
> > > > log4j-jpa: one for backwards compatibility with javax.* and another
> for
> > > > Jakarta EE. We might need to do this for other libs as well.
> > > >
> > > > Looking through the source, I only see "import javax.servlet" in:
> > > > log4j-samples
> > > > log4j-taglib
> > > > log4j-web
> > > > src/site/asciidoc
> > > >
> > > > I see "import javax.persistence in:
> > > > log4j-jpa
> > > > log4j-perf
> > > >
> > > > If I expand my search to "import javax", I see this many more
> places. I
> > > > don't think all of these are affected by the Jakarta EE change. If I
> > was
> > > at
> > > > a unix box I could slice and dice the imports, but here are the
> > packages
> > > > that might be affected.
> > > > log4j-1.2-api
> > > > log4j-core
> > > > log4j-flume-ng
> > > > log4j-jdbc
> > > > log4j-jms
> > > > log4j-jmx-guil
> > > > log4j-jpa
&

Re: [apache/logging-log4j2] Bump tomcat-catalina from 8.5.20 to 10.0.14 (PR #662)

[Tim Perry]

Incidentally, I'm in favor of adding jakarta as a suffix to the Java EE
module name so the Java EE and Jakarta EE module end up sorting next to
each other when listing the directories. It makes it easy to see which ones
have a Jakarta clone.

(I'm also in favor of learning to type Jakarta the first time rather than
always having the r in the wrong place with my usual typo: jarkata!)

On Thu, Jan 27, 2022 at 11:48 AM Matt Sicker  wrote:

> I think having the two different modules makes sense. That makes it
> easiest for users to use the version corresponding to their own
> environment. We still have support for Servlet 2.5, so as long as
> these modules don't see a lot of active development and continue to be
> releasable, then I don't see the problem with keeping both for a long
> time.
>
> On Thu, Jan 27, 2022 at 1:44 PM Tim Perry  wrote:
> >
> > The pom.xml changes look reasonable to me. I haven't checked it out and
> > played with it. I was thinking I'd have time, but I just got buried with
> > work elsewhere.
> >
> > I do wonder if log4j should have two modules:
> >  * log4j-appserver (Tomcat 9 or less)
> >  * log4j-appserver-jakarta (Tomcat 10 or greater)
> >
> > Are there enough users of log4j-appserver for this to be useful?
> > Eventually, the world will need to adapt the the Jarkarta EE reality but
> I
> > imagine it will take years or decades. Thoughts?
> >
> > Tim
> >
> >
> >
> >
> >
> >
> > On Wed, Jan 26, 2022 at 10:55 AM Volkan Yazıcı  wrote:
> >
> > > Mind somebody reviewing the following PR, please?
> > > https://github.com/apache/logging-log4j2/pull/730
> > > Time, what do you think?
> > >
> > > On Mon, Jan 24, 2022 at 10:21 PM Tim Perry  wrote:
> > >
> > > > Sorry Volkan, I think I somehow searched the wrong pom.xml. I was
> > > convinced
> > > > appserver was bringing in log4j-jms, but it isn't.
> > > >
> > > > You will need to update the servlet version. This won't work with
> Tomcat
> > > > 10:
> > > > 
> > > > javax.servlet
> > > > javax.servlet-api
> > > > 3.0.1
> > > > provided
> > > > 
> > > >
> > > > Sorry for the confusion.
> > > >
> > > > On Mon, Jan 24, 2022 at 1:09 PM Volkan Yazıcı 
> wrote:
> > > >
> > > > > This PR only addresses `log4j-appserver`, which doesn't have any
> > > `javax`
> > > > > package dependencies. Quoting from my comment to the PR:
> > > > >
> > > > > "AFAIK, Tomcat is only used by `log4j-appserver`. There I don't
> see any
> > > > > dependencies on the `javax` namespace, but just an implementation
> of
> > > > > `org.apache.juli.logging.Log` packaged by Tomcat. All CI checks
> also
> > > look
> > > > > green – note that there are no tests associated with
> `log4j-appserver`,
> > > > > though compilation succeeds. I don't see a reason not to upgrade.
> If
> > > the
> > > > > user wants to stick to a Tomcat version <10, they can still do so.
> > > > > `org.apache.juli.logging.Log` looks to be intact, hence I don't
> foresee
> > > > any
> > > > > compatibility issues."
> > > > >
> > > > > Hence I still think this is a legit upgrade. Am I missing
> something?
> > > > >
> > > > > On Sun, Jan 23, 2022 at 1:14 AM Tim Perry 
> wrote:
> > > > >
> > > > > > Many libraries are producing one code line for the javax.*
> > > environment
> > > > > and
> > > > > > another code line for the jakarta.* environment. This is because
> when
> > > > > > Oracle gave the Eclipse Foundation the J2EE code they required
> name
> > > > > > changes. This affects code using Servlet API, JPA, Bean
> Validation,et
> > > > > > cetera.
> > > > > >
> > > > > > Spring:
> > > > > > spring 5 uses javax.* and spring 6 will support jakarta.*
> > > > > >
> > > > > > Hibernate:
> > > > > > Hibernate Validator 6.x will keep the javax.* packages while
> > > Hibernate
> > > > > > Validator 7.x moved to the jakarta.* packages.
> > > > > >
> > > > > >
> > > > >
> > > >
> > >
> https://in.rel

Re: [apache/logging-log4j2] Bump tomcat-catalina from 8.5.20 to 10.0.14 (PR #662)

[Tim Perry]

I think the appserver is a special case where we’d need separate projects for 
Jakarta EE and Java EE.

For other sub-projects, I don’t know. I’ll leave that to someone who 
understands the direction Oracle is taking Java, the Java module system, et 
cetera. 


> On Jan 27, 2022, at 1:08 PM, Ralph Goers  wrote:
> 
> Is there a reason that the javax and Jakarta components can’t both reside 
> in the log4j-appserver module? The don’t share the same package space 
> so that shouldn’t be a problem.
> 
> Ralph
> 
>> On Jan 27, 2022, at 12:44 PM, Tim Perry  wrote:
>> 
>> The pom.xml changes look reasonable to me. I haven't checked it out and
>> played with it. I was thinking I'd have time, but I just got buried with
>> work elsewhere.
>> 
>> I do wonder if log4j should have two modules:
>> * log4j-appserver (Tomcat 9 or less)
>> * log4j-appserver-jakarta (Tomcat 10 or greater)
>> 
>> Are there enough users of log4j-appserver for this to be useful?
>> Eventually, the world will need to adapt the the Jarkarta EE reality but I
>> imagine it will take years or decades. Thoughts?
>> 
>> Tim
>> 
>> 
>> 
>> 
>> 
>> 
>>> On Wed, Jan 26, 2022 at 10:55 AM Volkan Yazıcı  wrote:
>>> 
>>> Mind somebody reviewing the following PR, please?
>>> https://github.com/apache/logging-log4j2/pull/730
>>> Time, what do you think?
>>> 
>>>> On Mon, Jan 24, 2022 at 10:21 PM Tim Perry  wrote:
>>> 
>>>> Sorry Volkan, I think I somehow searched the wrong pom.xml. I was
>>> convinced
>>>> appserver was bringing in log4j-jms, but it isn't.
>>>> 
>>>> You will need to update the servlet version. This won't work with Tomcat
>>>> 10:
>>>> 
>>>> javax.servlet
>>>> javax.servlet-api
>>>> 3.0.1
>>>> provided
>>>> 
>>>> 
>>>> Sorry for the confusion.
>>>> 
>>>> On Mon, Jan 24, 2022 at 1:09 PM Volkan Yazıcı  wrote:
>>>> 
>>>>> This PR only addresses `log4j-appserver`, which doesn't have any
>>> `javax`
>>>>> package dependencies. Quoting from my comment to the PR:
>>>>> 
>>>>> "AFAIK, Tomcat is only used by `log4j-appserver`. There I don't see any
>>>>> dependencies on the `javax` namespace, but just an implementation of
>>>>> `org.apache.juli.logging.Log` packaged by Tomcat. All CI checks also
>>> look
>>>>> green – note that there are no tests associated with `log4j-appserver`,
>>>>> though compilation succeeds. I don't see a reason not to upgrade. If
>>> the
>>>>> user wants to stick to a Tomcat version <10, they can still do so.
>>>>> `org.apache.juli.logging.Log` looks to be intact, hence I don't foresee
>>>> any
>>>>> compatibility issues."
>>>>> 
>>>>> Hence I still think this is a legit upgrade. Am I missing something?
>>>>> 
>>>>> On Sun, Jan 23, 2022 at 1:14 AM Tim Perry  wrote:
>>>>> 
>>>>>> Many libraries are producing one code line for the javax.*
>>> environment
>>>>> and
>>>>>> another code line for the jakarta.* environment. This is because when
>>>>>> Oracle gave the Eclipse Foundation the J2EE code they required name
>>>>>> changes. This affects code using Servlet API, JPA, Bean Validation,et
>>>>>> cetera.
>>>>>> 
>>>>>> Spring:
>>>>>> spring 5 uses javax.* and spring 6 will support jakarta.*
>>>>>> 
>>>>>> Hibernate:
>>>>>> Hibernate Validator 6.x will keep the javax.* packages while
>>> Hibernate
>>>>>> Validator 7.x moved to the jakarta.* packages.
>>>>>> 
>>>>>> 
>>>>> 
>>>> 
>>> https://in.relation.to/2021/01/06/hibernate-validator-700-62-final-released/
>>>>>> 
>>>>>> Tomcat:
>>>>>> When first released, Tomcat 9 and Tomcat 10 were functionally
>>>> identical,
>>>>>> except Tomcat 10 supported jakarta.* and Tomcat 9 supported javax.*.
>>>> They
>>>>>> have slowly diverged as more features have been added to Tomcat 10.
>>> The
>>>>>> difference isn't very big.
>>>>>> 
>>>>>&g

Re: Order of property sources

[Tim Perry]

In that case, should log4j log a warning if more than one provider is included?

Tim


> On Mar 15, 2022, at 1:44 PM, Ralph Goers  wrote:
> 
> I would not be in favor of this change. Providers have been designed since 
> day one that highest wins and it is documented - 
> https://logging.apache.org/log4j/2.x/manual/extending.html#LoggerContextFactory
>  
> .
>  
> 
> The fact that the user was surprised may be true, but the ordering was 
> intentional.  If you include log4j-to-slf4j it means you want to log to 
> SLF4J. In that case log4j-core shouldn’t even be present.
> 
> Ralph
> 
>> On Mar 14, 2022, at 4:37 AM, Piotr P. Karwasz  
>> wrote:
>> 
>>> On Mon, Mar 14, 2022 at 10:54 AM Volkan Yazıcı  wrote:
>>> Regarding your remark about providers... `Provider#getPriority()` has the
>>> following javadoc: *"Gets the priority (natural ordering) of this 
>>> Provider"*,
>>> hence I would have expected it to work the same lowest-comes-first way, but
>>> apparently not – relying on your assessment here. I am also inclined to
>>> align them with the lowest-comes-first strategy, though this might have a
>>> bigger impact if there are 3rd party providers out there in the wild. Maybe
>>> others can weigh in here?
>> 
>> I can provide some additional evidence supporting the necessity to
>> invert the order. At least one user on Stack Overflow was surprised
>> that after adding `log4j-core` to his project, the loggers are still
>> of type `org.apache.logging.slf4j.SLF4JLogger`:
>> 
>> https://stackoverflow.com/q/70487959/11748454
>> 
>> Piotr
> 

Re: Dependencies for `log4j-jakarta-smtp`

[Tim Perry]

Will including `com.sun.activation:jakarta.activation` and
`com.sun.mail:smtp` be a problem on application servers
 that already include implementations of them? My
knowledge of the J2EE application servers is incomplete
for the modern (Java 11+) versions.


On Tue, Apr 19, 2022 at 5:05 PM Matt Sicker  wrote:

> As with the old Java EE dependencies on the APIs, the same pattern
> still applies to Jakarta EE via the "provided" scope (or "compileOnly"
> configuration when using Gradle).
>
> For the second point, I'd lean toward including the dependencies in
> those cases. You could make them "runtime" scope to avoid using any
> classes from it at compile time, though.
>
> On Tue, Apr 19, 2022 at 3:44 AM Piotr P. Karwasz
>  wrote:
> >
> > Hello,
> >
> > I just pushed the `log4j-jakarta-smtp` to `release-2.x`. Thanks to many
> > comments and suggestions in the PR it works as expected, but I have some
> > doubts on the Maven dependency pattern to apply to the artifact.
> >
> > In practice the artifact:
> >
> > 1. Directly references classes from
> > `jakarta.activation:jakarta.activation-api` and
> > `jakarta.mail:jakarta.mail-api`, but since all known implementations of
> > those APIs shade those artifacts instead of depending on them, I put
> those
> > dependencies as optional compile dependencies. Should they be 'provided'
> > dependencies?
> > 2. There is in practice a single implementation of the two APIs above, so
> > to help users of standalone and light servlet container applications I
> > added `com.sun.activation:jakarta.activation` and `com.sun.mail:smtp` (a
> > lighter version of `com.sun.mail:jakarta.mail` with just the SMTP
> provider)
> > as non-optional runtime dependencies. Should they be optional
> dependencies?
> >
> > Piotr
>

Re: Dependencies for `log4j-jakarta-smtp`

[Tim Perry]

Good point. Thank you, Matt.

On Tue, Apr 19, 2022 at 10:49 PM Matt Sicker  wrote:

> If they cause an issue, it's trivial to exclude those dependencies in
> Maven, Gradle, or really any other build system. I suppose we can find
> out from users. :)
>
> On Tue, Apr 19, 2022 at 12:32 PM Tim Perry  wrote:
> >
> > Will including `com.sun.activation:jakarta.activation` and
> > `com.sun.mail:smtp` be a problem on application servers
> >  that already include implementations of them? My
> > knowledge of the J2EE application servers is incomplete
> > for the modern (Java 11+) versions.
> >
> >
> > On Tue, Apr 19, 2022 at 5:05 PM Matt Sicker  wrote:
> >
> > > As with the old Java EE dependencies on the APIs, the same pattern
> > > still applies to Jakarta EE via the "provided" scope (or "compileOnly"
> > > configuration when using Gradle).
> > >
> > > For the second point, I'd lean toward including the dependencies in
> > > those cases. You could make them "runtime" scope to avoid using any
> > > classes from it at compile time, though.
> > >
> > > On Tue, Apr 19, 2022 at 3:44 AM Piotr P. Karwasz
> > >  wrote:
> > > >
> > > > Hello,
> > > >
> > > > I just pushed the `log4j-jakarta-smtp` to `release-2.x`. Thanks to
> many
> > > > comments and suggestions in the PR it works as expected, but I have
> some
> > > > doubts on the Maven dependency pattern to apply to the artifact.
> > > >
> > > > In practice the artifact:
> > > >
> > > > 1. Directly references classes from
> > > > `jakarta.activation:jakarta.activation-api` and
> > > > `jakarta.mail:jakarta.mail-api`, but since all known implementations
> of
> > > > those APIs shade those artifacts instead of depending on them, I put
> > > those
> > > > dependencies as optional compile dependencies. Should they be
> 'provided'
> > > > dependencies?
> > > > 2. There is in practice a single implementation of the two APIs
> above, so
> > > > to help users of standalone and light servlet container applications
> I
> > > > added `com.sun.activation:jakarta.activation` and
> `com.sun.mail:smtp` (a
> > > > lighter version of `com.sun.mail:jakarta.mail` with just the SMTP
> > > provider)
> > > > as non-optional runtime dependencies. Should they be optional
> > > dependencies?
> > > >
> > > > Piotr
> > >
>

Re: Trying to Test and Build release-2.x Failing Locally

[Tim Perry]

Yes, you need JDK 8.
https://www.azul.com/downloads/?version=java-8-lts&package=jdk or similar.

On Thu, Apr 21, 2022 at 4:57 PM Mohammed Barayyan 
wrote:

> Does that mean I have to install JDK 8? or can I modify the toolchains or
> the pom.xml to achieve that?
>
> On Thu, Apr 21, 2022 at 7:56 PM Gary Gregory 
> wrote:
>
> > Use Java 8, not 14.
> >
> > Gary
> >
> > On Thu, Apr 21, 2022, 12:35 Mohammed Barayyan 
> > wrote:
> >
> > > Hello,
> > >
> > > Any advice on the issue please.
> > >
> > > I'm trying to test and build branch 2.x from source but I get an error:
> > >
> > > error: cannot find symbol
> > > [ERROR]   symbol:   class Reflection
> > > [ERROR]   location: package sun.reflect
> > >
> > > More details about my question in:
> > > stackoverflow question  > from
> > > source but I get an error: error: cannot find symbol [ERROR] symbol:
> > class
> > > Reflection [ERROR] location: package sun.reflect More details about my
> > > question in:
> > >
> > >
> >
> https://stackoverflow.com/questions/71957458/building-and-testing-log4j2-from-source-fails-cannot-find-symbol-symbol-clas
> > > >
> > >
> > > Thank you.
> > >
> >
>

Re: More test failures

[Tim Perry]

Do you remember the last release they worked in. I could use git bisect to find 
when they broke if I have a good starting point. I have some time to run it 
this afternoon…,

Tim


Tim Perry
(916) 505-3634

> On May 29, 2022, at 11:44 AM, Ralph Goers  wrote:
> 
> So I have been trying to fix the problems on Windows in the unit test I 
> added recently. I think I have finally fixed that but I just updated from 
> master and now I am getting all the errors below. Did something change?
> 
> Ralph
> 
> [ERROR] Failures:
> [ERROR]   RollingAppenderCronEvery2DirectTest.testAppender(Logger) 
> target\rolling-cron-every2Direct failed with 
> java.nio.file.FileSystemException: 
> target\rolling-cron-every2Direct\test1-05-29-22-12-28-46.log: The process 
> cannot access the file because it is being used by another process.
> 
> [ERROR]   RollingAppenderCronEvery2Test.testAppender(Logger) 
> target\rolling-cron-every2 failed with java.nio.file.FileSystemException: 
> target\rolling-cron-every2\rollingtest.log: The process cannot access the 
> file because it is being used by another process.
> 
> [ERROR]   
> RollingAppenderCronOnceADayTest.testAppender(Logger,RollingFileAppender) 
> target\rolling-cron-once-a-day failed with java.nio.file.FileSystemException: 
> target\rolling-cron-once-a-day\rollingtest.log: The process cannot access the 
> file because it is being used by another process.
> 
> [ERROR]   RollingAppenderCronTest.testAppender(LoggerContext) 
> target\rolling-cron failed with java.nio.file.FileSystemException: 
> target\rolling-cron\rollingtest.log: The process cannot access the file 
> because it is being used by another process.
> 
> [ERROR]   RollingAppenderDirectWrite1906Test.testAppender(LoggerContext) 
> target\rolling-direct-1906 failed with java.nio.file.FileSystemException: 
> target\rolling-direct-1906\rollingfile.2022-05-29-12-28-09.log: The process 
> cannot access the file because it is being used by another process.
> 
> [ERROR]   
> RollingAppenderDirectWriteTempCompressedFilePatternTest.testAppender(LoggerContext)
>  target\rolling-direct failed with java.nio.file.FileSystemException: 
> target\rolling-direct\test1-05-29-22-12-26-25.log: The process cannot access 
> the file because it is being used by another process.
> 
> [ERROR]   RollingAppenderDirectWriteTest.testAppender(LoggerContext) 
> target\rolling-direct failed with java.nio.file.FileSystemException: 
> target\rolling-direct\test1-05-29-22-12-10-20.log: The process cannot access 
> the file because it is being used by another process.
> 
> [ERROR]   
> RollingAppenderDirectWriteWithReconfigureTest.testRollingFileAppenderWithReconfigure(LoggerContext)
>  target\rolling-direct-reconfigure failed with 
> java.nio.file.FileSystemException: 
> target\rolling-direct-reconfigure\test1-05-29-22-12-18-18-203.log: The 
> process cannot access the file because it is being used by another process.
> 
> [ERROR]   RollingAppenderTimeAndSizeDirectTest.testAppender(Logger) 
> target\rolling3Direct failed with java.nio.file.FileSystemException: 
> target\rolling3Direct\test1-05-29-22-12-14-05-21.log: The process cannot 
> access the file because it is being used by another process.
> 
> [ERROR]   RollingAppenderTimeTest.testAppender(LoggerContext) target\rolling2 
> failed with java.nio.file.FileSystemException: 
> target\rolling2\rollingtest.log: The process cannot access the file because 
> it is being used by another process.
> 
> [ERROR]   RollingAppenderUncompressedTest.testAppender(Logger) 
> target\rolling4 failed with java.nio.file.FileSystemException: 
> target\rolling4\rollingtest.log: The process cannot access the file because 
> it is being used by another process.
> 
> [ERROR]   RollingNewDirectoryTest.streamClosedError(Logger) 
> target\rolling-new-directory failed with java.nio.file.FileSystemException: 
> target\rolling-new-directory\2022_05_29-17-15\messages-2022_05_29_12_17_15_048.log:
>  The process cannot access the file because it is being used by another 
> process.
> 
> [ERROR]   RollingRandomAppenderDirectWriteTest.testAppender(Logger) 
> target\rolling-random-direct failed with java.nio.file.FileSystemException: 
> target\rolling-random-direct\test1-05-29-22-12-20-20.log: The process cannot 
> access the file because it is being used by another process.
> 
> [ERROR]   RolloverWithDeletedOldFileTest.testAppender(Logger) 
> target\rolling-with-padding failed with java.nio.file.FileSystemException: 
> target\rolling-with-padding\rollingtest.log: The process cannot access the 
> file because it is being used by another process.
> 
> [ERROR]   RolloverWithPaddingTest.testAppender(Logger) 
> target\rolling-with-padding failed with java.nio.file.FileSystemE

Re: More test failures

[Tim Perry]

They referred to the tests. Do you know a commit they worked at?

Tim


> On May 29, 2022, at 12:43 PM, Ralph Goers  wrote:
> 
> Who is they?  You mean me? This is on the master branch. It has never been 
> released.
> 
> Ralph
> 
>> On May 29, 2022, at 12:31 PM, Tim Perry  wrote:
>> 
>> Do you remember the last release they worked in. I could use git bisect to 
>> find when they broke if I have a good starting point. I have some time to 
>> run it this afternoon…,
>> 
>> Tim
>> 
>> 
>> Tim Perry
>> (916) 505-3634
>> 
>>>> On May 29, 2022, at 11:44 AM, Ralph Goers  
>>>> wrote:
>>> 
>>> So I have been trying to fix the problems on Windows in the unit test I 
>>> added recently. I think I have finally fixed that but I just updated from 
>>> master and now I am getting all the errors below. Did something change?
>>> 
>>> Ralph
>>> 
>>> [ERROR] Failures:
>>> [ERROR]   RollingAppenderCronEvery2DirectTest.testAppender(Logger) 
>>> target\rolling-cron-every2Direct failed with 
>>> java.nio.file.FileSystemException: 
>>> target\rolling-cron-every2Direct\test1-05-29-22-12-28-46.log: The process 
>>> cannot access the file because it is being used by another process.
>>> 
>>> [ERROR]   RollingAppenderCronEvery2Test.testAppender(Logger) 
>>> target\rolling-cron-every2 failed with java.nio.file.FileSystemException: 
>>> target\rolling-cron-every2\rollingtest.log: The process cannot access the 
>>> file because it is being used by another process.
>>> 
>>> [ERROR]   
>>> RollingAppenderCronOnceADayTest.testAppender(Logger,RollingFileAppender) 
>>> target\rolling-cron-once-a-day failed with 
>>> java.nio.file.FileSystemException: 
>>> target\rolling-cron-once-a-day\rollingtest.log: The process cannot access 
>>> the file because it is being used by another process.
>>> 
>>> [ERROR]   RollingAppenderCronTest.testAppender(LoggerContext) 
>>> target\rolling-cron failed with java.nio.file.FileSystemException: 
>>> target\rolling-cron\rollingtest.log: The process cannot access the file 
>>> because it is being used by another process.
>>> 
>>> [ERROR]   RollingAppenderDirectWrite1906Test.testAppender(LoggerContext) 
>>> target\rolling-direct-1906 failed with java.nio.file.FileSystemException: 
>>> target\rolling-direct-1906\rollingfile.2022-05-29-12-28-09.log: The process 
>>> cannot access the file because it is being used by another process.
>>> 
>>> [ERROR]   
>>> RollingAppenderDirectWriteTempCompressedFilePatternTest.testAppender(LoggerContext)
>>>  target\rolling-direct failed with java.nio.file.FileSystemException: 
>>> target\rolling-direct\test1-05-29-22-12-26-25.log: The process cannot 
>>> access the file because it is being used by another process.
>>> 
>>> [ERROR]   RollingAppenderDirectWriteTest.testAppender(LoggerContext) 
>>> target\rolling-direct failed with java.nio.file.FileSystemException: 
>>> target\rolling-direct\test1-05-29-22-12-10-20.log: The process cannot 
>>> access the file because it is being used by another process.
>>> 
>>> [ERROR]   
>>> RollingAppenderDirectWriteWithReconfigureTest.testRollingFileAppenderWithReconfigure(LoggerContext)
>>>  target\rolling-direct-reconfigure failed with 
>>> java.nio.file.FileSystemException: 
>>> target\rolling-direct-reconfigure\test1-05-29-22-12-18-18-203.log: The 
>>> process cannot access the file because it is being used by another process.
>>> 
>>> [ERROR]   RollingAppenderTimeAndSizeDirectTest.testAppender(Logger) 
>>> target\rolling3Direct failed with java.nio.file.FileSystemException: 
>>> target\rolling3Direct\test1-05-29-22-12-14-05-21.log: The process cannot 
>>> access the file because it is being used by another process.
>>> 
>>> [ERROR]   RollingAppenderTimeTest.testAppender(LoggerContext) 
>>> target\rolling2 failed with java.nio.file.FileSystemException: 
>>> target\rolling2\rollingtest.log: The process cannot access the file because 
>>> it is being used by another process.
>>> 
>>> [ERROR]   RollingAppenderUncompressedTest.testAppender(Logger) 
>>> target\rolling4 failed with java.nio.file.FileSystemException: 
>>> target\rolling4\rollingtest.log: The process cannot access the file because 
>>> it is being used by another process.
>>> 
>>> [ERROR]   RollingNewDirectoryTest.streamClosedError(Logger) 
&

Re: Compile `release-2.x` with JDK 11

[Tim Perry]

I've run into an issue where javac 11+ will emit valid java 8 code for
functions that weren't added to Java until after java 8. When the code is
run on Java 8 runtime errors appear complaining about functions missing
from classes that are part of the JRE. Most recently I ran into this when I
used java.lang.String@strip() in a library cross-compiled to java 8
bytecode with Java 11 javac.

As long as there is some method to prevent the compiler from emitting code
that won't work on Java 8, I don't see a reason to compile with Java 8.
However, if this issue can't be prevented, then I think moving to Java 9+
will result in the accidental release of code that won't work on Java 8.

FWIW, I haven't compiled with anything but Java 11+ for the last year and a
half. It is possible more recent versions of the java compiler warn you if
you use functions or maybe there is a maven plugin that would help here.


On Sun, Jun 12, 2022 at 6:24 PM Volkan Yazıcı  wrote:

> Using a recent JDK, yet targeting 8 is a great idea Piotr! I brought up
> this subject a couple of times in the video calls, though, as far as I
> recall, the excuse was mostly the fact that Java 11 work is aimed for 3.x
> and the lack of time.
>
> Personally, I would even favor using Java 17 and configuring the target for
> both `release-2.x` and `master`. In particular, JEP 355 (Text Blocks)
> released in Java 13 can help us to replace the XML configuration files in
> tests and hardcode them into the code.
>
> I also totally agree with the point of Andrew Marlow that using a modern
> JDK for 2.x will significantly decrease the maintenance burden.
>
> On Sun, Jun 12, 2022 at 1:13 PM Piotr P. Karwasz 
> wrote:
>
> > Hi all,
> >
> > Currently the `release-2.x` branch must be run using JDK 8 and uses
> > Maven's toolchains to compile Java 9+ code. This has many drawbacks:
> >  * It is difficult for new contributors to run the build process,
> >  * It requires a change to `JAVA_HOME` each time we build
> > `release-2.x`. I suppose most of you use a different default Java
> > version,
> >  * the latest Maven Surefire plugin fails to run if there are Java 9+
> > classes on the classpath: it performs a class scan in the current JVM
> > and forks only afterwards.
> >
> > From my own tests the `--release 8` switch on JDK 9+ works quite well
> > (i.e. the signatures of the JRE methods are those from JRE 8, not JRE
> > 9+, the classes use the correct bytecode version and it produces
> > errors if we use Java 9+ language features). Maybe we could switch to
> > compiling `release-2.x` using Java 11 with the `--release 8` after
> > 2.18.0 is out? I would keep the toolchains configuration exclusively
> > for the Maven Surefire plugin: it would scan the test classes using
> > the current JVM and fork an authentic JVM 8 to run the tests. What do
> > you think?
> >
> > Piotr
> >
>

Re: Compile `release-2.x` with JDK 11

[Tim Perry]

Yes, that looks like it would address my concern. Thank you. 

> On Jun 12, 2022, at 11:43 AM, Volkan Yazıcı  wrote:
> 
> Can't we perform this check via configuring animal-sniffer-maven-plugin
> <https://www.mojohaus.org/animal-sniffer/animal-sniffer-maven-plugin> to
> work with the `java18` signature
> <https://mvnrepository.com/artifact/org.codehaus.mojo.signature/java18>?
> 
>> On Sun, Jun 12, 2022 at 8:32 PM Tim Perry  wrote:
>> 
>> I've run into an issue where javac 11+ will emit valid java 8 code for
>> functions that weren't added to Java until after java 8. When the code is
>> run on Java 8 runtime errors appear complaining about functions missing
>> from classes that are part of the JRE. Most recently I ran into this when I
>> used java.lang.String@strip() in a library cross-compiled to java 8
>> bytecode with Java 11 javac.
>> 
>> As long as there is some method to prevent the compiler from emitting code
>> that won't work on Java 8, I don't see a reason to compile with Java 8.
>> However, if this issue can't be prevented, then I think moving to Java 9+
>> will result in the accidental release of code that won't work on Java 8.
>> 
>> FWIW, I haven't compiled with anything but Java 11+ for the last year and a
>> half. It is possible more recent versions of the java compiler warn you if
>> you use functions or maybe there is a maven plugin that would help here.
>> 
>> 
>>> On Sun, Jun 12, 2022 at 6:24 PM Volkan Yazıcı  wrote:
>>> 
>>> Using a recent JDK, yet targeting 8 is a great idea Piotr! I brought up
>>> this subject a couple of times in the video calls, though, as far as I
>>> recall, the excuse was mostly the fact that Java 11 work is aimed for 3.x
>>> and the lack of time.
>>> 
>>> Personally, I would even favor using Java 17 and configuring the target
>> for
>>> both `release-2.x` and `master`. In particular, JEP 355 (Text Blocks)
>>> released in Java 13 can help us to replace the XML configuration files in
>>> tests and hardcode them into the code.
>>> 
>>> I also totally agree with the point of Andrew Marlow that using a modern
>>> JDK for 2.x will significantly decrease the maintenance burden.
>>> 
>>> On Sun, Jun 12, 2022 at 1:13 PM Piotr P. Karwasz <
>> piotr.karw...@gmail.com>
>>> wrote:
>>> 
>>>> Hi all,
>>>> 
>>>> Currently the `release-2.x` branch must be run using JDK 8 and uses
>>>> Maven's toolchains to compile Java 9+ code. This has many drawbacks:
>>>> * It is difficult for new contributors to run the build process,
>>>> * It requires a change to `JAVA_HOME` each time we build
>>>> `release-2.x`. I suppose most of you use a different default Java
>>>> version,
>>>> * the latest Maven Surefire plugin fails to run if there are Java 9+
>>>> classes on the classpath: it performs a class scan in the current JVM
>>>> and forks only afterwards.
>>>> 
>>>> From my own tests the `--release 8` switch on JDK 9+ works quite well
>>>> (i.e. the signatures of the JRE methods are those from JRE 8, not JRE
>>>> 9+, the classes use the correct bytecode version and it produces
>>>> errors if we use Java 9+ language features). Maybe we could switch to
>>>> compiling `release-2.x` using Java 11 with the `--release 8` after
>>>> 2.18.0 is out? I would keep the toolchains configuration exclusively
>>>> for the Maven Surefire plugin: it would scan the test classes using
>>>> the current JVM and fork an authentic JVM 8 to run the tests. What do
>>>> you think?
>>>> 
>>>> Piotr
>>>> 
>>> 
>> 

Re: Compile `release-2.x` with JDK 11

[Tim Perry]

Piotr,

I did a troll back through the history of the maven artifact in question
and I can see one of the admins removed the following line from the pom.xml
because it broke compilation with Java 8:

 8

As you correctly guessed, this meant API compatibility wasn't checked when
building with Java 11 and explains what went wrong. Thanks for reminding me
what the issue was. Sorry to have brought this up, but I suppose we are
better safe than sorry.

Happy Sunday.
Tim


On Sun, Jun 12, 2022 at 8:26 PM Piotr P. Karwasz 
wrote:

> Hi Tim,
>
> On Sun, 12 Jun 2022 at 20:32, Tim Perry  wrote:
> > I've run into an issue where javac 11+ will emit valid java 8 code for
> > functions that weren't added to Java until after java 8. When the code is
> > run on Java 8 runtime errors appear complaining about functions missing
> > from classes that are part of the JRE. Most recently I ran into this
> when I
> > used java.lang.String@strip() in a library cross-compiled to java 8
> > bytecode with Java 11 javac.
>
> Are you sure that the library was compiled with `--release 8` and not
> with `-source 8 -target 8`? `String#strip()` does not compile on my
> JDK 11, as well as `ByteBuffer#position(int)` uses the correct return
> type of `Buffer` (JRE 8) instead of `ByteBuffer` (JRE 11).
>
> Piotr
>

Re: Compile `release-2.x` with JDK 11

[Tim Perry]

Piotr,

It looks like the following will add the release flag to builds on Java 9+
but still work with Java 8. Should we use this?


  java-8-api
  
[9,)
  
  
8
  


On Sun, Jun 12, 2022 at 10:16 PM Matt Sicker  wrote:

> Just see some of the commits in master that made that buildable in newer
> JDKs for ideas on what things need to be updated in 2.x for similar
> convenience. I’m not so sure about the release process requiring Java 11,
> but I’d support having this offered for convenience. Really, any build
> enhancements that makes the 2.x branch maintainable while 3.x becomes the
> new current will be great since I assume we’d all like to continue
> maintaining bug fixes and such for 2.x for a few years at least.
>
> —
> Matt Sicker
>
> > On Jun 12, 2022, at 16:47, Tim Perry  wrote:
> >
> > Piotr,
> >
> > I did a troll back through the history of the maven artifact in question
> > and I can see one of the admins removed the following line from the
> pom.xml
> > because it broke compilation with Java 8:
> >
> > 8
> >
> > As you correctly guessed, this meant API compatibility wasn't checked
> when
> > building with Java 11 and explains what went wrong. Thanks for reminding
> me
> > what the issue was. Sorry to have brought this up, but I suppose we are
> > better safe than sorry.
> >
> > Happy Sunday.
> > Tim
> >
> >
> >> On Sun, Jun 12, 2022 at 8:26 PM Piotr P. Karwasz <
> piotr.karw...@gmail.com>
> >> wrote:
> >>
> >> Hi Tim,
> >>
> >>> On Sun, 12 Jun 2022 at 20:32, Tim Perry  wrote:
> >>> I've run into an issue where javac 11+ will emit valid java 8 code for
> >>> functions that weren't added to Java until after java 8. When the code
> is
> >>> run on Java 8 runtime errors appear complaining about functions missing
> >>> from classes that are part of the JRE. Most recently I ran into this
> >> when I
> >>> used java.lang.String@strip() in a library cross-compiled to java 8
> >>> bytecode with Java 11 javac.
> >>
> >> Are you sure that the library was compiled with `--release 8` and not
> >> with `-source 8 -target 8`? `String#strip()` does not compile on my
> >> JDK 11, as well as `ByteBuffer#position(int)` uses the correct return
> >> type of `Buffer` (JRE 8) instead of `ByteBuffer` (JRE 11).
> >>
> >> Piotr
> >>
>

Re: Dependencies for `log4j-jakarta-smtp`

[Tim Perry]

I kind of like Matt's solution of just telling people to exclude those
artifacts when they don't need them. Otherwise we'll have the same issue in
reverse: requests from people about how to include the "missing"
dependencies

On Thu, Jul 14, 2022 at 8:56 PM Piotr P. Karwasz 
wrote:

> Hi Tim,
>
> On Tue, 19 Apr 2022 at 19:32, Tim Perry  wrote:
> >
> > Will including `com.sun.activation:jakarta.activation` and
> > `com.sun.mail:smtp` be a problem on application servers
> >  that already include implementations of them? My
> > knowledge of the J2EE application servers is incomplete
> > for the modern (Java 11+) versions.
>
> You were right on the spot:
> https://issues.apache.org/jira/browse/LOG4J2-3554
>
> I believe we should remove the implementation artifacts from the POM
> and leave just the APIs (as `provided`).
>
> Piotr
>

Re: Javadoc HTML, JARs, and JXR

[Tim Perry]

I was going to say the same thing. I'm wondering if there is ever a
situation where someone can download the javadoc jars but not source jars.

On Tue, Feb 14, 2023 at 9:58 AM Piotr P. Karwasz 
wrote:

> Hi Matt,
>
> On Tue, 14 Feb 2023 at 17:58, Matt Sicker  wrote:
> >
> > ... I’m not sure if IDEs and such can figure out the javadocs from the
> source jar itself.
>
> I have disabled javadoc downloads in my Eclipse ages ago and it still
> shows me javadocs. Whenever sources are available, I don't believe
> that javadoc jars are useful.
>
> Piotr
>

Re: Javadoc HTML, JARs, and JXR

[Tim Perry]

Yes, that is true, but a different situation than I thought we were
addressing.  I meant my question to be "if both javadoc and source jars are
available, would anyone somehow be able to get javadoc jars but not source
jars?"

On Tue, Feb 14, 2023 at 11:01 AM Volkan Yazıcı  wrote:

> Yes, shareware. That is, Javadoc JARs allow you to share your documentation
> without sharing your sources. AFAIK, almost all modern IDEs (Eclipse,
> NetBeans, IDEA) prioritize displaying documentation from source JARs, if
> available. Since ASF makes it obligatory to share sources, I see no purpose
> for Javadoc JARs. As a matter of fact, displaying (java)docs from sources
> also enable IDEs to allow users to jump directly to sources too, which I
> think is tremendously convenient.
>
> On Tue, Feb 14, 2023 at 7:57 PM Tim Perry  wrote:
>
> > I was going to say the same thing. I'm wondering if there is ever a
> > situation where someone can download the javadoc jars but not source
> jars.
> >
> > On Tue, Feb 14, 2023 at 9:58 AM Piotr P. Karwasz <
> piotr.karw...@gmail.com>
> > wrote:
> >
> > > Hi Matt,
> > >
> > > On Tue, 14 Feb 2023 at 17:58, Matt Sicker  wrote:
> > > >
> > > > ... I’m not sure if IDEs and such can figure out the javadocs from
> the
> > > source jar itself.
> > >
> > > I have disabled javadoc downloads in my Eclipse ages ago and it still
> > > shows me javadocs. Whenever sources are available, I don't believe
> > > that javadoc jars are useful.
> > >
> > > Piotr
> > >
> >
>

Re: [log4j] Checkout from git is broken

[Tim Perry]

 The warning: remote HEAD refers to nonexistent ref, unable to checkout.
means that the remote (bare) repository contains *branch reference* in the
file called HEAD with a value that does not match any published branch in
the same repository. In practice, that file defines *which branch* should
be checked out by default after cloning the repository.

https://stackoverflow.com/questions/11893678/warning-remote-head-refers-to-nonexistent-ref-unable-to-checkout

On Thu, Mar 9, 2023 at 2:51 PM Gary D. Gregory  wrote:

> Hi All:
>
> My expectation is that:
>
> C:\Users\ggregory\git\a>git clone
> https://gitbox.apache.org/repos/asf/logging-log4j2.git logging-log4j3
>
> Would clone and checkout master (or main or whatever the default branch is
> called now) but instead, I get an empty folder and this output:
>
> Cloning into 'logging-log4j3'...
> remote: Enumerating objects: 262413, done.
> remote: Counting objects: 100% (262413/262413), done.
> remote: Compressing objects: 100% (68647/68647), done.
> remote: Total 262413 (delta 127358), reused 256238 (delta 122788)
> Receiving objects: 100% (262413/262413), 38.27 MiB | 11.31 MiB/s, done.
> Resolving deltas: 100% (127358/127358), done.
> warning: remote HEAD refers to nonexistent ref, unable to checkout
>
> This says nothing to help:
> https://github.com/apache/logging-log4j2/blob/2.x/BUILDING.md
> and nothing jumps out at me on the main page
> https://logging.apache.org/log4j/2.x/index.html
>
> So... what's the magic missing?
>
> TY!
> Gary
>
>

Re: Renaming `log4j-core`

[Tim Perry]

Can we publish log4j-core so it spits out a deprecation warning and just
pulls in log4j-impl or log4j-runtime or whatever? That might address
Ralph's concern. If we can't avoid the issues Ralph describes, then I'd
vote -1 too.

On Thu, Jun 22, 2023 at 8:13 AM Ralph Goers 
wrote:

> Pretend for a moment that you work for a company that has lots of shared
> components that don’t always do everything correctly and publish artifacts
> that declare a dependency on log4j-core (Note: I do).  If we change the
> name of log4j-core to something else then suddenly both an older version of
> log4j-core and the new artifact are going to be on the class path. The user
> will absolutely not know this until they start running their application
> and start to have weird problems.  This is exactly why when Commons
> components change the artifact name they also require the package names to
> be changed. However, this still would not really help in our case as now
> both log4j 2.x and log4j 3.x would be present which would undoubtedly
> create a host of problems.
>
> Without having an easy and foolproof way to deal with that I would have to
> vote -1 on changing the artifact name.
>
> Note that all the examples of projects renaming do not have the same
> problems Log4j will when both are present on the class path.
>
> However, I am in favor of splitting the web site in two between the API
> and the implementation, possibly even giving some of the optional modules
> their own web site or at least breaking them more clearly into their own
> pages.
>
> Ralph
>
> > On Jun 22, 2023, at 1:34 AM, Piotr P. Karwasz 
> wrote:
> >
> > Hi all,
> >
> > I think one of the main problems preventing Log4j API from being used
> > more wildly are naming problems and misinformation on many sites.
> >
> > Personally I find the name `log4j-core` for our implementation quite
> > unfortunate: this is often interpreted as "core Log4j classes", which
> > suggests that all artifacts including `log4j-api` should be considered
> > as a unit.
> >
> > I would profit from the major version bump to change it to
> > `log4j-impl` or `log4j-runtime`.
> >
> > Similar changes have occurred in other projects. For example JAXB
> > changed it's implementation from `jaxb-impl`[1] to `jaxb-runtime`[2]
> > (and also the group id), during the jakartification process.
> >
> > The Java EE Mail project used `javax.mail-api` for their API and
> > `javax.mail` for their implementation. Now they renamed their
> > implementation to `angus-mail`, which stresses the difference between
> > API and implementation more (although in this case Angus **is** the
> > only implementation available).
> >
> > So, what do you think about renaming `log4j-core`?
> >
> > Piotr
> >
> > [1] https://mvnrepository.com/artifact/com.sun.xml.bind/jaxb-impl
> > [2] https://mvnrepository.com/artifact/org.glassfish.jaxb/jaxb-runtime
> > [3] https://mvnrepository.com/artifact/javax.mail/javax.mail-api
> > [4] https://mvnrepository.com/artifact/com.sun.mail/javax.mail
> > [5] https://mvnrepository.com/artifact/org.eclipse.angus/angus-mail
>
>

Re: Renaming `log4j-core`

[Tim Perry]

> On Jun 26, 2023, at 1:38 PM, Ralph Goers  wrote:
> 
> 
> 
>> On Jun 26, 2023, at 12:17 PM, Piotr P. Karwasz  
>> wrote:
>> 
>> Hi Ralph,
>> 
>> On Mon, 26 Jun 2023 at 20:33, Ralph Goers  wrote:
>>>> On Jun 25, 2023, at 11:32 PM, Piotr P. Karwasz  
>>>> wrote:
>>>> 
>>>> Hi Tim,
>>>> 
>>>>> On Sat, 24 Jun 2023 at 22:00, Tim Perry  wrote:
>>>>>> 
>>>>>> Can we publish log4j-core so it spits out a deprecation warning and just
>>>>>> pulls in log4j-impl or log4j-runtime or whatever?
>>>>> 
>>>>> This is probably the best solution.
>>> 
>>> How can it be implemented?
>> 
>> We replace `log4j-core` with an empty JAR file with `log4j-impl` as 
>> dependency.
> 
> I can’t say I am thrilled with this idea but I also can’t point to any 
> concrete problems it would cause without testing

It would need a class which prints a depreciation warning at startup as well. 
Preferably with directions to fix. 

I’m not thrilled by this, but at least it helps them along. 

Possibly better to stick with log4j-core

Move Chainsaw to dormant: Q

[Tim Perry]

Do dormant projects clearly indicate they can be re-animated by a sufficient 
number of active committers?

Thanks, 
Tim

Re: A logger config named "root"

[Tim Perry]

How much code is it to support "root" fully? If this is trivial, fully
supporting "root" makes sense. If not, deprecating it makes sense. Heck, do
both! LOL.

On Tue, Nov 19, 2024 at 7:11 AM Piotr P. Karwasz 
wrote:

> Hi Gary,
>
> On 19.11.2024 13:08, Gary Gregory wrote:
> > This seems like a nice clean up that will break a lot of configurations.
> >
> > How about we issue a warning in 2.x like "The logger name 'root' is
> > deprecated and will be removed in 3.0"?
>
> I thought about going in the other direction: allow users to call
> `Configurator.setLevel("ROOT", Level.INFO)`.
>
> It is unlikely that "ROOT" will collide with any real logger.
>
> Piotr
>
>