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(-)
>>> 
>> 

Reply via email to