Bugzilla – Full Text Bug Listing
| Summary: | org.apache.lucene has inconsistent contract of optionally required bundles, and provided packages | ||||||
|---|---|---|---|---|---|---|---|
| Product: | [Tools] Orbit | Reporter: | David Williams <david_williams> | ||||
| Component: | bundles | Assignee: | Gunnar Wagenknecht <gunnar> | ||||
| Status: | RESOLVED FIXED | QA Contact: | |||||
| Severity: | normal | ||||||
| Priority: | P3 | CC: | dj.houghton, john.arthorne, robert.wetzold, tjwatson | ||||
| Version: | unspecified | ||||||
| Target Milestone: | Juno M6 | ||||||
| Hardware: | PC | ||||||
| OS: | Linux | ||||||
| Whiteboard: | |||||||
| Attachments: |
|
||||||
Assigning to Gunnar since he's shown as contact for 2.9.1, and adding DJ to CC since he's represented "the platform" with previous versions. It's a known bug in PDE. The "org.apache.lucene" is an "aggregate" bundle, i.e. it re-exports all packages from the Lucene "sub-" bundles it depends on. *** This bug has been marked as a duplicate of bug 259959 *** See also bug 329057. I am not convined this is a bug in PDE ... at least for this "optional" require bundle case, like org.apache.lucene has. I can imagine scenarios where things would not be resolved by p2 correctly. For example, if another bundle said it needed only o.a.lucene.analysis and o.a.lucene said it had it, then o.a.lucene would be installed ... but, if the optional bundle were not available, (or, if the were not installed greedily) then the package would not actually be there at the end of the install. The original dup bug, 259959, does not mention anything being "optional" so I could better understand why that case might be a bug. Plus, its use-case was to collect "split packages". I think the lucene case is there only for legacy apps that do not want to go to the trobule to require or imprort exaxctly what they need ... in that case we'd want users not NOT rerquire org.apache.lucene but instead the more specific things its needs. And, please remember, we will be getting rid of the "optional" implies "greedy" relationships in this dev cycle, so even if this worked right now, it might work once we change the greediness in the repo. Just commenting. I'm fine letting this ride ... until trouble bubbles up ... and if so, remind me to tell you "I told you so" (ha ha ha). (In reply to comment #4) > The original dup bug, 259959, does not mention anything being "optional" so I > could better understand why that case might be a bug. Plus, its use-case was to > collect "split packages". I think the lucene case is there only for legacy apps > that do not want to go to the trobule to require or imprort exaxctly what they > need ... in that case we'd want users not NOT rerquire org.apache.lucene but > instead the more specific things its needs. The "split package" case also applies. Some Lucene packages are split across multiple bundles. The think is that the lucene.core bundle exports all its packages with the split-package header. Thus, people that want just core (and nothing else) would have to add that very specific header as well. > And, please remember, we will be getting rid of the "optional" implies "greedy" > relationships in this dev cycle, so even if this worked right now, it might > work once we change the greediness in the repo. Good point. Should we remove the "optional" cause? I think if we were to remove the "optional" cause we'd get a few complains about too much stuff being brought in. (In reply to comment #5) > Good point. Should we remove the "optional" cause? I think if we were to remove > the "optional" cause we'd get a few complains about too much stuff being > brought in. If folks are complaining about too much being pulled in then they should not be requiring the lucene bundle directly. From my point of view the lucene bundle is providing an aggregated contract. The only way clients can consistently depend on that aggregated contract is for the lucene bundle to ensure that all of the contents of its aggregated contract are present. To me that sounds like non-optional dependencies. Adding Robert, as he is "contact" for version 3 of lucene bundles ... we'll want a consistent solution. I'll re-open this with different focus. Maybe the Red X's are a PDE bug :) ... but ... seems we've uncovered some inconsistency in how the "umbrella" bundle optionally requires bundles but exports all packages. I'm not sure what the right answer is, but seems the package exports should be removed from the umbrella bundle ... or, the required bundles not be optional. I'd lean towards removing the export packages from "org.apache.lucene" umbrella bundle. I think that when lucene was "split up", that umbrella bundle was left in place simply to satisfy "legacy" uses where people had "require bundle" on org.apache.lucene and it was desired they still end up with what they had before (where, some of the "required" bundles were new, from previous releases, so were thought could be optional). I think if someone "imports package" they expect to get a "finely tuned" set of bundles that satisfy the constraint ... not "everything" ... so they would not expect to get "org.apache.lucene". No? It appears there is no umbrella bundle for the 3.0.3 branch. I assume that is intentional? Should there be one, for compatibility? All this is complicated by changing how optional and greedy are treated in repository publishing. So, I'll reopen to continue discussion ... but, know of known problems this is causing right now. (In reply to comment #8) > ... but, know of known problems this is causing right now. Sorry, meant ... but, know of _no_ problems this is causing right now. Concerning the 3.0.3: the missing umbrella is intentionally currently, plainly speaking, for time and effort reasons. We required only 4 Apache bundles and also only have CQs for these. (In reply to comment #8) > > I'm not sure what the right answer is, but seems the package exports should be > removed from the umbrella bundle ... or, the required bundles not be optional. If I understand correctly the exports must remain. The are exporting the "complete" package as aggregated from the individual split parts. The split parts use mandatory directives to make sure casual Import-Package clients don't get wired to them unless they really mean to use the specific split part of the package. So normal Import-Package users that are expecting the complete package need the aggregate bundle to export the full package with no mandatory directives. > > I'd lean towards removing the export packages from "org.apache.lucene" umbrella > bundle. I think that when lucene was "split up", that umbrella bundle was left > in place simply to satisfy "legacy" uses where people had "require bundle" on > org.apache.lucene and it was desired they still end up with what they had > before (where, some of the "required" bundles were new, from previous releases, > so were thought could be optional). > > I think if someone "imports package" they expect to get a "finely tuned" set of > bundles that satisfy the constraint ... not "everything" ... so they would not > expect to get "org.apache.lucene". No? I don't think it is only to support legacy but to provide a proper Export-Package that gives a view of the "complete" package for clients to import using Import-Package. FWIW, the original bug discussing split packages is bug 260034. I think what's being said (here and there) is that the "require bundles" statements in the manifest really has nothing to do with the "export packages" statements ... given the way "child" bundles specify their "split packages"? If so, then I think things are fine the way they are ... and that the currently "optional" bundles should stay optional ... since that's the way it was in Indigo ... at least until someone complains of a specific case where they need ti changed. (And, they _might_ once we change so that optional bundles are no longer greedy?) So, I'll call this "second focus" as invalid, and misunderstanding on my part. Though the original purpose of the bug, described in comment 0 is still a dup of bug 259959. Thanks all for your comments. I still think the aggregate bundle could be left in a situation where it is not telling the truth about the package contracts it exports if it has optional require-bundle statements for bundles that provide part of the packages the aggregate bundle exports. I would not call this a best practice, but if the lucene owners and consumers are willing to accept that the API contract provided by the aggregate lucene bundle is fluid depending on the split parts that are installed then I guess we can live with it. Tom is correct on this one. The bundle is not telling the truth. It promises exports which it actually can't guarantee. I'm willing to fix this if people agree. I'll reopen ... admitting I've misunderstood two or three times all in the same bug :) Before you change anything, just make sure the platform's help system "uses it right" and won't suddenly get 3 times as many lucene bundles. (and/or post a note to cross-project about a potentially significant change coming ... at least, I think its significant ... unless I am misunderstanding again. :) We will move to new "publisher" added in Juno, so that our p2 repository, for Juno, will have "greedy=false" for optional dependencies (I think, if I am recalling things from last year) ... let me know if you think that's significant here ... such as you want your change in before publisher changes so you get some particular "before and after" version). Otherwise, I'll be changing that in next few weeks. Will this issue cause more problems now that our Orbit repo is "non greedy"? (bug 368999). I'd like to take a stab on fixing this when adding Lucene 3.5 to Orbit (bug 369792) for M6. Here is the plan: org.apache.lucene will no longer be the "catch-all" bundle. Instead it will become the "uniting" bundle for all Lucene packages which are split across different bundles. The Require-Bundle dependencies will be changed from optional to non-optional. Any other Lucene bundle that does not expert a split package will be removed from the Require-Bundle list. The change will only be implemented for 3.5 but not for lower versions. Using this strategy, org.apache.lucene will be a convenience bundle exporting split Lucene packages without split-package header. However, it won't be the catch-and-unite-all-if-available bundle. Thus, anybody importing Lucene packages without split-packages header will get the minimum required bundles for a united package space but not more. BTW, I'll also review carefully which packages are still split in 3.5. This has been revisited and changed with the Lucene 3.5 bundles (added with bug 369792). The new contract for org.apache.lucene is: Require-Bundle: org.apache.lucene.core;bundle-version="[3.5.0,4.0.0)";visibility:=reexport, org.apache.lucene.misc;bundle-version="[3.5.0,4.0.0)", org.apache.lucene.queries;bundle-version="[3.5.0,4.0.0)" Export-Package: org.apache.lucene.index;version="3.5.0", org.apache.lucene.search;version="3.5.0", org.apache.lucene.store;version="3.5.0" The dependencies aren't optional anymore to not violate the Export-Package contract. Anybody how does not wish to pull in these dependencies likely wants to specify a dependency on org.apache.lucene.core only. |
Created attachment 207956 [details] small screen shot showing errors in manifest.mf file. I was playing around with near M4 eclipse-SDK build (I20111205-2330) and "imported target bundles with source" and saw that the org.apache.lucene has lots of red X's in its manifest. Not sure if this is some new PDE check ... or if the issue has always existed, but many of the exported packages do not exist in org.apache.lucene. org.apache.lucene does have a lot of optional required bundles, which it reexports, since it is a "transitional bundle" ... but, it seems wrong for it to "claim" it exports these packages. It should leave that up to the bundles it reexports. I'll attach screen shot, since it better shows this issue visually.