Skip to content

Update LogAdapter to allow build-time code removal #29506

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

Closed

Conversation

sdeleuze
Copy link
Contributor

@sdeleuze sdeleuze commented Nov 17, 2022

Allow for example to remove those classes and 90 related methods when Logback is used:

  • org.apache.commons.logging.LogAdapter$JavaUtilAdapter
  • org.apache.commons.logging.LogAdapter$JavaUtilLog
  • org.apache.commons.logging.LogAdapter$LocationResolvingLogRecord
  • org.apache.commons.logging.LogAdapter$Log4jAdapter
  • org.apache.commons.logging.LogAdapter$Log4jLog
  • org.apache.commons.logging.LogAdapter$LogApi
  • org.apache.logging.log4j.message.ObjectMessage
  • org.apache.logging.log4j.message.ReusableObjectMessage org.apache.logging.log4j.simple.SimpleLoggerContext
  • org.apache.logging.log4j.simple.SimpleLoggerContextFactory

org.apache.logging.slf4j.SLF4JProvider extends org.apache.logging.log4j.spi.Provider, so a bunch of Log4j classes are still reachable, but looks like a useful improvement.

Allow for example to remove those classes and 90 related methods when Logback is used:
- org.apache.commons.logging.LogAdapter$JavaUtilAdapter
- org.apache.commons.logging.LogAdapter$JavaUtilLog
- org.apache.commons.logging.LogAdapter$LocationResolvingLogRecord
- org.apache.commons.logging.LogAdapter$Log4jAdapter
- org.apache.commons.logging.LogAdapter$Log4jLog
- org.apache.commons.logging.LogAdapter$LogApi
- org.apache.logging.log4j.message.ObjectMessage
- org.apache.logging.log4j.message.ReusableObjectMessage
- org.apache.logging.log4j.simple.SimpleLoggerContext
- org.apache.logging.log4j.simple.SimpleLoggerContextFactory
@sdeleuze sdeleuze added in: core Issues in core modules (aop, beans, core, context, expression) type: enhancement A general enhancement theme: aot An issue related to Ahead-of-time processing labels Nov 17, 2022
@sdeleuze sdeleuze added this to the 6.0.1 milestone Nov 17, 2022
@sdeleuze sdeleuze requested a review from jhoeller November 17, 2022 08:54
@sdeleuze sdeleuze self-assigned this Nov 17, 2022
Copy link
Contributor

@jhoeller jhoeller left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me, also the use of Function there instead of the enum-based delegation arrangement.

@sdeleuze sdeleuze closed this in 04366f4 Nov 21, 2022
@bclozel
Copy link
Member

bclozel commented Nov 22, 2022

This change has broken the Spring Boot build. The error is currently hard to reproduce locally, but the stacktrace is:

java -jar spring-boot-smoke-test-ant.jar 
Exception in thread "main" java.lang.reflect.InvocationTargetException
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:77)
	at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.base/java.lang.reflect.Method.invoke(Method.java:568)
	at org.springframework.boot.loader.MainMethodRunner.run(MainMethodRunner.java:49)
	at org.springframework.boot.loader.Launcher.launch(Launcher.java:95)
	at org.springframework.boot.loader.Launcher.launch(Launcher.java:58)
	at org.springframework.boot.loader.JarLauncher.main(JarLauncher.java:65)
Caused by: java.lang.NoClassDefFoundError: org/apache/logging/log4j/spi/Provider
	at java.base/java.lang.ClassLoader.defineClass1(Native Method)
	at java.base/java.lang.ClassLoader.defineClass(ClassLoader.java:1012)
	at java.base/java.security.SecureClassLoader.defineClass(SecureClassLoader.java:150)
	at java.base/java.net.URLClassLoader.defineClass(URLClassLoader.java:524)
	at java.base/java.net.URLClassLoader$1.run(URLClassLoader.java:427)
	at java.base/java.net.URLClassLoader$1.run(URLClassLoader.java:421)
	at java.base/java.security.AccessController.doPrivileged(AccessController.java:712)
	at java.base/java.net.URLClassLoader.findClass(URLClassLoader.java:420)
	at java.base/java.lang.ClassLoader.loadClass(ClassLoader.java:587)
	at org.springframework.boot.loader.LaunchedURLClassLoader.loadClass(LaunchedURLClassLoader.java:149)
	at java.base/java.lang.ClassLoader.loadClass(ClassLoader.java:520)
	at java.base/java.lang.Class.forName0(Native Method)
	at java.base/java.lang.Class.forName(Class.java:467)
	at org.apache.commons.logging.LogAdapter.isPresent(LogAdapter.java:100)
	at org.apache.commons.logging.LogAdapter.<clinit>(LogAdapter.java:43)
	at org.apache.commons.logging.LogFactory.getLog(LogFactory.java:67)
	at org.apache.commons.logging.LogFactory.getLog(LogFactory.java:59)
	at org.springframework.boot.SpringApplication.<clinit>(SpringApplication.java:183)
	at smoketest.ant.SampleAntApplication.main(Unknown Source)
	... 8 more
Caused by: java.lang.ClassNotFoundException: org.apache.logging.log4j.spi.Provider
	at java.base/java.net.URLClassLoader.findClass(URLClassLoader.java:445)
	at java.base/java.lang.ClassLoader.loadClass(ClassLoader.java:587)
	at org.springframework.boot.loader.LaunchedURLClassLoader.loadClass(LaunchedURLClassLoader.java:149)
	at java.base/java.lang.ClassLoader.loadClass(ClassLoader.java:520)
	... 27 more

@bclozel bclozel reopened this Nov 22, 2022
@wilkinsona
Copy link
Member

wilkinsona commented Nov 22, 2022

I think it's NoClassDefFoundError that needs to be caught in addition to the existing catch of ClassNotFoundException.

@sdeleuze sdeleuze closed this in 0b8000e Nov 22, 2022
@sdeleuze
Copy link
Contributor Author

I think this is now fixed, sorry for the regression, this one was pretty hard to anticipate. I think previously this potential bug was possible but hidden by the fact the classpath checks were done lazily in the static block. Thanks for catching this with Boot tests.

@sdeleuze sdeleuze deleted the log-adapter-refinement branch December 10, 2024 14:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: core Issues in core modules (aop, beans, core, context, expression) theme: aot An issue related to Ahead-of-time processing type: enhancement A general enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants