Bug 483383 - Allow p2 to prevent installation of IUs requiring an Execution Environment not compatible with running JRE
Summary: Allow p2 to prevent installation of IUs requiring an Execution Environment no...
Status: RESOLVED FIXED
Alias: None
Product: Equinox
Classification: Eclipse Project
Component: p2 (show other bugs)
Version: 4.5.0 Mars   Edit
Hardware: All All
: P3 enhancement (vote)
Target Milestone: 4.16 M3   Edit
Assignee: P2 Inbox CLA
QA Contact: Mickael Istria CLA
URL:
Whiteboard:
Keywords: api, Documentation, noteworthy
: 567947 (view as bug list)
Depends on:
Blocks: 561865
  Show dependency tree
 
Reported: 2015-12-01 10:06 EST by Rüdiger Herrmann CLA
Modified: 2020-12-15 12:45 EST (History)
17 users (show)

See Also:


Attachments
Example project and repo for tests (26.99 KB, application/zip)
2020-04-02 12:47 EDT, Mickael Istria CLA
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Rüdiger Herrmann CLA 2015-12-01 10:06:54 EST
It should not be possible to install features that require an execution environment that is not supported by the JRE currently running Eclipse.

Or, if for reasons I don't see, there should a t least be a warning.

For example, when running the Eclipse Platform SDK on a JRE v1.7 and installing a feature whose plug-ins require JRE 1.8, the installation succeeds. But after restarting Eclipse it seems as if the feature wasn't installed. None of the contributions which the plug-ins are meant to provide are visible. See here [1] for the full story.

The reason, of course, is that the bundles aren't started due to the unresolved requirement. But it takes some time and forth and back to find out what the reason is for the just installed plug-ins not showing up. Less patient or experienced users will likely just give up.


[1] https://github.com/rherrmann/gonsole/issues/59
Comment 1 Rüdiger Herrmann CLA 2016-01-27 06:28:47 EST
Here is another example of someone who walked into the same trap: https://github.com/rherrmann/gonsole/issues/66

Is someone reading the bugzilla inbox who could comment on why p2 doesn't prevent this?
Comment 2 Jonah Graham CLA 2016-02-24 13:00:08 EST
PyDev also has a similar warning on its download page http://www.pydev.org/download.html: 

Requirements

Java 7: Important: If you don't have java 7, the update process may appear to succeed, but PyDev will simply not show in the target installation. See PyDev does not appear after install! below for details on how to fix that.
Comment 3 Ed Merks CLA 2020-02-23 03:44:14 EST
With the upcoming support for being able to install JREs, this gets tricker because the update itself might update to a newer JRE version...
Comment 4 Mickael Istria CLA 2020-03-23 10:18:09 EDT
Bundles build with recent enough version of p2 have this requirement added (according to their BREE)

          <requiredProperties namespace='osgi.ee' match='(&amp;(osgi.ee=JavaSE)(version=1.8))'/>

However, this requirement is matched against the dummy `a.jre.javase` units that actually don't match anything concrete.
We could try to instruct p2 if ignoring such `a.jre.javase` units that don't match the currently running Java version during the dependency resolution. That should lead the install attempt to a failure which would show a p2 error message like "required capability not found namespace='osgi.ee' match='(&amp;(osgi.ee=JavaSE)(version=11))'", which is a bit helpful and non destructive.
Comment 5 Gunnar Wagenknecht CLA 2020-03-23 11:51:24 EDT
(In reply to Mickael Istria from comment #4)
> Bundles build with recent enough version of p2 have this requirement added
> (according to their BREE)
> 
>           <requiredProperties namespace='osgi.ee'
> match='(&amp;(osgi.ee=JavaSE)(version=1.8))'/>
> 
> However, this requirement is matched against the dummy `a.jre.javase` units
> that actually don't match anything concrete.

Wasn't there a bug that p2 should provide "real" IUs for the running (aka. runtime) JRE? I can't seem to find but but I think it would be good if p2 would dynamically create a proper IU based on the JRE it is running in and then match to it in the installation/update request.
Comment 6 Mickael Istria CLA 2020-03-23 12:01:16 EDT
(In reply to Gunnar Wagenknecht from comment #5)
> Wasn't there a bug that p2 should provide "real" IUs for the running (aka.
> runtime) JRE?

I think it's bug 346174, although it only mentions packages, not execution environment (but it's probably worth retitling it).
Comment 7 Mickael Istria CLA 2020-03-24 14:04:04 EDT
I'm looking into implementing proposal from comment 4, I think it can work but...
it requires an additional "flag" for the installation cases that would want to enforce a compatibility check against current JRE; this flag would be set in the p2 UI entry-points of the IDE (check for updates, install, marketplace...) but cannot be set in general at the risk of breaking backward-compatibility.

Many use-cases of p2 (like build stuff) actually require p2 to be able to provision bundles which have a BREE higher than the current one running p2. It's a very valid use-case and we don't want to break that and put extra load on build engines.

The flag would instruct p2 of ignoring all the incompatible "a.jre.javase" presenting a higher version of the osgi.ee capability than what's currently running.
I'm not sure we can regenerate those a.jre.javase units at install-time at the moment, indeed, Equinox/p2 doesn't contain the .profile definitions for latest JRE releases. Maybe it now generates the a.jre.javase unit more dynamically, but I don't know, and I don't think fixing that is a requirement to fix the current bug.
Comment 8 Ed Merks CLA 2020-03-26 07:12:59 EDT
I was investigating this issue from the installer point of view.  It seems in principle an issue such as this could be handled by requirement on a.jre.javase that restricts the version range to be bounded by the version of the currently running VM (or in the case of the installer, the version of the VM chosen by the user for the installation). 

As a result, it made me  have a close look at all the a.jre.javase versions available in all the p2 repositories.

For example, the latest release

https://download.eclipse.org/releases/2020-03/202003181000/

contains only version 14.0.0. 

While this corresponding EPP repo:

https://download.eclipse.org/technology/epp/packages/2020-03/

contains version 9.0.0 through 13.0.0 (no 14.0.0).

In fact, looking at all p2 repositories on download.eclipse.org,  I see also version 1.6.0 and 1.7.0, but nowhere do ai see a 1.8.0.

Locally I tried to configure Oomph's Tycho build like this:

        <plugin>
          <groupId>org.eclipse.tycho</groupId>
          <artifactId>tycho-p2-publisher-plugin</artifactId>
          <version>${tycho-version}</version>
          <configuration>
            <profiles>JavaSE-1.7, JavaSE-1.8, JavaSE-9, JavaSE-10, JavaSE-11, JavaSE-12, JavaSE-13, JavaSE-14</profiles>
          </configuration>
        </plugin>

But in the final repository I see only version 1.7.0, 1.8.0, 9.0.0, 11.0.0, 13.0.0, and 14.0.0.  So 10.0.0 and 12.0.0 are missing.  This is using tycho-version 1.6.0.

The bottom line is that it's not safe to assume that an appropriately exact a.jre.javase unit verison will be available in the p2 repos being used during resolution.   While one could implement a "negative" requirement that excludes all a.jre.javase IUs, i.e., ignore all the things in the p2 repositories, somewhere/somehow during resolution, a synthesized IU that satisfied requirements such as this must be available:

<requiredProperties namespace='osgi.ee' match='(&amp;(osgi.ee=JavaSE)(version=11))'/>

And of course if the VM is 1.8, that requirement would not be satisfied in order to prevent the update/install.

The only thing in p2 that I see able to synthesis such an IU is org.eclipse.equinox.p2.publisher.actions.JREAction.

I see profiles here:

https://git.eclipse.org/c/equinox/rt.equinox.framework.git/tree/bundles/org.eclipse.osgi

But that stops at Java9...

