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

WIP: Bump dependency versions #916

Closed
wants to merge 7 commits into from

Conversation

hwbllmnn
Copy link
Contributor

@hwbllmnn hwbllmnn commented Jul 23, 2018

This bumps lots of dependency versions, especially switches to log4j2 since log4j 1.x was EOLed in 2015.

Once deegree/deegree-maven-plugin#9 is merged and 2.0.1 is released, I can upgrade the plugin version here and hopefully have a green travis build (once the geotools repo is added to the nexus proxy as I commented in #903).

Copy link
Member

@tfr42 tfr42 left a comment

Choose a reason for hiding this comment

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

Thanks for your contribution. May it be possible to split the commits for changing travis-ci config, log4j upgrade and batik update into separate PR?

@tfr42 tfr42 added needs discussion requires discussion with contributor in progress labels Aug 24, 2018
@hwbllmnn
Copy link
Contributor Author

Sure, I can do that in the next days.

@stephanr
Copy link
Member

In the voice chat we discussed whether we have an advantage in replacing the generic SLF4J API with a concrete implementation of log4j2.

Here we have come to the point that if there is no reason (such as peroformance problems or similar) we would prefer to keep SLF4J.
We also found out that logback is already often used instead of log4j.

@hwbllmnn
Copy link
Contributor Author

Some arguments to use log4j2 over slf4j: https://logging.apache.org/log4j/2.0/faq.html#api-tradeoffs

Ultimately I really have no preference, but log4j2 seems to offer the most flexibility. You can easily use logback via the log4j2 API, but I'm not sure the other way round.

@copierrj
Copy link
Member

copierrj commented Sep 7, 2018

The TMC decided that we are not prepared to switch to a different logging api at this point in time. Are you able to prepare a PR without this change?

@hwbllmnn
Copy link
Contributor Author

hwbllmnn commented Sep 7, 2018

I'll try that when I find the time. Unfortunately that has a couple of consequences:

  • there's no log4j2 binding for slf4j AFAIK, so we're still stuck with a logging API that EOLed in 2015
  • Version 2 of the deegree maven plugin generates no more log4j.properties, but produces log4j2.xml files

@hwbllmnn
Copy link
Contributor Author

Unfortunately, the previous fix did not fix all (travis) issues.

I've added deegree/deegree-maven-plugin#10 to fix the test related issues. I guess me locally relying on having the proprietary libraries available did not produce those issues before. This does not seem to fix all issues related to generated log4j2.xml files, but it does fix the tests itself (without any oracle or mssql profiles). I'll have a look at those issues once I have the time (and you're willing to put up with it).

I've also added skipping the deegree-tools-config module to this PR if the oracle profile is not activated. Adding the oracle sql dialect as a dependency does not work, as I understand it, <optional> maven dependencies only have an influence if you don't explicitly require them in the code.

As a last point, I'm sorry to say, since I'm just doing this in my spare time, I'm not really willing to rewrite everything back to using slf4j, especially as that means dropping everything back to using log4j v1 (or a different slf4j compatible logging lib). Extracting the dependency bump is also not that easy, since the deegree-maven-plugin plays a rather central role.

Let me know if you want me to keep going at this, if you want to drop/close it, feel free to do so (please let me know by mail/comment, I may not be available at the next IRC chat).

@hwbllmnn hwbllmnn closed this Feb 7, 2019
tfr42 added a commit to tfr42/deegree3 that referenced this pull request Mar 28, 2019
florianesser pushed a commit to wetransform-os/deegree3 that referenced this pull request Dec 17, 2021
florianesser pushed a commit to wetransform-os/deegree3 that referenced this pull request Dec 17, 2021
@tfr42 tfr42 removed the in progress label Oct 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs discussion requires discussion with contributor
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants