| Summary: | product editor launch configuration should use feature-based launch if feature-based product | ||||||
|---|---|---|---|---|---|---|---|
| Product: | [Eclipse Project] PDE | Reporter: | Jan Sievers <jan.sievers> | ||||
| Component: | UI | Assignee: | Hannes Wellmann <wellmann.hannes1> | ||||
| Status: | VERIFIED FIXED | QA Contact: | |||||
| Severity: | normal | ||||||
| Priority: | P3 | CC: | bartoszpop, jeffmcaffer, julian.honnen, pelouas, wellmann.hannes1 | ||||
| Version: | 3.7 | Keywords: | noteworthy | ||||
| Target Milestone: | --- | ||||||
| Hardware: | All | ||||||
| OS: | All | ||||||
| See Also: |
https://bugs.eclipse.org/bugs/show_bug.cgi?id=548677 https://git.eclipse.org/r/c/pde/eclipse.pde.ui/+/190583 https://git.eclipse.org/c/pde/eclipse.pde.ui.git/commit/?id=81def39a7197325053f12080c6ca86be1222011a https://github.com/eclipse-platform/www.eclipse.org-eclipse-news/pull/24 |
||||||
| Whiteboard: | |||||||
| Bug Depends on: | |||||||
| Bug Blocks: | 570760 | ||||||
| Attachments: |
|
||||||
|
Description
Jan Sievers
does your product export properly and run? Feature based launching does things like automatically add missing dependencies (there's a bug for that). It may be the case that your .product is actually wrong but when you launch using features it works ok because the problem is being found and fixed for you under the covers. yes it exports OK and it runs OK. When exporting, transitive dependencies are added automatically. This is one of the reasons why I opened this bug: "Testing" suggests that what you test is the same thing that would later be exported, but it turns out that with feature-based products that's not true. Created attachment 191506 [details]
demo plugin, feature and product
attached demo bundle and feature with a .product file demonstrate the problem.
If you export the .product, it will add all transitive dependencies and the exported product runs OK (minus branding/splash, but that's not the point here).
If you launch an Eclipse Application from the "Testing" section of the product editor, it fails (see stacktrace from my previous comment).
Proposed fix is to generate a feature-based launch config for testing if the product is feature-based. You can verify this works by manually changing the launch config to be feature-based and only including productdemo.feature. This would be more consistent with the product export wizard behaviour.
forgot to mention I'm using 3.7 M5 Thanks for the demo projects. Very helpful. Looking at them I see that the .product file does no validate. Open the editor and click the validate icon in the top right (the one with the check mark). It reports that o.e.ui and o.e.core.runtime are missing Here is what is happening. When you run the export, the end result is "installed" by running p2's director against the exported repo and perhaps other repos. In your case, p2 is discovering core.runtime and ui and installing them. Similarly when you run a feature based launch, PDE is effectively doing a "Add Required Plugins" under the covers and finding core.runtime and ui and adding them. For better or worse the typical approach that we have taken is to have the base part of a system be well specified with explicit feature inclusions and then the upper parts of the system are added on top. So the expectation is that the product def bring together all the pieces that are needed. That is, the product should validate. Side note: The demo bundle manifest has duplicate entries for core.runtime and the ui bundles on the dependencies page. This is not likely to be an actual problem but it doesn't make sense. I also have this problem (my export RCP product, which is based on features, is working, but not launching it directly through the *.product). And I have the same problem for simple launch configurations based on features. After some search, I understood that eclipse is using the "selected_target_plugins" in the *.launch even if we choosed to be based on features. ... and my *.launch or my *.product are validating successfully. I had to check "clear the configuration area before launching" in the product launch configuration... 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. I think with the latest improvements of feature-based Eclipse/Equinox applications we can now flip the switch and make the launch configurations created for feature-based product based on features by default. The question is should only newly created launch-configs be set up to be feature-based or should also existing configurations be updated when they are again launched from the product? I tend to be in favour of the latter since the list of features/bundles is updated as well. If feature-based launches work well in the next time, we could also remove the population of a corresponding set of bundles for a bundle-based launch (e.g. after the next release). This would complete the second step from Bug 548677. @Julian what do you think? New Gerrit change created: https://git.eclipse.org/r/c/pde/eclipse.pde.ui/+/190583 (In reply to Hannes Wellmann from comment #10) > I think with the latest improvements of feature-based Eclipse/Equinox > applications we can now flip the switch and make the launch configurations > created for feature-based product based on features by default. I agree. > The question is should only newly created launch-configs be set up to be > feature-based or should also existing configurations be updated when they > are again launched from the product? I tend to be in favour of the latter > since the list of features/bundles is updated as well. I agree as well, the "update" of existing launch configs has always been quite destructive and it's easy enough to switch back. > If feature-based launches work well in the next time, we could also remove > the population of a corresponding set of bundles for a bundle-based launch > (e.g. after the next release). This would complete the second step from Bug > 548677. Do we gain anything by the removal? (In reply to Julian Honnen from comment #12) > (In reply to Hannes Wellmann from comment #10) > > > The question is should only newly created launch-configs be set up to be > > feature-based or should also existing configurations be updated when they > > are again launched from the product? I tend to be in favour of the latter > > since the list of features/bundles is updated as well. > I agree as well, the "update" of existing launch configs has always been > quite destructive and it's easy enough to switch back. That's right. One can easily switch back to a plug-in based launched in the Plug-ins tab. > > > If feature-based launches work well in the next time, we could also remove > > the population of a corresponding set of bundles for a bundle-based launch > > (e.g. after the next release). This would complete the second step from Bug > > 548677. > Do we gain anything by the removal? I would say a little less code to maintain and smaller launch-configurations (not the strongest arguments indeed). But as soon as Bug 544838 and Bug 570760 are resolved a user has the same set of bundles in the launched application when selecting to not include requirements automatically. Nevertheless maybe it is not a bad idea to keep it, in case one wants to adjust the launch configuration specifically, for whatever reason. But then I suggest to fill the set of bundles based on the result of BundleLauncherHelper.getWorkspaceBundleMap() to get a consistent result, after the recent changes have proofed to work well Gerrit change https://git.eclipse.org/r/c/pde/eclipse.pde.ui/+/190583 was merged to [master]. Commit: http://git.eclipse.org/c/pde/eclipse.pde.ui.git/commit/?id=81def39a7197325053f12080c6ca86be1222011a The behavioural change has been submitted. As suggested before set of bundles could be filled with the results of calling BundleLauncherHelper.getWorkspaceBundleMap(), which would ensure equivalence between both launch kinds and would save some code. But I would like to change that only after some time has passed and that method is battle-prooed. Should we keep this bug open until then? Or should I resolve this this as fixed and submit such patch later for this or a subsequent bug? (In reply to Hannes Wellmann from comment #15) > As suggested before set of bundles could be filled with the results of > calling BundleLauncherHelper.getWorkspaceBundleMap(), which would ensure > equivalence between both launch kinds and would save some code. Without having looked at the current implementation - I would expect that the selected bundles in the launch config exactly reflect the configured bundles from the product and nothing else. > Should we keep this bug open until then? Or should I resolve this this as > fixed and submit such patch later for this or a subsequent bug? Better to resolve this bug and open another if needed. (In reply to Julian Honnen from comment #16) > (In reply to Hannes Wellmann from comment #15) > > As suggested before set of bundles could be filled with the results of > > calling BundleLauncherHelper.getWorkspaceBundleMap(), which would ensure > > equivalence between both launch kinds and would save some code. > Without having looked at the current implementation - I would expect that > the selected bundles in the launch config exactly reflect the configured > bundles from the product and nothing else. Exactly. Once the change for Bug 544838 is submitted a Working-Coppy with AUTOMATIC_ADD_REQUIREMENTS=false can be passed to BundleLauncherHelper.getMergedBundleMap() which should lead to the same result. But yes, let's handle this in a separate bug. Verified with Eclipse SDK Version: 2022-06 (4.24) Build id: I20220516-1800 PR for the N&N entry: https://github.com/eclipse-platform/www.eclipse.org-eclipse-news/pull/24 |