So I have no good understanding of where the necessary profile definitions would come from nor why the Tycho publisher fails to generate IUs for certain profiles.
Comment 9 Mickael Istria CLA 2020-03-26 08:58:58 EDT
(In reply to Ed Merks from comment #8)
> The bottom line is that it's not safe to assume that an appropriately exact
> a.jre.javase unit verison will be available in the p2 repos being used
> during resolution.

OK, so we'll need to generate such the IU at resolution time...
 
> And of course if the VM is 1.8, that requirement would not be satisfied in
> order to prevent the update/install.
> 
> The only thing in p2 that I see able to synthesis such an IU is
> org.eclipse.equinox.p2.publisher.actions.JREAction.
> 
> I see profiles here:
> https://git.eclipse.org/c/equinox/rt.equinox.framework.git/tree/bundles/org.
> eclipse.osgi
> But that stops at Java9...

All that seems to demonstrate that we need to improve the JREAction so it can generate some decent IUs for Java 9 and later. I think the idea is that profiles were abandoned in Equinox because of JPMS and modular runtime, which make it totally unreliable to hardcode a list of packages and actually compatible Execution Environments... But still, we could ignore packages at the moment and only focus on the compatible JavaSE versions from bytecode execution POV. I don't think the result would be worse than the current state.
So the JREAction could still generate some IUs for Java 9+, ignoring packages and listing compatible bytecode versions (ie current and previous Java versions).

And the p2 planner would just add this unit dynamically when passed a flag saying "check against currently running runtime".
Comment 10 Ed Merks CLA 2020-03-26 12:23:32 EDT
Note that these generated a.jre.javase IUs are also how package imports are resolved:

<unit id='a.jre.javase' version='9.0.0' singleton='false'>
  <provides size='248'>
    <provided namespace='org.eclipse.equinox.p2.iu' name='a.jre.javase' version='9.0.0'/>
    <provided namespace='java.package' name='java.applet' version='0.0.0'/>
    <provided namespace='java.package' name='java.awt' version='0.0.0'/>
    <provided namespace='java.package' name='java.awt.color' version='0.0.0'/>
    <provided namespace='java.package' name='java.awt.datatransfer' version='0.0.0'/>
    <provided namespace='java.package' name='java.awt.desktop' version='0.0.0'/>
    <provided namespace='java.package' name='java.awt.dnd' version='0.0.0'/>
    <provided namespace='java.package' name='java.awt.event' version='0.0.0'/>

I.e., not only do we need something to resolve the osgi.ee rerequements, i.e., these capabilities:

    <provided namespace='osgi.ee' name='OSGi/Minimum' version='1.0.0'/>
    <provided namespace='osgi.ee' name='OSGi/Minimum' version='1.1.0'/>
    <provided namespace='osgi.ee' name='OSGi/Minimum' version='1.2.0'/>
    <provided namespace='osgi.ee' name='JRE' version='1.0.0'/>
    <provided namespace='osgi.ee' name='JRE' version='1.1.0'/>
    <provided namespace='osgi.ee' name='JavaSE' version='1.0.0'/>
    <provided namespace='osgi.ee' name='JavaSE' version='1.1.0'/>
    <provided namespace='osgi.ee' name='JavaSE' version='1.2.0'/>
    <provided namespace='osgi.ee' name='JavaSE' version='1.3.0'/>
    <provided namespace='osgi.ee' name='JavaSE' version='1.4.0'/>
    <provided namespace='osgi.ee' name='JavaSE' version='1.5.0'/>
    <provided namespace='osgi.ee' name='JavaSE' version='1.6.0'/>
    <provided namespace='osgi.ee' name='JavaSE' version='1.7.0'/>
    <provided namespace='osgi.ee' name='JavaSE' version='1.8.0'/>
    <provided namespace='osgi.ee' name='JavaSE' version='9.0.0'/>
    <provided namespace='osgi.ee' name='JavaSE/compact1' version='1.8.0'/>
    <provided namespace='osgi.ee' name='JavaSE/compact1' version='9.0.0'/>
    <provided namespace='osgi.ee' name='JavaSE/compact2' version='1.8.0'/>
    <provided namespace='osgi.ee' name='JavaSE/compact2' version='9.0.0'/>
    <provided namespace='osgi.ee' name='JavaSE/compact3' version='1.8.0'/>
    <provided namespace='osgi.ee' name='JavaSE/compact3' version='9.0.0'/>

We need something to resolve the package requirements as well.  Ideally it's not a hard coded list, but if it's not hard coded, somehow we must be able to generate this list of "exported package" from the VM itself.  I don't have any clue how to do that, other than hard coding it somewhere/somehow...
Comment 11 Mickael Istria CLA 2020-03-26 12:36:24 EDT
(In reply to Ed Merks from comment #10)
> Note that these generated a.jre.javase IUs are also how package imports are
> resolved:
> [...]
>     <provided namespace='java.package' name='java.applet' version='0.0.0'/>
> [...]
> We need something to resolve the package requirements as well.

In practice, I don't think any bundle we develop for the Eclipse IDE actually declares a requirement on those packages, and I think they could all resolve without them.
If I'm right (which is not something I'm certain at all), then we can just temporarily ignore this problem.

Also, I'm thinking the p2 UI should expose whether to "ensure compatibility with current execution environment" (default) or "ignore current execution environment" so there is a still a way to install stuff for the trickiest cases.

> Ideally it's
> not a hard coded list, but if it's not hard coded, somehow we must be able
> to generate this list of "exported package" from the VM itself.  I don't
> have any clue how to do that, other than hard coding it somewhere/somehow...

This part is specifically the topic of bug 346174
Comment 12 Ed Merks CLA 2020-03-26 12:41:59 EDT
Yes, I see the trafffic on the other bug.

I wonder though that imports of javax.xml.*, org.w3c.*, and perhaps other javax.* things are quite common, but mostly so that they resolve from Orbit bundles, though in principle they should resolve also to the JVM's packages....
Comment 13 Todor Boev CLA 2020-03-26 12:53:11 EDT
OSGi R7 specifies that after Java 9 'osgi.ee' capability describes only the JVM bytecode level. The set of system packages is extracted via reflection over the JPMS modules. Since Java 9 you can introspect the modules very easily. This is why equinox does not include hardcoded lists past Java 8.

It is easy for equinox to adapt to whatever the JVM is. It is not so easy for p2 because it's way of operation is geared to the publish->provision sequence, where p2 has full control over the provisioned content. The JRE has always been an outlier since it is not installed to disk from a p2 repo, but is available as is. What's worse I can boot the same p2 installation against a different JRE whenever I want. We can't defend against this.

With that said I think we should retain the publish->provision lifecycle for the JRE, don't change the planner itself to treat any IU in a special way - this is dangerous.

1. Fix JREAction to do a proper publication, including making 'a.jre.javase' a singleton and adding negative requirements to push away other versions.

2. Run the publisher on the JREs on local disk when you learn about them. The installer has a drop-down list of them for example. Treat the JRE directory as a p2 repository.

3. Support use cases where the JRE is installed very early - at the stage of an empty directory basically. This is very natural for the eclipse installer, less so with pre-zipped installations.

I have not worked out the details. Perhaps publish the JRE in a just in time manner like the /dropins does and then include it in provisioning as usual.
Comment 14 Todor Boev CLA 2020-03-26 12:56:54 EDT
Basically the problem with the JRE is that the metadata (in a p2 repo) is separate from the content (on disk) and they do not match. A good fix is to work towards uniting them and support those use cases. There will always be broken use cases like doing an initial installation against one JRE and later starting against another.
Comment 15 Mickael Istria CLA 2020-03-26 13:02:01 EDT
(In reply to Todor Boev from comment #13)
> don't change the planner itself to treat any IU in a special way -
> this is dangerous.

The current state is that the Eclipse SDK and packages have in there current Profile units defined for Java 13, just like if Java 13 is actually available, in any case, whether you run on Java 8, Java 9, Java 11, Java 13, Java 14...
So those IUs are actually faulty and bogus. They're somehow the root of evil that just made things work, but are now likely to break things, as it was anticipated in other disucssions.
I don't think we can properly cover this story without trying to get rid -at least optionally- of these bogus IUs in dependency resolution.

> 1. Fix JREAction to do a proper publication, including making 'a.jre.javase'
> a singleton and adding negative requirements to push away other versions.

Do you have any idea of what kind of requirement (positive or negative) could describe "only install a.jre.javase 11 because I'm running on Java 11" ?

> I have not worked out the details. Perhaps publish the JRE in a just in time
> manner like the /dropins does and then include it in provisioning as usual.

That's I believe the current plan: build the JRE IU from the current JRE during the provisioning step, ignore all bogus ones, and let p2 resolve.
Comment 16 Todor Boev CLA 2020-03-26 13:17:24 EDT
(In reply to Mickael Istria from comment #15)
> Do you have any idea of what kind of requirement (positive or negative)
> could describe "only install a.jre.javase 11 because I'm running on Java 11"
> ?

Yes. The use selects the real Java 11 during installation and p2 installs the corresponding 'a.jre.javase 11.0', which it may have to generate in a just in time manner.

This is how I solved most of our JRE problems. By simply installing explicitly the JRE "I'm running on" with the very first provisioning when the target directory is empty.

We also have logic to "publish" a JRE laying on local disk so it can be used for said installation.
Comment 17 Todor Boev CLA 2020-03-26 13:19:12 EDT
Just to add. Once the 'a.jre.javase' is installed as a p2 root - that is explicitly it can't be changed behind your back. And when it is a singleton that rejects everything else you can't install other JRE's which may be laying around in some p2 repo.
Comment 18 Mickael Istria CLA 2020-03-26 13:23:30 EDT
(In reply to Todor Boev from comment #16)
> Yes. The use selects the real Java 11 during installation and p2 installs
> the corresponding 'a.jre.javase 11.0', which it may have to generate in a
> just in time manner.

That is valid only for people using the installer. Eclipse is shipped also as zipped packages, as rpms/debs on some Linux distributions, as Flatpak, as .dmg.... There are also all the RCP applications that do not use the installer. A solution that only works for the installer is not enough.
And with all installations, users are allowed -and encouraged- to upgrade their JRE independently of upgrading the Eclipse-based app and we expect the Eclipse IDE to react to that and behave correctly, so p2 needs to be able to react to that as well.
Comment 19 Ed Merks CLA 2020-03-27 02:29:07 EDT
(In reply to Todor Boev from comment #13)

As you know, with regard to osgi.ee capabilities, in the end any IU could provide such capabilities, not just the somewhat evil a.jre.javase units, just as any IU can provide java.package capabilities, including ones that are also in the a.jre.javase unit. E.g., many IUs have this:

  <provided namespace='java.package' name='org.w3c.dom' version='0.0.0'/>

For publishing a.jre.javase, I think there are more "use cases" than the current "generate from a profile".  

a) Be able to publish such a thing from the currently running JVM, including from a Java 8 JVM (or even and older one than that). Perhaps for older ones (for Java 1.8), one could fall back to the hard-coded .profile definition. 

b) Be able to publish such a thing from a reference to an on-disk JRE/JDK.  I believe this would require launching a process in which the introspection could be done. We do stuff exactly like this already in Oomph to determine the version of a JRE/JDK.

Of course even for b), this process could be done once per new Java version and can be captured for reuse somewhere as suggested in comment #13 though one caveat is that this would assume the full set of modules with regard to generating java.package capabilities.

It's an interesting point about being singleton, but I'm not sure that would buy us much especially since there are already many a.jre.javase units in the wild any one of which might be available at install/update time...

Note that this was just approved yesterday:

  https://bugs.eclipse.org/bugs/show_bug.cgi?id=561493

So the possibility to install/update a JDK/JRE as an actual p2 IU (provided by Eclipse.org) will be a reality. These will definitely need to be singletons and we should definitely consider how this interacts with the current a.jre.javase design.  So far I've only considered that such an IU simply needs to be installed.  It doesn't actually export any packages and wouldn't actually be useful in a target platform.  And for the new JustJ project, it would be super interesting to consider how to separate out the modules into separate IUs, but that strays from the topic of this Bugzilla.

With regard to ignoring the a.jre.javase IUs, it's very easy to have a negative requirement on a.jre.javase by setting max=0 on the requirement.  This removes from possible consideration all these generated IUs from all p2 repositories.  Of course to get resolution to succeed, a different IU needs to be synthesized that provides the necessary capabilities.

I have simulated this with Oomph's installer, adding a negative requirement, and then resolution fails like this from the explainer:

  ERROR: org.eclipse.equinox.p2.director code=0 Software being installed: artificial_root 1.0.0.v1585286694014
  ERROR: org.eclipse.equinox.p2.director code=1 Cannot satisfy dependency:
    ERROR: org.eclipse.equinox.p2.director code=0 From: artificial_root 1.0.0.v1585286694014
    ERROR: org.eclipse.equinox.p2.director code=0 To: org.eclipse.equinox.p2.iu; a.jre.javase 0.0.0
  ERROR: org.eclipse.equinox.p2.director code=1 Cannot satisfy dependency:
    ERROR: org.eclipse.equinox.p2.director code=0 From: artificial_root 1.0.0.v1585286694014
    ERROR: org.eclipse.equinox.p2.director code=0 To: org.eclipse.equinox.p2.iu; epp.package.committers [4.15.0,5.0.0)
  ERROR: org.eclipse.equinox.p2.director code=1 Cannot satisfy dependency:
    ERROR: org.eclipse.equinox.p2.director code=0 From: Eclipse IDE for Eclipse Committers 4.15.0.20200313-1200 (epp.package.committers 4.15.0.20200313-1200)
    ERROR: org.eclipse.equinox.p2.director code=0 To: osgi.ee; JavaSE 0.0.0

We'd definitely need the explainer to do a better job.  E.g., an IRequirement has a getDescription that could be displayed.  And in the above it's not clear that the a.jre.javase IRequirement has max=0.

To get resolution to succeed, another IU must provide the osgi.ee capabilities and also the java.package capabilities.  This is where I have difficulty in prototyping further because I was just going prototype in a hack where I would just have a requirement on a.jre.javase that restricts the version range to a specific known Java version, but the absence of any a.jre.javase 1.8.0 version in any p2 repository makes that challenging and of course that's not the right solution anyway (and I can't convince Tycho to generate one either).

Imagine if p2 supported some type of "virtual" content metadata repository.  E.g., jre:/... where it might support URIs such as jre:/1.8.0, jre:/11.0.0, and jre:/self, where the latter is the most interesting/relevant one for self install/update; the others could be synthesized based on a known pre-computed profiles but the self one could be synthesize from introspection.  In all cases, this p2 repo could contain the necessary IU and a requirement to this IU could be included in the change request.  That requirement could be optional because something else will necessarily require it.  I say an optional requirement because if there exists also an IU for installing an actual JRE, one that could actually help update from the current self Java 8 to a new Java 11 for example, that IU should be used/resolved instead.

I totally agree that we must have a solution that works for all the use cases, including self install/update (which might also include installing/updating an actual JRE), as well as things like p2 director and the Eclipse Installer (for which you might want to ensure that the created installation does actually resolve all the osgi.ee and java.package requirements expected of that installation).  But as Mickael suggests, the self update issue is super important because we don't want people updating their Java 1.8-based installation to a version of Eclipse that requires Java 11 and then fails to start after the update.

I think all this conceptually possible/doable. The most important missing piece at this point is the sythesis of a p2 repository (IU) for the actual (expected) JRE version.
Comment 20 Mickael Istria CLA 2020-03-28 05:17:53 EDT
I've written a test to verify whether negative requirements against a.jre.javase can work for the case of "upgrading an app running on Java 8 to bundles that require Java 11, and guard against installation with somewhat useful message".
[The test is shown below]

The conclusion is that as a.jre.javase is pre-installed in the profile, we cannot add a negative requirement onto it, without uninstalling that unit from the profile if p2 remediation can do it or having the install operation failing because the negative requirement cannot be fulfilled (and no useful hint).
So negative requirement is IMO to exclude, and I still think that we need a specific flag to 1. exclude installed or available a.jre.javase from dependency resolution and 2. add a synthetic unit from current JRE to the dependnecy resolution context -and ideally have it *not* installed in the profile so it remains a dynamic/volatile unit-.


	@Test
	public void testRequireNewerJRE_withNewerJREUnitInProfile_restrictedToCurrentJRE() throws Exception {
		// install a.jre.javase with newer version in profile
		IInstallableUnit newerJREIU = createJreJavaSEUnit("1000");
		createTestMetdataRepository(new IInstallableUnit[] { newerJREIU });
		InstallOperation installOperation = new InstallOperation(
				new ProvisioningSession(profile1.getProvisioningAgent()),
				Collections.singleton(newerJREIU));
		installOperation.setProfileId(profile1.getProfileId());
		assertTrue(installOperation.resolveModal(null).isOK());
		ProvisioningJob provisioningJob = installOperation.getProvisioningJob(null);
		provisioningJob.schedule();
		provisioningJob.join();
		assertTrue(provisioningJob.getResult().isOK());
		profile1 = getProfile(profile1.getProfileId());
		assertFalse(profile1.query(QueryUtil.createIUAnyQuery(), null).isEmpty());
		//
		ProfileChangeRequest req1 = new ProfileChangeRequest(profile1);
		req1.addInstallableUnits(createIU("dummy-unit-requiring-1.8", Version.create("0.0.1"), new IRequirement[] {
				MetadataFactory.createRequirement("osgi.ee", "(&(osgi.ee=JavaSE)(version=1.8))",
						null, 1, 1, true) }));
		// Try negative requirement in request: does it prevent install at all?
		req1.addInstallableUnits(createIU("dummyNoReq", Version.create("0.0.1")));
		req1.addExtraRequirements(Collections.singleton(MetadataFactory.createRequirement("org.eclipse.equinox.p2.iu",
				"a.jre.javase", VersionRange.create("0.0.0"), null, 0, 0, false)));
		IProvisioningPlan plan1 = planner.getProvisioningPlan(req1, null, null);
		assertEquals(IStatus.OK, plan1.getStatus().getSeverity());
	}
Comment 21 Ed Merks CLA 2020-03-30 01:17:07 EDT
(In reply to Mickael Istria from comment #20)
> I've written a test to verify whether negative requirements against
> a.jre.javase can work for the case of "upgrading an app running on Java 8 to
> bundles that require Java 11, and guard against installation with somewhat
> useful message".
> [The test is shown below]
> 
> The conclusion is that as a.jre.javase is pre-installed in the profile, we
> cannot add a negative requirement onto it, without uninstalling that unit
> from the profile if p2 remediation can do it or having the install operation
> failing because the negative requirement cannot be fulfilled (and no useful
> hint).
> So negative requirement is IMO to exclude, and I still think that we need a
> specific flag to 1. exclude installed or available a.jre.javase from
> dependency resolution and 2. add a synthetic unit from current JRE to the
> dependnecy resolution context -and ideally have it *not* installed in the
> profile so it remains a dynamic/volatile unit-.
> 

Yes, I reached the same conclusion.  There are several issues with this a.jre.javase unit.  Firstly, there are many floating around in the wild (though none for Java 1.8!), and p2's general behavior is to pick the largest available version.  As such, generally I see version 14.0.0 in the profiles of all installations. This already kind of a bogus state because I see exactly that IU in the profile of each installation running on Java 1.8.  It would have been good if this IU were not included in the profile... 

The only purpose of these p2-repo-hosted IUs is of course to make resolution not fail.  But if we change our design goal now with the intent that resolution should fail in some cases, we must exclude somehow *all* the a.jre.javase IUs from all p2 "external" repositories.  A negative requirement to exclude *all* a.jre.javase IUs of course implies that we need to synthesize an IU with a different ID, otherwise the requirement would exclude the synthesized IU as well.  In principle we could instead have a requirement to a restricted version range of the a.jre.javase IU and we could synthesize an "internal" repo that contains correct IUs for the proper full range of Java versions.  But even then, if such an change operation is performed, the 14.0.0 a.jre.javase IU in the profile would have to be potentially downgraded to 1.8.0 (or uninstalled), and that's not really an update but rather involves a remediation.  And even if we did this and made it work in a non-ugly way, i.e., not go into remediation mode, it's still the case that it would be better if this "artificial" IU were not included in the profile *and* it's definitely still necessary to make this behavior optional so that things like the p2 director running on Java 1.8 can still make an installation the requires Java 11 to run.  And for this case, it would be better that rather than having the behavior be mere optional that it were configurable with the expected Java version of the target, which implies we can't just synthesize the IU from the runtime itself (at least with respect to the java.package capabilities)...

Is this test added to an existing class or is it in a new class?  I don't see the code for createJreJavaSEUnit anywhere...

> 
> 	@Test
> 	public void
> testRequireNewerJRE_withNewerJREUnitInProfile_restrictedToCurrentJRE()
> throws Exception {
> 		// install a.jre.javase with newer version in profile
> 		IInstallableUnit newerJREIU = createJreJavaSEUnit("1000");
> 		createTestMetdataRepository(new IInstallableUnit[] { newerJREIU });
> 		InstallOperation installOperation = new InstallOperation(
> 				new ProvisioningSession(profile1.getProvisioningAgent()),
> 				Collections.singleton(newerJREIU));
> 		installOperation.setProfileId(profile1.getProfileId());
> 		assertTrue(installOperation.resolveModal(null).isOK());
> 		ProvisioningJob provisioningJob =
> installOperation.getProvisioningJob(null);
> 		provisioningJob.schedule();
> 		provisioningJob.join();
> 		assertTrue(provisioningJob.getResult().isOK());
> 		profile1 = getProfile(profile1.getProfileId());
> 		assertFalse(profile1.query(QueryUtil.createIUAnyQuery(), null).isEmpty());
> 		//
> 		ProfileChangeRequest req1 = new ProfileChangeRequest(profile1);
> 		req1.addInstallableUnits(createIU("dummy-unit-requiring-1.8",
> Version.create("0.0.1"), new IRequirement[] {
> 				MetadataFactory.createRequirement("osgi.ee",
> "(&(osgi.ee=JavaSE)(version=1.8))",
> 						null, 1, 1, true) }));
> 		// Try negative requirement in request: does it prevent install at all?
> 		req1.addInstallableUnits(createIU("dummyNoReq", Version.create("0.0.1")));
> 	
> req1.addExtraRequirements(Collections.singleton(MetadataFactory.
> createRequirement("org.eclipse.equinox.p2.iu",
> 				"a.jre.javase", VersionRange.create("0.0.0"), null, 0, 0, false)));

This would also exclude the synthesized IU created by createJreJavaSEUnit wouldn't it?  Or does createJreJavaSEUnit create something with a different ID?

> 		IProvisioningPlan plan1 = planner.getProvisioningPlan(req1, null, null);
> 		assertEquals(IStatus.OK, plan1.getStatus().getSeverity());
> 	}
Comment 22 Todor Boev CLA 2020-03-30 05:14:28 EDT
Isn't it a well established invariant that any p2 installation has no unsatisfied requirements? Having this IU that vanishes after provisioning can cause problems in arbitrary unknown code which assumes this is the case.

E.g. I have had issues where an IU was only available during provisioning and was not recorded in the installation which caused *all* subsequent provisioning operations to immediately fail at the very start saying there is an unsatisfied requirement in the current installation.
Comment 23 Mickael Istria CLA 2020-03-30 05:20:35 EDT
(In reply to Todor Boev from comment #22)
> Isn't it a well established invariant that any p2 installation has no
> unsatisfied requirements? Having this IU that vanishes after provisioning
> can cause problems in arbitrary unknown code which assumes this is the case.

I sincerely don't know for sure...

> E.g. I have had issues where an IU was only available during provisioning
> and was not recorded in the installation which caused *all* subsequent
> provisioning operations to immediately fail at the very start saying there
> is an unsatisfied requirement in the current installation.

OK, that's good to know. I'll try to add some extra tests to ensure the installation is "sane" after this hack.
We could maybe try to restrain the check against current JRE as a check, ie apply the suggest filter and compute whether a provisioning plan can be built successfully without a.jre.javase but with the current "volatile" JRE unit, report if failing; and if it's successful, just install against current profile and a.jre.javase unit, so we don't change the actual installation behaviour and risk less regressions; we just add an extra check.
Comment 24 Ed Merks CLA 2020-03-30 05:32:13 EDT
(In reply to Todor Boev from comment #22)
> Isn't it a well established invariant that any p2 installation has no
> unsatisfied requirements? Having this IU that vanishes after provisioning
> can cause problems in arbitrary unknown code which assumes this is the case.
> 

Yes, that would be an inconsistency and that could lead to problems.  But it's also an inconsistency that what the actual runtime does in terms of resolving requirements and wiring packages is different than what's implied by the a.jre.javase IU in p2 profile.

In any case, having it in the profile is not in and of itself a problem.  It's the fact that p2 will tend to want to keep that version or a higher version in the profile that's a problem.  I.e., if one could pretend it's not present in the profile at the start of any install/update operation (just as the actual runtime behaves as if it weren't an actual useful thing to consider for resolution and wiring), all would be better behaved.

> E.g. I have had issues where an IU was only available during provisioning
> and was not recorded in the installation which caused *all* subsequent
> provisioning operations to immediately fail at the very start saying there
> is an unsatisfied requirement in the current installation.

Yes, we have to make sure that the cure is not worse than the disease.  But right now the disease is that there is an IU that says all is great in the profile, but the actual runtime does not thinks all is great because it's using the actual running JRE to satisfy requirements and to do wiring, not the misleading a.jre.javase version in the profile.

Better would be as Mickael appears to be suggesting to record the "volatile" one because at least that one would be accurate at that point in time.  And if it's somehow ignored the next time, an accurate-at-that-point-in-time one would always be recorded.
Comment 25 Todor Boev CLA 2020-03-30 09:02:06 EDT
I am sort of starting to loose the thread of the discussion (sorry for that) so I'll make one last remark.

How about
1) Make sure every provisioning operation has a repository with the "current" JRE added. It can be dynamically built from the current JRE (like the eclipse updater), or statically available when external provisioning is done (like the installer)

2) Make sure each change request contains forces p2 to install that JRE.

3) Let p2 sort the rest: install the JRE if needed, ignore if it's already there, downgrade... And along the way make sure all the other IUs are consistent. Fail if necessary.

