Gary, To be clear, you are proposing that we create the same log4j-api-test and log4j-core-test modules that exist in master? If you want to do that work I won’t object. But I myself would prefer to focus on master at this point as much as possible so we can get 3.0 out in a reasonable time frame. Obviously, I have Jira issues in my queue that also need to be addressed so I will also still have to commit to release-2.x.
Ralph > On Jan 6, 2022, at 6:53 AM, Gary Gregory <[email protected]> wrote: > > On Mon, Jan 3, 2022 at 7:30 AM Volkan Yazıcı <[email protected]> wrote: > >> Fantastic work Ralph! Please see my comments below: >> >> On Tue, Dec 28, 2021 at 9:39 AM Ralph Goers <[email protected]> >> wrote: >>> Most of the components that were generating test jars have been split >> into two modules - the main component, >>> which only builds the jar, and a -test component that builds a -test jar >> and then runs the unit tests. >> >> For completeness and consistency across all modules *and* to (hopefully) >> mitigate the problem you have mentioned in #2, I think we should place all >> tests to their own modules. Even though we all agree that this is ugly, >> IIRC, our most recent Maven and JPMS research pointed us in this direction. >> >> I also propose backporting every "moving tests to a module" change to >> `release-2.x`. Otherwise, it will be really difficult to synchronize >> changes on both branches in the future. >> > > I agree with Volkan. Whatever nightmare of a mess JPMS is making in master, > it is no longer possible to cherry-pick what should be simple commits from > release-2.x to master. Patches/diff files are not even possible, it is all > manual, this is a f*g PITA for both development and maintenance as I am > currently experiencing :-( This is not a good use of time. > > Frustrated, > Gary > > >> >>> 1. log4j-core uses toolchains on MacOS to use Java 17. There is a bug in >> javac on Macs that causes compilation >>> to fail. It is fixed in Java 17 but was not back-ported to Java 11. >> >> For one, I am fine if we set the build (not target!) JDK requirement to >> Java 17 for all platforms to work around this problem. This said, I am >> confused on why we still need Maven toolchains in `master`. Shouldn't we >> simply remove all Maven toolchains configuration and configure >> maven-compiler-plugin and maven-enforcer plugin to require 17 for build and >> target 11? >> >>> 2. In general the test jars have a second compilation step to create the >> module-info.java. Unfortunately, the unit >>> tests cannot be modularized because: >>> a. the unit tests open the main module (i.e. they use the same >> package names). >>> b. the test jar requires the main module. >>> c. the unit tests require the test module. >>> This creates a circularity because the unit tests depend on the test >> module and it requires the same packages >>> as the unit tests. >> >> Will this still pose a problem if we move all tests to their own module? >> >>> 3. I tried building entirely with Java 17 but there were some unit tests >> in log4j-core-test that fail. There is one class >>> that is trying to modify a final variable in the Constant class and >> it seems that the technique it is using is no >>> longer valid. >>> 4. log4j-jul must use Junit 4. It seems Junit 5 is hard-wired to use >> java.util.logging and so it isn’t possible to set the >>> system property in time to have the unit tests use Log4j’s LogManager. >> >> Do we have tickets for these two? >> >>> 5. JsonTemplateLayout is still creating a test-jar. I haven’t validated >> if anything needs it. If it does than it will need to >>> be split to have a -test module as well. >> >> JTL test-jar is used by log4j-perf. >> I have created LOG4J2-3308 >> <https://issues.apache.org/jira/browse/LOG4J2-3308> to address this. >> >>> 6. I split the annotation processor into its own module - >> log4j-plugin-processor. Note that with modular compiles >>> annotation processors are no longer added automatically so this >> should be clearer. >>> 7. I have not added module-info.java files to anything beyond log4j-api, >> log4j-plugins, and log4j-core. That will be >>> coming next. >>> 8. log4j-api-test has a few tests that seem to randomly fail. I didn’t >> look into them. >> >> Again, great work Ralph! >> Thanks so much for pushing `master` further. >>
