Re: [PR] Turn on the CI build for PRs (logging-log4net)
gdziadkiewicz commented on code in PR #204: URL: https://github.com/apache/logging-log4net/pull/204#discussion_r1836828847 ## .github/workflows/build.yaml: ## @@ -47,11 +55,20 @@ jobs: with: node-version: 20 - - name: Build and test -env: - DOTNET_SKIP_FIRST_TIME_EXPERIENCE: 1 - DOTNET_CLIE_TELEMETRY_OPTOUT: 1 + - name: Setup non-windows build +if: matrix.os != 'windows-latest' +run: | + echo "DOTNET_CORE=1" >> $GITHUB_ENV + + - name: Install build dependencies run: | npm update -g npm --no-progress npm install --no-progress - npm test + + - name: Build Review Comment: That's true, but it's easier to tell what failed when the phases are separated. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: notifications-unsubscr...@logging.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [I] Replace field injection with setter injection in `3.x` (logging-log4j2)
ppkarwasz commented on issue #2769: URL: https://github.com/apache/logging-log4j2/issues/2769#issuecomment-2468934317 @jaykataria, > Hi @ppkarwasz, had a general question about how Pull Requests and Code reviews work in this repo, do I have to reach out to folks to drive it or, someone volunteering will just pickup the review on their own time. We don't have a lot of active maintainers (more maintainers are always welcome, see [becoming a committer](https://community.apache.org/contributors/becomingacommitter.html#becoming-a-committer) on the ASF website), but we'll look at your PR as soon as we have time. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: notifications-unsubscr...@logging.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Fix(#2769): Modify the annotation processor in 2.x to give a warning if a plugin builder attribute does not have a public setter. (logging-log4j2)
github-actions[bot] commented on PR #3195: URL: https://github.com/apache/logging-log4j2/pull/3195#issuecomment-2468966307 Job Requested goals Build Tool Version Build Outcome Build Scan® build-macos-latest clean install 3.9.8 :white_check_mark: https://ge.apache.org/s/2titc2ozzjdfm"; rel="nofollow">https://img.shields.io/badge/Build%20Scan%C2%AE-PUBLISHED-06A0CE?logo=Gradle"; alt="Build Scan PUBLISHED" /> build-ubuntu-latest clean install 3.9.8 :white_check_mark: https://ge.apache.org/s/k6rjxuc63xaei"; rel="nofollow">https://img.shields.io/badge/Build%20Scan%C2%AE-PUBLISHED-06A0CE?logo=Gradle"; alt="Build Scan PUBLISHED" /> build-windows-latest clean install 3.9.8 :white_check_mark: https://ge.apache.org/s/h5wganur6uro6"; rel="nofollow">https://img.shields.io/badge/Build%20Scan%C2%AE-PUBLISHED-06A0CE?logo=Gradle"; alt="Build Scan PUBLISHED" /> ## Generated by gradle/develocity-actions -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: notifications-unsubscr...@logging.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [I] Replace field injection with setter injection in `3.x` (logging-log4j2)
jaykataria commented on issue #2769: URL: https://github.com/apache/logging-log4j2/issues/2769#issuecomment-2468973925 Hi @ppkarwasz , it would be awesome I can contribute more, let me see the steps to become a contributor, this is awesome thanks a lot 🙂 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: notifications-unsubscr...@logging.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[PR] Removes weak references from `LoggerRepository` (logging-log4j2)
ppkarwasz opened a new pull request, #3199: URL: https://github.com/apache/logging-log4j2/pull/3199 Removes weak references to `Logger`s in `LoggerRepository`. The usage of weak references in `LoggerRepository` might cause `null` to be returned by `LogManager.getLogger()` of all Log4j Core versions up to `2.24.1`. Versions of Log4j API up to `2.24.0` did hold **hard** references to all the registered loggers, so the change will not alter the previous behavior. This PR also inverts the order of the `String` and `MessageFactory` keys to the `LoggerRepository` multi-map to limit the number of internal maps. The external map is a `WeakHashMap` to allow logger-specific message factories to be GC-ed. Closes #3143. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: notifications-unsubscr...@logging.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Removes weak references from `LoggerRepository` (logging-log4j2)
ppkarwasz commented on PR #3199: URL: https://github.com/apache/logging-log4j2/pull/3199#issuecomment-2468593620 The weak reference to `MessageFactory` might require additional explanations. Unlike a weak reference to `Logger`, it will not cause `null` to be returned by `LoggerContext#getLogger`, since the method keeps a hard reference to the factory: https://github.com/apache/logging-log4j2/blob/8ee9387d9ec2063ab11f27eaa43e44a13f4c9935/log4j-core/src/main/java/org/apache/logging/log4j/core/LoggerContext.java#L526-L536 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: notifications-unsubscr...@logging.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [I] log4j-api 2.24.1 causes getLogger() to return null when using log4j-core 2.24.0 (logging-log4j2)
vy commented on issue #3196: URL: https://github.com/apache/logging-log4j2/issues/3196#issuecomment-2467547812 @centic9, #2936 is probably fixed your problem. Could you check if you can still reproduce the problem using `2.25.0-SNAPSHOT`, please? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: notifications-unsubscr...@logging.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Bump github/codeql-action from 3.27.0 to 3.27.1 [logging-parent]
github-actions[bot] merged PR #283: URL: https://github.com/apache/logging-parent/pull/283 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: notifications-unsubscr...@logging.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Removes weak references from `LoggerRepository` (logging-log4j2)
vy commented on code in PR #3199: URL: https://github.com/apache/logging-log4j2/pull/3199#discussion_r1837099466 ## log4j-api/src/main/java/org/apache/logging/log4j/spi/LoggerRegistry.java: ## @@ -43,7 +41,7 @@ @NullMarked public class LoggerRegistry { -private final Map>> loggerRefByMessageFactoryByName = new HashMap<>(); +private final Map> loggerByNameByMessageFactory = new WeakHashMap<>(); Review Comment: Doesn't this change imply that a `Logger` will never be freed unless the associated `MF` becomes unreachable? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: notifications-unsubscr...@logging.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Removes weak references from `LoggerRepository` (logging-log4j2)
ppkarwasz commented on code in PR #3199: URL: https://github.com/apache/logging-log4j2/pull/3199#discussion_r1837111329 ## log4j-api/src/main/java/org/apache/logging/log4j/spi/LoggerRegistry.java: ## @@ -43,7 +41,7 @@ @NullMarked public class LoggerRegistry { -private final Map>> loggerRefByMessageFactoryByName = new HashMap<>(); +private final Map> loggerByNameByMessageFactory = new WeakHashMap<>(); Review Comment: The entire `LoggerRegistry` will be freed, when the associated `LoggerContext` becomes unavailable. The three lines: ```java logger = newInstance(this, name, messageFactory); loggerRegistry.putIfAbsent(name, effectiveMessageFactory, newLogger); return loggerRegistry.getLogger(name, effectiveMessageFactory); ``` have been in Log4j Core since 2016. These lines should not return `null`. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: notifications-unsubscr...@logging.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Turn on the CI build for PRs (logging-log4net)
gdziadkiewicz commented on PR #204: URL: https://github.com/apache/logging-log4net/pull/204#issuecomment-2468412716 > > > @gdziadkiewicz can you test how just using "dotnet test" as a single step would look like? @fluffynuts do you have an idea why the windows output is much longer? (the macos and linux output are missing important inforamtion) > > > > > > Will do. > > the windows output looks like the output from `npm test` where the other two look like the output from `dotnet test` - the default verbosity on `dotnet test` will only dump out failing tests. > > I'd like to suggest that you install the Quackers test logger (https://www.nuget.org/packages/quackers.testlogger) - in conjunction with `zarro`, you'll get easier-to-read output (colored, with pass/fail icons) and it will suppress duplicate logging from the dotnet logger. This mechanism also allows for faster testing by running test assemblies in parallel, if log4net is ever looking at having a second tests assembly (or looking to reduce overall test-time: split existing tests across multiple test assemblies, as long as they all have a name ending in `.Tests`. Parallel testing is a fairly large reason why I use this approach everywhere. > > Alternatively, if y'all really want to eschew `npm test`, I suggest raising the verbosity for `dotnet test` - but I still heartily recommend the `zarro` / `quackers` route (both are written by me, and are our defaults at work) as you'll get the best output that way (eg: see https://github.com/fluffynuts/NExpect/actions/runs/11629746737/job/32387287891 for an example) WIP -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: notifications-unsubscr...@logging.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Turn on the CI build for PRs (logging-log4net)
gdziadkiewicz commented on code in PR #204: URL: https://github.com/apache/logging-log4net/pull/204#discussion_r1836834683 ## .github/workflows/build-dotnet-solo.yaml: ## @@ -0,0 +1,59 @@ +# +# Licensed to the Apache Software Foundation (ASF) under one or more +# contributor license agreements. See the NOTICE file distributed with +# this work for additional information regarding copyright ownership. +# The ASF licenses this file to you under the Apache License, Version 2.0 +# (the "License"); you may not use this file except in compliance with +# the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +# + +name: build-dotnet-solo + +on: + workflow_dispatch: + push: +branches: + - master + pull_request: + +jobs: + + build: + +runs-on: ${{ matrix.os }} + +strategy: + fail-fast: false + matrix: +os: [ macos-latest, ubuntu-latest, windows-latest ] + +env: + DOTNET_SKIP_FIRST_TIME_EXPERIENCE: 1 + DOTNET_CLI_TELEMETRY_OPTOUT: 1 + +steps: + + - name: Checkout repository +uses: actions/checkout@692973e3d937129bcbf40652eb9f2f61becf3332 # 4.1.7 + + - name: Set up dotnet +uses: actions/setup-dotnet@6bd8b7f7774af54e05809fcc5431931b3eb1ddee # 4.0.1 +with: + dotnet-version: 8 + dotnet-quality: ga + + - name: Build Review Comment: I would rather not do that. With separate steps, it's easy to see without digging into whether it's a build issue or a failing tests issue. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: notifications-unsubscr...@logging.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Turn on the CI build for PRs (logging-log4net)
FreeAndNil commented on code in PR #204: URL: https://github.com/apache/logging-log4net/pull/204#discussion_r1836837063 ## .github/workflows/build-dotnet-solo.yaml: ## @@ -0,0 +1,59 @@ +# +# Licensed to the Apache Software Foundation (ASF) under one or more +# contributor license agreements. See the NOTICE file distributed with +# this work for additional information regarding copyright ownership. +# The ASF licenses this file to you under the Apache License, Version 2.0 +# (the "License"); you may not use this file except in compliance with +# the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +# + +name: build-dotnet-solo + +on: + workflow_dispatch: + push: +branches: + - master + pull_request: + +jobs: + + build: + +runs-on: ${{ matrix.os }} + +strategy: + fail-fast: false + matrix: +os: [ macos-latest, ubuntu-latest, windows-latest ] + +env: + DOTNET_SKIP_FIRST_TIME_EXPERIENCE: 1 + DOTNET_CLI_TELEMETRY_OPTOUT: 1 + +steps: + + - name: Checkout repository +uses: actions/checkout@692973e3d937129bcbf40652eb9f2f61becf3332 # 4.1.7 + + - name: Set up dotnet +uses: actions/setup-dotnet@6bd8b7f7774af54e05809fcc5431931b3eb1ddee # 4.0.1 +with: + dotnet-version: 8 + dotnet-quality: ga + + - name: Build Review Comment: Good point. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: notifications-unsubscr...@logging.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [I] Broken logger initialization in 2.24.1 (logging-log4j2)
ppkarwasz commented on issue #3143: URL: https://github.com/apache/logging-log4j2/issues/3143#issuecomment-2468608905 @anuragagarwal561994, The bug was introduced in #2961 and it only affects Log4j API 2.24.1 and all the implementations that use `LoggerRegistry`. Regarding the `2.25.0`, we have a backlog of releases to make, so it might not happen before December. @vy, what do you think about releasing `2.24.2` with a fix to this problem? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: notifications-unsubscr...@logging.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [I] Replace field injection with setter injection in `3.x` (logging-log4j2)
jaykataria commented on issue #2769: URL: https://github.com/apache/logging-log4j2/issues/2769#issuecomment-2468909929 Hi @ppkarwasz, had a general question about how Pull Requests and Code reviews work in this repo, do I have to reach out to folks to drive it or, someone volunteering will just pickup the review on their own time. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: notifications-unsubscr...@logging.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Removes weak references from `LoggerRepository` (logging-log4j2)
vy commented on code in PR #3199: URL: https://github.com/apache/logging-log4j2/pull/3199#discussion_r1837128394 ## log4j-api/src/main/java/org/apache/logging/log4j/spi/LoggerRegistry.java: ## @@ -43,7 +41,7 @@ @NullMarked public class LoggerRegistry { -private final Map>> loggerRefByMessageFactoryByName = new HashMap<>(); +private final Map> loggerByNameByMessageFactory = new WeakHashMap<>(); Review Comment: Why don't we fix the existing code, which allows `Logger`s to be freed when they become unreachable? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: notifications-unsubscr...@logging.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Removes weak references from `LoggerRepository` (logging-log4j2)
vy commented on code in PR #3199: URL: https://github.com/apache/logging-log4j2/pull/3199#discussion_r1837128394 ## log4j-api/src/main/java/org/apache/logging/log4j/spi/LoggerRegistry.java: ## @@ -43,7 +41,7 @@ @NullMarked public class LoggerRegistry { -private final Map>> loggerRefByMessageFactoryByName = new HashMap<>(); +private final Map> loggerByNameByMessageFactory = new WeakHashMap<>(); Review Comment: Why aren't we instead trying to fix the existing code, which allows `Logger`s to be freed when they become unreachable? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: notifications-unsubscr...@logging.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [I] DefaultMergeStrategy - not accounting for AsyncWaitStrategyFactory when merging configurations (logging-log4j2)
ppkarwasz commented on issue #3172: URL: https://github.com/apache/logging-log4j2/issues/3172#issuecomment-2469764544 > I am not sure why the bot didn't mark this as "waiting for maintainer" - not sure if it pops up for you? The workflow is in [`labeler.yaml`](https://github.com/apache/logging-log4j2/blob/2.x/.github/workflows/labeler.yaml) and is based on the affiliation of the reporter with the organization/repository/issue. I am not sure how GitHub assigns them. IIRC I had to add `CONTRIBUTOR` to the list, since some maintainers didn't show up as `MEMBER`. If you know a better way to check if the issue author has write permission to the repo, feel free to submit a PR. > If however, two configurations had an `AsyncWaitStrategyFactory` element: Personally I think that the `AsyncWaitStrategyFactory` needlessly complicates the configuration. There is a [`log4j2.asyncLoggerConfigWaitStrategy`](https://logging.apache.org/log4j/2.x/manual/systemproperties.html#log4j2.asyncLoggerConfigWaitStrategy) Log4j configuration property with a similar purpose. Anyway, except ``, ``, ``, `` and filters, all the other **direct** children of `` should replace the children of the same type IMHO. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: notifications-unsubscr...@logging.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [I] Validate scripts in global `` container (logging-log4j2)
ppkarwasz commented on issue #3176: URL: https://github.com/apache/logging-log4j2/issues/3176#issuecomment-2469764936 > @ppkarwasz what should be done with this ticket? see last comment. > > Of course, the Script builders and AbstractScript constructor can be modified... Since `AbstractScript.getName()` is used extensively accross the code, I believe that we need an additional property (`getConfiguredName()`?) to allow the `ScriptsPlugin` to remove the scripts without an explicit name. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: notifications-unsubscr...@logging.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [I] DefaultMergeStrategy - problem merging filters (logging-log4j2)
ppkarwasz commented on issue #3173: URL: https://github.com/apache/logging-log4j2/issues/3173#issuecomment-2469768022 > One other thing I noticed in my testing, the DefaultMergeStrategy uses the root node of the first configuration to merge all the others - this results in the first configuration being modified (regardless of type). > > For example, if the first is a XmlConffiguration and a second XmlConfiguration is merged - the internal node structure of the first is modified. > > I was wondering why, when a "reconfigure" is started on a CompositeConfiguration, isn't a new empty root-node used as the target for the merges? That it probably a mistake, the target should be the root node of the `CompositeConfiguration`. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: notifications-unsubscr...@logging.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [I] AsyncWaitStrategyFactoryConfig - potential NPE when creating programatically (logging-log4j2)
ppkarwasz commented on issue #3159: URL: https://github.com/apache/logging-log4j2/issues/3159#issuecomment-2469785110 I am seriously considering deprecating `AsyncWaitStrategyFactoryConfig`. I will discuss it with the other maintainers (especially @remkop, who introduced it), but it could probably be replaced with the [`log4j2.asyncLoggerConfigWaitStrategy`](https://logging.apache.org/log4j/2.x/manual/systemproperties.html#log4j2.asyncLoggerConfigWaitStrategy) Log4j configuration property. The presence of both a Log4j configuration property and a direct child of `` IMHO needlessly complexifies the logic to determine which waiting strategy to use. In `main` we have a simple DI system and the [`WaitStrategy` beans](https://github.com/apache/logging-log4j2/blob/main/log4j-async-logger/src/main/java/org/apache/logging/log4j/async/logger/internal/AsyncLoggerDefaultBundle.java) are the only ones that need a `Configuration` parameter. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: notifications-unsubscr...@logging.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [I] AsyncWaitStrategyFactoryConfig - potential NPE when creating programatically (logging-log4j2)
ppkarwasz commented on issue #3159: URL: https://github.com/apache/logging-log4j2/issues/3159#issuecomment-2469793900 > I am not sure why the bot didn't mark this as "waiting for maintainer" - not sure if it pops up for you? You are a `CONTRIBUTOR` now! :wink: The labels are pretty much a work in progress and "waiting for maintainer" is mostly there to make a triage and ask the user some basic questions. Your issues usually don't need more information, so you can consider them accepted by default. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: notifications-unsubscr...@logging.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [I] Standalone version 2.1.0 fails to start [logging-chainsaw]
tarekahf commented on issue #22: URL: https://github.com/apache/logging-chainsaw/issues/22#issuecomment-2469608764 I used Java 11 and it worked. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: notifications-unsubscr...@logging.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[I] Loading text based log file is extremely slow [logging-chainsaw]
tarekahf opened a new issue, #28: URL: https://github.com/apache/logging-chainsaw/issues/28 I managed to use Chainsaw successfully to read a text based log file with the following configuration XML. The only problem that the file is loaded one line at a time and it is extremely slow. Any idea what is worng? ``` http://jakarta.apache.org/log4j/"; debug="true"> ``` -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: notifications-unsubscr...@logging.apache.org.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Removes weak references from `LoggerRepository` (logging-log4j2)
ppkarwasz commented on code in PR #3199: URL: https://github.com/apache/logging-log4j2/pull/3199#discussion_r1837508024 ## log4j-api/src/main/java/org/apache/logging/log4j/spi/LoggerRegistry.java: ## @@ -43,7 +41,7 @@ @NullMarked public class LoggerRegistry { -private final Map>> loggerRefByMessageFactoryByName = new HashMap<>(); +private final Map> loggerByNameByMessageFactory = new WeakHashMap<>(); Review Comment: The current code is fixed: https://github.com/apache/logging-log4j2/blob/89c9b9f81cb12a6a217788a71832bfbd3b6d07b2/log4j-core/src/main/java/org/apache/logging/log4j/core/LoggerContext.java#L527-L531 The changes in this PR are mainly there to allow `log4j-api` version `2.24.2/2.25.0` to work with any `log4j-core` version `2.24.x`. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: notifications-unsubscr...@logging.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Removes weak references from `LoggerRepository` (logging-log4j2)
ppkarwasz commented on PR #3199: URL: https://github.com/apache/logging-log4j2/pull/3199#issuecomment-2469657752 In order to preserve backward compatibility and allow `Logger`s to be garbage-collected, I will: - [ ] Copy `LoggerContext` to an internal package of `log4j-core`. - [ ] Restore the usage of `WeakReference` in the copied class. - [ ] Remove the new methods in `LoggerRepository` in the original class. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: notifications-unsubscr...@logging.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [I] Implement `MessageFactory`-namespaced logger registry (blocked by `2.24.1` release) (logging-log4j2)
ppkarwasz commented on issue #2962: URL: https://github.com/apache/logging-log4j2/issues/2962#issuecomment-2469688771 We should probably abstain as much as possible from using helper methods in `log4j-api`, because these give a rather unnatural dependency of `log4j-core` on a recent version of `log4j-api`. I would put the revamped `LoggerRegistry` in `log4j-kit`. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: notifications-unsubscr...@logging.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org