|
Description
Thomas Watson
New Gerrit change created: https://git.eclipse.org/r/90289 New Gerrit change created: https://git.eclipse.org/r/90288 I opened felix issue https://issues.apache.org/jira/browse/FELIX-5514 to track getting this fixed at apache. Meanwhile I have a fix I will release to Equinox shortly (see gerrit reviews) Gerrit change https://git.eclipse.org/r/90289 was merged to [master]. Commit: http://git.eclipse.org/c/equinox/rt.equinox.framework.git/commit/?id=fcab3e35945c3d10db5b22838c1230c67afdaee2 Gerrit change https://git.eclipse.org/r/90288 was merged to [master]. Commit: http://git.eclipse.org/c/equinox/rt.equinox.framework.git/commit/?id=f9777863a046cb6516c5846f2d592ff6aa25fb15 I have submitted the fixes to Equinox. New Gerrit change created: https://git.eclipse.org/r/90635 New Gerrit change created: https://git.eclipse.org/r/90634 This problem is severe and should be considered for Neon.3. Gerrit change https://git.eclipse.org/r/90635 was merged to [R4_6_maintenance]. Commit: http://git.eclipse.org/c/equinox/rt.equinox.framework.git/commit/?id=c8d441fca3a1343225d40663ccbc8b77807dcfda Gerrit change https://git.eclipse.org/r/90634 was merged to [R4_6_maintenance]. Commit: http://git.eclipse.org/c/equinox/rt.equinox.framework.git/commit/?id=8e71dd54cca09480967bcc5f7984d2c49ffe40d1 (In reply to Thomas Watson from comment #9) > This problem is severe and should be considered for Neon.3. +1 Done for Neon.3. org.eclipse.osgi.container.ModuleWiring#getSubstitutionWires() is a new API method that gave API Tools errors in R4_6_maintenance (bad @since 3.12, wants to increase minor version from 3.11.3). The API addition looks OK, so I've added the necessary filters: http://git.eclipse.org/c/equinox/rt.equinox.framework.git/commit/?id=4fa9c28f322c3691a43ac71663a735b3a2790e85 I guess Dani will have to add an exclusion rule for the API change report. (In reply to Markus Keller from comment #14) > org.eclipse.osgi.container.ModuleWiring#getSubstitutionWires() is a new API > method that gave API Tools errors in R4_6_maintenance (bad @since 3.12, > wants to increase minor version from 3.11.3). > > The API addition looks OK, so I've added the necessary filters: > http://git.eclipse.org/c/equinox/rt.equinox.framework.git/commit/?id=4fa9c28f322c3691a43ac71663a735b3a2790e85 > > > I guess Dani will have to add an exclusion rule for the API change report. Looks like we're not producing the freeze report for M-builds for quite some time. I've created bug 511963 as a reminder to do it for the Oxygen M-builds. (In reply to Markus Keller from comment #14) > org.eclipse.osgi.container.ModuleWiring#getSubstitutionWires() is a new API > method that gave API Tools errors in R4_6_maintenance (bad @since 3.12, > wants to increase minor version from 3.11.3). > > The API addition looks OK, so I've added the necessary filters: > http://git.eclipse.org/c/equinox/rt.equinox.framework.git/commit/ > ?id=4fa9c28f322c3691a43ac71663a735b3a2790e85 > > I guess Dani will have to add an exclusion rule for the API change report. I better get some javadoc in there. One thing that concerns me. Here we need to have ModuleWiring (considered API) implement FelixWiring (considered internal) so we can implement the getSubstitutionWires to improve performance of the resolver since the wiring implementation already has to keep track of this knowledge. Is it a problem that our API implements a internal type, one that will hopefully go away in the future? In the future (OSGi R7) I plan to propose the method getSubstitutionWires as a method on Wiring and then I would get rid of the FelixWiring interface altogether. New Gerrit change created: https://git.eclipse.org/r/90743 Gerrit change https://git.eclipse.org/r/90743 was merged to [master]. Commit: http://git.eclipse.org/c/equinox/rt.equinox.framework.git/commit/?id=1bc36f585d4c1e6e329b0d584c9a649f0330416a (In reply to Thomas Watson from comment #16) > One thing that concerns me. Here we need to have ModuleWiring (considered > API) implement FelixWiring (considered internal) so we can implement the > getSubstitutionWires to improve performance of the resolver since the wiring > implementation already has to keep track of this knowledge. > > Is it a problem that our API implements a internal type, one that will > hopefully go away in the future? In the future (OSGi R7) I plan to propose > the method getSubstitutionWires as a method on Wiring and then I would get > rid of the FelixWiring interface altogether. In general it is a considered an API leak and API Tools reports this. As long as you can make sure this does not leak to the clients by adapting ModuleWiring to changes, it is fine, since clients are not allowed to reference FelixWiring. (In reply to Dani Megert from comment #19) > (In reply to Thomas Watson from comment #16) > > One thing that concerns me. Here we need to have ModuleWiring (considered > > API) implement FelixWiring (considered internal) so we can implement the > > getSubstitutionWires to improve performance of the resolver since the wiring > > implementation already has to keep track of this knowledge. > > > > Is it a problem that our API implements a internal type, one that will > > hopefully go away in the future? In the future (OSGi R7) I plan to propose > > the method getSubstitutionWires as a method on Wiring and then I would get > > rid of the FelixWiring interface altogether. > > In general it is a considered an API leak and API Tools reports this. > > As long as you can make sure this does not leak to the clients by adapting > ModuleWiring to changes, it is fine, since clients are not allowed to > reference FelixWiring. I was thinking about how I am going to propose this back to OSGi for R7 and I think I am going to take a different approach. Instead of implementing this on the ModuleWiring/FelixWiring interface I'm going to add a method to the FelixResolveContext interface. One issue is that I don't think we can add a new method to the Wiring interface in OSGi because it is a consumer type. Instead I would add a method to the ResolveContext for OSGi R7 since that is an abstract class. This will keep all of this solution out of the API altogether and allow us to internally give the information to the resolver. (In reply to Thomas Watson from comment #16) > I better get some javadoc in there. API Javadocs for ModuleWiring are currently not generated; I filed bug 511984 for that. If you're pretty sure that "Collection<Wire> getSubstitutionWires()" will be the API that eventually gets added to BundleWiring, or if you think it's necessary to allow other clients to refer to this method already now, then some Javadoc would be good. Otherwise, you can also mark ModuleWiring#getSubstitutionWires() as @noreference for now. New Gerrit change created: https://git.eclipse.org/r/90761 Gerrit change https://git.eclipse.org/r/90761 was merged to [master]. Commit: http://git.eclipse.org/c/equinox/rt.equinox.framework.git/commit/?id=71394a2cbca386ea5b4a2acc3d807be51336baca New Gerrit change created: https://git.eclipse.org/r/90766 Gerrit change https://git.eclipse.org/r/90766 was merged to [R4_6_maintenance]. Commit: http://git.eclipse.org/c/equinox/rt.equinox.framework.git/commit/?id=f5262c4a0110dcc27aeb553e720e07e67a7e6ae2 (In reply to Markus Keller from comment #14) > org.eclipse.osgi.container.ModuleWiring#getSubstitutionWires() is a new API > method that gave API Tools errors in R4_6_maintenance (bad @since 3.12, > wants to increase minor version from 3.11.3). > > The API addition looks OK, so I've added the necessary filters: > http://git.eclipse.org/c/equinox/rt.equinox.framework.git/commit/ > ?id=4fa9c28f322c3691a43ac71663a735b3a2790e85 > > I guess Dani will have to add an exclusion rule for the API change report. I reverted the filters and I released a solution that involves no added API. |