DO NOT REPLY [Bug 28237] - [PATCH] Use the commons logging LogFactory also in Fop.java

2004-04-19 Thread bugzilla
DO NOT REPLY TO THIS EMAIL, BUT PLEASE POST YOUR BUG 
RELATED COMMENTS THROUGH THE WEB INTERFACE AVAILABLE AT
http://issues.apache.org/bugzilla/show_bug.cgi?id=28237.
ANY REPLY MADE TO THIS MESSAGE WILL NOT BE COLLECTED AND 
INSERTED IN THE BUG DATABASE.

http://issues.apache.org/bugzilla/show_bug.cgi?id=28237

[PATCH] Use the commons logging LogFactory also in Fop.java





--- Additional Comments From [EMAIL PROTECTED]  2004-04-19 11:30 ---
Glen,

One more...

3) setLevel() has been part of the Log4J API since before Logger was born; i.e.
when Logger was Category.


What Sun is up to is summed up neatly in the java.util.logging description:
http://java.sun.com/j2se/1.4.2/docs/api/java/util/logging/package-summary.html#package_description

The Log4J documentation also mentions the critical importance of logging for
debugging, quoting Kernighan and Pike on the superiority of judicious logging
over interactive debuggers.  (Yes!)

I discovered the limitations because I started the process of converting the
recently introduced 1.4 logging in alt-design to use commons-logging.  At some
stage I will probably subclass commons-logging and apply that, so that at least
the logging invocations will look the same.

Thanks, Glen, for taking the trouble to look up the details of the usage of
commons-logging.  Unfortunately, Craig doesn't address the issue.  Currently,
commons-logging *forces* us to use the kind of work-around he describes.  But
that work-around is only necessary because of the absence of setLevel().  Not
including it is an ideological decision which cripples the logging.

In the absence of commons-logging, users of 1.4, Log4J and Lumberjack can all
setLevel dynamically.  Craig might have decided that that is a Very Bad Thing,
and that we must be protected from ourselves.  But the four points have no
bearing on whether to provide setLevel(); they are all about getting the native
logger, which users are forced to do by the refusal to include setLevel in the
interface.  If it were included, there would be no need to get the native
logger.  The real workaround is to modify the interface and the implementations.

All any of us have to do is understand the arguments.  If they persuade us, it
doesn't matter how many or how few have adopted it.  Likewise if they fail to
persuade.


DO NOT REPLY [Bug 28237] - [PATCH] Use the commons logging LogFactory also in Fop.java

2004-04-18 Thread bugzilla
DO NOT REPLY TO THIS EMAIL, BUT PLEASE POST YOUR BUG 
RELATED COMMENTS THROUGH THE WEB INTERFACE AVAILABLE AT
http://issues.apache.org/bugzilla/show_bug.cgi?id=28237.
ANY REPLY MADE TO THIS MESSAGE WILL NOT BE COLLECTED AND 
INSERTED IN THE BUG DATABASE.

http://issues.apache.org/bugzilla/show_bug.cgi?id=28237

[PATCH] Use the commons logging LogFactory also in Fop.java





--- Additional Comments From [EMAIL PROTECTED]  2004-04-18 18:05 ---
--- Additional Comments From [EMAIL PROTECTED] 2004-04-18 03:27 ---

Glen, Simon, et al.

I have a problem with commons-logging.  It deliberately leaves the configuration
to the user and the underlying logging system.  One example of this is in the
dynamic setting of log levels.  FOP allows a command line (or user
configuration) override of the logging level.  This code has been the focus of
some discussion here.  


To my way of thinking, such an override is not a
configuration issue per se, but extends to be a run-time issue.  


Perhaps, but not many others appear to be thinking this way.  A considerable
statement, given all the apps--Tomcat, Axis, Struts, POI, Tapestry, Turbine,
etc., etc.--currently using commons-logging as-is.

The logging level *is* the prerogative of the user--they decide whether or not
they want to see info or debug statements.  We don't adjust the log level of
the logger, instead we declare the desired level of the message to be displayed.
 People choose debug level, or info level, and the app stays with that level.  

When there *are* misbehaving instances, those error messages should be given
warn, error, or fatal levels--which guarantees that they will show up in
any logger with a debug or an info setting.  (i.e., Loggers *will* print out
the messages of more severe errors than that of what they are assigned for.  It
is just that they will not display less severe errors.)

