log4j should not be a compile dependency

[dsmiley]

log4j is marked as a compile dependency because of KmlMetaDump.java has log4j import statements even though it doesn't use Log4j. This should be fixed; remove the import statements and either mark it as runtime and/or optional

[docjason]

KmlMetaDump uses a custom Appender added to the log4j root logger that uses the logging system to generate more statistics on errors and warnings at lower levels of geodesy and giscore packages useful for debugging.

It does use log4j at the compile time to do this:

import org.apache.log4j.AppenderSkeleton; import org.apache.log4j.Level; import org.apache.log4j.Logger; import org.apache.log4j.spi.LocationInfo; import org.apache.log4j.spi.LoggingEvent; import org.apache.log4j.spi.ThrowableInformation;

//Get the root logger
Logger root = Logger.getRootLogger();
if (root != null) {
    MetaAppender appender = new MetaAppender();
    appender.setThreshold(Level.WARN);
    root.addAppender(appender);
}
    ...
private class MetaAppender extends AppenderSkeleton {
    protected void append(LoggingEvent event) {
             ...

[dsmiley]

Ok; weird I didn't notice that. Any way, it would be nice if this could be done in an optional way as to simply not do this if Log4j is not available. It's annoying to exclude log4j dependencies down-stream from giscore.

[docjason]

KmlMetaDump is standalone class. If too much trouble down stream can move that single class to sub-module/sub-project.

[dsmiley]

I think that proposed solution is too heavyweight to warrant it -- it's just a single class. I propose simply marking log4j as an optional dependency. If the user wants to use KmlMetaDump, then they need Log4j (unfortunately). Perfection would be making KmlMetaDump work when Log4j isn't present.

[rdrand]

I had looked at that Appender dependency a while ago at that time it wasn't trivial to just remove the dependency – I think getting the root logger wasn't supported at that time.

I have a couple of ideas of how to change this in what might be better ways:

1. Take advantage of the visitor pattern in KmlOutputStream and make this a shim IGisOutputStream. Then each of your checkXXX can be a visitXXX and just call the corresponding accept method in KmlOutputStream. The setup can be done by a variant DocumentType. So:

os = GISFactory.getOutputStream(DocumentType.ITaggingKML, ….)

This results in getting:

KMLMetaStream( KMLOutputStream()) instead of just KMLOutputStream()

1. Change KMLMetaDump to use a perfectly normal registered appender with slf4j that you've written as in KmlMetaDump. I don't see why you can't use exactly the same inner class trick with slf4j of creating an appender and adding it to the root logger.

Doug

From: Jason Mathews notifications@github.com<mailto:notifications@github.com> Reply-To: OpenSextant/giscore reply@reply.github.com<mailto:reply@reply.github.com> Date: Monday, December 2, 2013 6:08 PM To: OpenSextant/giscore giscore@noreply.github.com<mailto:giscore@noreply.github.com> Subject: Re: [giscore] log4j should not be a compile dependency (#4)

KmlMetaDump is standalone class. If too much trouble down stream can move that single class to sub-module/sub-project.

— Reply to this email directly or view it on GitHubhttps://github.com/OpenSextant/giscore/issues/4#issuecomment-29667928.

[docjason]

Changed log4j to "provided" so hidden from published maven pom.xml Ideally change to "optional" when gradle natively supports optional dependencies.