|
Description
Ed Merks
The getFeaturePaths was re-implemented so that we could break a dependency on update.configurator for PDE's use. However, there is an implementation of P2 that provides the IBundleGroupProvider in the update.configurator bundle which hasn't been back-ported yet, so provided that the installation still has that bundle in it then services that depend on it should still work as expected. This is probably the specific change that introduced the delta in behavour: https://git.eclipse.org/r/c/pde/eclipse.pde.ui/+/164035/12/ui/org.eclipse.pde.core/src/org/eclipse/pde/internal/core/target/ProfileBundleContainer.java The comment in the deleted code suggests the plan was to drop support for platform.xml since that was only used by the old Eclipse update manager, rather than P2. Regarding the package installed; do the features show up in the list of the 'about' section of the running Eclipse (i.e. outside of PDE)? If they're not showing there, does the update.configurator bundle exist in the runtime and started? There's a small possibility that if the list of features in the parent Eclipse is empty then they won't be propagated to the running instance, in which case the fix might be to ensure that the parent instance has that bundle installed and running. (PDE brought it in as a dependency before; it may be that having removed that dependency, it's no longer included and in which case the list of features might not be showing elsewhere.) If the update.configurator bundle is present, and the list of features aren't shown, then it may be due to the change above which stopped scanning the platform.xml file for a list of features but only looked in the 'features/' directory instead. Unfortunately if that doesn't work then we might need to revert a subset of the changes for bug 527378: 386e461afa5eefb733cb0a2ac864dc230b0fe05b 11a09692b07fd57ba1e9e85f225edff215eb5a0f a113ae7bf4dc063e902ba7a1e700e80d8f40570b Unfortunately these steps will revert the performance benefits as they'll pull back in the dependencies to update.configurator but at this point in the release lifecycle I'm not sure if there is a more appropriate way of resolving the issues. (In reply to Alex Blewitt from comment #2) > Regarding the package installed; do the features show up in the list of the > 'about' section of the running Eclipse (i.e. outside of PDE)? Yes. And that was answered in the forum thread. The profile definitely knows about the features. > If they're not > showing there, does the update.configurator bundle exist in the runtime and > started? I'm not sure this is a relevant question. But yes, it's present. > There's a small possibility that if the list of features in the > parent Eclipse is empty then they won't be propagated to the running > instance, in which case the fix might be to ensure that the parent instance > has that bundle installed and running. It's present in the profile. > (PDE brought it in as a dependency > before; it may be that having removed that dependency, it's no longer > included and in which case the list of features might not be showing > elsewhere.) > > If the update.configurator bundle is present, and the list of features > aren't shown, then it may be due to the change above which stopped scanning > the platform.xml file for a list of features but only looked in the > 'features/' directory instead. > Yes, that's exactly which the debugger tells me. > Unfortunately if that doesn't work then we might need to revert a subset of > the changes for bug 527378: > It doesn't work in general because the bundle pool can be shared instead of collocated. > 386e461afa5eefb733cb0a2ac864dc230b0fe05b > 11a09692b07fd57ba1e9e85f225edff215eb5a0f > a113ae7bf4dc063e902ba7a1e700e80d8f40570b > > Unfortunately these steps will revert the performance benefits as they'll > pull back in the dependencies to update.configurator but at this point in > the release lifecycle I'm not sure if there is a more appropriate way of > resolving the issues. It seems risky to try to change it back now. I'm not sure the best alternative going forward. Certainly the p2 profile knows exactly what's installed and I think it knows where those bundles and features are physically located... This can only looks for features, not bundles, so should be capable of resolving bundles in a shared bundle pool. It’s just that it won’t find expanded features this way, which in turn may not include those bundles from the pool. What kind of platform.xml does Oomph write out? Is it just a list of features that don’t happen to live in the platform itself? At the end of the day the method listed in my first comment returns a list of features which could be re-implemented in an alternate way to restore the behaviour before. (In reply to Alex Blewitt from comment #4) > This can only looks for features, not bundles, so should be capable of > resolving bundles in a shared bundle pool. It’s just that it won’t find > expanded features this way, which in turn may not include those bundles from > the pool. > Yes, the bundles are in the TP, but the features are not. I'm not sure if any bundles are missing... > What kind of platform.xml does Oomph write out? In both cases, the features are present in the platform.xml. But with the new logic, nothing looks in that file. > Is it just a list of > features that don’t happen to live in the platform itself? No, generally the platform.xml should be all features, but with a shared bundle pools, all features in in the shared pool. E.g., it looks like this: <?xml version="1.0" encoding="UTF-8"?> <config date="1599028765140" transient="false"> <site updateable="true" url="file:/D:/Users/merks/.p2/pool/" enabled="true" policy="MANAGED-ONLY"> <feature id="org.eclipse.emf.codegen.ecore.source" version="2.23.0.v20200701-0840" url="features/org.eclipse.emf.codegen.ecore.source_2.23.0.v20200701-0840/"> </feature> <feature id="org.eclipse.emf.codegen.ecore.ui.source" version="2.23.0.v20200703-0737" url="features/org.eclipse.emf.codegen.ecore.ui.source_2.23.0.v20200703-0737/"> </feature> > At the end of the > day the method listed in my first comment returns a list of features which > could be re-implemented in an alternate way to restore the behaviour before. Yes, somehow the features need to be computed without assuming they are located in the feature folder of the installation (collocated). I expect/suspect that the p2 profile could be used more directly, but I'd need to investigate if the artifact locations are known in that manner. I expect that is the case because Oomph's bundle pool analyzer knows how to find all the artifact from the profiles... Is the platform.xml file always at configuration/org.eclipse.update/platform.xml? I could have a go at using that is present in that location to parse the file to compute the list of features. What’s the best way of getting an environment installed to test this? (In reply to Alex Blewitt from comment #6) > Is the platform.xml file always at > configuration/org.eclipse.update/platform.xml? > > I could have a go at using that is present in that location to parse the > file to compute the list of features. > > What’s the best way of getting an environment installed to test this? Yes it's there in the packaged downloads and in the installations created by the installer. Any environment created by the installer based on the 2020-09/4.17 version of Eclipse will illustrate the problem. The installer ends up calling the following to fix the problems in that file in the case of a shared bundle pool: https://git.eclipse.org/c/oomph/org.eclipse.oomph.git/tree/plugins/org.eclipse.oomph.p2.core/src/org/eclipse/oomph/p2/internal/core/AgentImpl.java#n1130 This code also illustrates how the platform.xml can be (and is) loaded via org.eclipse.equinox.internal.p2.update.Configuration.load(File, URL). For a packaged download the platform.xml looks like this: <?xml version="1.0" encoding="UTF-8"?> <config date="1598532645972" transient="false"> <site updateable="true" url="platform:/base/" enabled="true" policy="USER-EXCLUDE"> <feature id="org.eclipse.equinox.p2.discovery.feature" version="1.2.700.v20200705-1016" url="features/org.eclipse.equinox.p2.discovery.feature_1.2.700.v20200705-1016/"> </feature> Note in particular the url="platform:/base/" which must be resolved to a file: URL to find the actual files referenced by the features' relative URLs, i.e., url="features/*" For an installation produced by the installer with a shared bundle pool, the platform.xml looks like this: <?xml version="1.0" encoding="UTF-8"?> <config date="1599028765140" transient="false"> <site updateable="true" url="file:/D:/Users/merks/.p2/pool/" enabled="true" policy="MANAGED-ONLY"> <feature id="org.eclipse.emf.codegen.ecore.source" version="2.23.0.v20200701-0840" url="features/org.eclipse.emf.codegen.ecore.source_2.23.0.v20200701-0840/"> </feature> Note that fixing the site url to be a proper absolute file URI with an absolute path (rather than relative) is what org.eclipse.oomph.p2.internal.core.AgentImpl.adjustPlatformXML(String, File) does. Also the policy is changed to MANAGED-ONLY so that only the explicitly listed features are included in the installation, not all features in the shared pool. I imagine this will be tricky to debug/test changes. What I did for that purpose is add this to the eclipse.ini of the Oomph installed version of 2020-09: -Xdebug -Xrunjdwp:transport=dt_socket,server=y,suspend=y,address=8123 And the remote debugged it from my Eclipse SDK IDE. That works well if the source is in sync with what version of PDE/p2 is actually installed in that installation... That being said, if you ensure that the platform.xml is properly processed in terms of how the url of the site(s) and the features are resolved, the same logic should work well for collocated and shared bundle pools. (In reply to Ed Merks from comment #7) > Also the policy is changed to MANAGED-ONLY so that only the > explicitly listed features are included in the installation, not all > features in the shared pool. I'm trying to figure out where that aspect was handled previously. AFAICS PDE simply consumed the IPlatformConfiguration: https://github.com/eclipse/eclipse.pde.ui/blob/38ca2f903f105ac23b0d576c3d8fb3749fbc4ea7/ui/org.eclipse.pde.core/src/org/eclipse/pde/internal/core/PluginPathFinder.java#L242 In update.configurator itself, I can't find anything that handles the site policy for features (only for plugins): https://github.com/eclipse/eclipse.platform/blob/d447133afceb72ed54eac0a0cdb5dfd6aa420544/update/org.eclipse.update.configurator/src/org/eclipse/update/internal/configurator/SiteEntry.java#L126 I guess for PDE the only feasible solution is to copy the required code from update.configurator to parse that file. Relying on IBundleGroupProvider to get the features is not an option because it a) is just a masked dependency on update.configurator b) only works for the ${eclipse_home} installation, not external ones New Gerrit change created: https://git.eclipse.org/r/c/pde/eclipse.pde.ui/+/170146 New Gerrit change created: https://git.eclipse.org/r/c/pde/eclipse.pde.ui/+/170148 New Gerrit change created: https://git.eclipse.org/r/c/pde/eclipse.pde.ui/+/170149 New Gerrit change created: https://git.eclipse.org/r/c/pde/eclipse.pde.ui/+/170147 Gerrit change https://git.eclipse.org/r/c/pde/eclipse.pde.ui/+/170146 was merged to [master]. Commit: http://git.eclipse.org/c/pde/eclipse.pde.ui.git/commit/?id=d4a49972c4610da37f102c8755a54e88d73f86b5 Gerrit change https://git.eclipse.org/r/c/pde/eclipse.pde.ui/+/170148 was merged to [master]. Commit: http://git.eclipse.org/c/pde/eclipse.pde.ui.git/commit/?id=87c008e3edd6715b9d4a4320417875042a917446 I can try to test this when it's an an IBuild... New Gerrit change created: https://git.eclipse.org/r/c/pde/eclipse.pde.ui/+/170232 Gerrit change https://git.eclipse.org/r/c/pde/eclipse.pde.ui/+/170232 was merged to [master]. Commit: http://git.eclipse.org/c/pde/eclipse.pde.ui.git/commit/?id=f03516f8e776c403293a9c6f1a923d2deaa60c14 New Gerrit change created: https://git.eclipse.org/r/c/pde/eclipse.pde.ui/+/170238 Gerrit change https://git.eclipse.org/r/c/pde/eclipse.pde.ui/+/170238 was merged to [master]. Commit: http://git.eclipse.org/c/pde/eclipse.pde.ui.git/commit/?id=23239f8df4dc8f6081f8abb3d0ca403999724bd8 Julian, can you please verify this fix. A target based on an external oomph-based installation finds the correct features with I20201006-1800. |