Community
Participate
Working Groups
The publisher does not produce any metadata for the Eclipse-GenericRequire and Eclipse-GenericCapability headers. It should. A peek at GenericSpecificationImpl#isSatisfiedBy shows how these are matched up. A name, a type and an LDAP filter.
Note that OSGi now has standard headers for this: Provide-Capability and Require-Capability. I prefer we recognize these headers over the Eclipse specific ones.
For folks that are wondering. The specification for the new headers Require-Capability/Provide-Capability can be found in the OSGi Service Platform Release 4 Version 4.3 Core Specification found at: http://www.osgi.org/Download/Release4V43 Look at section 3.3 for the details.
*** Bug 324352 has been marked as a duplicate of this bug. ***
Could someone give me news about the support of Provide-Capability and Require-Capability in Equinox/P2?
As you can see from the status of this bug, the publishers (= translators from OSGi to p2 metadata) don't take them into account yet. AFAIK, the p2 metadata format should be powerful enough to express the generic provides/requires.
Given that the p2 publisher (metadata generator) does not read those attributes, you can express additional dependencies using the p2.inf mechanism (http://wiki.eclipse.org/Equinox/p2/Customizing_Metadata). It is a bit cumbersome but it gets the job done.
(In reply to comment #6) > Given that the p2 publisher (metadata generator) does not read those > attributes, you can express additional dependencies using the p2.inf > mechanism (http://wiki.eclipse.org/Equinox/p2/Customizing_Metadata). It is a > bit cumbersome but it gets the job done. Pascal, one year ago when I moved Gemini JPA to Tycho, I tried that p2.inf stuff. I spent one entire day trying to figure out a way to build Gemini JPA Provide-Capability and Require-Capability support test project. I couldn't make it work in that day. So I gave up.
> Pascal, one year ago when I moved Gemini JPA to Tycho, I tried that p2.inf > stuff. I spent one entire day trying to figure out a way to build Gemini JPA > Provide-Capability and Require-Capability support test project. I couldn't > make it work in that day. So I gave up. I agree that it is really painful to put together. If you still need this (and I assume you do), open a bug in your bugzilla with all the details and I can try to take a look at it.
FWIW, I ran into a weird problem today while trying to publish (generate) metadata for a bundle containing a Require-Capability directive using Tycho 0.19.0. The stacktrace is: Caused by: java.lang.IllegalArgumentException at org.eclipse.tycho.p2.impl.publisher.P2GeneratorImpl.getCanonicalArtifact(P2GeneratorImpl.java:156) at org.eclipse.tycho.p2.impl.publisher.P2GeneratorImpl.generateMetadata(P2GeneratorImpl.java:109) at org.eclipse.tycho.plugins.p2.P2MetadataMojo.attachP2Metadata(P2MetadataMojo.java:143) at org.eclipse.tycho.plugins.p2.P2MetadataMojo.execute(P2MetadataMojo.java:104) So for some reason no metadata is generated if a Require-Capability is present. Not sure yet what's going on. As soon as I remove the Require-Capability header, the error goes away and proper p2 metadata is generated.
(In reply to Gunnar Wagenknecht from comment #9) > FWIW, I ran into a weird problem today while trying to publish (generate) > metadata for a bundle containing a Require-Capability directive using Tycho > 0.19.0. Looks like an incorrect header. Unfortunately, the error is suppressed by BundlesAction (bug 428950).
(In reply to Thomas Watson from comment #1) > Note that OSGi now has standard headers for this: > > Provide-Capability and Require-Capability. I prefer we recognize these > headers over the Eclipse specific ones. I updated the title accordingly.
Is there any hope that p2 meta-data for Provide/Require-Capability could be addressed in the Mars/4.5 release? Why this matters to me/ECF: as an R6 enterprise-spec-compliant impl of OSGi Remote Services/RSA we have spec-defined Provide-Capability headers in our discovery and distribution/rs providers (newly added in R6 specification). It would make us and our consumers...some of which use p2 for installation of ECF RS/RSA...very happy to be able to use Require-Capability in our/their plugins and/or products to specify the requirement for specific providers(s). Then their consumers could install...and then be able to resolve at runtime...specific discovery and/or distribution providers. Addressing this bug would allow them to easily create the necessary meta-data from their builds simply by using the standard Require-Capability header. If desired, we could contribute some to the implementation. Thanks.
(In reply to comment #12) > If desired, we could contribute some to the implementation. Thanks. This would be good. I'd also like to see this bug fixed, but it just isn't high enough on my list. I'd review and submit good patches though. Bug 346174 comment #31 may have some useful pointers of how to implement this. [1] may also come in handy. [1] http://wiki.eclipse.org/Query_Language_for_p2
I am submitting for review a batch of 10 changes that implement this issue: https://git.eclipse.org/r/#/c/98871/
Gerrit change https://git.eclipse.org/r/98861 was merged to [master]. Commit: http://git.eclipse.org/c/equinox/rt.equinox.p2.git/commit/?id=70c366fd4d0475ed1fd826cbf2283f8ed3b3ebec
Gerrit change https://git.eclipse.org/r/98862 was merged to [master]. Commit: http://git.eclipse.org/c/equinox/rt.equinox.p2.git/commit/?id=02e018c88f5307fc754ae8bc0c0467eb209dd982
Gerrit change https://git.eclipse.org/r/98871 was merged to [master]. Commit: http://git.eclipse.org/c/equinox/rt.equinox.p2.git/commit/?id=d2c97e4e8f866fd8233c5ed67a99907b34e00ab4
Gerrit change https://git.eclipse.org/r/98866 was merged to [master]. Commit: http://git.eclipse.org/c/equinox/rt.equinox.p2.git/commit/?id=780d9bedcc73cf2daf808b1a16d68565daba871a
New Gerrit change created: https://git.eclipse.org/r/104433
Gerrit change https://git.eclipse.org/r/104433 was merged to [master]. Commit: http://git.eclipse.org/c/equinox/rt.equinox.p2.git/commit/?id=6c0135edf1bd792bf107d96a5fa53681dd1ce6f1
Half of the commits are presently merged. If there anything required on my part now for rest to be merged?
Gerrit change https://git.eclipse.org/r/98865 was merged to [master]. Commit: http://git.eclipse.org/c/equinox/rt.equinox.p2.git/commit/?id=8e6983eab7b45fbe90282303641fe76d2d3b4971
Gerrit change https://git.eclipse.org/r/98863 was merged to [master]. Commit: http://git.eclipse.org/c/equinox/rt.equinox.p2.git/commit/?id=aa4390e03ef4b5f6676c9e2991779df068b38f2b
Gerrit change https://git.eclipse.org/r/98864 was merged to [master]. Commit: http://git.eclipse.org/c/equinox/rt.equinox.p2.git/commit/?id=872431089780603782ca6de7c774059ed8620e3c
Gerrit change https://git.eclipse.org/r/98867 was merged to [master]. Commit: http://git.eclipse.org/c/equinox/rt.equinox.p2.git/commit/?id=508127d71a2a2cbc3aad554f2931219ae190a410
New Gerrit change created: https://git.eclipse.org/r/106191
Gerrit change https://git.eclipse.org/r/98868 was merged to [master]. Commit: http://git.eclipse.org/c/equinox/rt.equinox.p2.git/commit/?id=719079bb53131a19435e345b0e58228dedb75dbc
What would be the best way to test it in real life? I think once Tycho includes a p2 version with the provided patch, we can remove all the blocks such as http://git.eclipse.org/c/platform/eclipse.platform.ui.git/tree/tests/pom.xml#n74 that should be automatically resolved with those changes; shouldn't they? Note that it would still require the most dynamic bundles to properly define capabilities (bug 424104). Those should all be covered, but it's probably that some were missed or forgotten and that this patches unveil some missing declarations inside Platform.
Gerrit change https://git.eclipse.org/r/106191 was merged to [master]. Commit: http://git.eclipse.org/c/equinox/rt.equinox.p2.git/commit/?id=1c02b17b8348fc8a3e04a05db1240a28ecc352cf
(In reply to Mickael Istria from comment #28) > What would be the best way to test it in real life? > I think once Tycho includes a p2 version with the provided patch, we can > remove all the blocks such as > http://git.eclipse.org/c/platform/eclipse.platform.ui.git/tree/tests/pom. > xml#n74 that should be automatically resolved with those changes; shouldn't > they? > Note that it would still require the most dynamic bundles to properly define > capabilities (bug 424104). Those should all be covered, but it's probably > that some were missed or forgotten and that this patches unveil some missing > declarations inside Platform. I am hoping the solution to this bug will also avoid the need for hacks like this one in ECF: http://git.eclipse.org/c/ecf/org.eclipse.ecf.git/tree/examples/bundles/com.mycorp.examples.timeservice.consumer.filediscovery/pom.xml#n18
(In reply to Mickael Istria from comment #28) > What would be the best way to test it in real life? > I think once Tycho includes a p2 version with the provided patch, we can > remove all the blocks such as > http://git.eclipse.org/c/platform/eclipse.platform.ui.git/tree/tests/pom. > xml#n74 that should be automatically resolved with those changes; shouldn't > they? > Note that it would still require the most dynamic bundles to properly define > capabilities (bug 424104). Those should all be covered, but it's probably > that some were missed or forgotten and that this patches unveil some missing > declarations inside Platform. Hi Mickael, this is really cool news. The workaround you mention is the one to be done only in the pom.xml. Another workaround solution that works since Oxygen is the usage of p2 capabilities. For this I added the p2.inf files in a lot of Equinox bundles to be able to specify a capability without the need to modify the pom.xml. IIRC I even modified some of the platform bundles for this to avoid pom.xml configurations. Anyhow, the correct way is the usage of OSGi capabilities to avoid additional configuration in any way. And yes I think you are right, you can only test this when Tycho is ready to consume the patched p2. You could also try to work with p2 repositories and enable "Include required software" in the target definition to see if that works. For example, create a p2 repository or use an existing one that contains a bundle with a Require-Capability header for DS 1.3. The target platform should point to the Oxygen repository, but not including the Felix SCR bundle directly. When adding your bundle to the target definition, the newly introduced p2 mechanism should identify and add the Felix SCR bundle to your target platform as required bundle. Hope it is clear what I mean and it is possible to test it this way. At least it should in theory. :) When this is working, the following tickets would also be nice to get implemented: Bug 513216 - the Require-Capability header for DS 1.3 should be created automatically Bug 490063 - automatically add the Provide-Capability header for the included services And maybe we also need to think about some additional support to specify capabilities via editor. Dirk
What are the remaining work items on this issue? I see capabilities are well parsed, read in MANIFEST, read in metadata and published to metadata... Is there anything else to do in order to complete this story?
(In reply to Mickael Istria from comment #32) > What are the remaining work items on this issue? I see capabilities are well > parsed, read in MANIFEST, read in metadata and published to metadata... Is > there anything else to do in order to complete this story? The changes merged so far make the processing of Provide/Require-Capability fully functional, but it is tested in so far as all current tests were updated to pass. The issue may be closed and another can be opened for the automated tests.
If you plan to work on some tests soon, let's keep it open.
(In reply to Dirk Fauth from comment #31) > (In reply to Mickael Istria from comment #28) > > What would be the best way to test it in real life? > > I think once Tycho includes a p2 version with the provided patch, we can > > remove all the blocks such as > > http://git.eclipse.org/c/platform/eclipse.platform.ui.git/tree/tests/pom. > > xml#n74 that should be automatically resolved with those changes; shouldn't > > they? > > Note that it would still require the most dynamic bundles to properly define > > capabilities (bug 424104). Those should all be covered, but it's probably > > that some were missed or forgotten and that this patches unveil some missing > > declarations inside Platform. > > Hi Mickael, > > this is really cool news. The workaround you mention is the one to be done > only in the pom.xml. Another workaround solution that works since Oxygen is > the usage of p2 capabilities. For this I added the p2.inf files in a lot of > Equinox bundles to be able to specify a capability without the need to > modify the pom.xml. IIRC I even modified some of the platform bundles for > this to avoid pom.xml configurations. > > Anyhow, the correct way is the usage of OSGi capabilities to avoid > additional configuration in any way. And yes I think you are right, you can > only test this when Tycho is ready to consume the patched p2. You could also > try to work with p2 repositories and enable "Include required software" in > the target definition to see if that works. > For example, create a p2 repository or use an existing one that contains a > bundle with a Require-Capability header for DS 1.3. The target platform > should point to the Oxygen repository, but not including the Felix SCR > bundle directly. When adding your bundle to the target definition, the newly > introduced p2 mechanism should identify and add the Felix SCR bundle to your > target platform as required bundle. > > Hope it is clear what I mean and it is possible to test it this way. At > least it should in theory. :) > > When this is working, the following tickets would also be nice to get > implemented: > Bug 513216 - the Require-Capability header for DS 1.3 should be created > automatically > Bug 490063 - automatically add the Provide-Capability header for the > included services > > And maybe we also need to think about some additional support to specify > capabilities via editor. > > Dirk Be aware that there are no guarantees as to how the capabilities/requirements added via p2.inf interact with those extracted from the Provide/Require-Capability headers. E.g. for "osgi.service" capabilities it is not possible to publish the metadata from Provide-Capability as a p2 (namespace, name, version) tuple. As a result the name is a unique synthetic string and the version may be "0.0.0". Such a capability can be matched by a requirement extracted from the Require-Capability header, but not from p2.inf. The p2.inf format is not rich enough to allow you to declare a requirement with an LDAP filter (this should be a different bug). I think the capabilities/requirements added so far to the set of equinox bundles via p2.inf work independently from Provide/Require-Capability. I.e. you can use either to provision. However it will be best to stop using p2.inf to reflect Require/Provide-Capability since this can cause conflicts in the future.
(In reply to Todor Boev from comment #35) > (In reply to Dirk Fauth from comment #31) > > (In reply to Mickael Istria from comment #28) > > > What would be the best way to test it in real life? > > > I think once Tycho includes a p2 version with the provided patch, we can > > > remove all the blocks such as > > > http://git.eclipse.org/c/platform/eclipse.platform.ui.git/tree/tests/pom. > > > xml#n74 that should be automatically resolved with those changes; shouldn't > > > they? > > > Note that it would still require the most dynamic bundles to properly define > > > capabilities (bug 424104). Those should all be covered, but it's probably > > > that some were missed or forgotten and that this patches unveil some missing > > > declarations inside Platform. > > > > Hi Mickael, > > > > this is really cool news. The workaround you mention is the one to be done > > only in the pom.xml. Another workaround solution that works since Oxygen is > > the usage of p2 capabilities. For this I added the p2.inf files in a lot of > > Equinox bundles to be able to specify a capability without the need to > > modify the pom.xml. IIRC I even modified some of the platform bundles for > > this to avoid pom.xml configurations. > > > > Anyhow, the correct way is the usage of OSGi capabilities to avoid > > additional configuration in any way. And yes I think you are right, you can > > only test this when Tycho is ready to consume the patched p2. You could also > > try to work with p2 repositories and enable "Include required software" in > > the target definition to see if that works. > > For example, create a p2 repository or use an existing one that contains a > > bundle with a Require-Capability header for DS 1.3. The target platform > > should point to the Oxygen repository, but not including the Felix SCR > > bundle directly. When adding your bundle to the target definition, the newly > > introduced p2 mechanism should identify and add the Felix SCR bundle to your > > target platform as required bundle. > > > > Hope it is clear what I mean and it is possible to test it this way. At > > least it should in theory. :) > > > > When this is working, the following tickets would also be nice to get > > implemented: > > Bug 513216 - the Require-Capability header for DS 1.3 should be created > > automatically > > Bug 490063 - automatically add the Provide-Capability header for the > > included services > > > > And maybe we also need to think about some additional support to specify > > capabilities via editor. > > > > Dirk > > Be aware that there are no guarantees as to how the > capabilities/requirements added via p2.inf interact with those extracted > from the Provide/Require-Capability headers. > > E.g. for "osgi.service" capabilities it is not possible to publish the > metadata from Provide-Capability as a p2 (namespace, name, version) tuple. > As a result the name is a unique synthetic string and the version may be > "0.0.0". Such a capability can be matched by a requirement extracted from > the Require-Capability header, but not from p2.inf. The p2.inf format is not > rich enough to allow you to declare a requirement with an LDAP filter (this > should be a different bug). > > I think the capabilities/requirements added so far to the set of equinox > bundles via p2.inf work independently from Provide/Require-Capability. I.e. > you can use either to provision. However it will be best to stop using > p2.inf to reflect Require/Provide-Capability since this can cause conflicts > in the future. Once it works I will be happily remove the p2.inf stuff from the equinox bundles again
Removing the p2.inf capabilities is a breaking change. After all they were added to Oxygen specifically so that third parties can require them.(In reply to Dirk Fauth from comment #36) > (In reply to Todor Boev from comment #35) > > (In reply to Dirk Fauth from comment #31) > > > (In reply to Mickael Istria from comment #28) > > > > What would be the best way to test it in real life? > > > > I think once Tycho includes a p2 version with the provided patch, we can > > > > remove all the blocks such as > > > > http://git.eclipse.org/c/platform/eclipse.platform.ui.git/tree/tests/pom. > > > > xml#n74 that should be automatically resolved with those changes; shouldn't > > > > they? > > > > Note that it would still require the most dynamic bundles to properly define > > > > capabilities (bug 424104). Those should all be covered, but it's probably > > > > that some were missed or forgotten and that this patches unveil some missing > > > > declarations inside Platform. > > > > > > Hi Mickael, > > > > > > this is really cool news. The workaround you mention is the one to be done > > > only in the pom.xml. Another workaround solution that works since Oxygen is > > > the usage of p2 capabilities. For this I added the p2.inf files in a lot of > > > Equinox bundles to be able to specify a capability without the need to > > > modify the pom.xml. IIRC I even modified some of the platform bundles for > > > this to avoid pom.xml configurations. > > > > > > Anyhow, the correct way is the usage of OSGi capabilities to avoid > > > additional configuration in any way. And yes I think you are right, you can > > > only test this when Tycho is ready to consume the patched p2. You could also > > > try to work with p2 repositories and enable "Include required software" in > > > the target definition to see if that works. > > > For example, create a p2 repository or use an existing one that contains a > > > bundle with a Require-Capability header for DS 1.3. The target platform > > > should point to the Oxygen repository, but not including the Felix SCR > > > bundle directly. When adding your bundle to the target definition, the newly > > > introduced p2 mechanism should identify and add the Felix SCR bundle to your > > > target platform as required bundle. > > > > > > Hope it is clear what I mean and it is possible to test it this way. At > > > least it should in theory. :) > > > > > > When this is working, the following tickets would also be nice to get > > > implemented: > > > Bug 513216 - the Require-Capability header for DS 1.3 should be created > > > automatically > > > Bug 490063 - automatically add the Provide-Capability header for the > > > included services > > > > > > And maybe we also need to think about some additional support to specify > > > capabilities via editor. > > > > > > Dirk > > > > Be aware that there are no guarantees as to how the > > capabilities/requirements added via p2.inf interact with those extracted > > from the Provide/Require-Capability headers. > > > > E.g. for "osgi.service" capabilities it is not possible to publish the > > metadata from Provide-Capability as a p2 (namespace, name, version) tuple. > > As a result the name is a unique synthetic string and the version may be > > "0.0.0". Such a capability can be matched by a requirement extracted from > > the Require-Capability header, but not from p2.inf. The p2.inf format is not > > rich enough to allow you to declare a requirement with an LDAP filter (this > > should be a different bug). > > > > I think the capabilities/requirements added so far to the set of equinox > > bundles via p2.inf work independently from Provide/Require-Capability. I.e. > > you can use either to provision. However it will be best to stop using > > p2.inf to reflect Require/Provide-Capability since this can cause conflicts > > in the future. > > Once it works I will be happily remove the p2.inf stuff from the equinox > bundles again Removing the p2.inf capabilities is a breaking change. After all they were added specifically for third parties to require them. The p2.inf requirements can be removed.
A note about it should be added to the N&N ( https://git.eclipse.org/r/#/admin/projects/www.eclipse.org/eclipse/news ) as it can impact -positively- many RCP developers.
New Gerrit change created: https://git.eclipse.org/r/110671
Gerrit change https://git.eclipse.org/r/110671 was merged to [master]. Commit: http://git.eclipse.org/c/www.eclipse.org/eclipse/news.git/commit/?id=54c70d9eb5766fecd47c381ced8336d49ecd8575
In this commit: https://git.eclipse.org/c/equinox/rt.equinox.p2.git/commit/bundles/org.eclipse.equinox.p2.publisher.eclipse/src/org/eclipse/equinox/p2/publisher/eclipse/BundlesAction.java?id=aa4390e03ef4b5f6676c9e2991779df068b38f2b the addGenericRequirements method was added. It uses MetadataFactory.createRequirement(matchExpr, null, 0, 1, true) which does new RequiredCapability(requirement, filter, minCard, maxCard, greedy, null) but the match expression is one without parameters (at least sometimes) so that methods such as these will fail: @Override public String getName() { return (String) matchExpression.getParameters()[0]; } @Override public String getNamespace() { return (String) matchExpression.getParameters()[1]; } Clients cannot expect something that implements IRequiredCapability to throw exceptions when any of those methods are called. But that's exactly what can and will happen: https://bugs.eclipse.org/bugs/show_bug.cgi?id=528266 It seems that the generic requirement for the BREE is being handled: Require-Capability: osgi.ee; filter="(&(osgi.ee=JavaSE)(version=1.8))" with these as the ns, ldap, and matcher: osgi.ee (&(osgi.ee=JavaSE)(version=1.8)) providedCapabilities.exists(pc | pc.namespace == 'osgi.ee' && pc.attributes ~= filter('(&(osgi.ee=JavaSE)(version=1.8))')) Should I open a separate bug or will this be addressed in this bug? I really hope this doesn't end up in Photon M4 leaving Oomph Targlets broken for the next 6 weeks.
I can see that in this commit: https://git.eclipse.org/c/equinox/rt.equinox.p2.git/commit/bundles/org.eclipse.equinox.p2.publisher/src/org/eclipse/equinox/p2/publisher/AbstractPublisherAction.java?id=aa4390e03ef4b5f6676c9e2991779df068b38f2b that it appears you have have already run into this problem yourselves and added the method toRequiredCapability to detected instances of ICapabilityRequirement that will throw exceptions when getNamespace() getName() are called on them. But other places like org.eclipse.equinox.internal.p2.ui.query.CategoryElementWrapper.shouldWrap(Object) and org.eclipse.equinox.internal.p2.metadata.RequirementChange.matches(IRequiredCapability) don't look safe anymore. Surely the current approach can't be a proper design. Either these methods must return the correct sensible results or the instance should be just an IRequirement but isn't an IRequiredCapability. For this BREE generic requirement one would expect getNamespace() to return osgi.ee, getName() to return JavaSE with a getRange() that returns a range with an inclusive lower bound of 1.8 and no upper bound.
The RequiredCapability implementation already had support to act both as IRequiredCapability and only as an IRequirement - thus latent bugs like this were there prior to this bug. I decided not to change it because I couldn't be sure if this won't break something else in the convoluted p2 codebase let alone in other projects. Particularly it seems common to cast IRequirement straight to RequiredCapability without instanceof checks. Seems everyone just assume there is only one implementation for both IRequirement and IRequiredCapability - RequiredCapability. So I had to make a choice to either break all sites that make the erroneous assumption or workaround the issue on a case by case basis. I fixed the one case where p2 tests failed. In light of all this it may be better to open a new bug to fix RequiredCapability. E.g. extract the IRequirement support into a base class and use that base class where appropriate in the p2 codebase. Then hope this will not cause further bugs.
Yes, I imagine that some clients don't do an instanceof test first, but we always do so that would have helped. It's impossible to know if implementing an IRequirement that isn't an IRequiredCapability will be better or worse in general. Certainly for us it would be better, but as you suggest, who knows what bad assumptions lurk out there (or more likely internally in the p2 code). Though arguably anyone downcasting without checking should expect that this might become broken (which is defensible in the p2 code because the implementation is controlled there). Definitely anyone casting with checking should be able to assume that the methods don't throw exceptions and don't return null. So another possible alternative is to add guards and to return an empty string or empty version range. All that is to say, I'm not really sure it's best to extract a Requirement base class. In fact for the case of this BREE I think it should be properly represented as a IRequiredCapability with proper namespace, name, and version range. After all, there really do exist such capabilities to which they must resolve. But I don't understand the other cases where generic requirements are processed. It strike me though that the API does have getType(), getName(), and getVersionRange() so one would kind of expect those to be properly populated and properly used during conversion. That would of course be the best solution... Will these new structures actually end up in p2 update sites? Because if so it's highly likely to break updates (p2 itself) in older environment isn't it?
I will start researching a fix for this issue as soon as possible.
I'm more than happy to help with this effort and to review any of your proposed changes. What I would really appreciate is an example of what other types of generic requirements might be processed so I can better understand the full scope of the problem. As I said, I think the BREE should be properly converted to an IRequiredCapability and it looks to me like all that information is known in org.eclipse.osgi.internal.resolver.StateBuilder.convertBREEs(String[], List<GenericSpecification>) such that org.eclipse.equinox.p2.publisher.eclipse.BundlesAction.addGenericRequirement(List<IRequirement>, GenericSpecification, ManifestElement[]) could use the getName(), getType() and getVersionRange() of the arguments. But I don't understand what all might be in the org.eclipse.osgi.service.resolver.GenericSpecification.getMatchingFilter() for other cases such as the following: org.eclipse.osgi.internal.resolver.StateBuilder.createEquinoxRequires(ManifestElement[]) org.eclipse.osgi.internal.resolver.StateObjectFactoryImpl.createGenericRequires(GenericSpecification[]) org.eclipse.osgi.internal.resolver.StateObjectFactoryImpl.createGenericSpecification(String, String, String, boolean, boolean) org.eclipse.osgi.internal.resolver.StateBuilder.createOSGiRequires(ManifestElement[], List<GenericSpecification>)org.eclipse.osgi.internal.resolver.StateReader.readGenericSpecification(DataInputStream) This last method looks like it would never know the name nor the version range of what it reads back in... Are there one or more sample projects that would contain examples of all these various cases? I'd really really appreciate such an example! In the meantime I will add guards to the Oomph code for all places where we process an IRequiredCapability so that we don't end up in a broken state for Photon M4. To summarize, I think an optimal solution would properly support the IRequiredCapability to the best extent possible for all the cases where a namespace, name, and version range are properly/sensibly known. After all, a GenericSpecification is a VersionConstraint, so the API for the source of the information suggests that this information ought to be present... And to repeat myself, I'm more than happy to help!
(In reply to Ed Merks from comment #41) > In this commit: > > https://git.eclipse.org/c/equinox/rt.equinox.p2.git/commit/bundles/org. > eclipse.equinox.p2.publisher.eclipse/src/org/eclipse/equinox/p2/publisher/ > eclipse/BundlesAction.java?id=aa4390e03ef4b5f6676c9e2991779df068b38f2b > > the addGenericRequirements method was added. > > It uses MetadataFactory.createRequirement(matchExpr, null, 0, 1, true) which > does new RequiredCapability(requirement, filter, minCard, maxCard, greedy, > null) but the match expression is one without parameters (at least > sometimes) so that methods such as these will fail: > > @Override > public String getName() { > return (String) matchExpression.getParameters()[0]; > } > > @Override > public String getNamespace() { > return (String) matchExpression.getParameters()[1]; > } > > Clients cannot expect something that implements IRequiredCapability to throw > exceptions when any of those methods are called. But that's exactly what can > and will happen: > > https://bugs.eclipse.org/bugs/show_bug.cgi?id=528266 > > It seems that the generic requirement for the BREE is being handled: > > Require-Capability: osgi.ee; filter="(&(osgi.ee=JavaSE)(version=1.8))" > > with these as the ns, ldap, and matcher: > osgi.ee > (&(osgi.ee=JavaSE)(version=1.8)) > providedCapabilities.exists(pc | pc.namespace == 'osgi.ee' && pc.attributes > ~= filter('(&(osgi.ee=JavaSE)(version=1.8))')) > > Should I open a separate bug or will this be addressed in this bug? > > I really hope this doesn't end up in Photon M4 leaving Oomph Targlets broken > for the next 6 weeks. Not only Oomph Targlets, also the CBI aggregator fails on this new meta data, see bug 528266. Outch.
Note that I'm quite paranoid that this data will end up in p2 repositories, ones that have a long timetime, at which point we will never be able to get rid of this problematic data breaking existing applications. I have asked already "Will these new structures actually end up in p2 update sites?" but I've not seen the answer. Please, please let the answer be no, please please delete repositories that contain such data, and please for goodness sake don't produce any! This brings to mind the bad version of httpcomponents that ended up in the release train repos and that took months to get rid of. Let's anticipate that this p2 publishing problem too is likely to be a serious problem that would should nip it in the bud.
(In reply to Ed Merks from comment #44) > So another possible alternative is > to add guards and to return an empty string or empty version range. That would seem like an easy thing to do without much risk. @Ed: Do you think it would fix everything for the current version? At least remove the blockers and possible hard failures in p2, while discussion can go on about better implementation? > Will these new structures actually end up in p2 update sites? They already do: looking at the the metadata for org.eclipse.e4.core.services in current Platform repository we can see that MANIFEST.MF's: Require-Capability: osgi.extender; filter:="(&(osgi.extender=osgi.component)(version>=1.2)(!(version>=2.0)))" is exposed as <required namespace='osgi.extender' name='osgi.component' range='[1.2.0,2.0.0)'/> I didn't find an other instance of Require-Capability in Platform code (but I did only look at usual suspects - Equinox, Runtime - so could have very easily missed something); except one: org.apache.ant. The org.apache.ant does specify a Require-Capability for Java 8 (using osgi.ee and so on), but since this bundle wasn't modified for a while, its metadata were reused and not rebuilt, so the requirement doesn't appear in the p2 repo; but it may appear as soon as something modifies the bundle. > Because if so > it's highly likely to break updates (p2 itself) in older environment isn't > it? That's one good question: @Todor: before your patches, what would be the behaviour with those new requirements? Would they be ignored, or would older p2 attempt to parse them, and fail because of the bug in getName/getNamespace ?
The p2 metadata repository parser ignores elements which it doesn't understand. I have only extended the metadata format for capabilities. Now they contain a map of attributes additional to (namespace; name; version). The parser will ignore these new elements and end up with a standard capability The new requirements (which cause the issues) were always supported, but no one used them up until now. I deliberately write them as optional+greedy so that the provisioning will not fail if there is no matching capability - which there won't be when the capability attributes are not parsed. The result is that old p2 runtimes will load new p2 metadata and will provision as if there are no generic requirements/capabilities. Additionally: The generic requirements/capabilities model is at the heart of the OSGi framework since OSGi 5. The public OSGi framework API expresses the runtime model of the bundles is *only* generic requirements/capabilities. In fact they are not called "generic" - this is a legacy term. They are simply requirements and capabilities. All OSGi service specifications now define requirements and capabilities to express non-class dependencies like a dependency to an OSGi service or an OSGi extender (e.g. the Declarative Services runtime) or any number of other things. P2 has fallen behind this trend to the point that it becomes very hard to provision sets of bundles designed around these modern trends. Just one of the issues is that the OSGi BREE header is deprecated. All modern bundle builds tools express the BREE requirements in generic format in the "osgi.ee" namespace. When such bundles are loaded into an OSGi runtime they fail. P2 needs to catch this at provisioning time.
I opened Bug 528408 to address the RequiredCapability issue since this issue did not cause it. It simply exposed it by using the current p2 support to express requirements in it's full capacity.
Thanks for the update. So I feel much better to know that older runtimes will ignore this newer information when they read it!
Thanks for your answers Todor. I've downloaded the Platform (just Platform, not SDK so no JDT, no PDE...) from Oxygen.1a, and tried ./eclipse -consoleLog -application org.eclipse.equinox.p2.director -repository http://download.eclipse.org/eclipse/updates/4.8milestones/S-4.8M4-201712062000/ -uninstallIU org.eclipse.platform.ide -installIU org.eclipse.platform.ide to update to 4.8.M4. It basically uses an old p2 director (pre-change) to parse some post-change metadata for Required-Capability (bundle org.eclipse.e4.core.services in the one I use as example). p2 metadata are successfully read (or not, but at least nothing fails) by the older p2 and upgrade works as expected. So, unless someone has an example which does make the upgrade story fail here from a released version, I think we can consider that the concerns raised are "internal" to 4.8.M4 and are better being tracked on bug 528408.
(In reply to Mickael Istria from comment #53) > I've downloaded the Platform (just Platform, not SDK so no JDT, no PDE...) > from Oxygen.1a, and tried > ./eclipse -consoleLog -application org.eclipse.equinox.p2.director -repository > http://download.eclipse.org/eclipse/updates/4.8milestones/S-4.8M4-201712062000/ > -uninstallIU org.eclipse.platform.ide -installIU org.eclipse.platform.ide > > to update to 4.8.M4. ... As far as I can see, the M4 repository was still created by the Oxygen.0 version of p2 (from glancing over how tycho itself is built). I'm Cc:ing tycho-inbox to request confirmation that tycho 1.1.0-SNAPSHOT still uses p2 Oxygen.0 under the hood, right? If that is true we may just have to ask tycho to never link against M3 or M4 version of p2 - to protect the majority of projects from producing meta data that cannot yet be safely consumed.
(In reply to Stephan Herrmann from comment #54) > I'm Cc:ing tycho-inbox to request confirmation that tycho 1.1.0-SNAPSHOT > still uses p2 Oxygen.0 under the hood, right? Tycho is still using p2 from Oxygen.0 https://github.com/eclipse/tycho/blob/e5380daafa1200338017f1ba0aa1bb1427b39240/tycho-bundles/tycho-bundles-target/tycho-bundles-target.target#L15 which bug should we be watching so we know when it's safe to upgrade p2 to a Photon milestone?
(In reply to Jan Sievers from comment #55) > (In reply to Stephan Herrmann from comment #54) > > > I'm Cc:ing tycho-inbox to request confirmation that tycho 1.1.0-SNAPSHOT > > still uses p2 Oxygen.0 under the hood, right? > > Tycho is still using p2 from Oxygen.0 > > https://github.com/eclipse/tycho/blob/ > e5380daafa1200338017f1ba0aa1bb1427b39240/tycho-bundles/tycho-bundles-target/ > tycho-bundles-target.target#L15 > > which bug should we be watching so we know when it's safe to upgrade p2 to a > Photon milestone? Jan, bug 528408 should be the one.
New Gerrit change created: https://git.eclipse.org/r/142177
Gerrit change https://git.eclipse.org/r/142177 was merged to [master]. Commit: http://git.eclipse.org/c/ecf/org.eclipse.ecf.git/commit/?id=d247d372af4caef07f09ec25cdb4a197b3633884
