docx4j 2.7.1 rc - feedback - logging

[mpierce]

Could you remove the auto-configuration of Log4j? It's rather unfortunate of a library to configure my logging for me, especially in a hard-coded way that I cannot easily disable. I think it's not a controversial statement to say that logging is something that should be left to the deployer of the code to decide. What if I wanted to configure logging in my own code so that all logging went to a database or something like that? Now I'd have to be very careful to run my logging setup code before your Context static initializer runs, otherwise I'd have a few stray logging statements that didn't use my preferred logging system.

This also creates a runtime dependency on Log4j specifically, which crashes with java.lang.NoSuchMethodError: org.apache.log4j.Logger.getAllAppenders()Ljava/util/Enumeration; if log4j is not present. I prefer to use Logback, but the same problem would exist if I chose to use any other logging library.

Ideally you could switch your logging dependency to SLF4J to make it easy for your users to use whichever logging library they choose (the suggested path for any library), but not crashing would be a good start.

[jason]

Hi, and thanks for the feedback.

I agree, slf4j is probably the right thing to do.

In the meantime, log4j is a dependency, and has been since docx4j's inception. Unless i'm mistaken, docx4j won't run without it, with or without the autoconfig.

I'm interested in what others think on this topic. What if the log4j auto-config remained, but could be disabled in docx4j.properties?

cheers .. Jason

[jason]

OK, see now http://www.docx4java.org/trac/docx4j/changeset/1675 for new property:

Code: Select all

docx4j.Log4j.Configurator.disabled=false


[Pierre]

Hi,

Setting docx4j.Log4j.Configurator.disabled=false doesn't fix the getAllAppenders issues, since the property is tested after the call to isConfigured method that makes that call.

On simple fix, would be to move some lines in Log4jConfigurator, as followed :

Code: Select all

public class Log4jConfigurator {
    public synchronized static void configure() {
   boolean disabled = Boolean.parseBoolean(
         Docx4jProperties.getProperties().getProperty("docx4j.Log4j.Configurator.disabled", "false"));            
   if (disabled) return;
        if (!isConfigured()) {
        [...]   


does it make sense ?

[jason]

Thanks for the suggestion; Committed as revision 1779.