Consider the
situation in which we wish to allow the logging level to be changed during the
lifetime of a single FOP instance; to allow, e.g., an administrator to get more
information about a misbehaving instance.


Actually, maybe we don't really need to consider it.  After all, you think the
folks at Tomcat, Turbine, et. al. have never considered that situation--have
never needed to?  All of them came to same conclusion--even under presumably
much more demanding circumstances that FOP would have:  the logging level is to
be set by the user.   Craig McClanahan of Sun is probably the lead developer on
Tomcat, Struts, and Commons-Logging, and has been on all three for several years
now.  If programs not being able to set the logging level was an actual problem,
it probably would have been long identified and fixed by now.  I believe the C-L
team came to the conclusion that would cause more headaches than it solves.

I see two logical problems with the argument that the logging requirements of
FOP go beyond what might be needed by a servlet container, J2EE server, or Web
framework environment--i.e., the motivation for subclassing: 

(1)  FOP primarily runs *on* servlet containers, J2EE servers, and in Web
framework.  But these servers don't need the ability to set the log
level--they're not asking for it--so why do we? 

(2) Our programmatic cousins--Xalan and Batik--don't even use logging to begin
with.  (Sometimes I'm unsure if we even need it ourselves!)  If it is indeed
true that it would be unworkable if FOP cannot set the logging level, would it
not follow that Xalan and Batik are immediately DOA because they don't even have
loggers?

What *is* important for us, however, is that the error messages printed out for
the misbehaving instance be detailed enough so the situation can be reproduced.

SimpleLog provides this facility, and it is (was?) used in the -d switch
processing.  


No, that's not a runtime option--that's a command-line option that is fixed for
the life of the report generation.  This option was removed in my last patch,
because it was quietly overriding what they have set up in C-L locally, and it
was only there for reasons of backward compatibility, which can't be deemed much
of an issue anyway given that we've switched the logger.


I can't do that if I am using Jdk14Logger, Jdk13LumberjackLogger or
Log4JLogger.  I don't know whether Log4JLogger supports log level changes, and
it doesn't partucularly matter.  Jdk14Logger and presumably
Jdk13LumberjackLogger do, and it's a very useful facility, even if only to
support command line settings.


I moved the settings into the fop.bat and fop.sh files in my last (attached)
patch.  I'm currently unsure how sufficient that is.  You do raise a valid point
here, however, CL will not allow us to use command line settings to change the
logging level.  The user needs to read up on the logger that they have chosen,
and configure it according to that logger's directions.  Logging is truly a
configuration item in the Java world.



I've taken the issue up on the (overcrowded) jakarta-commons dev 

DO NOT REPLY [Bug 28237] - [PATCH] Use the commons logging LogFactory also in Fop.java

2004-04-18 Thread bugzilla
DO NOT REPLY TO THIS EMAIL, BUT PLEASE POST YOUR BUG 
RELATED COMMENTS THROUGH THE WEB INTERFACE AVAILABLE AT
http://issues.apache.org/bugzilla/show_bug.cgi?id=28237.
ANY REPLY MADE TO THIS MESSAGE WILL NOT BE COLLECTED AND 
INSERTED IN THE BUG DATABASE.

http://issues.apache.org/bugzilla/show_bug.cgi?id=28237

[PATCH] Use the commons logging LogFactory also in Fop.java





--- Additional Comments From [EMAIL PROTECTED]  2004-04-18 19:28 ---
Glen,

On Sat, Apr 17, 2004 at 08:36:13AM -, [EMAIL PROTECTED] wrote:
 --- Additional Comments From [EMAIL PROTECTED]  2004-04-17 08:36 ---
 Simon (and others),
 
 For the remaining three issues remaining, please take a look at the patch I just
 added.
 
 1.)  I removed the -debug and -quiet options, because they just cover over the
 commons-logging settings.  Also I'm leery of switching to SimpleLog as a
 default, seeing the Commons Logging will normally use JDK1.4 logging.
 
 Rather than use an API that covers up Commons-Logging functionality (i.e., -d
 and -q), I would rather have us make it very simple for the user to use/learn
 the exact commons-logging method, which brings up:

