-
Notifications
You must be signed in to change notification settings - Fork 24.7k
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
Avoid shading of org.joda.convert package #3557
Comments
what is the actual problem that you experienced with the current shading of joda? |
There is no problem with shaded joda-time, there is problem with relocation of org.joda.convert to org.elasticsearch.common.joda.convert. Bytecode points to this package however elasticsearch distribution does not contain it. |
we can do that, though I am still confused to be honest, what error does it cause? I would suspect it will cause the same problem when someone simply uses the joda time jar, cause it doesn't include those annotations as well... |
You are right, if you are using elasticsearch/joda-time jar in standard environment like J2SE or J2EE it doesn't matter. However for OSGi tools we have it is not so obvious, so we need to mark package org.elasticsearch.common.joda.convert as optional all the time when we try to use elasticsearch. The benefit of not shading org.joda.convert will be that all who wants to use joda-convert will be able to use it together with elasticsearch. Currently shaded joda time objects will be unknown for joda-convert since annotation names are not org.joda.convert.ToString but org.elasticsearch.common.joda.convert.ToString. This package does not exist, annotation will be dropped. Code below will not work differently for each instance: org.joda.convert.StringConvert.convertToString(org.elasticsearch.common.joda.time.DateTime.now());
org.joda.convert.StringConvert.convertToString(org.joda.time.DateTime.now()); |
coming back to this, we added Joda convert because of this issue #4660, a bit of conflicting requirements.... |
See PR #3558 |
Annotations from package org.joda.convert package are used by couple of org.joda.time classes. However there is no other direct reference between these two projects. Annotations from joda-convert project are used only by this project, mainly to have unified way to call appropriate .parse(String) variants in classes like DateTime/DateMidnight/Period.
According to Java language specification annotations which are not present at the runtime are simply ignored and do not break linkage process, thus dependency is optional. As long as elasticsearch does not use org.joda.convert package this dependency may be omitted and excluded from shading.
The text was updated successfully, but these errors were encountered: