-
Notifications
You must be signed in to change notification settings - Fork 1.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix Android-related issues in Log4j Core #3071
Conversation
This fixes three issues encountered in the [`log4j-samples-android`](https://github.com/apache/logging-log4j-samples/tree/main/log4j-samples-android) test project: 1. Disables the `jvmrunargs` lookup on Android and fixes it on the other platforms. Previously, the lookup always returned `null`. 2. Switches the default context selector to `BasicContextSelector` on Android. `StackLocator` is broken on Android: it cannot use our JDK 8 code (missing `sun.reflect` classes), but also it cannot use our JDK 11+ code (missing multi-release JAR support). This causes `ClassLoaderContextSelector` to use two different logger contexts for the same classloader. 3. Fixes a `ParserConfigurationException` caused by the lack of XInclude capabilities in Android's XML parser. The fix to [LOG4J2-3531](https://issues.apache.org/jira/browse/LOG4J2-3531) didn't cover all the cases. Closes #3056. Part of #2832.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. 👍
I would also like to see a F.A.Q. entry for Android (e.g., Does Log4j Core work on Android?
) and document that
- Yes, it works out-of-the-box
- XInclude is not available
BasicContextSelector
[link toBCS
docs] will be used
...-core/src/main/java/org/apache/logging/log4j/core/lookup/JmxRuntimeInputArgumentsLookup.java
Show resolved
Hide resolved
@vy, I added a FAQ entry in fbd4ea0. Since this PR can not be tested, until we publish a snapshot and the external |
The CI build 11273365001 confirms that a simple application on Android functions correctly with Log4j Core. |
This fixes three issues encountered in the [`log4j-samples-android`](https://github.com/apache/logging-log4j-samples/tree/main/log4j-samples-android) test project: 1. Disables the `jvmrunargs` lookup on Android and fixes it on the other platforms. Previously, the lookup always returned `null`. 2. Switches the default context selector to `BasicContextSelector` on Android. `StackLocator` is broken on Android: it cannot use our JDK 8 code (missing `sun.reflect` classes), but also it cannot use our JDK 11+ code (missing multi-release JAR support). This causes `ClassLoaderContextSelector` to use two different logger contexts for the same classloader. 3. Fixes a `ParserConfigurationException` caused by the lack of XInclude capabilities in Android's XML parser. The fix to [LOG4J2-3531](https://issues.apache.org/jira/browse/LOG4J2-3531) didn't cover all the cases. Closes #3056. Part of #2832.
When will this merge be released? |
Please open a GitHub Discussion or ask on the Personally I won't have time to make a |
This fixes three issues encountered in the
log4j-samples-android
test project:jvmrunargs
lookup on Android and fixes it on the other platforms. Previously, the lookup always returnednull
.BasicContextSelector
on Android.StackLocator
is broken on Android: it cannot use our JDK 8 code (missingsun.reflect
classes), but also it cannot use our JDK 11+ code (missing multi-release JAR support). This causesClassLoaderContextSelector
to use two different logger contexts for the same classloader.ParserConfigurationException
caused by the lack of XInclude capabilities in Android's XML parser. The fix to LOG4J2-3531 didn't cover all the cases.Closes #3056.
Part of #2832.