I thought that we should retain backward compatibility, but if that is
not (yet) the case for the development branch, I am most happy to see
these options be dropped.
 
 2.)  In fop.bat and fop.sh, (I only tested the former, I'll need someone to
 check the latter), I created commented-out lines that will allow the user to
 choose a different logger from the default JDK1.4 and the logging level (the
 latter, iff they're using SimpleLog).

That is very helpful. Frankly I think that users who are not satisfied
with the defaults really should study log configuration. But giving
them a headstart does not hurt us.
 
 3.)  Per your last comment below, on using .info() instead of .debug() in the
 dumpConfiguration() nee debug() method--I agree, that should be the message
 level instead because that is what the user is specifically requesting.  My
 patch does this, but without switching to a SimpleLog() to guarantee output. 
 The drawback is that if the user specifically chooses error or fatal, etc.,
 as a level, that will take precedence over setting this field, and as a
 consequence this information won't be shown.  OTOH, the logger that the user
 specifically chose externally won't be overridden.  I suspected the latter was
 the lesser of two evils, and so went with that.  Your thoughts?

With the info level, it is OK not to check whether the user has info
level enabled; it is the default.

I am quite happy with your patch.

Peter,

I generally agree with Glen's point of view. With this solution we
have chosen a good and well-respected logging solution, which has the
good sense to act as a middle man who gives the user a choice of
logging implementation. Answering queries about logging behaviour and
features is no longer on our shoulders.

Regards, Simon


DO NOT REPLY [Bug 28237] - [PATCH] Use the commons logging LogFactory also in Fop.java

2004-04-18 Thread bugzilla
DO NOT REPLY TO THIS EMAIL, BUT PLEASE POST YOUR BUG 
RELATED COMMENTS THROUGH THE WEB INTERFACE AVAILABLE AT
http://issues.apache.org/bugzilla/show_bug.cgi?id=28237.
ANY REPLY MADE TO THIS MESSAGE WILL NOT BE COLLECTED AND 
INSERTED IN THE BUG DATABASE.

http://issues.apache.org/bugzilla/show_bug.cgi?id=28237

[PATCH] Use the commons logging LogFactory also in Fop.java

[EMAIL PROTECTED] changed:

   What|Removed |Added

 Status|NEW |RESOLVED
 Resolution||FIXED



--- Additional Comments From [EMAIL PROTECTED]  2004-04-18 22:41 ---
Patch applied.  I'm closing Simon's issue here, but Peter's concerns can still
be discussed on this bug page or on the FOP-DEV list directly.

Glen


DO NOT REPLY [Bug 28237] - [PATCH] Use the commons logging LogFactory also in Fop.java

2004-04-18 Thread bugzilla
DO NOT REPLY TO THIS EMAIL, BUT PLEASE POST YOUR BUG 
RELATED COMMENTS THROUGH THE WEB INTERFACE AVAILABLE AT
http://issues.apache.org/bugzilla/show_bug.cgi?id=28237.
ANY REPLY MADE TO THIS MESSAGE WILL NOT BE COLLECTED AND 
INSERTED IN THE BUG DATABASE.

http://issues.apache.org/bugzilla/show_bug.cgi?id=28237

[PATCH] Use the commons logging LogFactory also in Fop.java





--- Additional Comments From [EMAIL PROTECTED]  2004-04-18 23:19 ---
Two related questions:

If logging levels are supposed to be set through configuration files, then

1) why did the Sun guys put setLevel() in the API for java.util.logging?
2) why did the commons-logging guys put setLevel() in SimpleLog?

You're all individuals Brian
Yes! We're all individuals Crowd
You're all different Brian
Yes! We are all different Crowd
I'm not Dissenter
Monty Python's Life of Brian


DO NOT REPLY [Bug 28237] - [PATCH] Use the commons logging LogFactory also in Fop.java

2004-04-18 Thread bugzilla
DO NOT REPLY TO THIS EMAIL, BUT PLEASE POST YOUR BUG 
RELATED COMMENTS THROUGH THE WEB INTERFACE AVAILABLE AT
http://issues.apache.org/bugzilla/show_bug.cgi?id=28237.
ANY REPLY MADE TO THIS MESSAGE WILL NOT BE COLLECTED AND 
INSERTED IN THE BUG DATABASE.

http://issues.apache.org/bugzilla/show_bug.cgi?id=28237

[PATCH] Use the commons logging LogFactory also in Fop.java





--- Additional Comments From [EMAIL PROTECTED]  2004-04-19 03:42 ---
1) why did the Sun guys put setLevel() in the API for java.util.logging?
2) why did the commons-logging guys put setLevel() in SimpleLog?

