Property log4j.skipJansi should have a default of true

[AndyGee]

The current default forces Win platforms to install an obscure jansi
native library (for pretty print console messages) or always set this
property to true, even for trivial unit tests.

[remkop]

Andy, this pull request has not gotten the attention it deserves. The change itself is trivial but more importantly the community should be okay with it.
Please create a Jira ticket on the Log4j 2 issue tracker so it can be discussed.

[garydgregory]

Wait a sec, that does not make sense to me. If I do have Jansi on my classpath, you're suggesting that I also have to have this system property set to true? That sounds pretty cumbersome and a step backwards.

[jcetkov]

it suggest not to require the jansi - as if you don't have it (on windows system) it dies horribly, just because it's not able to format output

[jvz]

I'm looking at ConsoleAppender right now, and I see that if the class is not installed, it still returns the proper OutputStream instead of blowing up. Is there a bug elsewhere?

[mikaelstaldal]

@jcetkov Can you give more details on how it "dies horribly"? What does the configuration look like?

[jcetkov]

@mikaelstaldal I'm not able to reproduce as I'm not running windows - I've seen it in stacktrace from another user where the (TomEE) startup died with the following error

java.lang.UnsatisfiedLinkError: Could not load library. Reasons: [no jansi64-1.8 in java.library.path, no jansi-1.8 in java.library.path, no jansi in java.library.path, Native Library C:\Users\user\AppData\Local\Temp\jansi-64-1.8.dll already loaded in another classloader]
 at org.fusesource.hawtjni.runtime.Library.doLoad(Library.java:184)
 at org.fusesource.hawtjni.runtime.Library.load(Library.java:142)
 at org.fusesource.jansi.internal.Kernel32.<clinit>(Kernel32.java:37)
 at org.fusesource.jansi.WindowsAnsiOutputStream.<clinit>(WindowsAnsiOutputStream.java:52)
 at sun.reflect.NativeConstructorAccessorImpl.newInstance0(Native Method)
 at sun.reflect.NativeConstructorAccessorImpl.newInstance(NativeConstructorAccessorImpl.java:62)

[rgoers]

Jansi requires native code? That sucks. That means that Jansi can only be loaded by a single class loader and has to be in Tomcat's classpath. Given that, Jansi should default to being disabled.

[mikaelstaldal]

It would quite strange to have a property with name "skipXXX" with default value true. And it would also not be a very good idea to change default value of an existing property.

If we want Jansi to be disabled by default, I would suggest that we rather add another property "enableJansi" with default false.

While at it, since this is only used by the ConsoleAppender, maybe it would be better to have it as a parameter to ConsoleAppender in configuration, rather than a system property?

[garydgregory]

What if we wanted to use JAnsi with the status logger? Gary

…

On Wed, Aug 16, 2017 at 1:44 PM, Mikael Ståldal ***@***.***> wrote: It would quite strange to have a property with name "skipXXX" with default value true. And it would also not be a very good idea to change default value of an existing property. If we want Jansi to be disabled by default, I would suggest that we rather add another property "enableJansi" with default false. While at it, since this is only used by the ConsoleAppender, maybe it would be better to have it as a parameter to ConsoleAppender in configuration, rather than a system property? — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#29 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABIfN0afO2NLkauxosRI2E1xzvlku_Isks5sY0aIgaJpZM4IhcOF> .

[mikaelstaldal]

@garydgregory OK, maybe keep it as system property then.

BTW, is there a way to safely try to load it on Windows, and gracefully do without it if the native library is missing (but the Java library is there)?

[asfbot]

Gary Gregory on dev@logging.apache.org replies:
JAnsi uses the same trick JNA uses: The native libraries it needs for each
OS is embedded inside the JAR itself. At runtime, the library detects the
OS and extracts the right native library from itself.

Gary

[rgoers]

@mikaelstaldal We have to change the default value as it can cause application startup to fail.
@garydgregory I don't think it is a great loss for the status logger not to support it.

[garydgregory]

Sure, I was just looking ahead... ;-)

[asfbot]

Remko Popma on dev@logging.apache.org replies:
I like the idea of having it as a parameter to ConsoleAppender, that would
make it easy for users to find.

No problem changing the default behaviour to not using Jansi.

Do we still retain the current system property with its current semantics
(so setting skipJansi=false would also enable colorized output)? That would
allow existing setups to continue functioning. In the documentation we can
replace mentions of this system property with a link to the upcoming
ConsoleAppender attribute.

[mikaelstaldal]

If should add a parameter to ConsoleAppender, I think it should be named "enableJansi" (or similar) and have default false.

[AndyGee]

I just stopped using the library, so I'll close this and let a dev decide.

[remkop]

FYI: work on this is now in progress under JIRA ticket https://issues.apache.org/jira/browse/LOG4J2-2087

[remkop]

Merged into master.
Please verify and close.