Community
Participate
Working Groups
Build Identifier: BundleInfo classes in framework admin and simple configurator define bundle hascode on top of bundle version and symbolic name only. It means that two bundles with the same name and version will be treated as equal no matter than they could have different locations. Fwk admin and simple configurator should be modified to align with OSGi spec and allow installation of bundles with different locations when org.osgi.framework.bsnversion property is set to multiple. The attached patches contain test cases validating the expected result and initial proposal of fixing the bundles. It's still inital because during the investigation some questions pop up and I'm not sure what is expected and whether the issue is in test or in the code. Basically the problem is that simple configurator doesn't recognize equinox and bundles listed in osgi.bundles property. If some of those are listed in bundles.info, there is no clear way to detect whether they are already installed or not which is an issue in multiple bsnversion scenario. BundlesTxtTest.testSimpleConfiguratorInstallsMultipleBSN illustrates that - starting equinox with: - simple configurator installed - org.osgi.framework.bsnversion=multiple - bundles.info containing equinox,sc and two other test bundles results in 6 installed bundles instead of 4 (equinox and sc are installed twice). So 4 or 6 is the expected result in that situation from your point of view? SC is responsible to identify the new stuff for install based on the bundles.info but since the differentiation between already installed and new bundles cannot be easily done do you think sc should keep some kind of mapping to deal with those special bundles (which are not installed by it)? This is a blocker for integrating p2 in Virgo and every help, advice or comment on the topic will be greatly appreciated. Reproducible: Always
Created attachment 198597 [details] Initial proposal of multiple bsnversion support not final
Created attachment 198598 [details] Test cases
I took a quick look at the code and here is some initial feedback. Keep in mind that I'm not super familiar with the spec so I'm a bit fuzzy about some details. - Don't change the BundleInfo hashcode to include the bundlelocation. Hashing the bundle location does not work because the URI for the same location on disk can be encoded differently depending on the input string, how it has been munged, etc. See that the equals method is not a simple string compare. For reference, bug #218046 was were some of this burnt us in the past. - In fwkadmin, checking the FRAMEWORK_BSNVERSION property against the system property will not lead to the desired behaviour if the installer does not have this property enabled (e.g. if I install using the director shipped with eclipse). Instead the property lookup should be done against the value contained in ConfigData. If making the assumption that this property is set is acceptable, then it could be fine for a while but probably not for a final deliverable. - It looks like the check in simpleconfigurator for isMultipleBSNVersionAllowed is incorrect since by default bsnVersionMode would be null and thus isMultipleBSNVersionAllowed would be null. - Should anything be done wrt uninstallBundle in both the simpleconfigurator and fwkadmin?
I see now that framework admin and simple configurator view the system as a flat framework and model it that way. frameworkadmin.equinox loads up a resolver State and seems to do some resolution of the state. This resolution will completely ignore any regions configured by virgo. I'm not sure how important this is or even if resolution done at the framework admin level is important to the overall provisioning operation. The real issue seems to be simple configurator which will install duplicate bundles that were installed from the EclipseStarter launcher. It would be good to be able to determine if the "initial" locations used to install these bundles were ones managed by simple configurator or not. I am not sure what the easiest way to do that is. Even if we can determine this situation we would have to make the simplifying assumption that any bundle installed this way (from EclipseStarter etc.) is intended to only have a single instance installed in the whole framework no matter what.
(In reply to comment #3) > - Should anything be done wrt uninstallBundle in both the simpleconfigurator > and fwkadmin? Don't think so - sc uninstalls everything which is not listed in bundles.info (the exclusive mode) so if bundle locations are reliable there shouldn't be a special handling.
(In reply to comment #4) It would be good > to be able to determine if the "initial" locations used to install these > bundles were ones managed by simple configurator or not. I am not sure what > the easiest way to do that is. Even if we can determine this situation we > would have to make the simplifying assumption that any bundle installed this > way (from EclipseStarter etc.) is intended to only have a single instance > installed in the whole framework no matter what. What if we go for the following approach: Bundles with initial locations shouldn't be presented in bundles.info. If they are, they will treated as requests for separate installation? It's a limitation since now sc manages to cope with bundles listed in both places (because of the bundle existence check). A drawback is that there won't be one place containing all the bundles - they would have to be gathered from bundles.info(s) and config.ini - this could cause confusion. Do you think the the handling of "initial" locations should be the same in both multiple and single bsn_version cases?
(In reply to comment #4) Tom, do you think there's a chance (and easy solution) equinox to provide info about osgi.bundles in terms of Bundle objects or list of IDs for example? That would let us get rid of error-prone location comparison at least for initial bundles? As for the other bundles if simple configurator and simpe configurator manipulator use the same base to map relative to absolute URLs and vice versa there should not be an issue.
(In reply to comment #7) > (In reply to comment #4) > Tom, do you think there's a chance (and easy solution) equinox to provide info > about osgi.bundles in terms of Bundle objects or list of IDs for example? That > would let us get rid of error-prone location comparison at least for initial > bundles? Today we use "initial@" location prefix to indicate the "initial bundles" managed by the osgi.bundles property. Are you looking for some other mechanism other than looking at the installed bundles location string and finding the prefix "initial@"? What do you have in mind? A property (accessed through BundleContext.getProperty) that lists the location strings? Not very nice since you would have to parse it out into a list of locations from a single string. Other than that we would have to either invent a new framework service or stick this information on an existing framework service. Both options are not very appealing to me. > As for the other bundles if simple configurator and simpe configurator > manipulator use the same base to map relative to absolute URLs and vice versa > there should not be an issue. I don't know the details here, but I think Pascal has mentioned in some gotchas the p2 team encountered trying to determine if two bundles are equal based on the location string and relative paths etc. Not sure if the same issues could arise here or not.
(In reply to comment #8) > Today we use "initial@" location prefix to indicate the "initial bundles" > managed by the osgi.bundles property. Are you looking for some other mechanism > other than looking at the installed bundles location string and finding the > prefix "initial@"? What do you have in mind? A property (accessed through > BundleContext.getProperty) that lists the location strings? Not very nice since > you would have to parse it out into a list of locations from a single string. > Other than that we would have to either invent a new framework service or stick > this information on an existing framework service. Both options are not very > appealing to me. I was thinking about a property to get a list of initally installed bundle IDs instead of location list. Don't want to go in "parse string" direction since there is nothing standard to rely on. PackageAdmin returns a list of bundles that should be compared with BundleInfo objects by sc. Currently the check is: bundle.getLocation().equals(bundleInfo.getLocation) which cause issues with initial bundles but it seems to work for others. The check could be enhaced with !osgiBundles.contain(bundleInfo)
Some comments on your concerns: (In reply to comment #3) > I took a quick look at the code and here is some initial feedback. Keep in mind > that I'm not super familiar with the spec so I'm a bit fuzzy about some > details. > > - Don't change the BundleInfo hashcode to include the bundlelocation. Hashing > the bundle location does not work because the URI for the same location on disk > can be encoded differently depending on the input string, how it has been > munged, etc. See that the equals method is not a simple string compare. For > reference, bug #218046 was were some of this burnt us in the past. We need to change BundleInfo hash code - otherwise adding bundle info to config data set overwrites previous entries with the same location. This is not desired when having multiple bsn version: private LinkedHashSet bundlesList = new LinkedHashSet(); ConfigData#addBundle(BundleInfo bundleInfo) { bundlesList.add(bundleInfo); } I checked the bug you referred to: As clarified in that discussion: http://dev.eclipse.org/mhonarc/lists/equinox-dev/msg06887.html these locations: [plugins\org.eclipse.pde.ui_3.4.0.v20080204-0800.jar] [file:plugins\org.eclipse.pde.ui_3.4.0.v20080204-0800.jar] are different so both should be presented in bndles.info Since locations are pure strings I guess these [plugins\org.eclipse.pde.ui_3.4.0.v20080204-0800.jar] [plugins/org.eclipse.pde.ui_3.4.0.v20080204-0800.jar] are different, too according to the spec. Also p2 tests pass so I assumed modified hash code calculation is not a breaking change. Are you aware of scenario where Eclipse update would break? It would be very helpful to illustrate Eclipse expectations. I saw there's URIUtil class responsible to check whether URIs are the same - probably we can change it to cover both the spec and Eclipse cases . > - In fwkadmin, checking the FRAMEWORK_BSNVERSION property against the system > property will not lead to the desired behaviour if the installer does not have > this property enabled (e.g. if I install using the director shipped with > eclipse). Instead the property lookup should be done against the value > contained in ConfigData. If making the assumption that this property is set is > acceptable, then it could be fine for a while but probably not for a final > deliverable. > > - It looks like the check in simpleconfigurator for isMultipleBSNVersionAllowed > is incorrect since by default bsnVersionMode would be null and thus > isMultipleBSNVersionAllowed would be null. I'll update the patch as soon as we agree on how this bug should be fixed.
There seem to be a couple of misapprehensions in comment 10. Firstly, bundle location is just a string and this must be unique across the whole framework even when ...bsnversion=multiple. Secondly, bundle location is just a string and need not be a URI. For example, in Virgo we have to make bundle locations unique by prepending a region identifier (something like "user-region:file:..."). So it seems wrong that BundleInfo's equals method is not a simple string compare as this will presumably barf if it is given a non-URI string. See the javadoc for BundleContext.installBundle which says that String location is typically in the form of a URL, but is actually just a string. The stream-based variant of the method is what Virgo uses to pass a non-URI location.
Here are the problems[P] and resolutions[R] related to this bug which were discussed during last equinox "office hours": [P]Ecipse should remain stable [R]The new behavior proposed here won't be activated by default in Eclipse. [R]Pascal expressed concerns that since framework admin code is very brittle even a small change could result in issues and asked for commitment on supporting every problematic scenarios (related to this change) that may appear. I commit on that. Currently known problematic areas are garbage collection and dropins usage. [P]Clashing bundle locations [R]No explicit need to replace context.installBundle(location) with context.installBundle(location, stream) [R]Paths in bundles.info should stay relative in order to allow moving the whole installation (Eclipse, Virgo, etc) trasparently. Since p2 is a single provisioner and assumes it controls the location strings of the bundles it installs, it's responsible for transforming OSGi String location into URI. [R]Hashing bundle locations may not be changed as soon as we change equals to be based on the region a bundle belongs to or actual bundle location (if the new behavior is enabled) [P]New behavior enablement [R]It would enabled if there's a non-null value set for the property "bsnversion" and this value is != "single". It's not enough to check for "multiple" because soon there would be another value "managed" that allows multiple installations of same bsnversion. [P]Retriving the value of "bsnversion" [R]Instead of System.getProperty(..), bundleContext.getProperty(..) should be used. This is necessary in case when p2 is embeded in a VM with other frameworks. [R]On top of the previous, bsnversion value should be passed to framework admin (set in ConfigData for example) [P]Installing Virgo with standard tools (e.g. Eclipse director) [R]Set a system property in p2 director arguments during Virgo installation would be enough. The drawback is that if it's forgotten, Virgo installation would be broken. This is not the perfect solution but now, since we're focused on releasing Virgo milestone with p2, we could live with the drawback and looking for a better solution on the way. Please look at the mm and check whether I've missed or misinterpret something.
Multiple bsn version support in general would reqire revising and changing location string handling in p2. Currently there are many p2 components dealing with it in non-unified way: (simple configurator, simple configurator manipulator, framework admin) and there's no concept how Equinox installed bundle locations are interpreted - e.g. initial@file://../org.eclipse.osgi_3.7.0.v20110524.jar The inital idea of this bug was to provide a prototype which handles only the Virgo case with its specific assumptions (different bundle pools for different regions, etc). This would have resulted in a clear vision what should be changed in general. Since "managed" will soon become the default value in Eclipse, it will be impossible to guarantee that only Virgo consumes the new logic (if a find hook is introduced for some reason, the new behavior will be activated). We consider change of that kind too risky for Eclipse since p2 is basic functionality and Virgo-related assumptions won't be valid in all the cases. The hook provider in Eclipse will have to take care of Eclipse internals (bundle info handling) in order to guarantee alignment with current p2 - which seems as no clear responsibility who does what. With the new direction of Virgo, multiple bsn version support is not critical anymore and the clean solution could be postponed until it becomes priority for the p2 community. If this happens, I would be happy to continue working on the topic.
This bug hasn't had any activity in quite some time. Maybe the problem got resolved, was a duplicate of something else, or became less pressing for some reason - or maybe it's still relevant but just hasn't been looked at yet. If you have further information on the current state of the bug, please add it. The information can be, for example, that the problem still occurs, that you still want the feature, that more information is needed, or that the bug is (for whatever reason) no longer relevant. -- The automated Eclipse Genie.
This bug was marked as stalebug a while ago. Marking as worksforme. If this report is still relevant for the current release, please reopen and remove the stalebug whiteboard tag.