You caught me on the first question--I noticed I was in error when I wrote:

 What they (and Sun, with its 1.4 logger) are telling you--logging
is a configuration and not a runtime issue--should carry some weight with you. 

Sun is *not* telling us this, precisely because they do have a setLevel().  (But
that does not necessarily mean they advocate calling programs from switching
it--the user could be setting it for his own embedded program.)

In the meantime, it may be helpful for you to look at C-L's team's thoughts on
having a setLevel() below, and in the last link, a potential workaround as well
as Craig McClanahan's concerns with modifying the underlying implementation from
C-L:

http://marc.theaimsgroup.com/?l=jakarta-commons-devm=102571996601634w=2
http://marc.theaimsgroup.com/?l=jakarta-commons-devm=101096333702331w=2
http://marc.theaimsgroup.com/?l=jakarta-commons-devm=101097934025726w=2
http://marc.theaimsgroup.com/?l=jakarta-commons-devm=101068848405397w=2
http://marc.theaimsgroup.com/?l=jakarta-commons-devm=101061362702721w=2 (point 2)
http://nagoya.apache.org/bugzilla/show_bug.cgi?id=14155 (sample workaround, also
Craig's four problems with a setLevel())

Also, to give you a better idea of the usage of C-L, here's a list of the 88
projects currently linking with it:
 
http://lsd.student.utwente.nl/gump/jakarta-commons/commons-logging/index.html#Project+Dependees

Glen


DO NOT REPLY [Bug 28237] - [PATCH] Use the commons logging LogFactory also in Fop.java

2004-04-17 Thread bugzilla
DO NOT REPLY TO THIS EMAIL, BUT PLEASE POST YOUR BUG 
RELATED COMMENTS THROUGH THE WEB INTERFACE AVAILABLE AT
http://issues.apache.org/bugzilla/show_bug.cgi?id=28237.
ANY REPLY MADE TO THIS MESSAGE WILL NOT BE COLLECTED AND 
INSERTED IN THE BUG DATABASE.

http://issues.apache.org/bugzilla/show_bug.cgi?id=28237

[PATCH] Use the commons logging LogFactory also in Fop.java





--- Additional Comments From [EMAIL PROTECTED]  2004-04-17 08:21 ---
Created an attachment (id=11267)
another logging solution


DO NOT REPLY [Bug 28237] - [PATCH] Use the commons logging LogFactory also in Fop.java

2004-04-17 Thread bugzilla
DO NOT REPLY TO THIS EMAIL, BUT PLEASE POST YOUR BUG 
RELATED COMMENTS THROUGH THE WEB INTERFACE AVAILABLE AT
http://issues.apache.org/bugzilla/show_bug.cgi?id=28237.
ANY REPLY MADE TO THIS MESSAGE WILL NOT BE COLLECTED AND 
INSERTED IN THE BUG DATABASE.

http://issues.apache.org/bugzilla/show_bug.cgi?id=28237

[PATCH] Use the commons logging LogFactory also in Fop.java





--- Additional Comments From [EMAIL PROTECTED]  2004-04-17 08:36 ---
Simon (and others),

For the remaining three issues remaining, please take a look at the patch I just
added.

1.)  I removed the -debug and -quiet options, because they just cover over the
commons-logging settings.  Also I'm leery of switching to SimpleLog as a
default, seeing the Commons Logging will normally use JDK1.4 logging.

Rather than use an API that covers up Commons-Logging functionality (i.e., -d
and -q), I would rather have us make it very simple for the user to use/learn
the exact commons-logging method, which brings up:

2.)  In fop.bat and fop.sh, (I only tested the former, I'll need someone to
check the latter), I created commented-out lines that will allow the user to
choose a different logger from the default JDK1.4 and the logging level (the
latter, iff they're using SimpleLog).

3.)  Per your last comment below, on using .info() instead of .debug() in the
dumpConfiguration() nee debug() method--I agree, that should be the message
level instead because that is what the user is specifically requesting.  My
patch does this, but without switching to a SimpleLog() to guarantee output. 
The drawback is that if the user specifically chooses error or fatal, etc.,
as a level, that will take precedence over setting this field, and as a
consequence this information won't be shown.  OTOH, the logger that the user
specifically chose externally won't be overridden.  I suspected the latter was
the lesser of two evils, and so went with that.  Your thoughts?

Thanks,
Glen