In that way handling the JRE bis converted to the issue of providing the correct flavor of JRE p2 repo.

There is perhaps the issue of replacing the bogus JRE with a correct one (under a new name if needed) for existing insallations.

Indeed to handle this I did introduce a new name for the JRE IU and only than made that new IU reject the old 'a.jre.javase', be a singleton, be generatet from a real JRE and also made sure to explicitly request that is is installed.
Comment 26 Mickael Istria CLA 2020-03-30 09:11:55 EDT
(In reply to Todor Boev from comment #25)
> I am sort of starting to loose the thread of the discussion (sorry for that)
> so I'll make one last remark.
> 
> How about
> [...]

As far as I understand, this would break the (currently valid) case of someone willing to install an IU that's only compatible with a newer Java version, which is common at build time.
So this behaviour cannot be the default, and I think for this behavior to remain usable, we need to keep the greedy "a.jre.javase" units in the profile as they enable capability to install incompatible IUs from osgi.ee perspective. I don't think we can just remove the a.jre.javase IU without breaking exist cases.

I'm getting closer from a solution, without adding too much complexity or inconsistency. The general idea is that similarly to ProvisioningContext.setExtraInstallableUnits() that augment the context for 1 resolution without altering the profile, there could be a ProvisioningContext.setIgnoredInstallableUnits(iuQueries) that allow to exclude some IUs from the resolution.
Then, if this works, checking against current JRE becomes only a matter of making a 1st test against a provisioning context that 1. adds current JRE IUs and 2. removes all a.jre.javase.
This check would be optional, controlled in UI, and would be just a check; the actual provisioning steps will remain the same one as usual (against a.jre.javase that's in the profile).
Comment 27 Mickael Istria CLA 2020-03-30 09:53:03 EDT
Ok, I got something that does the job! Assuming the current profile contains a a.jre.javase unit that defines a newer version of the JavaSE compatibility (like JavaSE-1000)

	ProfileChangeRequest req = new ProfileChangeRequest(profile);
	req.addInstallableUnits(actualUnitsToInstall);
	req.addExtraRequirements(Collections.singleton(MetadataFactory.createRequirement("org.eclipse.equinox.p2.iu",
			"a.jre.javase", VersionRange.create("0.0.0"), null, 0, 0, false)));
	req.removeInstallableUnits(aJreJavaSEQuery.toSet().toArray(IInstallableUnit[]::new));
	req.add(createCurrentJavaSEUnit()); // that's still a TODO, I only have a poor hack for this at the moment
	IProvisioningPlan plan1 = planner.getProvisioningPlan(req1, null, null);
	assertEquals(IStatus.OK, plan1.getStatus().getSeverity());

That does check whether the current installation can work against the current JRE, excluding the a.jre.javase.
So if all the units to install match eg. JavaSE-8, status is OK => we can continue install (as usual with the "legacy" way, no need to propagate that "test" plan).
If one unit requires, let's say JavaSE-1000 (which is available in the profile), then status is error with as 1 item in its message:
"""
Cannot satisfy dependency:
  From: dummy-unit-requiring-JavaSE-1000 0.0.1
  To: osgi.ee; (&(osgi.ee=JavaSE)(version=1000))
"""

So it seems like this is a valid way to check compatibility.

1 very good thing with that is that it's just playing with the ProfileChangeRequest, no new API is even necessary!
Comment 28 Ed Merks CLA 2020-04-01 06:18:31 EDT
I think this approach is promising and have also been experimenting with this approach in the context of Oomph's p2 director task...

Firstly there is the issue of synthesizing the a.jre.javase IU.  The osgi.ee capabilities follow a predictable pattern so synthesizing those is pretty straightforward.  The java.package capabilities are more problematic, but using reflective hacking, it's possible to extract this from Storage.findVMProfile or Storage.calculateVMProfile.  For Java 9 or higher the profile properties for the packages are calculated whereas for 1.8 or earlier they loaded from a *.profile.

So I wrote a small application that computes a "self profile", I downloaded all the adoptOpenJDK JREs version 1.8, 9 through 14, and ran that application to capture a *.profile for each one.  I.e., I've captured statically exactly what Equinox actually uses at runtime for the known released Java versions.   For a future version, one could predict that it will have the same package capabilities as the most-recent statically-computed released version and as mentioned already, the osgi.ee capabilities follow a predictable pattern so those too can be computed for a future version.  I also hacked it to produce profiles for 1.5 through 1.7; which just duplicates the profiles in org.eclipse.osgi.  But all this matters only for applications like the director on the installer...

In terms of dynamic synthesis for the self-hosted case on which you are focused,  you can use these constants to access what OSGi (Storage) has computed:

    String systemPackages = System.getProperty(Constants.FRAMEWORK_SYSTEMPACKAGES);
    String systemPackagesExtra = System.getProperty(Constants.FRAMEWORK_SYSTEMPACKAGES_EXTRA);
    String systemCapabilities = System.getProperty(Constants.FRAMEWORK_SYSTEMCAPABILITIES);
    String systemCapabilitiesExtra = System.getProperty(Constants.FRAMEWORK_SYSTEMCAPABILITIES_EXTRA);

The Java version is a little trickier because I don't see a public constants for "java.specification.version" nor for "java.vm.specification.version".  But there's always "java.version" which is parsed like this:

        String javaSpecVersionProp = System.getProperty(EquinoxConfiguration.PROP_JVM_SPEC_VERSION);
        StringTokenizer st = new StringTokenizer(javaSpecVersionProp, " _-"); //$NON-NLS-1$
        javaSpecVersionProp = st.nextToken();
        try {
            String[] vComps = javaSpecVersionProp.split("\\."); //$NON-NLS-1$
            // only pay attention to the first three components of the version
            int major = vComps.length > 0 ? Integer.parseInt(vComps[0]) : 0;
            int minor = vComps.length > 1 ? Integer.parseInt(vComps[1]) : 0;
            int micro = vComps.length > 2 ? Integer.parseInt(vComps[2]) : 0;
            javaVersion = new Version(major, minor, micro);
        } catch (IllegalArgumentException e) {
            // do nothing
        }

This is the same information used by JREAction.  In the ideal world, JREAction could be used to synthesize an IU given a Version as input to the constructor and it could use profiles cases such as the director and use the above information for the case of self hosting; perhaps with a null Version...

So let's assume that we can 1) synthesize dynamically an accurate a.jre.javase IU or 2) can load statically an accurate a.jre.javase IU and can even predict a reasonably accurate future version as well.

What I tried then is exactly like what you did.  I.e., modify the profile change request to remove all a.jre.javase IUs in the profile and then to add my specific synthetic a.jre.javase IU.  But for me, the negative requirement also excludes the IU that I added, so it seems to me that a negative requirement that excludes all a.jre.javase IUs isn't workable.  I.e., it failed like this:

  ERROR: org.eclipse.equinox.p2.director code=0 Software being installed: a.jre.javase 1.8.0.v201003310000
  ERROR: org.eclipse.equinox.p2.director code=0 Software being installed: artificial_root 1.0.0.v1585733168611
  ERROR: org.eclipse.equinox.p2.director code=1 Cannot satisfy dependency:
    ERROR: org.eclipse.equinox.p2.director code=0 From: artificial_root 1.0.0.v1585733168611
    ERROR: org.eclipse.equinox.p2.director code=0 To: org.eclipse.equinox.p2.iu; a.jre.javase 0.0.0

Note here too that I changed my synthesized IU so that it has a qualifier in the version number; otherwise it's problematic that via InstallableUnit.equals the synthesized IU is exactly equal to some IU from a p2 repository all of which have no qualifier in the version.  What I tried then instead is to add an extra requirement on the synthesized IU with version range restriction so that only the synthesized IU can possibly resolve the requirement.   But that doesn't work either, because then we've done nothing to exclude the a.jre.javase IU's from the repos and p2 happily installs both and both end up in the profile.  So I changed the synthesized IU to be a singleton.   Then it's strictly required by the version range constraint, and because it's a singleton it excludes all other a.jre.javase versions.  

To test this further, I created a sample repository with an IU that requires Java 11 and tried to install that IU at the same time.  That fails like this:

  ERROR: org.eclipse.equinox.p2.director code=0 Software being installed: a.jre.javase 1.8.0.v201003310000
  ERROR: org.eclipse.equinox.p2.director code=0 Software being installed: artificial_root 1.0.0.v1585734013695
  ERROR: org.eclipse.equinox.p2.director code=1 Only one of the following can be installed at once:
    ERROR: org.eclipse.equinox.p2.director code=0 a.jre.javase 1.8.0.v201003310000
    ERROR: org.eclipse.equinox.p2.director code=0 a.jre.javase 1.8.0
  ERROR: org.eclipse.equinox.p2.director code=1 Cannot satisfy dependency:
    ERROR: org.eclipse.equinox.p2.director code=0 From: artificial_root 1.0.0.v1585734013695
    ERROR: org.eclipse.equinox.p2.director code=0 To: org.eclipse.equinox.p2.iu; org.example.sample 0.0.0
  ERROR: org.eclipse.equinox.p2.director code=1 Cannot satisfy dependency:
    ERROR: org.eclipse.equinox.p2.director code=0 From: Sample 1.0.0.202003251448 (org.example.sample 1.0.0.202003251448)
    ERROR: org.eclipse.equinox.p2.director code=0 To: osgi.ee; (&(osgi.ee=JavaSE)(version=11))

So I think your approach is promising and I think synthesizing the self-host IU is easily possible using the information in the System.getProperty().  I expect that it having a version qualifier (likely based on the system time) and making it a singleton would be good things.  I expect that this new behavior must remain optional so there must be some new minimal API for enabling/disabling this somewhere...  Perhaps a profile property?

The longer term question is the non-self-hosting case.  Should p2 support that so that the director application can be enhanced to support an argument that can validate/ensure that the installation it creates will actually launch for a given specified Java version? 

There's also the concern about the case where the profile change request will install/update a real JRE that might well be a higher version than the current JRE.   Perhaps that would be addressed if that real JRE IU specified its osgi.ee capabilities and if the requirement we add to the synthesized IU was optional greedy, but I doubt it.
Comment 29 Mickael Istria CLA 2020-04-02 12:47:49 EDT
Created attachment 282321 [details]
Example project and repo for tests

This is just a backup of the project I use to build a repo that allows to test various BREE requirements.
Comment 30 Eclipse Genie CLA 2020-04-03 11:28:19 EDT
New Gerrit change created: https://git.eclipse.org/r/160445
Comment 31 Mickael Istria CLA 2020-04-03 14:06:16 EDT
(In reply to Eclipse Genie from comment #30)
> New Gerrit change created: https://git.eclipse.org/r/160445

This is good for testing. ideally running against 8 < Java version < 14 to test the various combinations against repo built by attachment 282321 [details] .
This gives some good results for installation and updates. It uses former suggestion of creating an extra check derived from the initial provisioning plan, with tweaks to test against current JRE. This is non-intrusive in the application profile as this is only an extra check, nothing more or less or different is installed compared to what's currently happening.
The patch itself is probably not perfect in term of separation and reusability; but it's all internal, so even if we don't improve it in the meantime, it's fixable. However any suggestion is welcome (ideally on Gerrit review) to better organize the code for this new feature, or just to improve labels or whatever. A test is currently failing, I'll work on it. I'll also check how much would MPC reuse of this.
Comment 32 Ed Merks CLA 2020-04-04 03:37:55 EDT
(In reply to Mickael Istria from comment #31)
> (In reply to Eclipse Genie from comment #30)
> > New Gerrit change created: https://git.eclipse.org/r/160445
> 
> This is good for testing. ideally running against 8 < Java version < 14 to
> test the various combinations against repo built by attachment 282321 [details]
> [details] .
> This gives some good results for installation and updates. It uses former
> suggestion of creating an extra check derived from the initial provisioning
> plan, with tweaks to test against current JRE. This is non-intrusive in the
> application profile as this is only an extra check, nothing more or less or
> different is installed compared to what's currently happening.
> The patch itself is probably not perfect in term of separation and
> reusability; but it's all internal, so even if we don't improve it in the
> meantime, it's fixable. However any suggestion is welcome (ideally on Gerrit
> review) to better organize the code for this new feature, or just to improve
> labels or whatever. A test is currently failing, I'll work on it. I'll also
> check how much would MPC reuse of this.

This looks like a good approach for testing the self-hosted resolution case via a separate plan to analyze just the JRE issue resolution issues.  The only incomplete thing I see here is the capabilities synthesis, but that could be addressed quite easily by replicating the logic in org.eclipse.equinox.p2.publisher.actions.JREAction.parseSystemCapabilities(String, MultiStatus, List<IProvidedCapability>).  Perhaps some public static method there could provide for better reuse of this logic, which is quite complex...
Comment 33 Mickael Istria CLA 2020-04-07 10:36:17 EDT
The patch is ready for merge. We'll merge it after M1 is produced.
I'll prepare the N&N entry in the meantime and will push it to Gerrit as well.
I've started looking at MPC, and it unfortunately doesn't reuse the typical ProvisioningOperationWizard, so it doesn't inherit from that change. MPC will have to be modified when the change is available in p2. I'm opening a separate ticket for that.
Comment 34 Eclipse Genie CLA 2020-04-07 15:56:19 EDT
New Gerrit change created: https://git.eclipse.org/r/160613
Comment 35 Matthias Becker CLA 2020-04-08 01:59:41 EDT
Can the error message be improved. I think most user won't know that they have to update their Java VM from that very technical message.
Comment 36 Mickael Istria CLA 2020-04-08 02:50:05 EDT
(In reply to Matthias Becker from comment #35)
> Can the error message be improved. 

Probably, but not from plain p2 engine messages (which need to remain technical since they have more "payload" as it)

> I think most user won't know that they
> have to update their Java VM from that very technical message.

The message mentions "require [...] Java [...] 14", so curious users have a hint.
Others will have to search for this message in their favorite search engine.
It's acceptable IMO for now, but we could open a separate ticket to improve that. A simple string replacement might work.
Comment 39 Michael Keppler CLA 2020-04-11 03:12:15 EDT
Just saw the new and noteworthy. I'm not a native English speaker, but I think the grammar of new the label is not right. Shouldn't it be "...current_ly_ running JRE"?
Comment 40 Eclipse Genie CLA 2020-04-11 16:16:32 EDT
New Gerrit change created: https://git.eclipse.org/r/160799
Comment 42 Eclipse Genie CLA 2020-04-13 08:31:07 EDT
New Gerrit change created: https://git.eclipse.org/r/160807
Comment 44 Leif Geiger CLA 2020-05-26 01:56:07 EDT
Some general thought about the approach used here, that I stumble about when adding the MPC support (bug 561865):
I wonder if it would make more sense to also respect JRE constraints while the planning process in p2 and not in a separated step afterwards. That way, if there is a feature set, that contains optional dependencies, that do not match the JRE constraints, those IUs can easily be left out in the remediation step. That way also a good solution can be presented to the user containing all error messages for missing dependencies including that to the JRE. This would also prevent bugs like 563564. Should we open a new ticket for this?
Comment 45 Ed Merks CLA 2020-05-26 02:37:47 EDT
(In reply to Leif Geiger from comment #44)
> Some general thought about the approach used here, that I stumble about when
> adding the MPC support (bug 561865):
> I wonder if it would make more sense to also respect JRE constraints while
> the planning process in p2 and not in a separated step afterwards. That way,
> if there is a feature set, that contains optional dependencies, that do not
> match the JRE constraints, those IUs can easily be left out in the
> remediation step. That way also a good solution can be presented to the user
> containing all error messages for missing dependencies including that to the
> JRE. This would also prevent bugs like 563564. Should we open a new ticket
> for this?

Yes, I've tried to make that point.  If we synthesized an appropriate IU to replace/supersede the "allow everything" ones in the p2 repositories, we could better reuse the constraint mechanism (though the failures have very technical information).
Comment 46 Mickael Istria CLA 2020-05-26 07:12:15 EDT
(In reply to Leif Geiger from comment #44)
> Some general thought about the approach used here, that I stumble about when
> adding the MPC support (bug 561865):
> I wonder if it would make more sense to also respect JRE constraints while
> the planning process in p2 and not in a separated step afterwards. That way,
> if there is a feature set, that contains optional dependencies, that do not
> match the JRE constraints, those IUs can easily be left out in the
> remediation step. That way also a good solution can be presented to the user
> containing all error messages for missing dependencies including that to the
> JRE. This would also prevent bugs like 563564.

I agree.
But that requires much more work, and more complex one (including new APIs) than the current state. So it's may not be profitable since...

> Should we open a new ticket for this?

I think bug 563564 is enough at the moment.

(In reply to Ed Merks from comment #45)
> If we synthesized an appropriate IU to
> replace/supersede the "allow everything" ones in the p2 repositories, we
> could better reuse the constraint mechanism (though the failures have very
> technical information).

I expect when JustJ is ready and publishes units that contain the packages and osgi.ee capabilities and EPP starts leveraging them, then the packages will stop publishing/including the a.jre.javase stuff and then the issue will be less probable for end-users (it would become a provider issue) and will be less profitable to fix.
Comment 47 Felix Klar CLA 2020-05-28 09:31:47 EDT
Hi. I just tested a fix for a related issue in the Eclipse Marketplace Client  https://bugs.eclipse.org/bugs/show_bug.cgi?id=561865
There (since Eclipse 2020-06 M3) the p2 code that parses the Java version (see Comment 28 of Bug 483383) is called, which throws an exception, when an app is installed with the Marketplace Client under Ubuntu 19.04/19.10 using OpenJDK Java 14 (package 'openjdk-14-jdk') 'openjdk version "14-ea" 2020-03-17; OpenJDK Runtime Environment (build 14-ea+18-Ubuntu-1)'.
There, "java.version" is "14-ea" (early access), which leads to a parsing error, here:

java.lang.IllegalArgumentException: Format "format(n[.n=0;[.n=0;[.S=[A-Za-z0-9_-];='';]]])" was unable to parse 14-ea.0.0
	at org.eclipse.equinox.internal.p2.metadata.VersionFormat.parse(VersionFormat.java:281)
	at org.eclipse.equinox.internal.p2.metadata.VersionParser.parse(VersionParser.java:79)
	at org.eclipse.equinox.p2.metadata.Version.create(Version.java:96)
	at org.eclipse.equinox.internal.p2.ui.ProvUI.getCurrentJavaSEVersion(ProvUI.java:387)
	at org.eclipse.equinox.internal.p2.ui.ProvUI.createCurrentJavaSEUnit(ProvUI.java:352)
	at org.eclipse.equinox.internal.p2.ui.ProvUI.toCompabilityWithCurrentJREProvisioningPlan(ProvUI.java:324)
	at org.eclipse.epp.internal.mpc.ui.wizards.MarketplaceWizard.lambda$5(MarketplaceWizard.java:872)
	at org.eclipse.jface.operation.ModalContext$ModalContextThread.run(ModalContext.java:122)

In Java 14 the API for java.lang.Runtime.Version did change a bit, see https://docs.oracle.com/en/java/javase/14/docs/api/java.base/java/lang/Runtime.Version.html
"A version string consists of a version number optionally followed by pre-release and build information." and see also section "Version strings".

So, "14-ea" is a valid Java 'version string' and should be parsed into "14" plus the pre-release identifier "ea", instead of "14-ea".
Comment 48 Mickael Istria CLA 2020-05-28 09:36:24 EDT
> There, "java.version" is "14-ea" (early access), which leads to a parsing
> error, here:

Thanks. This was already reported in bug 562652 and fixed with https://git.eclipse.org/c/equinox/rt.equinox.p2.git/commit/?id=3076d30d6f0339f204f2b4b5450e3d3f4657ed2d
Comment 49 Jonah Graham CLA 2020-06-18 06:35:54 EDT
This fix has had a side-effect to be aware of. As it prevents installing Java 11 content on Java 8 it actually prevents CDT from installing on Java 8.

CDT has a bundle fragment that requires Java 11 (Bundle-RequiredExecutionEnvironment: JavaSE-11). The original intention of this fragment was to workaround the removal of jaxb in Java 11.

This fragment, on Java 8, is silently ignored by OSGi, but not by p2.

The problem now is that Java 8 users cannot install CDT, or anything else that comes with CDT already as the BREE of Java 11 prevents any installation. (See Bug 564407 Comment 0)

For now I am putting this here so that you are aware of the problem and p2/OSGi mismatch side-effect.
Comment 50 Mickael Istria CLA 2020-06-18 07:49:33 EDT
(In reply to Jonah Graham from comment #49)
> This fix has had a side-effect to be aware of. As it prevents installing
> Java 11 content on Java 8 it actually prevents CDT from installing on Java 8.
> 
> CDT has a bundle fragment that requires Java 11
> (Bundle-RequiredExecutionEnvironment: JavaSE-11). The original intention of
> this fragment was to workaround the removal of jaxb in Java 11.
> 
> This fragment, on Java 8, is silently ignored by OSGi, but not by p2.
> 
> The problem now is that Java 8 users cannot install CDT, or anything else
> that comes with CDT already as the BREE of Java 11 prevents any
> installation. (See Bug 564407 Comment 0)

That's bad.
A workaround is to go to preferences > Install/Update and deselect "Verify that provisioning operation is compatible with current JRE".
In this fragment marked as optional in CDT? I think that would allow p2 to just ignore it.
Comment 51 Jonah Graham CLA 2020-06-18 08:10:42 EDT
(In reply to Mickael Istria from comment #50)
> That's bad.
> A workaround is to go to preferences > Install/Update and deselect "Verify
> that provisioning operation is compatible with current JRE".

Thank you - this workaround is much better. I updated Bug 564407 Comment 2

> In this fragment marked as optional in CDT? I think that would allow p2 to
> just ignore it.

The reason the fragment was not optional was to ensure that if a user upgraded from Java 8 to Java 11 their IDE would not stop working. i.e. the Java 11 bits were designed to sit dormant on Java 8 and activate on Java 11.
Comment 52 Carsten Reckord CLA 2020-06-18 13:36:23 EDT
(In reply to Mickael Istria from comment #46)
> I agree.
> But that requires much more work, and more complex one (including new APIs)
> than the current state. So it's may not be profitable since...
> 
> (In reply to Ed Merks from comment #45)
> > If we synthesized an appropriate IU to
> > replace/supersede the "allow everything" ones in the p2 repositories, we
> > could better reuse the constraint mechanism (though the failures have very
> > technical information).
> 
> I expect when JustJ is ready and publishes units that contain the packages
> and osgi.ee capabilities and EPP starts leveraging them, then the packages
> will stop publishing/including the a.jre.javase stuff and then the issue
> will be less probable for end-users (it would become a provider issue) and
> will be less profitable to fix.

Sorry, I'm very late to the party and don't want to play the party pooper now, but:

I don't think this is a question of "profitable". This is a question of contracts and breaking them in really "interesting" ways.

To be honest, when Leif and I discussed the proposed change in MPC, had it not been already too late in the release to change anything meaningful, I would have advocated for rejecting it outright. But at the time, I guess I long missed my window of opportunity both here and in the larger discussion of Platform pushing the need for this.

But back to P2: While the current approach more or less does the job for the cases at hand (current CDT issues notwithstanding), my issue with it is that it violates the general contract of the P2 planner in multiple ways:

1. The planner is supposed to transform a valid profile and a change request into a new valid profile, or fail if the change would violate the given constraints. Now we suddenly have constraints outside of the planner's domain that decide about the plan's viability. That's bad, because every P2 client will have to actively mind that. And if they don't, they'll break the system. So I'd say you did actually change the API, just not in the strict, technical sense.

2. Since the planner doesn't know about these additional constraints, it can't come up with a solution that fulfills them. So we might see plans that get rejected by the JVM constraint afterwards, where the planner could have found an alternative solution that would have fulfilled it.

Case in point for 1: MPC. And I guess remediation is a good example for both. Not sure what other P2 consumers are out there, but they have a good chance of wreaking havoc as well.

I hope that JustJ puts us in a position where we don't have to worry about this anymore (what do you think would be a time frame to see this?). But I would advocate strongly for a bug to track this and revisit a proper fix - maybe with the benefit that JustJ makes the problem easier to solve in the future (or to decide that the current solution is not needed anymore and can be removed).
Comment 53 Mickael Istria CLA 2020-06-18 14:41:07 EDT
(In reply to Carsten Reckord from comment #52)
> 1. The planner is supposed to transform a valid profile and a change request
> into a new valid profile, or fail if the change would violate the given
> constraints. Now we suddenly have constraints outside of the planner's
> domain that decide about the plan's viability. That's bad, because every P2
> client will have to actively mind that. And if they don't, they'll break the
> system. So I'd say you did actually change the API, just not in the strict,
> technical sense.

The reality is that without this check, the planner would be turning a valid profile and a change request into a non-valid profile that failed at starting the IDE. This issue was critical, especially with upcoming release of bundles with Java 11 as BREE.
Sure, it could probably have been done better; p2 should probably not have dummy JRE units statically defined in the profile, and so on... But realistically, after multiple attempts to tweak things here and there, making p2 "natively" understand BREEs and JREs is/was quite difficult and would have induced a lot of other troubles, mainly at build-time; so the extra check which happens only on UI side was the only affordable working way to do it at that point of time.
But it's not set in stone and the current design isn't really essential here, as long as there is a solution. Any contribution to improve that is welcome. Probably the planner is the best place where to add such extra-check...
 
> 2. Since the planner doesn't know about these additional constraints, it
> can't come up with a solution that fulfills them. So we might see plans that
> get rejected by the JVM constraint afterwards, where the planner could have
> found an alternative solution that would have fulfilled it.

Same as above: contributions to improve this are really welcome

> I hope that JustJ puts us in a position where we don't have to worry about
> this anymore (what do you think would be a time frame to see this?).

JustJ has produced some builds and EPP is open to contributions. I have it in my bucket list, but it's relatively deep under a lot of other more important issues. So the best way to shorten the timeframe is to try contributing it to EPP right now.

> But I
> would advocate strongly for a bug to track this and revisit a proper fix -

Sure, please open such bug.
Comment 54 Ed Merks CLA 2020-06-19 02:57:38 EDT
I agree that the current solution is less that ideal, but this in a context that is far from ideal to begin with.   Firstly, as Mickael mentions, these "fake" a.jre.javase IUs are questionable at best.  Their only purpose is to provide capabilities such at that p2 is always is able to satisfy package imports that are in principle available in the JRE and is able to satisfy all possible BREE requirements (EE requirements) regardless of what JRE is/will actually be used.  

So the notion that p2 can properly resolve such requirements (and find alternatives) is completely broken by those fake IUs.  They serve the opposite role of mostly just disabling that by satisfying everything possible.  So the only "proper" solution is to have negative requirements to exclude all of them and then to provide "proper" IUs based on the actual/expect JRE.  Doing that for the actual JRE is quite easy, and Mickael has implemented that in his solution.  Doing that for the expected JRE (as when the director or installer is creating/materializing an installation) is way more challenging, i.e., where to get the correct information for Java 9, 10, 11, 12, 13, and 14? And for future versions not yet known/available.

Given that OSGi (Equinox) no longer provides profiles for these because they are computed dynamically from the actual JRE, somewhere else static profiles are needed.  I've investigated how do that in Oomph and have a prototype solution. But given that my spare time is not copious and given that everything I do is paid for by me, there is a limit, especially when an alternative compromise solution is chosen despite effort invested in a more proper solution...

The JustJ JREs do specify negative requirements on the fake JRE IUs, but this leads to Tycho problems as seen in this thread:

https://www.eclipse.org/lists/justj-dev/msg00006.html

My impression is that Tycho is completely confused by negative requirements (or is hard-coded to add a requirement on the "expected profile").  In one case, adding a profile property to filter out the negative requirements helps, but in the case of an actual <target>, that doesn't work and resolveWithExecutionEnvironmentConstraints needs to be disabled completely.  Which is annoying because I want it to resolve/test those properly using the JustJ JREs which provide exactly the correct java.package capabilities and ee capabilities for the actual real JRE that will be installed in the product.

I'm not sure how best to move forward from this current situation especially in a context where real JRE IUs are available and should be what's used to do proper resolution.
Comment 55 Mickael Istria CLA 2020-11-01 12:27:37 EST
*** Bug 567947 has been marked as a duplicate of this bug. ***