Avoid shading of org.joda.convert package, fixes #3557

[splatch]

Another fix for shading issue.

[clintongormley]

Hi @splatch

Sorry it has a taken a whil to get to this, but is this still an issue?

[splatch]

hey @clintongormley, I think it is still a case, since fix for #4660 was a brute force. The issue described in my comment last year was ignored:
#3557 (comment)

I'm not sure if Scala still has the issue with optional annotations and if elasticsearch still needs fix introduced in #4660, because as I said - by JVM spec annotations having runtime retention which classes could not be found during class resolving should be dropped.

Cheers,
Lukasz

[s1monw]

I am trying to understand the issue here, if we only shade the org.joda.time package what are the implications? I am not a shading expert but afaik we then just keep the org.joda package for classes outside org.joda.time right? this might break bw compatibilty though, is this really such a big issue?

[splatch]

Hey @s1monw,
You are right, sharding org.joda.time itself will be sufficient. Before elasticsearch used outdated maven-shade-plugin which was not relocating constant pool resulting half-shaded artifacts from bytecode point of view. After update of shade-plugin it's safe to have org.joda.time shaded back. All annotation references to org.joda.convert will work properly - just not sure how scala compiler will live with it. Maybe after 2.11/2.12 updates Scala got smarter? ;-)

[s1monw]

@splatch I dont' have any way to see if this fixes anything - can you confirm your PR fixes the problem?

[clintongormley]

@splatch could you confirm whether this patch fixes the problem you're experiencing?

[splatch]

Yes, this pull request solves my problem. I checked it against 1.3.4 - commit can be cherry-picked.

[clintongormley]

w00t - thanks

[martijnvg]

Pushed via: b0a14f0

[splatch]

Thx a lot!