[prev in list] [next in list] [prev in thread] [next in thread] 

List:       jakarta-commons-dev
Subject:    Re: [logging] Split Log4JLogger into Log4J12Logger and
From:       robert burrell donkin <robertburrelldonkin () blueyonder ! co ! uk>
Date:       2005-06-29 21:25:29
Message-ID: 1120080329.5021.53.camel () knossos ! elmet
[Download RAW message or body]

On Wed, 2005-06-29 at 21:58 +1200, Simon Kitching wrote:

<snip>

> > > The breakage is more when people refer to it in config files. Or
> > > possible call 
> > >   LogFactory.setAttribute(...,
> > >    "org.apache.commons.logging.impl.Log4JLogger");
> > 
> > that's the point: i was wondering whether we could preserve
> > compatibility by having some mechanism that guesses which implementation
> > to use. (we'll need something like this to support dynamic binding.)
> 
> The obvious solution is just to provide
>   /** @deprecated */
>   Log4JLogger extends Log4J12Logger {}

i've come round to thinking that this is probably the most reasonable
solution.

> > > We also need to consider that Log4J 1.3 is likely to be released within
> > > the lifetime of this release of commons-logging. If we provide a class
> > > called "Log4JLogger" that is not compatible with it, I'm concerned we
> > > will cause more pain than by simply removing that class. Remember that
> > > every person we would have "broken" by not including Log4JLogger would
> > > get broken anyway if they try to use Log4J 1.3, as Log4JLogger is not
> > > compatible with Log4J 1.3: we have a choice of breakage only:
> > >  * message "Log4JLogger does not exist" (if we remove the class)
> > >  * message "Log4JLogger cannot be initialised: log4j 1.2 not found"
> > >    (if we include it)
> > 
> > by removing support for log4jlogger, we break support for existing
> > deployments: it's no longer a drop in replacement. anyone upgrading to
> > the new log4j version is going to expect a lot of pain including having
> > to upgrade. 
> 
> I don't believe "a lot of pain" is a fair assessment. They upgrade to
> log4j 1.3, and run their app.

sorry: didn't explain myself very well. i meant that anyone wanted to
upgrade an enterprise class application running in a container should
probably expect quite a bit of pain when they upgrade to an incompatible
version of a key library (such as log4j).

my concern is for users who are just upgrading JCL (whilst maintaining
their existing version of log.

> If they just use standard discovery then the new log4j 1.3 is
> discovered, and all works fine.

that's certainly the way i'd like things to work :)

> If they use a commons-logging.properties file, a service file, or a
> system property to specify Log4JLogger class then they get the message
>   Class not found: Log4JLogger does not exist
> so they update the relevant config file and all works fine.
> 
> If they have code that does  
>    LogFactory.setAttribute("o.a.c.l.i.Log4JLogger")
> then they get the message
>   Class not found: Log4JLogger does not exist
> and have to either update their code, or create a .java file containing
>   public class Log4JLogger extends Log4J12Logger {}
> and drop the relevant .class file into their app.

removing log4jlogger seems likely to cause a lot of pain for those just
upgrading JCL without really getting much gain. this release is a big
improvement on the old and i'd really prefer not to give people any
reason not to upgrade.

> The last is the only case that is at all awkward, and the fix still
> takes less than 10 minutes and requires no immediate code changes
> (though the original code should of course be updated sometime).

it's not that it's particularly awkward: it's the fact that the release
isn't (in practical terms) compatible with the last one. 
 
> > > Given that we're going to break their apps either way (due to log4j's
> > > binary incompatibility) it seems sensible to do it right from the code
> > > point of view.
> > 
> > i'm not sure i parse this correctly. could you expand?
> 
> When a user uses a config file or call to setAttribute to specify
> Log4JLogger as the adapter class:
>  * If we drop Log4JLogger from the jar we cause the problems 
>    listed above.
>  * If we include a Log4JLogger that only works with Log4J 1.2 
>    then when they put log4j 1.3 in their classpath their app 
>    will also stop working. Ok, this is only triggered once they
>    upgrade to log4j1.3 rather than when they upgrade to JCL 1.1,
>    but it's going to happen eventually.

let the future take care of itself :)

maybe log4j 1.3 will be compatible with 1.2, maybe incompatible. i'm
definitely in favour of adopting the change to log412logger as soon as
possible whether 1.3 gets released or not.

we deprecate log4jlogger and make it very clear about this future
strategy in the release notes. but don't force the huge number of JCL
users to change configuration files today without warning because
something might happen tomorrow.

> So we break such user code either way. What the user needs to do to fix
> their app in either case is pretty similar. But the latter solution has
> us shipping a junk class in JCL.

the problem of junk classes is one of releases why shipping the optional
jar was proposed. many (most?) users of JCL don't care if there are a
few extra classes in there for backwards compatibility and some are very
glad of them.

> > > Alternatively we could provide the Log4JLogger that is currently in SVN
> > > which is compatible with both log4j versions. But it is only compatible
> > > when we compile it against log4j1.3-alpha6 or later, and I'm really not
> > > keen on releasing a Log4JLogger class compiled like that. I'm wavering
> > > on the idea of releasing a Log4J13Logger compiled like that; depends how
> > > confident the log4j team are that they will stick with their API change
> > > plan. Currently, however, there seems to be confusion over when/if they
> > > are going to change the Category/Logger class hierarchy so I think
> > > there's still the possibility of further API changes from the log4j team
> > > in which case we should NOT include log4j1.3 support (ie Log4J13Logger)
> > > in the next release.
> > 
> > i think that we need to work on log4j support and think about the right
> > way to make this change. however, until there is a full release it would
> > be wrong to include support within the core jar.
> > 
> > i wouldn't object to support within an optional jar or ask those
> > requiring support to use the source release. we could then release a new
> > version once a full log4j 1.3 release is available. 
> 
> Agreed. I personally wouldn't even offer an optional jar; we can just
> provide the source and let people compile it themselves as the fix is
> just one class.

there are a number of classes in a similar situation. moving them into
the optional jar seems to me a bit like deprecating them.

> And obviously we can get ready when log4j 1.3 RCs start, so JCL can be
> released on the same day.

+1

might be good to coordinate release candidates

- robert


---------------------------------------------------------------------
To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-dev-help@jakarta.apache.org

[prev in list] [next in list] [prev in thread] [next in thread]