You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@commons.apache.org by robert burrell donkin <ro...@blueyonder.co.uk> on 2005/06/26 21:56:45 UTC
Re: [logging] Split Log4JLogger into Log4J12Logger and
Log4J13Logger
On Wed, 2005-06-22 at 21:55 +0200, Dennis Lundberg wrote:
> Simon Kitching wrote:
> > Hi,
> >
> > Currently the Log4JLogger code in svn has this horrible stuff all
> > through it:
> > if (is12) {
> > ...
> > } else {
> > ...
> > }
> >
> > This is to handle the fact that log4j versions 1.2 and 1.3 are expected
> > to be binary incompatible in both directions, ie code compiled against
> > 1.2 won't work against 1.3 and code compiled against 1.3 won't work
> > against 1.2.
> >
> > The hack does allow code compiled against 1.3 to run against 1.3.
> > However there are a number of disadvantages:
> > * it requires compiling against 1.3 which isn't released yet
> > and may be many months away.
> > * it has a performance hit
> > * it isn't very readable
> > * it isn't future proof; when log4j 1.3 comes out, this code
> > won't work as the Priority class will go away completely.
> >
> > I would therefore prefer to split this class into separate Log4J12Logger
> > and Log4J13Logger. A static initializer block in each will check which
> > version of log4j is available, and throw an exception to declare
> > themself "unavailable" if the wrong log4j version is present. Both
> > adapters can also be included in the list of discoverable logging
> > classes.
>
> I think it would be a wise move to split the class in two. A while back
> I spent a fair amount of time trying to understand the differences
> betwen log4j 1.2 and 1.3 and also to get commons-logging to work with
> both log4j versions. It was confusing to say the least, and splitting
> this class would at least make the commons-logging code more understandable.
+1
> > Of course removing a class will mean a 1.1 version number, but that's
> > good for a number of reasons.
> >
> > The only other major concern I see is config files or system properties
> > that explicitly request the old logadapter by name. But that should be
> > easily fixable, and a config change seems a reasonable thing to do in
> > a .x release to me. Alternatively we could provide a trivial Log4JLogger
> > class that just extends Log4J12Logger; however I suspect this will cause
> > more pain and confusion than simply reporting a failure, esp. when log4j
> > 1.3 is released and starts to become widely used.
>
> I agree that, unless people have extended Log4JLogger, the only problems
> that could arise for users of commons-logging would be to change their
> config-files.
>
> If we hold on to the Log4JLogger class for a while and let it extend
> Log4J12Logger, as you suggest, we might get away with doing a 1.0.X release.
i think that the next release should be a 1.1 release in any case.
however, i would prefer retaining Log4JLogger so that it can be properly
deprecated.
- robert
---------------------------------------------------------------------
To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-dev-help@jakarta.apache.org
Re: [logging] Split Log4JLogger into Log4J12Logger and
Log4J13Logger
Posted by robert burrell donkin <ro...@blueyonder.co.uk>.
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
Re: [logging] Split Log4JLogger into Log4J12Logger and
Log4J13Logger
Posted by Simon Kitching <sk...@apache.org>.
On Tue, 2005-06-28 at 22:32 +0100, robert burrell donkin wrote:
> > The problem is that people won't be referring to it in the normal way,
> > via an import in their code (at least it's *theoretically* possible, but
> > I can't imagine anyone actually subclassing Log4JLogger.
>
> IMHO we should have marked them all final to exclude this possibility.
> JCL is one of the cases when creating a new implementation based on the
> source of another is better...
Agreed.
>
> > 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 {}
> > 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.
If they just use standard discovery then the new log4j 1.3 is
discovered, and all works fine.
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.
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).
>
> > 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.
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.
> > 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.
And obviously we can get ready when log4j 1.3 RCs start, so JCL can be
released on the same day.
Regards,
Simon
---------------------------------------------------------------------
To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-dev-help@jakarta.apache.org
Re: [logging] Split Log4JLogger into Log4J12Logger and
Log4J13Logger
Posted by robert burrell donkin <ro...@blueyonder.co.uk>.
On Mon, 2005-06-27 at 11:22 +1200, Simon Kitching wrote:
> On Sun, 2005-06-26 at 20:56 +0100, robert burrell donkin wrote:
>
> > i think that the next release should be a 1.1 release in any case.
> > however, i would prefer retaining Log4JLogger so that it can be properly
> > deprecated.
>
> The problem is that people won't be referring to it in the normal way,
> via an import in their code (at least it's *theoretically* possible, but
> I can't imagine anyone actually subclassing Log4JLogger.
IMHO we should have marked them all final to exclude this possibility.
JCL is one of the cases when creating a new implementation based on the
source of another is better...
> 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.)
> In neither case do we have any real mechanism for indicating deprecation
> to the user.
we still have the old form of deprecation: the release notes :)
> 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.
> 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?
> 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.
- robert
---------------------------------------------------------------------
To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-dev-help@jakarta.apache.org
Re: [logging] Split Log4JLogger into Log4J12Logger and
Log4J13Logger
Posted by Simon Kitching <sk...@apache.org>.
On Sun, 2005-06-26 at 20:56 +0100, robert burrell donkin wrote:
> i think that the next release should be a 1.1 release in any case.
> however, i would prefer retaining Log4JLogger so that it can be properly
> deprecated.
The problem is that people won't be referring to it in the normal way,
via an import in their code (at least it's *theoretically* possible, but
I can't imagine anyone actually subclassing Log4JLogger.
The breakage is more when people refer to it in config files. Or
possible call
LogFactory.setAttribute(...,
"org.apache.commons.logging.impl.Log4JLogger");
In neither case do we have any real mechanism for indicating deprecation
to the user.
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)
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.
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.
Regards,
Simon
---------------------------------------------------------------------
To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-dev-help@jakarta.apache.org