Update OWLAPI to version 4.5.29 #1200
Conversation
|
Not specific to this PR, but it made me notice that we have tons of references to a nonexistent FOAF property ("depicted_by"). It should be |
pom.xml
Outdated
| @@ -204,12 +204,12 @@ | |||
| <dependency> | |||
| <groupId>ch.qos.logback</groupId> | |||
| <artifactId>logback-classic</artifactId> | |||
| <version>1.2.13</version> | |||
| <version>1.3.0</version> | |||
There was a problem hiding this comment.
This is fine.
But just FYI, it would also be possible to make ROBOT compile against the new OWLAPI without having to drastically bump the versions of the logging libraries (Logback and Log4-over-SLF4J): simply add an explicit dependency to org.slf4j:slf4j-api version 1.7.x.
The reason the build fails if we do not update the logging libraries is because the OWLAPI declares a dependency to org.slf4j:slf4j-api 2.x. ROBOT “inherits” that dependency (since it depends on the OWLAPI), so when we build the robot.jar archive, that archive ends up containing a copy of the slf4j-api 2.x (inherited from the OWLAPI) and a copy of Logback-classic (explicitly declared here as a direct dependency) — and those two versions are not compatible.
But SLF4J-API is strictly backwards compatible. Just because the OWLAPI declares it depends on slf4j-api >= 2.x does not mean it cannot work with slf4j-api 1.x.
So we could explicitly declare slf4j-api 1.7.x as a direct dependency of ROBOT here (alongside Logback-classic and Log4J-over-SLF4J). We would then end up with a robot.jar archive containing only versions of logging libraries that are compatible with each other (SLF4J-API < 2 and Logback-classic < 1.3).
Not saying that we should do that, of course. It is perfectly fine to bump to Logback-classic 1.3 and Log4J-over-SLF4J 2.0. Just mentioning the other possibility.
There was a problem hiding this comment.
Oh thanks @gouttegd I didnt know that!
@jamesaoverton what is your preference? Bumping versions up, or adding this additional (transitive) dependency as a constraint?
There was a problem hiding this comment.
Thanks @gouttegd for the explanation.
I despise Java logging, which may be clouding my judgement. But I think the simplest thing is just to bump these two dependencies to their latest versions:
- pkg:maven/ch.qos.logback/logback-classic@1.5.6
- pkg:maven/org.slf4j/log4j-over-slf4j@2.0.13
This built and passed the test suite for me locally, so I don't see the benefit of adding yet another dependency (slf4j-api) that I don't understand. If I misunderstood something, please let me know.
There was a problem hiding this comment.
I despise Java logging
I just spent the last 24 hours debugging issues caused by Java logging libraries, so… I share your feeling! :)
I don't see the benefit of adding yet another dependency (slf4j-api) that I don't understand. If I misunderstood something, please let me know.
Just that ROBOT is already dependent on slf4j-api – it’s just that currently that dependency is an indirect one.
But I fully agree that adding the explicit dependency on slf4j-api just to avoid bumping logback-classic and log4-over-slf4j would have no particular benefit in ROBOT’s case. It would only be useful if for some reason we wanted to keep using older versions of those libraries.
There was a problem hiding this comment.
The slf4j upgrade has been removed. Only the logback upgrade remains now.
@gouttegd explained the fact that, for some reason, the built passed without this change because of some issue with the shading plugin. We still need this upgrade
|
@matentzn Why not use the latest versions of these dependencies?
|
|
I have no opinion on these matters, other than not constraining the dependencies more than absolutely necessary- I leave it to you and am very happy to support using the most up to date versions! |
|
Ok. Since we're updating, I'd like to update to the latest stable releases of these dependencies. |
jamesaoverton
changed the title
Update OWLAPI to version 4.5.28
docs/have been added/updatedmvn verifysays all tests passmvn sitesays all JavaDocs correctCHANGELOG.mdhas been updated