DO NOT REPLY [Bug 28237] - [PATCH] Use the commons logging LogFactory also in Fop.java

2004-04-17 Thread bugzilla
DO NOT REPLY TO THIS EMAIL, BUT PLEASE POST YOUR BUG 
RELATED COMMENTS THROUGH THE WEB INTERFACE AVAILABLE AT
http://issues.apache.org/bugzilla/show_bug.cgi?id=28237.
ANY REPLY MADE TO THIS MESSAGE WILL NOT BE COLLECTED AND 
INSERTED IN THE BUG DATABASE.

http://issues.apache.org/bugzilla/show_bug.cgi?id=28237

[PATCH] Use the commons logging LogFactory also in Fop.java





--- Additional Comments From [EMAIL PROTECTED]  2004-04-18 03:27 ---
Glen, Simon, et al.

I have a problem with commons-logging.  It deliberately leaves the configuration
to the user and the underlying logging system.  One example of this is in the
dynamic setting of log levels.  FOP allows a command line (or user
configuration) override of the logging level.  This code has been the focus of
some discussion here.  To my way of thinking, such an override is not a
configuration issue per se, but extends to be a run-time issue.  Consider the
situation in which we wish to allow the logging level to be changed during the
lifetime of a single FOP instance; to allow, e.g., an administrator to get more
information about a misbehaving instance.

SimpleLog provides this facility, and it is (was?) used in the -d switch
processing.  I can't do that if I am using Jdk14Logger, Jdk13LumberjackLogger or
Log4JLogger.  I don't know whether Log4JLogger supports log level changes, and
it doesn't partucularly matter.  Jdk14Logger and presumably
Jdk13LumberjackLogger do, and it's a very useful facility, even if only to
support command line settings.

I've taken the issue up on the (overcrowded) jakarta-commons dev list, and been
told that it's a configuration issue, and that SimpleLog has the facility
because there is no underlying logger.  I don't see it, but it's not worth
arguing about there, especially as it was suggested that I subclass the relevant
interfaces and classes if I wanted the facility.  That seems to be the way to go.

The alternative is to drop back to the native logger whenever a change of level
is required, which seems to me to rather defeat the purpose.

Simon's original point I think ran counter to this proposal (create a
hard-coded SimpleLog instance with the appropriate log level set).  Note,
though, that the behaviour of the default common-logging LogFactory is to create
a single log instance *of any given name*.  If we create a log named FOP (or
org.apache.fop if we are following the 1.4 recommendations), it is not going
to interfere with the logging of any other applications, but the user will
expect to see changes to the level reflected in the Fop log (but see
configuration details below).

If users have set up debugging in the user configuration file, we ought to give
them debug logging.  If there is a conflict with some other setting, it is the
user's responsibility.  If there is a command line switch for debugging, the
requirement is even more definite.

The default logging selection behaviour is quite complex:
Find a LogFactory
  a system property named org.apache.commons.logging.LogFactory
  JDK 1.3 JAR Services Discovery - a resource named
   META-INF/services/org.apache.commons.logging.LogFactory
  properties file commons-logging.properties:
   property org.apache.commons.logging.LogFactory
  else default LogFactory

The default LogFactory looks for:
  configuration attribute org.apache.commons.logging.Log
  else system property org.apache.commons.logging.Log
  else if Log4J available, Log4JLogger
  else if 1.4, Jdk14Logger
  else SimpleLog

I suspect that Jdk13LumberjackLogger might be in there between Jdk14Logger and
SimpleLog.

Whether the LogFactory that is in use is well behaved or not will depend on
configuration details which are deliberately beyond our control, but the JDK and
default LogFactory both guarantee a single instance of a Log of a given name. 
This seems to satisfy one of Simon's difficulties.
2. The command line options -d and -e are used to configure the Log
   object of the options object. This log is not passed on to driver,
   and thus gets lost.

If users are specifying a logging implementation which does *not* behave in this
way, and in obeying their instructions about, say, debugging, we go over the top
of their entire server logging, they need to fix their instructions to Fop or
use another logger.

I agree with Simon that, in the absence of any specifics, we make no changes to
the inherited logging environment.

3. Commons logging is also configured by Java system properties. A
   user may configure it to use a certain logging implementation,
   e.g. Log4J. Hard coding a SimpleLog instance goes against
   reasonable expectations of users when we advertise that we use
   commons logging. If a user does nothing he gets the JDK 1.4 logging
   if he runs that version, and SimpleLog otherwise.
