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 <[email protected]> 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 <[email protected]> wrote: > > > > Is it okay to import hamcrest to use in the tests? > > > > <dependency> > > <groupId>org.hamcrest</groupId> > > <artifactId>hamcrest</artifactId> > > <version>2.2</version> > > <scope>test</scope> > > </dependency> > > > > > > On Mon, Jan 25, 2021 at 10:44 AM Tim Perry <[email protected]> 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 <[email protected]> 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 <[email protected]> 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 <[email protected]> > 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 <[email protected]> > 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 <[email protected]> > > >> 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 <[email protected]> > > >> 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 > > >> > > > > > > >> > > > > >> > > > >
