Thank you Eric preparing this PR! Regarding the version bump of the exported package. IIUC three classes are exported, two of which are implementations of "ch.qos.logback.core.joran.action.Action", an abstract class that has changed between logback 1.x and 2.x. Furthermore, the javadocs of both of these classes state "This class is for configuration reference only. Consumers are not intended to instantiate or extend from it."
The third class is the "ConfigProvider" interface, which allows for the registration of services that provide "configuration fragments", remains unchanged. It is unfortunate that the "ConfigProvider" API is in the same package as these two "Action" implementations. Maybe this can be improved while we are already contemplating a major version bump. I don't understand if it is necessary to export the "Action" implementation classes at all. Does someone know? I suspect not. If they don't need to be exported, we should move them to an internal package. Or does that break something? Considering the name of the new package "org.apache.sling.commons.log.logback.spi", I was wondering if it needs to be exported? It currently does not have a package-info.java file. If it is not exported, I wonder why the package is called "spi". Regarding the spi-fly discussion, I could imagine going with Carsten's suggestion of creating a "light" and a "heavy-all-dependencies-included" variant of the jar, provided that is reasonably straight forward and doesn't require too many hacks. Regards Julian On Fri, 10 Jan 2025 at 08:51, Carsten Ziegeler <cziege...@apache.org> wrote: > > I think there are two potential issues. One is the breaking change of > the exported package org.apache.sling.commons.log.logback (major version > change). Not sure, about the impact. > > The other one is SPIFLy. I agree, requiring it is probably better than > not supporting slf4j 2.x. However, I still think we should try to avoid it. > > The only idea that comes to my mind is to do the same as we do with > Apache Felix Jetty. We release two jars, the "normal" one where we embed > jetty and the "light" one which does not. If you choose the light one, > you have to install all the jetty bundles yourself, SPIFly and > potentially other things. On the other hand, it allows you to update > Jetty without waiting for the next Apache Felix Jetty release. > > This way we might catch two cookies with one fork. > > Regards > Carsten > > On 09.01.2025 17:05, Robert Munteanu wrote: > > Hi, > > > > Eric has been working on a large PR to move our commons.log bundle to > > slf4j 2.0 . I think this is a very welcome improvement. > > > > The only open topic (for me) is the new requirement of using the Aries > > SPIFly bundle. Slf4j 2.0 is now using the ServiceLoader mechanism to > > discover logging implementations and the OSGi answer to that is SPIFly. > > > > SPIFly is unfortunately heavy on dependencies and will need to be > > started very early on for logging to be functional. We have > > historically tried to avoid it, for example by creating our Johnzon > > wrappers. This might be more important for implementations like sling- > > mini. > > > > The details about the requirements start at [1] and I would encorage > > anyone that has an idea (or the cycles to implement a solution) to do > > so on the PR. > > > > I think supporting slf4j 2.x with the SPIFly requirement is better than > > slf4j 1.x without SPIFly, but maybe we can avoid it entirely. > > > > Patches welcome ;-) > > > > Thanks, > > Robert > > > > [1]: > > https://github.com/apache/sling-org-apache-sling-commons-log/pull/18#issuecomment-2088735403 > > -- > Carsten Ziegeler > Adobe > cziege...@apache.org >