Agree.

SimpleLog logs to stderr; only redirection sends it to a file (and
that is impossible under MSWindows command and cmd shells).
I didn't realise there was no redirection facility for 

DO NOT REPLY [Bug 28237] - [PATCH] Use the commons logging LogFactory also in Fop.java

2004-04-09 Thread bugzilla
DO NOT REPLY TO THIS EMAIL, BUT PLEASE POST YOUR BUG 
RELATED COMMENTS THROUGH THE WEB INTERFACE AVAILABLE AT
http://issues.apache.org/bugzilla/show_bug.cgi?id=28237.
ANY REPLY MADE TO THIS MESSAGE WILL NOT BE COLLECTED AND 
INSERTED IN THE BUG DATABASE.

http://issues.apache.org/bugzilla/show_bug.cgi?id=28237

[PATCH] Use the commons logging LogFactory also in Fop.java





--- Additional Comments From [EMAIL PROTECTED]  2004-04-09 10:09 ---
Glen,

 (a) why place within isDebugEnabled() and isWarnEnabled()
 conditionals, why does this matter?

To be as reticent as possible. If a user uses -d but his configuration
also sets the log level to debug, he already has what he wants; skip
the command line option silently. Similarly with -q.
 
 (b) one conditional is !isDebugEnabled() with a negative but the other
 is the positive isWarnEnabled()--was this a bug?

-d and !isDebugEnabled() means that the user does not yet have debug
log level but requests it, so we have to do something. -q and
isWarnEnabled() means that the user still has warning level or a more
verbose level on but requests quiet mode (error level), so we have to
do something.
 
 (c) Finally, did you actually mean isErrorEnabled() for the latter?

No, isErrorEnabled() means error level enabled and perhaps more
verbose. We need to ensure that we are not more verbose than error
level, that is, !isWarnEnabled().

For the dumpConfiguration method of CommandLineOptions, if the user
requests -x but does not set debug log level, he sees no configuration
dump. That does not serve the user well. My patch checked that and
used a debug enabled SimpleLog in that case. Alternatively, it might
be a good idea to show the configuration at info log level; after all,
this is information that the user explicitly requested.

Regards, Simon


DO NOT REPLY [Bug 28237] - [PATCH] Use the commons logging LogFactory also in Fop.java

2004-04-08 Thread bugzilla
DO NOT REPLY TO THIS EMAIL, BUT PLEASE POST YOUR BUG 
RELATED COMMENTS THROUGH THE WEB INTERFACE AVAILABLE AT
http://issues.apache.org/bugzilla/show_bug.cgi?id=28237.
ANY REPLY MADE TO THIS MESSAGE WILL NOT BE COLLECTED AND 
INSERTED IN THE BUG DATABASE.

http://issues.apache.org/bugzilla/show_bug.cgi?id=28237

[PATCH] Use the commons logging LogFactory also in Fop.java





--- Additional Comments From [EMAIL PROTECTED]  2004-04-09 03:18 ---
Thanks for the explanation, Simon.  I understand a lot more now and have
committed most of your patch, but with a few changes.  Please take a look at the
new version and let me know what you think.

Changes I didn't commit were a couple of minor issues I had some question
on--namely, your wrapping and creating of new logs based on -d and -q options in
CLO.parseOptions(): 

(a) why place within isDebugEnabled() and isWarnEnabled() conditionals, why does
this matter?

(b) one conditional is !isDebugEnabled() with a negative but the other is the 
positive isWarnEnabled()--was this a bug?  

(c) Finally, did you actually mean isErrorEnabled() for the latter?

BTW, please always use { } for the conditionals instead of single-lines [1]:
if (a  b) {
c = d;
}

http://xml.apache.org/fop/dev/conventions.html

Thanks again,
Glen


DO NOT REPLY [Bug 28237] - [PATCH] Use the commons logging LogFactory also in Fop.java

2004-04-07 Thread bugzilla
DO NOT REPLY TO THIS EMAIL, BUT PLEASE POST YOUR BUG 
RELATED COMMENTS THROUGH THE WEB INTERFACE AVAILABLE AT
http://issues.apache.org/bugzilla/show_bug.cgi?id=28237.
ANY REPLY MADE TO THIS MESSAGE WILL NOT BE COLLECTED AND 
INSERTED IN THE BUG DATABASE.

