[CURATOR-464] attach orignal artifacts with classifier original#355
[CURATOR-464] attach orignal artifacts with classifier original#355asfgit merged 1 commit intoapache:masterfrom
Conversation
|
I'm -1 on this PR. The vast majority don't use OSGI and don't want to have to manage a separate artifact. If I understand alternative 2, publish a new artifact without the shaded code, I'd be OK with that. |
|
Updating the title and the commit of this PR. Following alternative 2, I propose a new change where projects of type jar or bundle would have original artifacts attached, with classifier 'original'. It's better than 'slim', as shading does more than simply including. The byte code of curator classes is also modified during shading, e.g. imports of google guava packages become imports of relocated guava packages. Though listed in supported types in the bundle plugin, projects of type war are not included in this change, because those projects simply pack everything in the dependency tree into artifacts. Projects of type jar, on the other hand, could produce artifacts usable as bundles, by planting headers required by OSGi runtime into manifest during packaging, so they are included. Thanks! |
|
May I ask for some review and comments? |
|
@Randgalt, may I ask for your review around this change? |
|
Sorry - I've been tied up. I'll look at this soon. |
|
A few things:
|
|
I don't understand how this got merged - did I do it by accident? I didn't intend to. I'm going to open another PR to revert changes so we can continue the discussion. |
Would like to revisit the following issue.
https://issues.apache.org/jira/browse/CURATOR-464
The current approach would be preventing shaded artifacts from replacing original artifacts. Instead, shaded artifacts are now also attached with classifier uber, next to original artifacts, so clients could integrate either based on their need.
This approach is my preference while it'll be a little expensive in adoption, as new classifier must be added during upgrade. If this really bothers, here are 2 alternatives we may go:
Removing the reloaction in shading. Packages like
org.apache.curator.shaded.com.googlecould be found nowhere but in curator-client, which doesn't export these relocated packages in its manifest. Without relocation, these packages could be fulfilled by some other bundle, following the import instruction inside the manifest. However, if shaded classes take priority, problems would be expected in OSGi runtime because same classes are loaded by different bundles and they become different classses. This decision belongs to the implementation of OSGi runtime.Attaching original artifact with some classifier like 'slim'. This is essentially the reverse way of the proposed approach, which costs little in adoption as no new classifier is required during upgrade. Personally, I don't like this way as we don't usually put classifier on original artifacts. However, if compared with this issue not being fixed, this alternative would be better.
Please review the change and any comment is welcome.