Skip to content

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