-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 Hi Simon, Hi everybody,
Simon Kitching wrote: > Well, rain has unexpectedly started this afternoon, so I've got some > time to talk about this now... > > On Sat, 2005-10-01 at 12:49 +1200, Simon Kitching wrote: > >>On Fri, 2005-09-30 at 13:30 +0200, Boris Unckel wrote: >> >>>Hello, >>> >>> >>>>>Level.Trace was introduced in 1.2.12 >>>>> >>>> >>>>http://logging.apache.org/log4j/docs/api/org/apache/log4j/Level.html#TRACE >>>>You are right. Thanks! >>>>And 1.2.12 is released. As soon as it is in the ibiblio, I will update the >>>>version in project.xml >>>>Only a little confusing that Log4J13Logger is for Log4j 1.2.12+ >>>>I will also add this as comment to the Log4J13Logger. >>> >>>Since these are internal classes I would recommend to detect the log4j >>>Feature, not it's version, and rename the Log4J??Logger to >>>feature dependend name (i.E. Log4JWithTraceLogger and Log4JLegacyLogger). >>>The older one could be depraceted (first step/version) and afterwards >>>made be final to make users who extended it, are forced to migrate >>>in a definded timeframe. > > > I suggest you look at the current code again. Log4J12Logger has: > <code> > // Releases of log4j1.2 >= 1.2.12 have > // Priority.TRACE available, earlier > // versions do not. If TRACE is not available, then we have to map > // calls to Log.trace(...) onto the DEBUG level. > > try { > traceLevel = (Priority) Priority.class.getDeclaredField("TRACE").get(null); > } catch(Exception ex) { > // ok, trace not available > traceLevel = Priority.DEBUG; > } > </code> Jep, I have looked at the code before. But only the use of Priority makes this be incompatible with log4j 1.3 This check could be changed so it is only using Level and it would still compile. The check would not work as easy then, but it is still possible to figure this out without using Priority. So my suggestion is to kick out the dependency on Priority and change it to Level but keeping the behaviour in Log4j12Logger. I could do that if you want. > > So TRACE is supported in log4j12Logger as long as there is a TRACE member on > the > Priority class, ie it is already implemented as you request above: detecting > the feature, not the version. > > The Log4J13Logger class can (and does) simply assume TRACE is available. > > >>Basically, Log4j 1.2 and log4j 1.3 are not binary compatible. There is >>no way for us to write a single class that will run against both these >>log4j versions. This is due to the way that the log4j team decided to >>phase out the Category/Priority classes. This was discussed at length on >>the log4j lists, and they were basically committed to that approach. So >>we *must* have separate logger implementations for these two lib >>versions. And we must compile each logger class against the appropriate >>lib version, which basically makes using Maven to do the builds >>impossible; Maven assumes one classpath is used to compile all the code. >>So builds are ant-only. > > > This binary incompatibility is the reason the two logger classes exist, > not to support the TRACE feature. The two different classes need to be > compiled against different versions of log4j. As I wrote above my oppinion is that we can change the code so jcl will compile with all log4j > 1.2.12 and the resulting Log4j12Logger will work with older versions. Since Log4J12Logger is using the log4j Logger interface it will not work with log4j versions before Level was added because Level was introduced together with Logger as far as I know. So there will be no incompatibility added by my suggestion. > > >>Whether log4j12logger should be called Log4JLogger is debatable. There >>are drawbacks to this. Yes, renaming the class will break setups where >>someone has a config file that specifies Log4JLogger. However this setup >>will fail anyway if they try to use log4j 1.3 in the classpath instead >>of log4j 1.2. Given log4j's binary incompatibility between the two >>versions we *can't* guarantee smooth upgrades for all combinations, and >>I think the rename approach currently implemented will give the most >>obvious error message, and have the most obvious fix for the users. This >>is up for debate though. >> > > > To expand on this: assume for the moment that we provide a Log4JLogger > class compiled against the 1.2 log4j library, and a Log4J13Logger > compiled against the 1.3 log4j library. Anyways, I liked the suggestion from Boris for this issue. If I do not get it wrong, Log4J13Logger works fine with log4j 1.2.12 so the Names Boris suggested will make more sense. > > An app which has a commons-logging.properties file that specifies > Log4JLogger as the logger to use will continue to work fine when > commons-logging is upgraded. But when they upgrade log4j, they will > start getting strange "incompatible class definition" or "no such > method" exceptions, because the code is compiled to expect to call > Category.log(Priority p, ...) but that method no longer exists [the > details might not be entirely correct here; I'm working from memory]. > That's a rather confusing message to deal with, and no hint that the > fix is to change their commons-logging configuration. > > Now if we instead provide only Log4J12Logger/Log4J13Logger then when > they upgrade their commons-logging implementation they get the message > "Log4JLogger not found" which seems reasonably clear. They will > (hopefully) immediately refer to the commons-logging release notes which > will tell them how to resolve this by changing their > commons-logging.properties file. Yes, this is a nuisance, but it's one > imposed by the fact that the log4j releases are binary incompatible. > > It's also confusing to provide "Log4JLogger" when it doesn't work > against the current Log4J release; I think the > Log4J12Logger/Log4J13Logger names are clearer. Of course this will get > interesting when/if Log4J 1.4 is released... > > It would be possible to provide a Log4JLogger class that auto-detected > which version of log4j is in the classpath, and delegated to the > relevant implementation, but that would have to be implemented by > delegating calls at runtime as far as I can tell, which would have a > performance impact. If someone could come up with an auto-detect > solution that *doesn't* have a runtime impact I would be very happy to > see that included. Note that here we're *not* talking about the generic > commons-logging auto-detection, but where an app has explicitly provided > a logger class name that happens to be o.a.c.l.impl.Log4JLogger. Well after all I think it is okay if we do that change and people have to edit one line of config as far as we point this out with the release notes and in bold letters on the download page promoting the new release. > > Regards, > > Simon Regards Jörg -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.1 (GNU/Linux) Comment: Using GnuPG with Thunderbird - http://enigmail.mozdev.org iD8DBQFDP5j6mPuec2Dcv/8RAp/SAJ958Pr/yiJKd/OPrgLFkkbFsJ4/mgCfYqbH K/oR2l9DkP/JjyBmNKnrpEw= =PdAR -----END PGP SIGNATURE----- --------------------------------------------------------------------- To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
