@philwebb @wilkinsona After a deeper analysis to understand why Logback logging was broken on spring-graalvm-native, I found that the classpah checks previously performed during class initialization (allowing build time check against the whole classpath with reflection and build time code removal) are now done at runtime which is IMO an unwanted regression introduced during this refactoring.

Like this has been done recently for LiquibaseServiceLocatorApplicationListener, could you please bring back the classpath check to a static field for those classes: - org.springframework.boot.logging.java.JavaLoggingSystem$Factory - org.springframework.boot.logging.log4j2.Log4J2LoggingSystem$Factory - org.springframework.boot.logging.logback.LogbackLoggingSystem$Factory

We could then configured those factory classes to be initialized at build time, allowing proper logging system detection without having to introduce artificially reflection configuration.

Comment From: philwebb

I don't think we should have a static boolean for the logging system. I think we need to use the classloader that's passed in. I think the original design (as discussed in https://github.com/spring-projects/spring-boot/issues/23387#issuecomment-695357547) assumed that the entire LoggingSystemFactory would be replaced allowing you to return whatever you want from getLoggingSystem. Will that not work?

Comment From: sdeleuze

I think there is a misunderstanding here.

In #22594 implemented by @wilkinsona, the changes allowed to reduce reflection usage and make Spring Boot logging support native friendly by doing 2 changes: - Use a build time accessible classloader (LoggingSystem.class.getClassLoader() in that case) to perform the ClassUtils.isPresent checks at build time (where reflection on the whole classpath is possible in native regardless of the code removed by the static analysis) similar to what has been done recently for LiquibaseServiceLocatorApplicationListener - Use logging system constructor instead of reflection, notice that here the classloader that's passed in is used otherwise indeed that would be not correct.

This first attempt was not ok because it introduced a package tangle hence the need for #23387 that fixes the tangle by using spring.factories for logging system, but rollbacking the build time classpath check is unexpected and we still need that on spring-graalvm-native side.

We need for the 2 use cases we have: - In order to make reflection default mode working correctly without adding artificial reflection configuration - But we also need it for the functional mode where a programmatic equivalent of spring.factories will be generated at build time. The confusion comes maybe from the fact that the manually crafted version we have now performs before those classpath checks before instantiating let's say LogbackLoggingSystem.Factory but there should be no handcrafted code in the generated version we plan to have in 0.9 just a programmatic version to avoid reflection entries for classes listed in spring.factories.

Our goal is really to collaborate with you on getting something maintainable that can either work out of the box or being generated automatically without additional logic. We don't want to duplicate the Boot logic with some handcrafted logic hence my ask.

So what I ask concretely is something like that which is IMO consistent with the previous other changes that we did as part of the 2.4 native theme, including the one done on #22594 that has never been mentioned as being a problem (only the tangle was):

public static class Factory implements LoggingSystemFactory {

    private static final boolean LOGBACK_PRESENT = ClassUtils.isPresent("ch.qos.logback.core.Appender",
            Factory.class.getClassLoader());

    @Override
    public LoggingSystem getLoggingSystem(ClassLoader classLoader) {
        if (LOGBACK_PRESENT) {
            return new LogbackLoggingSystem(classLoader);
        }
        return null;
    }
}

Comment From: philwebb

Got it. I just assumed you'd be replacing that factory entirely so it didn't matter how our own implementations worked. I wish we had a better way to test this stuff so we could catch it earlier.

Comment From: sdeleuze

Thanks, I wish as well, we are collaborating actively with GraalVM team on JUnit 5 native support.