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

Use only one modern Logging-API and remove transitive dependency to implementation #254

Closed
HannesWell opened this issue Aug 25, 2021 · 13 comments

Comments

@HannesWell
Copy link
Collaborator

HannesWell commented Aug 25, 2021

In general it is recommended for libraries to compile/program against the slf4j API facade instead of a specific logging framework implementation. See for example:

This approach does not force users of a library to use the logging framework of the library or even worse to have multiple logging frameworks because different libraries use different frameworks.

At the moment symja uses a mishmash of the org.slf4j.Logger and org.apache.logging.log4j.Logger APIs.

I suggest to use only slf4j and to remove all specific bindings for slf4j-simple or log4j as dependencies. So the actual binding is not defined before a application that uses symja is deployed or build.

Only matheclipse modules that are stand-alone applications that are not consumed by users should have a dependency to a specific framework implementation.
As far as I can oversee, these stand-alone modules could be (but I'm not sure about that)

  • matheclipse-api
  • matheclipse-beakerx
  • matheclipse-discord
  • matheclipse-jar

And those modules should only bind against one logging framework implementation.

For the external code in matheclipse-external and -gpl a bridge module could maybe be used, as described here (http://www.slf4j.org/legacy.html) but I have to figure out the details.

@HannesWell
Copy link
Collaborator Author

There has been other issues regarding logging in the past: #63

Furthermore I can ask in the libraries symja depends one (namely jas and jml) to also use slf4j as well and to not bind against a specific framework, if they don't do magic it should be straight forward.

I would like to resolve this issue before #253 not be sure the logging is not broken.

@axkr
Copy link
Owner

axkr commented Aug 25, 2021

I think it will be difficult to solve this issue before #253 because it depends on other issues:

IMO only the matheclipse-io module uses a concrete logging.

@HannesWell
Copy link
Collaborator Author

Thanks for the hint. While I think that mavenicing all dependencies would be great, however I think that is not a blocker for this issue.

The reason is that there are bridge modules that mimic the log4j-1 respectively log4j-2 API but actually wrap a a slf4j Logger. So code that is compiled/programmed against log4j-1 or -2 can be used in the slf4j system with all it's benefits, namely that an application using symja/matheclipse can choose any compatible logging framework they want.

The situation is a bit confusing because log4j-1 (whose main package is org.apache.log4j) and log4j-2 (whose main package is org.apache.logging.log4j) are used together. The migration guide from log4j-1 to log4-2 gives an overview about the difference:
http://logging.apache.org/log4j/2.x/manual/migration.html

However

  • jml from Tilmann Neumann uses log4j-1, which can be bridged to slf4j using org.slf4j:log4j-over-slf4j.
    This bridge is provided by slf4j.
  • jas from Heinz Kredel uses log4j-2, which can be bridged to slf4j using org.apache.logging.log4j:log4j-to-slf4j.
    This bridge is provided by log4j.

At the moment, when I depend on matheclipse-core I also get the dependency to log4j-core (which is the log4j-2 implementation) with it.
With the bridges above this can be avoided and symja could behave as it where written purely against the slf4j-API. The classes in core can be easily adapted to use slf4j (at the moment they use slf4j-1 and -2 in different classes).

I'm working on a PR and it looks very good so far, I can submit it and we could discuss it there further? The only question I have is which matheclipse modules should require an actual implementation because they are stand-alone applications?

@axkr
Copy link
Owner

axkr commented Aug 25, 2021

The matheclipse-io and matheclipse-api module contains the standalone apps:

@HannesWell
Copy link
Collaborator Author

The matheclipse-io and matheclipse-api module contains the standalone apps:

* https://github.com/axkr/symja_android_library/wiki/Installation

Thanks for that information.

Do you have a preference which logging framework should finally be used for those applications?
At the moment symja uses slf4j-simple for tests but also has the log4j-core framework as compile dependency.

I would like to resolve this issue before #253 not be sure the logging is not broken.

Because I think I'm now sure that I have fully understand the logging situation in Symja, I solved issue #253 before.

@axkr
Copy link
Owner

axkr commented Aug 26, 2021

Do you have a preference which logging framework should finally be used for those applications?
At the moment symja uses slf4j-simple for tests but also has the log4j-core framework as compile dependency.

I prefer Log4J

@HannesWell
Copy link
Collaborator Author

If you prefer Log4J then it is probably simpler to use the log4j-2-api in Symja. With log4j-2 the logging framework is split into api and core (the implementation) so log4j-api can be used similar like slf4j as logging facade (there are also bridges in both directions).
Because the different dependencies of Symja use different Logging-APIs (log4j-1, log4j-2 and slf4j) in the end Logging-bridges have to be used anyway. Especially since new dependencies can pull in new logging-apis again.

There exist many Logging-Bridges to combine almost every possible combination of logging-api and -implentation/binding, but it depends on the actual case which bridge is the right one. Some combinations of bridges and implementation can even be harmful and cause endless loops (e.g. when you combine the bridge from log4j-api to slf4j and the binding of slf4j to log4j-2). Therefore those Symja modules that are not applications (i.e. external, frontend, core, gpl) should not have any (transitive!) to a logging bridge or implementation and should just transparently pass through all different apis that are used by themself or their dependencies. Then each user that depends on a symja module as well as Symja's 'application' modules (like io and api, and those that depend on them) can choose the right logging-bridges for the implementation they use.
If the non-application modules of symja would have transitive dependencies to bridges users would have to exclude them again, in case they are harmful in combination with their desired logging framework. Which would make everything just more complicated than it already is.

Therefore it actually does not matter if symja uses slf4j-api or log4j-2-api, because dependencies use both anyway. The important thing is just to drop log4j-1, because it is not split into api and implementation and it is outdated, and to not depend transitively on any logging-implementation.

I have also verified that it is also possible to bridge from log4j-2-api to slf4j without too much effort, in case Symja is used as OSGi bundle in Eclipse (which is how we use Symja).

Until now log4-2-api (org.apache.logging.log4j) is used in most cases in Symja. Therefore I think you prefer it? Furthermore it would be the most natural choice if you would like to use log4j as implementation in the end.

@HannesWell HannesWell changed the title Use only slf4j API for logging Use only one modern Logging-API and remove transitive dependency to implementation Sep 3, 2021
@axkr
Copy link
Owner

axkr commented Sep 3, 2021

I have not that deep understanding of the logging mechanism. If you think log4-2-api is best solution, we should use it.

@HannesWell
Copy link
Collaborator Author

I have not that deep understanding of the logging mechanism. If you think log4-2-api is best solution, we should use it.

OK. Yes if slf4j or log4j-api is used, IMHO does not matter in the end as explained above. So yes I think we can go with that.
In fact log4j-2 is even newer than slf4j, but Symja does not use the new features so there is effectively no really noticeable difference.
Then I will complete the PR as soon I have completed my testing.

HannesWell added a commit to HannesWell/symja_android_library that referenced this issue Sep 3, 2021
HannesWell added a commit that referenced this issue Sep 3, 2021
@HannesWell
Copy link
Collaborator Author

@axkr have you verified that the log4j2.properties are picked up by log4j at runtime?
Because the file is in the matheclipse root module (which has packaging type pom and therefore does not create any jar) it doesn't end up in any of the build jars.

@axkr
Copy link
Owner

axkr commented Sep 3, 2021

@axkr have you verified that the log4j2.properties are picked up by log4j at runtime?

No, I didn t checked that.

@HannesWell
Copy link
Collaborator Author

@axkr have you verified that the log4j2.properties are picked up by log4j at runtime?

No, I didn t checked that.

I have verified that it is not found at the moment. I think the reason is that the parent Maven module org.matheclipse:matheclipse is not on the classpath of the sub-module like org.matheclipse:matheclipse-api or -core.

I will work out a solution and submit a corresponding PR.

However this issue is completed.

@HannesWell
Copy link
Collaborator Author

For the record: Here is a good comparison if slf4j or log4j-2 API should be used:
https://stackoverflow.com/questions/41498021/is-it-worth-to-use-slf4j-with-log4j2

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants