We could consolidate the Log4j-private classes into the internal package and export it to other Log4j modules. That makes sense.
And LoggingSystem probably doesn’t need a Lazy instance of itself, right. — Matt Sicker > On Oct 31, 2022, at 20:58, Apache <ralph.go...@dslextreme.com> wrote: > > > > See below > >> On Oct 31, 2022, at 6:49 PM, Matt Sicker <mattsic...@sickmatter.com> wrote: >> >> The util3 package is a collection of all the private classes we have. We >> discussed the general idea on a recent conference call. These are pseudo >> private classes. They’re meant for internal use only and are guarded by the >> module info. JUnit does the same thing > > If they are private then they should be under a package named internal. It > won’t be obvious to devs not using JPMS that they are private. > >> The point of Lazy is ensuring thread safe publication of initialized >> variables that may be initialized by multiple threads. There are two main >> variants: one that guarantees the supplier function is executed only once >> and one that may run the supplier more than once but has a single return >> value. > > But why is it necessary? When getInstance is called that operation should > only be performed once. GetInstance is already lazy so why do we need lazy > initialization of the stuff in it? > > Ralph > >> >> — >> Matt Sicker >> >>>> On Oct 31, 2022, at 17:53, Ralph Goers <ralph.go...@dslextreme.com> wrote: >>> >>> Also, in LoggingSystem.java >>> >>> private static final String[] COMPATIBLE_API_VERSIONS = {"2.6.0"}; >>> >>> absolutely needs to be changed to version “3.0.0”. >>> >>> Ralph >>> >>>>> On Oct 31, 2022, at 3:49 PM, Ralph Goers <ralph.go...@dslextreme.com> >>>>> wrote: >>>> >>>> Matt, >>>> >>>> I absolutely would have preferred that you had created a PR for this and >>>> asked for a review before committing it as I have a few concerns: >>>> >>>> 1. It is a massive commit. It is going to take time to digest exactly what >>>> you have done. >>>> 2. You have created a util3 package in API with no discussion of this on >>>> the dev list. I assume that it contains artifacts that were cleaned up >>>> from util and are now no longer compatible? Or does it only contain >>>> classes that are exportable to other Log4j artifacts (I do see that in >>>> module-info.java). If these classes are meant to be private then I would >>>> have preferred that it be moved to internal/util. >>>> 3. I no longer understand the point of Lazy<>. LogManager now performs no >>>> static initialization that I can see. Instead, it calls >>>> LoggingSystem.getInstance(). What is the point of making initialization of >>>> INSTANCE lazy? You accomplished what you wanted to by moving the static >>>> initialization to another class. >>>> 4-10. I have no idea as it will take me a week to muddle through the rest >>>> of all these changes to figure out what happened. >>>> >>>> Ralph >>>> >>>>>> On Oct 30, 2022, at 6:24 PM, mattsic...@apache.org wrote: >>>>> >>>>> This is an automated email from the ASF dual-hosted git repository. >>>>> >>>>> mattsicker pushed a commit to branch master >>>>> in repository https://gitbox.apache.org/repos/asf/logging-log4j2.git >>>>> >>>>> >>>>> The following commit(s) were added to refs/heads/master by this push: >>>>> new aab23d5c05 [LOG4J2-3459] - Add LoggingSystemProvider SPI >>>>> aab23d5c05 is described below >>>>> >>>>> commit aab23d5c05bff2915c290dff3e73a0ae03caf43a >>>>> Author: Matt Sicker <mattsic...@apache.org> >>>>> AuthorDate: Sun Oct 30 20:24:17 2022 -0500 >>>>> >>>>> [LOG4J2-3459] - Add LoggingSystemProvider SPI >>>>> >>>>> This helps clean up a lot of static state in Log4j API along with >>>>> enabling further extensibility by exposing >>>>> LoggingSystemProvider::getInstance which can be implemented using >>>>> Injector in Log4j Core. The general utility class for working with this >>>>> provider is available as an internal (util3) class in LoggingSystem. Test >>>>> fixture helper classes have also been updated to use LoggingSystem >>>>> directly. >>>>> >>>>> Signed-off-by: Matt Sicker <mattsic...@apache.org> >>>>> --- >>>>> .../junit/LogManagerLoggerContextFactoryRule.java | 11 +- >>>>> .../test/junit/LoggerContextFactoryExtension.java | 12 +- >>>>> .../{TestProvider.java => TestSystemProvider.java} | 23 +- >>>>> .../log4j/spi/DefaultThreadContextMapTest.java | 10 +- >>>>> ....apache.logging.log4j.spi.LoggingSystemProvider | 17 ++ >>>>> .../services/org.apache.logging.log4j.spi.Provider | 1 - >>>>> log4j-api/src/main/java/module-info.java | 5 +- >>>>> .../java/org/apache/logging/log4j/LogManager.java | 76 +------ >>>>> .../org/apache/logging/log4j/ThreadContext.java | 133 +++++------ >>>>> .../org/apache/logging/log4j/internal/Binding.java | 32 ++- >>>>> .../apache/logging/log4j/internal/BindingMap.java | 59 +++++ >>>>> .../log4j/simple/SimpleLoggingSystemProvider.java | 24 +- >>>>> .../log4j/spi/AbstractLoggingSystemProvider.java | 60 +++++ >>>>> .../CopyOnWriteSortedArrayThreadContextMap.java | 68 ++---- >>>>> .../log4j/spi/DefaultContextMapFactory.java | 102 +++++++++ >>>>> .../log4j/spi/DefaultContextStackFactory.java | 50 ++++ >>>>> .../logging/log4j/spi/DefaultThreadContextMap.java | 36 +-- >>>>> .../GarbageFreeSortedArrayThreadContextMap.java | 51 ++--- >>>>> .../log4j/spi/LegacyLoggingSystemProvider.java | 125 ++++++++++ >>>>> .../logging/log4j/spi/LoggingSystemProvider.java | 82 +++++++ >>>>> .../org/apache/logging/log4j/spi/Provider.java | 253 >>>>> --------------------- >>>>> .../logging/log4j/spi/ThreadContextMapFactory.java | 134 ----------- >>>>> .../org/apache/logging/log4j/util/LazyInt.java | 52 ----- >>>>> .../org/apache/logging/log4j/util3/Activator.java | 57 +++-- >>>>> .../org/apache/logging/log4j/util3/Constants.java | 43 ++-- >>>>> .../apache/logging/log4j/util3/LoggingSystem.java | 223 >>>>> ++++++++++++++++++ >>>>> .../apache/logging/log4j/util3/PropertiesUtil.java | 31 +-- >>>>> .../apache/logging/log4j/util3/ProviderUtil.java | 143 ------------ >>>>> .../core/test/junit/ContextSelectorCallback.java | 9 +- >>>>> .../core/test/junit/LoggerContextResolver.java | 10 +- >>>>> .../log4j/core/test/junit/LoggerContextRule.java | 5 +- >>>>> ....apache.logging.log4j.spi.LoggingSystemProvider | 17 ++ >>>>> .../services/org.apache.logging.log4j.spi.Provider | 1 - >>>>> log4j-core/src/main/java/module-info.java | 7 +- >>>>> .../log4j/core/impl/Log4jSystemProvider.java | 75 ++++++ >>>>> .../apache/logging/log4j/core/osgi/Activator.java | 4 +- >>>>> ....apache.logging.log4j.spi.LoggingSystemProvider | 17 ++ >>>>> .../services/org.apache.logging.log4j.spi.Provider | 1 - >>>>> .../CopyOnWriteOpenHashMapThreadContextMap.java | 3 +- >>>>> .../GarbageFreeOpenHashMapThreadContextMap.java | 3 +- >>>>> log4j-to-jul/src/main/java/module-info.java | 7 +- >>>>> .../{JULProvider.java => JULSystemProvider.java} | 24 +- >>>>> ....apache.logging.log4j.spi.LoggingSystemProvider | 17 ++ >>>>> .../services/org.apache.logging.log4j.spi.Provider | 18 -- >>>>> .../org/apache/logging/slf4j/MDCContextMap.java | 19 +- >>>>> .../apache/logging/slf4j/SLF4JSystemProvider.java | 30 ++- >>>>> ....apache.logging.log4j.spi.LoggingSystemProvider | 17 ++ >>>>> .../services/org.apache.logging.log4j.spi.Provider | 1 - >>>>> src/changes/changes.xml | 3 + >>>>> src/site/asciidoc/manual/extending.adoc | 20 +- >>>>> src/site/asciidoc/runtime-dependencies.adoc | 4 +- >>>>> 51 files changed, 1202 insertions(+), 1023 deletions(-) >>>> >>> >