Are you using your old email address? I keep having to moderate them through.

Yes they should be in an appropriately named package under the internal package.

Ralph

> On Oct 31, 2022, at 7:36 PM, Matt Sicker <mattsic...@sickmatter.com> wrote:
> 
> 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(-)
>>>>> 
>>>> 
>> 

Reply via email to