Bugzilla – Full Text Bug Listing
| Summary: | [target] Default target platform doesn't list all installed bundles | ||
|---|---|---|---|
| Product: | [Eclipse Project] PDE | Reporter: | Mickael Istria <mistria> |
| Component: | UI | Assignee: | Mickael Istria <mistria> |
| Status: | VERIFIED FIXED | QA Contact: | |
| Severity: | normal | ||
| Priority: | P3 | CC: | akurtakov, alexander.fedorov, aniefer, carsten.pfeiffer, christian.dietrich.opensource, christian.k.2510, curtis.windatt.public, julian.honnen, krikava, lerch, lorenzo.bettini, pwebster, rsternberg, t-oberlies, Vikas.Chandra |
| Version: | 3.7 | Keywords: | noteworthy |
| Target Milestone: | 4.14 M3 | ||
| Hardware: | All | ||
| OS: | All | ||
| See Also: |
https://bugs.eclipse.org/bugs/show_bug.cgi?id=343152 https://git.eclipse.org/r/143596 https://git.eclipse.org/r/151355 https://git.eclipse.org/c/pde/eclipse.pde.ui.git/commit/?id=056665a6195a846594221d3cbd5dfb52b21cafc1 https://bugs.eclipse.org/bugs/show_bug.cgi?id=559290 https://bugs.eclipse.org/bugs/show_bug.cgi?id=560068 https://bugs.eclipse.org/bugs/show_bug.cgi?id=567674 |
||
| Whiteboard: | |||
| Bug Depends on: | |||
| Bug Blocks: | 422952 | ||
The default target platform is an 'installation' location pointing at ${eclipse_home}. An installation location will look for a config.ini and bundles.info file and load the list of of installed bundles.
Closing as WORKSFORME as the current behaviour is exactly what you are asking for.
If install metadata can't be found, we will fall back on loading all plug-ins in the directory.
Now that I saw your post on the newsgroup I see that there is a deeper issue here. ${eclipse_home} isn't resolving to something useful.
However, I can't think of anything reasonable for PDE to do in this case. ${eclipse_home} is assumed to be your running install. I'm not sure how else we can get that information.
(In reply to comment #2) > Now that I saw your post on the newsgroup I see that there is a deeper issue > here. ${eclipse_home} isn't resolving to something useful. > > However, I can't think of anything reasonable for PDE to do in this case. > ${eclipse_home} is assumed to be your running install. I'm not sure how else > we can get that information. I tried to create an installation-based target platform by pointing on the Tycho "work/configuration" folder, which is equivalent to the classic "configuration" folder; and PDE did not succeed to resolve the bundle as specified in the config.ini. Should I reopen this bug? (In reply to comment #3) > Should I reopen this bug? You can reopen it, but PDE isn't going to change the default target platform as stated in the title. I still feel the solution lies with the launcher, but perhaps the installation bundle container (location) should be updated to support the folder layout you describe. Ok, I understand this is not so easy. Then if we think about bug #343152, do you see another way to have the Target Platform during Tycho tests working rather than adding a warkaround for this bug in Tycho? When using the current install (${eclipse_home}), seems like PDE tries to list all bundles in the plugins folder. Am I right?
Instead, it should rather read the config.ini file or the bundles.info file to have an exhaustive list of what is running on this platform. In the case of Tycho, the bundles to use are listed in config,ini.
What Mickael wants is that the PDE can use the running OSGi runtime as target platform. It should be easy to get the list of installed bundles from the OSGi instrospection API and use that. Also this would be a lot more robust than reading relying on configuration files. (In reply to comment #6) > When using the current install (${eclipse_home}), seems like PDE tries to list > all bundles in the plugins folder. Am I right? > Instead, it should rather read the config.ini file or the bundles.info file to > have an exhaustive list of what is running on this platform. In the case of > Tycho, the bundles to use are listed in config,ini. For installation locations (such as the default one) we try to read bundles.info to get configuration information. If that fails we fall back on reading the plug-ins directory. I previously had some code to read information out of config.ini but it was dropped due to the instability of the changes I had made. It certainly makes sense to do so for install locations that do not have a bundles.info or platform.xml available. (In reply to comment #7) > What Mickael wants is that the PDE can use the running OSGi runtime as target > platform. It should be easy to get the list of installed bundles from the OSGi > instrospection API and use that. Also this would be a lot more robust than > reading relying on configuration files. I can see how this could be implemented. We would need a new location type that looked at the current running application. This would only be useful for the default target platform and this is the first bug where reading the configuration files has been impossible. Changing this would also have an effect on Target Weaving (mixing workspace plug-ins in the target when self hosting). I agree with the idea, but it is a lot of work for no visible gain for nearly all users. (In reply to comment #8) > (In reply to comment #6) > > When using the current install (${eclipse_home}), seems like PDE tries to list > > all bundles in the plugins folder. Am I right? > > Instead, it should rather read the config.ini file or the bundles.info file to > > have an exhaustive list of what is running on this platform. In the case of > > Tycho, the bundles to use are listed in config,ini. > > For installation locations (such as the default one) we try to read > bundles.info to get configuration information. Then maybe Tycho could generate the list of bundles to load in a bundle.info instead of setting them in the config.ini? For what I understood, that would resolve the initial use case. I gave a try to generate a bundles.info in Tycho rather than listing bundles in config.ini, and PDE does not succeed to generate an accurate target platform. IThe workaround for all project using PDE is to make sure the Test suite sets up a target platform: git.eclipse.org/c/gmf-tooling/org.eclipse.gmf-tooling.git/tree/tests/org.eclipse.gmf.tests/src/org/eclipse/gmf/tests/AllTests.java and http://git.eclipse.org/c/gmf-tooling/org.eclipse.gmf-tooling.git/tree/tests/org.eclipse.gmf.tests/src/org/eclipse/gmf/tests/Utils.java#n146 (In reply to comment #11) Thanks for sharing this utility. I have one addition: in the last line, the method schedules a job that loads the new target asynchronously. To ensure that the target is activated before running any test code, I'd suggest the following change: - LoadTargetDefinitionJob.load(targetDef); + Job job = new LoadTargetDefinitionJob(targetDef); + job.schedule(); + job.join(); Thanks Mickael However, from what I understand, this uses internal/provisional API. Indeed the code does not compile at least using Kepler. I had to modify the two lines: AbstractBundle bundleImpl = (AbstractBundle) bundle; BaseData bundleData = (BaseData) bundleImpl.getBundleData(); // EquinoxBundle bundleImpl = (EquinoxBundle) bundle; // Generation generation = (Generation) bundleImpl.getModule().getCurrentRevision().getRevisionInfo(); or did I miss something in your code? And would it be do-able, in case there is no bundles.info file, to fallback to the value of osgi.bundles property from config.ini? (In reply to Mickael Istria from comment #14) > And would it be do-able, in case there is no bundles.info file, to fallback > to the value of osgi.bundles property from config.ini? If it sounds fine to PDE committers, I'll relabel the issue accordingly. I had a quick look at it, and I'm missing a nice existing way to parse the value of the osgi.bundles property in config.ini. I could find some pieces of code doing this in Equinox, but they're all private. Is anyone aware of some other pieces of code dealing with this `osgi.bundles` property that we could reuse? I'm widening a bit the scope here as I'm not sure changing the existing "Installation" TP resolution is best. I think we could simply introduce a non-parameterized, singleton "Installed bundles in currently running IDE" that would dynamically be populated with the list of bundles by asking OSGi Platform more than by looking at whichever configuration files. Removing from my plan. I have a beginning of a draft, but cannot commit to proper resolution for 4.13. I'd gladly let anyone take my draft and continue the work. New Gerrit change created: https://git.eclipse.org/r/143596 (In reply to Eclipse Genie from comment #19) > New Gerrit change created: https://git.eclipse.org/r/143596 Mickael, any idea how this OSGi-based TP could find the host's features? (In reply to Julian Honnen from comment #20) > Mickael, any idea how this OSGi-based TP could find the host's features? It wouldn't be really OSGi as features are Eclipse-specific, but I guess looking at how the "Features" tab of Installation Dialog is implemented can give hints on how to introspect the running application for features. (In reply to Mickael Istria - away until July 15th from comment #21) > (In reply to Julian Honnen from comment #20) > > Mickael, any idea how this OSGi-based TP could find the host's features? > > It wouldn't be really OSGi as features are Eclipse-specific, but I guess > looking at how the "Features" tab of Installation Dialog is implemented can > give hints on how to introspect the running application for features. the "Features" tab of Installation Dialog uses dark magic of org.eclipse.core.runtime.IBundleGroup interface. The only its implementation for today uses "pre-p2" approach implemented by org.eclipse.update.internal.configurator.FeatureEntry I've touched this area recently during attempts to make About dialog available for E4 @Julian I think there should be a task to provide a p2-based implementation for feature-related information. (In reply to Alexander Fedorov from comment #22) > @Julian I think there should be a task to provide a p2-based implementation > for feature-related information. Something like var profileRegistry = context.getService(IProfileRegistry.class); var profile = profileRegistry.getProfile(IProfileRegistry.SELF); var results = profile.query(QueryUtil.createIUGroupQuery()); From the IUs, you can get the ArtifactKeys, that with some more magic I don't have beforehand you should be able to use as input to retrieve the actual location on filesystem. Giving 2nd thoughts about it, p2 shouldn't be involved in this resolution process as some products may not enable p2 (test runtime for instance). Using BundleGroups seems OK, and we can try downcasting to FeatureEntry and retrieving the path on filesystem with featureEntry.getSite().getURL() + featureEntry.getURL(). (In reply to Mickael Istria - away until July 15th from comment #24) > Giving 2nd thoughts about it, p2 shouldn't be involved in this resolution > process as some products may not enable p2 (test runtime for instance). > Using BundleGroups seems OK, and we can try downcasting to FeatureEntry and > retrieving the path on filesystem with featureEntry.getSite().getURL() + > featureEntry.getURL(). FeatureEntry implements deprecated interface IPlatformConfiguration.IFeatureEntry (Bug 311590), we shouldn't introduce new usages for it. From my POV the right way is to provide alternative implementation for IBundleGroupProvider service. May be p2 will be too heavy for it, but it seems to be the best candidate for today. If you know that p2 is not available for some configurations we may think about some lightweigt stub that just parse the feature manifests on some location. Not introducing dependency on p2 for RCP apps makes sense but for PDE I don't see a reason why we should prevent using it. pde.core already has hard dep on p2 bundles after all. (In reply to Alexander Fedorov from comment #25) > If you know that p2 > is not available for some configurations we may think about some lightweigt > stub that just parse the feature manifests on some location. In Tycho and PDE tests, which happen to be the initial reason behind this report, p2 is not enabled, trying to get the profile or services would usually return null. Also some RCP that don't want to enable extensions and upgrades do disable and remove p2. So to me,p2 isn't a viable solution at all to fix the initial problem in the general case while relying on deprecated or internal code (that happens to build at the same time as PDE) fixes the problem in all cases. Reading a feature location woukd also not work in all cases, as features can live in different folders (esp in the test execution). I think the best solution consists in using those APIs and -if they happen to properly.fix this issue- remove the deprecation note for them; or to make the IBundleGroup support a new accessor to get the location. (In reply to Mickael Istria - away until July 15th from comment #27) > I think the best solution consists in using those APIs and -if they happen > to properly.fix this issue- remove the deprecation note for them; or to make > the IBundleGroup support a new accessor to get the location. More specifically, I believe calling Arrays.stream(Platform.getBundleGroupProviders()).forEach(IBundleGroupProvider::getBundleGroups); // initialize statez PlatformConfiguration.getCurrent().getConfiguredFeatureEntries() could work to get the list of features with install locations (url). Although it's deprecated, as things build and are shipped together, having a test case covering it would be enough to ensure things work sustainably without risk of undetected issues. PDE tests are indeed my intended usecase, so I don't mind black magic and/or internal API. But: both IBundleGroupProvider::getBundleGroups and PlatformConfiguration.getCurrent().getConfiguredFeatureEntries() are empty in the test runtime. (In reply to Julian Honnen from comment #29) > But: both IBundleGroupProvider::getBundleGroups and > PlatformConfiguration.getCurrent().getConfiguredFeatureEntries() are empty > in the test runtime. Does you test runtime actually include features? ie is your PDE test launch/debug configuration content feature-based (in the Plugins tab, are you using "Launch With > features selected below"? (In reply to Mickael Istria from comment #30) > Does you test runtime actually include features? > ie is your PDE test launch/debug configuration content feature-based (in the > Plugins tab, are you using "Launch With > features selected below"? No, I'm using a regular PDE test launching with 'everything'. (In reply to Julian Honnen from comment #31) > No, I'm using a regular PDE test launching with 'everything'. The "everything" doesn't include features AFAIK, it's stating "all workspace and enabled target *plugins*". I didn't dig in the code, but I don't think the launch includes the features in that mode; and I'm not even sure it does when using the "selected features" mode (maybe they're expanded by the launch and the runtime still receives a list of bundles). (In reply to Mickael Istria from comment #32) > (maybe they're expanded by the > launch and the runtime still receives a list of bundles). Exactly. The 'Running Platform' TP in an eclipse started from feature-based launch config also doesn't contain any features. At this point, wouldn't it be worth merging this proposal which only has bundles at the moment? I think it would already provide a lot of value. (In reply to Mickael Istria from comment #34) > At this point, wouldn't it be worth merging this proposal which only has > bundles at the moment? I think it would already provide a lot of value. What's your personal usecase? I wouldn't mind merging it, if it stays internal. Maybe via TargetPlatformService::newOsgiTarget (not exposed on the interface). (In reply to Julian Honnen from comment #35) > What's your personal usecase? I'm using Flatpak distribution so the newly installed bundles are not part of TP (they're not in the same location). Also, in Tycho, when I was working on GMF Tools (generating Eclipse plugins), the target platform was empty in Tycho because the bundles are not in a folder/ plugin and are instead loaded from various paths configured in the config.ini. > I wouldn't mind merging it, if it stays internal. > Maybe via TargetPlatformService::newOsgiTarget (not exposed on the > interface). Actually, I looked at the documentation of TargetPlatformService::newDefaultTarget and it seems like this bug makes that the method is currently not conform to its contract. So I'll try changing the newDefaultTarget so it uses the new Target Platform location. I'll try one of the approaches to get features in, and if we can get bundles dynamically, and features as it's currently done (until we find better), then I think we'd reach of state that makes the method implementation more consistent with the contract and default target platform working better with Flatpak. Latest patchset on https://git.eclipse.org/r/143596 contains what I perceive as a good enough fix for this bug. I'm going to work from scratch on a new and better patch that will change existing ProfileBundleContainer (near line 128) to read bundles from configuration/comfig.ini#osgi.bundles property in case no other source is working. This should make the TP non-empty with Tycho and others. New Gerrit change created: https://git.eclipse.org/r/151355 Gerrit change https://git.eclipse.org/r/151355 was merged to [master]. Commit: http://git.eclipse.org/c/pde/eclipse.pde.ui.git/commit/?id=056665a6195a846594221d3cbd5dfb52b21cafc1 Thanks Julian! Mickael, can you please verify this defect? Verified with 0. Remove eclipse.pde.ui/ui/org.eclpse.pde.ui.tests/src/org/eclipse/pde/ui/tests/launcher/LaunchConfigurationMigrationTest.java and references to it (or we cannot diff test stuff) 1. In eclipse.pde.ui/ui/org.eclpse.pde.ui.tests/src/org/eclipse/pde/ui/tests/launcher/AbstractLaunchTest.java , comment "// TargetPlatformUtil.setRunningPlatformAsTarget();" to remove explicit setting of target platform 2. verify that this would lead test to failure previously with `mvn clean verify -Dtest=FeatureBasedLaunchTest -Pbuild-individual-bundles -Declipse-p2-repo.url=http://download.eclipse.org/eclipse/updates/4.14milestones/S-4.14M1-201910091800/` => 3 test failures "entry not found" because of empty target platform 3. Try build with patch included `mvn clean verify -Dtest=FeatureBasedLaunchTest -Pbuild-individual-bundles` => all tests pass, meaning that default target platform is populated. That proves that the default target platform is now populated in case when no configuration/org.eclipse.equinox.simpleconfigurator/bundles.info is present. Thank you Mickael for fixing this! I seem to understand that the fix is part of Tycho 1.5.0 since it works in my projects, but that's not mentioned in the released notes https://wiki.eclipse.org/Tycho/Release_Notes/1.5 |
When creating a target platform from current running instance, it is set to ${eclipse_home} installation. It is the same for the default platform. Instead, this target platform should resolve the list of bundles that are currently installed in the running instance. For the cases where bundles are set to external folders in config.ino or bundles.info or wherever. In most cases, ${eclipse_home} is fine, but it happens that people leverage equinox in a more flexible way, making the ${eclipse_home} variable irrelevant. This is the case when using Tycho. See http://bugs.eclipse.org/343152 for a use-case (Tycho maven-osgi-test-plugin).