Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix for upstream log4cxx #33

Merged
merged 1 commit into from
Feb 3, 2020

Conversation

flixr
Copy link
Contributor

@flixr flixr commented Jan 12, 2020

Fixes empty log lines on the console with upstream version of log4cxx.

In upstream log4cxx the calls log4cxx::Level::getDebug(), etc. now return new pointers on every call.
See https://issues.apache.org/jira/browse/LOGCXX-394

This resulted in rosconsole Formatter::print being called with an invalid level (levels::Count)
and hence color was NULL and nothing printed (except the newline).

So instead of calling getDebug(), directly compare the integer level which works in the released version and upstream.
It should also be more efficient to directly compare integer level when the upstream version of log4cxx is used.

While here also change/fix console printer to print unknown levels. see #34

@flixr flixr force-pushed the fix_for_upstream_log4xx branch from 58c1e10 to 194a1ff Compare January 12, 2020 15:27
Copy link
Member

@dirk-thomas dirk-thomas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With what versions of log4cxx is this change backwards compatible?

src/rosconsole/impl/rosconsole_log4cxx.cpp Outdated Show resolved Hide resolved
src/rosconsole/impl/rosconsole_log4cxx.cpp Outdated Show resolved Hide resolved
src/rosconsole/rosconsole.cpp Outdated Show resolved Hide resolved
@flixr flixr force-pushed the fix_for_upstream_log4xx branch 2 times, most recently from 8639733 to cd7d06d Compare January 22, 2020 23:02
@flixr
Copy link
Contributor Author

flixr commented Jan 22, 2020

This is tested and compatible with the released version 0.10.0 (from 2010) that is also in Debian/Ubuntu as well as the latest master from upstream.

Fixes empty log lines on the console with upstream version of log4cxx.

In upstream log4cxx the calls log4cxx::Level::getDebug(), etc. now return new pointers on every call.
See https://issues.apache.org/jira/browse/LOGCXX-394

This resulted in rosconsole `Formatter::print` being called with an invalid level (`levels::Count`)
and hence `color` was NULL and nothing printed (except the newline).

So instead of calling getDebug(), directly compare the integer level which works in the released version and upstream.
It should also be more efficient to directly compare integer level when the upstream version of log4cxx is used.
@flixr flixr force-pushed the fix_for_upstream_log4xx branch from cd7d06d to 3efe3a5 Compare February 1, 2020 12:18
@dirk-thomas dirk-thomas added the bug Something isn't working label Feb 3, 2020
@dirk-thomas
Copy link
Member

Thank you for the contribution.

@dirk-thomas dirk-thomas merged commit a4d634b into ros:melodic-devel Feb 3, 2020
@flixr flixr deleted the fix_for_upstream_log4xx branch February 3, 2020 19:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants