Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.

Bug 340350

Summary: product editor launch configuration should use feature-based launch if feature-based product
Product: [Eclipse Project] PDE Reporter: Jan Sievers <jan.sievers>
Component: UIAssignee: Hannes Wellmann <wellmann.hannes1>
Status: VERIFIED FIXED QA Contact:
Severity: normal    
Priority: P3 CC: bartoszpop, jeffmcaffer, julian.honnen, pelouas, wellmann.hannes1
Version: 3.7Keywords: 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 Flags
demo plugin, feature and product none

Description Jan Sievers CLA 2011-03-17 13:24:03 EDT
While preparing a demo for EclipseCon 2011, we noticed that the product editor is inconsistent when using feature-based products.

In our example feature-based product ( see https://github.com/jsievers/tycho-demo/blob/master/exercises/Exercise_05_Solution/tychodemo.repository/tychodemo.product ), we have one feature with one plugin defining a feature-based product.
When exporting the product, all transitive dependencies (which are not included directly or indirectly in the feature) are added to the installation as expected.

However when testing the product using the "play" button on the Overview tab, the launch fails with 

!SESSION Thu Mar 17 18:18:27 CET 2011 ------------------------------------------
!ENTRY org.eclipse.equinox.launcher 4 0 2011-03-17 18:18:27.281
!MESSAGE Exception launching the Eclipse Platform:
!STACK
java.lang.ClassNotFoundException: org.eclipse.core.runtime.adaptor.EclipseStarter
	at java.net.URLClassLoader$1.run(Unknown Source)
	at java.security.AccessController.doPrivileged(Native Method)
	at java.net.URLClassLoader.findClass(Unknown Source)
	at java.lang.ClassLoader.loadClass(Unknown Source)
	at java.lang.ClassLoader.loadClass(Unknown Source)
	at org.eclipse.equinox.launcher.Main.invokeFramework(Main.java:617)
	at org.eclipse.equinox.launcher.Main.basicRun(Main.java:575)
	at org.eclipse.equinox.launcher.Main.run(Main.java:1408)
	at org.eclipse.equinox.launcher.Main.main(Main.java:1384)

The strange thing I noticed is:

If I manually create a _feature-based_ "Eclipse Application" launch configuration with just my demo feature enabled, then

1. the launch works fine and more astonishing
2. the "play" button in the product editro now also suddenly works...
Comment 1 Jeff McAffer CLA 2011-03-17 14:02:52 EDT
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.
Comment 2 Jan Sievers CLA 2011-03-18 04:25:23 EDT
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.
Comment 3 Jan Sievers CLA 2011-03-18 10:09:14 EDT
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.
Comment 4 Jan Sievers CLA 2011-03-18 10:19:30 EDT
forgot to mention I'm using 3.7 M5
Comment 5 Jeff McAffer CLA 2011-03-18 13:22:34 EDT
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.
Comment 6 Guillaume P. CLA 2011-11-17 10:31:19 EST
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.
Comment 7 Guillaume P. CLA 2011-11-17 10:41:02 EST
... and my *.launch or my *.product are validating successfully.
Comment 8 Guillaume P. CLA 2011-11-24 07:12:11 EST
I had to check "clear the configuration area before launching" in the product launch configuration...
Comment 9 Eclipse Genie CLA 2018-11-27 15:21:20 EST
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.
Comment 10 Hannes Wellmann CLA 2022-02-08 13:24:51 EST
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?
Comment 11 Eclipse Genie CLA 2022-02-08 13:36:53 EST
New Gerrit change created: https://git.eclipse.org/r/c/pde/eclipse.pde.ui/+/190583
Comment 12 Julian Honnen CLA 2022-02-09 02:50:31 EST
(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?
Comment 13 Hannes Wellmann CLA 2022-02-10 14:02:18 EST
(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
Comment 15 Hannes Wellmann CLA 2022-03-10 18:13:53 EST
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?
Comment 16 Julian Honnen CLA 2022-03-11 02:30:58 EST
(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.
Comment 17 Hannes Wellmann CLA 2022-03-11 03:19:35 EST
(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.
Comment 18 Hannes Wellmann CLA 2022-05-17 01:38:18 EDT
Verified with Eclipse SDK

Version: 2022-06 (4.24)
Build id: I20220516-1800
Comment 19 Hannes Wellmann CLA 2022-05-17 18:55:24 EDT
PR for the N&N entry:
https://github.com/eclipse-platform/www.eclipse.org-eclipse-news/pull/24