http://issues.apache.org/bugzilla/show_bug.cgi?id=28237

[PATCH] Use the commons logging LogFactory also in Fop.java





--- Additional Comments From [EMAIL PROTECTED]  2004-04-07 20:37 ---
Glen,

SimpleLog log = new SimpleLog(FOP);
log.setLevel(SimpleLog.LOG_LEVEL_INFO);
driver.setLogger(log);

driver.getLogger().info(version);
options = new CommandLineOptions(args);

This code in Fop.java has three problems:

1. SimpleLog is configured by Java system properties. The second line
   above overrules that, against reasonable expectations of users. If
   a user does nothing, SimpleLog selects a reasonable default: info.

2. The command line options -d and -e are used to configure the Log
   object of the options object. This log is not passed on to driver,
   and thus gets lost.

3. Commons logging is also configured by Java system properties. A
   user may configure it to use a certain logging implementation,
   e.g. Log4J. Hard coding a SimpleLog instance goes against
   reasonable expectations of users when we advertise that we use
   commons logging. If a user does nothing he gets the JDK 1.4 logging
   if he runs that version, and SimpleLog otherwise.

I believe that my patch makes Fop's logging behaviour consistent with
the claim that we use commons logging, and with the documented
configuration options of commons logging. And it makes it consistent
internally. Note that Jeremias changed the code of many classes to use
commons logging.
 
SimpleLog logs to stderr; only redirection sends it to a file (and
that is impossible under MSWindows command and cmd shells). For
command-line users that seems appropriate; other users should select
and configure a log implementation that suits them. With commons
logging configuring logging via command line options is not really
appropriate.

This is all I discovered. I went into this because I was hit by the
inconsistencies in the current logging. For example, currently
getlogger().debug(xxx) in FOText.java will not give you anything for
the reasons I described under 1 and 2 above.

Regards, Simon


DO NOT REPLY [Bug 28237] - [PATCH] Use the commons logging LogFactory also in Fop.java

2004-04-06 Thread bugzilla
DO NOT REPLY TO THIS EMAIL, BUT PLEASE POST YOUR BUG 
RELATED COMMENTS THROUGH THE WEB INTERFACE AVAILABLE AT
http://issues.apache.org/bugzilla/show_bug.cgi?id=28237.
ANY REPLY MADE TO THIS MESSAGE WILL NOT BE COLLECTED AND 
INSERTED IN THE BUG DATABASE.

http://issues.apache.org/bugzilla/show_bug.cgi?id=28237

[PATCH] Use the commons logging LogFactory also in Fop.java





--- Additional Comments From [EMAIL PROTECTED]  2004-04-06 17:36 ---
Created an attachment (id=11157)
The patch as described


DO NOT REPLY [Bug 28237] - [PATCH] Use the commons logging LogFactory also in Fop.java

2004-04-06 Thread bugzilla
DO NOT REPLY TO THIS EMAIL, BUT PLEASE POST YOUR BUG 
RELATED COMMENTS THROUGH THE WEB INTERFACE AVAILABLE AT
http://issues.apache.org/bugzilla/show_bug.cgi?id=28237.
ANY REPLY MADE TO THIS MESSAGE WILL NOT BE COLLECTED AND 
INSERTED IN THE BUG DATABASE.

http://issues.apache.org/bugzilla/show_bug.cgi?id=28237

[PATCH] Use the commons logging LogFactory also in Fop.java





--- Additional Comments From [EMAIL PROTECTED]  2004-04-06 21:48 ---
Simon, if I understand you correctly (and pardon me, I'm not a logging expert):

1.) With our current process of setting the logger in FOP.java directly, we end 
up overriding whatever the command-line user may have specified on their 
machine?  (Remember, FOP embedded users set Logging via the Driver class 
anyway.)

2.) Will FOP still quietly default to SimpleLog if the user doesn't enter 
anything explicitly?  I would guess 95%* of our **command-line** user base 
either doesn't care about logging, or if they did care they want the command-
line output (i.e., SimpleLog) anyway--does your change reflect this?  We don't 
want users practicing with the command line to have to distract themselves with 
the logger.

3.)  As another option for the command-line user, given only a small minority 
want to log to a file, would it be better to provide a -logfile option, that 
once specified, logs to that file, and absent the setting of that option, will 
go quietly go to the SimpleLog?

Thanks,
Glen