Ok, that’s the one I reverted. I’ll split it up into more digestible pieces.
> On Nov 1, 2022, at 6:40 PM, Ralph Goers <ralph.go...@dslextreme.com> wrote:
>
> Thanks Matt. I was referring to the one in the first email of this thread.
>
> Ralph
>
>> On Nov 1, 2022, at 1:41 PM, Matt Sicker <m...@musigma.org> wrote:
>>
>> I’ve reverted the last two commits as they were basically the last commit,
>> though I’m not exactly sure if you’re referring to that one in particular.
>> I’ll break it down into smaller pieces, though, and submit the non-trivial
>> parts as a PR.
>> —
>> Matt Sicker
>>
>>> On Nov 1, 2022, at 10:21, Ralph Goers <ralph.go...@dslextreme.com> wrote:
>>>
>>> I have more questions on this.
>>>
>>> 1. PropertiesUtil cannot be deprecated. It is used by Spring Boot to
>>> register its PropertiesSource.
>>> 2. All these classes that you have marked internal are opened to various
>>> Log4j modules. That means they are NOT open to any other implementation of
>>> the Log4j-API. Can we really do that?
>>>
>>> The more I am looking at this I would prefer that you revert this commit
>>> and put it on a branch as several PRs that split it up into things we can
>>> really review.
>>>
>>> So I am -1 on this commit.
>>>
>>> Ralph
>>>
>>>> On Oct 31, 2022, at 7:40 PM, Apache <ralph.go...@dslextreme.com> wrote:
>>>>
>>>> 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(-)
>>>>>>>>>
>>>>>>>>
>>>>>>
>>>>
>>>
>>
>
