It sounds like we need a JIRA to look at the implementation and figure out how disruptive this change actually is. I created https://issues.apache.org/jira/browse/CURATOR-200 to investigate.
On Wed, Apr 1, 2015 at 11:09 AM, Jordan Zimmerman < [email protected]> wrote: > Sorry if I’m not being clear. I don’t mind changing the APIs. My only > concern is disrupting the current community. I’m hoping that we can add new > APIs and deprecate the old ones. I don’t know if OSGi supports this or not. > > -JZ > > > > On April 1, 2015 at 11:08:09 AM, Simon Kitching ( > [email protected]) wrote: > > Hi All, > > This problem would have occurred with or without OSGi. However without > OSGi, it wouldn't be solvable at all :-) > > I need to write code that: > * uses a library that needs a very old version of guava, ie is not > compatible with guava 16.0.1. > * uses curator which relies on guava 16.0.1 or later. > > With a "flat" classpath, this just cannot be solved. With OSGi, it can be > solved _if_ one or both of these libraries only relies "internally" on > guava. > > If curator had used guava everywhere throughout its public API, I wouldn't > bother asking here. But curator is _very close_ to merely using guava as an > internal detail, and so it seemed worth asking if the dev team were > interested in making the few changes necessary. > > Note that this problem is partially self-inflicted due to my employer's > requirement to use this ugly library that relies on very old guava > (actually, "com.google.collections:google-collections" which is the earlier > name for guava and sadly guava uses the same com.google.common.* > package-names). However every library makes incompatible changes from time > to time, so this will probably also bite someone else sometime. > > Thanks for at least discussing this. As there's apparently no great > enthusiasm for the idea, I'll need to solve my immediate problem some other > way (eg by forking curator locally or using maven-shade-plugin or similar) > for now. I'll try to find time next month (May) to see if I can come up > with some patches that might be acceptable (as mentioned in "compatible > API" below). > > Regards, > Simon > > ________________________________ > From: Jordan Zimmerman [[email protected]] > Sent: Wednesday, April 01, 2015 17:47 > To: [email protected]; Sean Busbey > Cc: Mike Drob; Simon Kitching > Subject: Re: Proposal : remove references to guava library from public APIs > > The problem, though, was with OSGi. OSGi support for Curator was added by > Curator users and not the core team. Curator has never committed to OSGi > support. Of course, we could change that. > > -JZ > > > > > On April 1, 2015 at 10:41:51 AM, Sean Busbey ([email protected]<mailto: > [email protected]>) wrote: > > This thread started with Simon running in to a Guava problem while trying > to use Curator. > > On Wed, Apr 1, 2015 at 10:33 AM, Jordan Zimmerman < > [email protected]> wrote: > > > I know Guava has caused problems elsewhere. But, I don’t recall any > > problems with Curator. > > > > -JZ > > > > > > > > On April 1, 2015 at 10:32:04 AM, Mike Drob ([email protected]) wrote: > > > > Respectfully disagree. Guava issues have plagued Hadoop in the past (and > to > > an extent, still do). Guava versions tend to iterate relatively quickly > and > > don't always have the keenest eye on backwards compatibility. When you > > expose your guava implementation, that locks all of your users into the > > same version because newer versions may no longer work with your library > > (an issue which osgi seeks to address). > > > > The JDK, on the other hand, goes to painstaking lengths to ensure > backwards > > compat for the past 20+ years. > > > > On Wed, Apr 1, 2015 at 9:55 AM, Jordan Zimmerman < > > [email protected] > > > wrote: > > > > > I consider Guava to be part of the JDK so I disagree. We haven’t had > many > > > issues with Guava compatibility. In fact, I can’t think of one Jira > > > reported on it. So, my vote would be to leave things as they are. > > > > > > -JZ > > > > > > > > > > > > On April 1, 2015 at 3:09:49 AM, Simon Kitching ( > > > [email protected]) wrote: > > > > > > Thanks Jordan. > > > > > > The root cause of the problem isn't really anything osgi-specific; it's > > > the fact that curator uses another library (guava) as part of its > > _public_ > > > API. > > > > > > Imagine you wanted to change from using guava to some other collections > > > library - it wouldn't be possible without breaking the public API of > > > curator. The question is whether guava really should be part of the > > curator > > > API, or should be just an implementation detail. I would suggest that > the > > > use of guava is really an implementation detail that should be > > > private/hidden - unlike use of jaxws for example, which really is an > > > externally-defined abstract API and is reasonable to include as part of > > the > > > public API of curator. > > > > > > This difference between used-in-the-impl and used-in-the-api doesn't > > > matter so much in a java application with one big classloader that has > > > every single jarfile in it; if you need the guava library in the > > classpath > > > for internal use by curator, then it will automatically also be visible > > to > > > all other classes and so it is impossible to have a different version > of > > > the library also present. Using OSGi (which creates multiple > > classloaders) > > > can allow multiple versions of the same lib - but only when the lib is > > only > > > used-in-the-impl (ie is for "internal" usage by a jarfile). > > > > > > Re keeping a compatible API: possibly all classes in package > > > "org.apache.curator.framework.listen" could be copied into a new > package, > > > and then the ListenerContainer class updated to not expose guava. All > > > classes in "org.apache.curator.framework.listen" could then be > > deprecated. > > > As long as OSGi code avoids using any code from the old package, there > > > would be no binding to the guava library used by curator. I don't know > if > > > that would be better than simply changing the API for a couple of > methods > > > or not. > > > > > > I will create a JIRA issue to update the maven-build-plugin version; > > > that's trivial and is not a binary incompatibility. > > > > > > Unless somebody objects within the next few days, I will also create a > > > JIRA issue regarding the APIs that expose guava. I might have time to > > work > > > on this myself next month (may). > > > > > > By the way, if you're interested in how OSGi classloading works, this > may > > > be helpful: http://moi.vonos.net/java/osgi-classloaders/ > > > > > > Thanks & Regards, > > > Simon (aka skitching at apache.org) > > > > > > ________________________________ > > > From: Jordan Zimmerman [[email protected]] > > > Sent: Tuesday, March 31, 2015 17:46 > > > To: [email protected]; Simon Kitching > > > Subject: Re: Proposal : remove references to guava library from public > > APIs > > > > > > I don’t have an objection in general. The biggest problem for me is > that > > I > > > know very little about OSGI. All of the OSGI work has been contributed > so > > > it’s hard to make sure that we keep it working. That said, changing > > > existing APIs is very disruptive to the Curator community. I’d like to > > see > > > someone (Simon?) commit to maintaining the OSGi compatibility of > Curator > > > and make sure releases don’t introduce issues. Also, can the existing > > APIs > > > remain and new, OSGi compatible parallel APIs be added? > > > > > > -JZ > > > > > > > > > > > > On March 31, 2015 at 7:39:08 AM, Simon Kitching ( > > > [email protected]<mailto: > > > [email protected]>) wrote: > > > > > > Hi, > > > > > > I've noticed that several curator classes expose the use of classes > from > > > google's guava library [1] as part of their *public* api. > > > > > > [1] maven artifact "com.google.guava:guava" which contains java > packages > > > com.google.common.* > > > > > > In an OSGi environment, it is possible to load multiple different > > versions > > > of the same library, as long as that library is a purely internal > > > implementation detail. Unfortunately, as curator exposes its use of > > guava, > > > this makes it impossible for code that uses curator to also use a > > different > > > version of Guava for its own purposes. Unfortunately, this has just > > bitten > > > me : I need to write code that uses both curator (requires guava 16.0 > or > > > later) and a third-party library that requires an earlier version of > > guava. > > > > > > Are there any objections to me raising an enhancement issue in JIRA for > > > this? Note that this change would be a binary incompatibility (though > > > fairly limited). > > > > > > The problem classes that I have found are: > > > * curator-framework: > > org.apache.curator.framework.listen.ListenerContainer > > > : method forEach takes a parameter of type > > com.google.common.base.Function > > > * curator-framework: > > > org.apache.curator.framework.api.transaction.CuratorTransactionResult : > > > method ofTypeAndPath returns com.google.common.base.Predicate > > > * curator-x-discovery-server: > > > org.apache.curator.x.discovery.server.contexts.GenericDiscoveryContext > : > > > constructor takes param of type com.google.common.reflect.TypeToken > > > * curator-x-discovery: org.apache.curator.x.discovery.InstanceFilter : > > > inherits from com.google.common.base.Predicate > > > > > > And by the way, I noticed that org.codehaus.jackson types are also used > > in > > > public APIs (at least, GenericDiscoveryContext). It may also be worth > > > looking into whether it is really necessary to expose this dependency. > > > > > > The goal of the change would be to ensure that in the MANIFEST.MF file > > for > > > each curator bundle (jarfile), the Export-Packages line minimises the > > > "uses:=" entries which refer to non-curator packages. A uses-constraint > > on > > > a package should only be needed when something in the package being > > > exported uses an external type in its public API. > > > > > > As a separate problem, I have noticed that with the 2.7.1 release (at > > > least), the "bnd" tool (via maven-bundle-plugin) is adding entries to > the > > > "uses" entries even when the referenced library is purely used > > internally. > > > I have found a reference (https://github.com/emlun/bnd-uses-strange) > > that > > > suggests this is a bug which is fixed in later bnd releases. > > Unfortunately > > > I can find no release-notes for bnd, nor any source-code repository so > > > cannot confirm this. However updating curator/pom.xml to specify the > > > following fixes the "uses" clauses: > > > <maven-bundle-plugin-version>2.5.3</maven-bundle-plugin-version> > > > > > > Thanks & Regards, > > > Simon > > > > > > ________________________________ > > > The information in this email is confidential and may be legally > > > privileged. It is intended solely for the addressee. Access to this > email > > > by anyone else is unauthorised. If you are not the intended recipient, > > any > > > disclosure, copying, distribution or any action taken or omitted to be > > > taken in reliance on it, is prohibited and may be unlawful. SmartStream > > > Technologies GmbH, Vienna Twin Tower, Wienerbergstrasse 11, 1100 > Vienna, > > > Austria, FN 194340w, HG Wien > > > ________________________________ > > > The information in this email is confidential and may be legally > > > privileged. It is intended solely for the addressee. Access to this > email > > > by anyone else is unauthorised. If you are not the intended recipient, > > any > > > disclosure, copying, distribution or any action taken or omitted to be > > > taken in reliance on it, is prohibited and may be unlawful. SmartStream > > > Technologies GmbH, Vienna Twin Tower, Wienerbergstrasse 11, 1100 > Vienna, > > > Austria, FN 194340w, HG Wien > > > > > > > > > -- > Sean > ________________________________ > The information in this email is confidential and may be legally > privileged. It is intended solely for the addressee. Access to this email > by anyone else is unauthorised. If you are not the intended recipient, any > disclosure, copying, distribution or any action taken or omitted to be > taken in reliance on it, is prohibited and may be unlawful. SmartStream > Technologies GmbH, Vienna Twin Tower, Wienerbergstrasse 11, 1100 Vienna, > Austria, FN 194340w, HG Wien >
