Always consider fragments that specify a platform filter

[laeubi]

A fragment can declare a Eclipse-PlatformFilter to be only installed on a given platform. If such fragment is currently wired to a host bundle, it is quite likely that fragment is needed but the host usually do not declare an Eclipse-ExtensibleAPI so we currently miss to add such fragment making the native code loading fails.

This now adds some code to store the platform filter and if one is given the fragment is always included.

Fix #1929

[github-actions]

Test Results

  324 files   -   441    324 suites   - 441   40m 1s ⏱️ - 12m 2s
  961 tests  - 2 651    928 ✅  - 2 629  30 💤  - 24  2 ❌ +2  1 🔥 ±0 
2 883 runs   - 7 951  2 785 ✅  - 7 885  91 💤  - 72  6 ❌ +6  1 🔥 ±0 

For more details on these failures and errors, see this check.

Results for commit 3a11ebb. ± Comparison against base commit ebb6634.

This pull request removes 2651 tests.

[22: Menu contribution using 4.x API] ‑ Unknown test
org.eclipse.pde.api.tools.anttasks.tests.ApiToolingAnalysisAntTaskTests ‑ test1
org.eclipse.pde.api.tools.anttasks.tests.ApiToolingAnalysisAntTaskTests ‑ test2
org.eclipse.pde.api.tools.anttasks.tests.ApiToolingAnalysisAntTaskTests ‑ test3
org.eclipse.pde.api.tools.anttasks.tests.ApiToolingAnalysisAntTaskTests ‑ test4
org.eclipse.pde.api.tools.anttasks.tests.ApiToolingAnalysisAntTaskTests ‑ test5
org.eclipse.pde.api.tools.anttasks.tests.ApiToolingAnalysisAntTaskTests ‑ test6
org.eclipse.pde.api.tools.anttasks.tests.ApiToolingAnalysisAntTaskTests ‑ test7
org.eclipse.pde.api.tools.anttasks.tests.ApiToolingApiFreezeAntTaskTests ‑ test1
org.eclipse.pde.api.tools.anttasks.tests.ApiToolingApiFreezeAntTaskTests ‑ test2
…

♻️ This comment has been updated with latest results.

[laeubi]

This should be included once master is open unless someone things we should have this in RC1

[HannesWell]

As elaborated in #1929 (comment) this is IMO the wrong approach and we should not extend the set of included fragments based on a heuristic that might or might not be correct. And incorrect inclusions can really ruin the runtime without any possibility to intervene except for specifying a launch completely manually.

Furthermore this now affects all calls to the computation method that don't specify any options. If at all this should only be done on explicit request, that could then be done for Plugin test launches.
But still I think the alternatives I suggested above are more suitable.

[laeubi]

@HannesWell can you please give an example where "incorrect inclusions can really ruin the runtime" I can not really think about any case where this would be wrong (but many cases where it is missing).

For me this is not really any heuristic, it is actually the reverse of Extensible-Api header, I never seen a case where one defines a Eclipse-PlatformFilter but don't want the fragment included, it is more the other way round that we take many efforts (e.g. additional p2.inf) to make sure the fragment is not badly missed.

[merks]

I'm personally fine with this approach.

[laeubi]

Furthermore this now affects all calls to the computation method that don't specify any options.

I now introduced a new option that is only activated in the BundleLauncherHelper so other callers should not be affected (even though I could not find it would harm anything on their call sites).

[HannesWell]

For me this is not really any heuristic, it is actually the reverse of Extensible-Api header, I never seen a case where one defines a Eclipse-PlatformFilter but don't want the fragment included, it is more the other way round that we take many efforts (e.g. additional p2.inf) to make sure the fragment is not badly missed.

All of this is in my opinion just a workaround because it's currently not possible to declare a requirement from a host to one of it's fragments and building the same in one reactor with Maven/Tycho.

Please see my answer in #1929 (comment)

Furthermore this now affects all calls to the computation method that don't specify any options.

I now introduced a new option that is only activated in the BundleLauncherHelper so other callers should not be affected (even though I could not find it would harm anything on their call sites).

As written in the issue I'll probably reluctantly approve this, if this is only enabled for shrieked JUnit Plugin Test runtimes. But for general app/product's that's a no-go for me, for the mentioned reasons and as explicit means to include required fragments already exists e.g. for Products.
But ideally that's still just a workaround until the IMO better solutions are available.

[laeubi]

All of this is in my opinion just a workaround because it's currently not possible to declare a requirement from a host to one of it's fragments and building the same in one reactor with Maven/Tycho.

Actually native fragments (and Eclipse-PlatformFilter) are the "workaround", OSGi has standard means to declare and select appropriate native code by platform, anyways its an Eclipse Extension and PDE can/should support it.

"required fragments" is really something completely alien from OSGi point-of-view ... if it is required then it should be part of the bundle, if it is truly optional then it can be a fragment, but both does not really make sense.

As written in the issue I'll probably reluctantly approve this, if this is only enabled for shrieked JUnit Plugin Test runtimes. But for general app/product's that's a no-go for me, for the mentioned reasons and as explicit means to include required fragments already exists e.g. for Products.

The whole purpose (and power) of "include required automatically" is that it works without specify it explicitly. SO of course one wants this for products and plain OSGi launches as well!

I really don't understand the concerns mentioned in the ticket they are talking about fragments that pull in more dependencies, about test fragments and so one none of this applies to the proposed solution here:

1. Native fragments only carry native code for the whole purpose of not shipping a full set of natives to get smaller bundles and have well defined meta-data carried with
2. They never have any additional dependencies
3. We perform additional work to make sure they are always installed
4. I have never seen a native test-fragment