Bug 569871 - p2 silently installs justj & replaces JDK 11 by JDK 15 from justj
Summary: p2 silently installs justj & replaces JDK 11 by JDK 15 from justj
Status: CLOSED WONTFIX
Alias: None
Product: Equinox
Classification: Eclipse Project
Component: p2 (show other bugs)
Version: 4.19   Edit
Hardware: All All
: P3 critical (vote)
Target Milestone: ---   Edit
Assignee: P2 Inbox CLA
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2020-12-22 05:16 EST by Andrey Loskutov CLA
Modified: 2024-05-13 10:07 EDT (History)
5 users (show)

See Also:


Attachments
crash on restart (53.04 KB, image/png)
2020-12-22 05:16 EST, Andrey Loskutov CLA
no flags Details
Update site that results in sporadic JustJ installation. (29.12 KB, application/zip)
2021-01-20 06:12 EST, Simeon Andreev CLA
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Andrey Loskutov CLA 2020-12-22 05:16:50 EST
Created attachment 285098 [details]
crash on restart

1) Download 4.19 SDK nightly build from https://download.eclipse.org/eclipse/downloads/drops4/I20201221-1800/

2) Extract & start Eclipse (using Java 11)

3) Download the following file: http://git.eclipse.org/c/platform/eclipse.platform.ui.git/plain/releng/org.eclipse.ui.releng/platformUiTools.p2f

4) Change 
<repository location='https://download.eclipse.org/webtools/downloads/drops/R3.19.0/I-latest/repository/'/>
to 
<repository location='https://download.eclipse.org/webtools/downloads/drops/R3.20.0/I-latest/repository/'/>

5) Select and install all plug-ins described by this file by using File>Import...>Install>Install Software Items from File.

6) Select "Install latest versions of selected software"

7) Do NOT select "Contact all update sites", continue

8) After installer fetches information about available software, before final install step, inspect proposed to update items: *no* justj anywhere.

9) Proceed with the wizard & restart Eclipse if requested after update. Note: the download takes a lot longer compared to "usual" plugins install - p2 silently loads ~120 MB JRE package from justj.

10) After proposed restart, Eclipse crashes immediately on next startup. Note that the error dialog already points to justj jre. Unfortunately no log.

11) Now inspect eclipse.ini: it uses justj JDK 15:

-vm
plugins/org.eclipse.justj.openjdk.hotspot.jre.full.linux.x86_64_15.0.1.v20201027-0507/jre/bin

12) Second attempt to start Eclipse succeeds, but unfortunately Eclipse now uses JDK 15 and not the original one.

If using "old" webtools update site in step 4, no unattended JDK update & no crash on restart happens. One could blame webtools, but I can't imagine they require Java 15 to be installed, so I assume some weird p2 issue.

1) The system should not silently replace my JDK.
2) There should be no crash if user wants to update JDK.

My main concern now is 1). I don't want to use any 3rd party JDK's and I don't want that they are *silently* installed. I should be either be able to confirm/deny this installation or (ideally) it should not be proposed at all if not explicitly requested.
Comment 1 Andrey Loskutov CLA 2020-12-23 07:54:17 EST
Ed, do you have any idea why webtools would cause p2 to install justj?
Comment 2 Andrey Loskutov CLA 2020-12-23 08:15:19 EST
Another related issue is bug 567504.

I don't want that *any* of engineers in my lab or our customers end up running Eclipse/our product on unsupported JVM after some "innocently looking" plugin installation.

And it looks like the only way to prevent it now is to switch Internet off (or fully disable updates), because any arbitrary update site might cause p2 to silently install JustJ).
Comment 3 Ed Merks CLA 2020-12-23 08:35:53 EST
(In reply to Andrey Loskutov from comment #1)
> Ed, do you have any idea why webtools would cause p2 to install justj?

I think it was tracked down to this:

https://bugs.eclipse.org/bugs/show_bug.cgi?id=567504#c16

I.e., it happens when there are package imports that aren't satisfied by the fake and incomplete a.jre.javase IUs but are satisfied by the accurately-computed JustJ JRE IUs.


I think this Bugzilla suggests that a IU be synthesized from the actual current JRE of the installation so that package imports are properly resolved against the JRE that will actually be used at runtime:

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

But in all cases, it's hard to image how it can be resolved and installed unless there is content metadata about its existence and artifact metadata its location.  This is why I suggested during the last release cycle that the EPP repo not contain the JustJ JREs.
Comment 4 Andrey Loskutov CLA 2020-12-23 09:00:45 EST
(In reply to Ed Merks from comment #3)
> (In reply to Andrey Loskutov from comment #1)
> > Ed, do you have any idea why webtools would cause p2 to install justj?
> 
> I think it was tracked down to this:
> 
> https://bugs.eclipse.org/bugs/show_bug.cgi?id=567504#c16
> 
> I.e., it happens when there are package imports that aren't satisfied by the
> fake and incomplete a.jre.javase IUs but are satisfied by the
> accurately-computed JustJ JRE IUs.

So having a "dummy" bundle that provides all system packages would prevent p2 to install bundles like JustJ? But now someone adds a package import that resolves in Java 15 but not in Java 11 and JustJ will be still installed...

> I think this Bugzilla suggests that a IU be synthesized from the actual
> current JRE of the installation so that package imports are properly
> resolved against the JRE that will actually be used at runtime:
> 
> https://bugs.eclipse.org/bugs/show_bug.cgi?id=568150
> 
> But in all cases, it's hard to image how it can be resolved and installed
> unless there is content metadata about its existence and artifact metadata
> its location.  This is why I suggested during the last release cycle that
> the EPP repo not contain the JustJ JREs.

This is also my point here: *any* 3rd party update site can now install & replace current JDK, JustJ or not, irrelevant.

<offtopic>
Is this not a critical security issue? With "patched" JDK that is happily installed by p2 one can do basically everything.

OK, probably this was possible already before, but I've realized it only now.
So one installs a bundle that contained some binaries and p2 instructions to use that binaries as JRE for the current Eclipse, and on next restart the current installation is hacked :-(.
</offtopic>

Back to the JustJ case (which is what is more likely to happen now with webtools update site).

Do we have a way in p2 to black list some bundles from being installed? I could add some preference/whatever needed in our custom product to do just that, so that JustJ would never be a choice for p2?

Should we add a preferences for p2 to:
1) On resolve also check current JRE packages?
2) Black/white list for namespaces/bundles for update process?
Comment 5 Ed Merks CLA 2020-12-23 09:30:17 EST
(In reply to Andrey Loskutov from comment #4)
> So having a "dummy" bundle that provides all system packages would prevent
> p2 to install bundles like JustJ? But now someone adds a package import that
> resolves in Java 15 but not in Java 11 and JustJ will be still installed...
> 

The purpose of the a.jre.javase IUs is to satisfy the EE requirements and java.package requirements that will be satisfied by a real JRE at runtime, so yes, normally having such things should suffice (and are necessary for resolution to succeed).

> This is also my point here: *any* 3rd party update site can now install &
> replace current JDK, JustJ or not, irrelevant.
> 

Yes, this has always been true.

> <offtopic>
> Is this not a critical security issue? With "patched" JDK that is happily
> installed by p2 one can do basically everything.
> 

Content is signed and one must trust the origin.

> OK, probably this was possible already before, but I've realized it only now.
> So one installs a bundle that contained some binaries and p2 instructions to
> use that binaries as JRE for the current Eclipse, and on next restart the
> current installation is hacked :-(.
> </offtopic>
> 
Well, it's not as if the IDE runs with a security manager so any bundle can hack the system, not just the JRE.

> Back to the JustJ case (which is what is more likely to happen now with
> webtools update site).
> 

Where is the system find the JustJ JRE?

> Do we have a way in p2 to black list some bundles from being installed? I
> could add some preference/whatever needed in our custom product to do just
> that, so that JustJ would never be a choice for p2?
> 

You can have negative requirements max = 0.

> Should we add a preferences for p2 to:
> 1) On resolve also check current JRE packages?
> 2) Black/white list for namespaces/bundles for update process?

There was such a check as 1) added, and at the time I suggested it would be better to synthesize an IU from the running JRE, but that wasn't chosen as the solution.

And however much I'd like to help, I have no spare resource to invest anymore.
Comment 6 Simeon Andreev CLA 2021-01-04 08:40:41 EST
Any code area I can debug in, to find out why JustJ is added to the list of plug-ins to install?

Looking at the update site contents and Import-Package statements in WebTools plug-ins (or plug-ins required by WebTools, that are installed along), I find nothing odd. At least nothing that the "fake JRE IU" doesn't provide already.
Comment 7 Mickael Istria CLA 2021-01-04 09:22:52 EST
Could it be related to https://bugs.eclipse.org/bugs/show_bug.cgi?id=567504#c15 ? Is CloudFoundry Tools still there?

(In reply to Ed Merks from comment #5)
> There was such a check as 1) added, and at the time I suggested it would be
> better to synthesize an IU from the running JRE, but that wasn't chosen as
> the solution.

Yet I think there was/is a general agreement that this approach of re-synthetizing IUs at runtime (which I tried to capture in bug 568150) is actually the best one forward; it appeared to be too difficult to implement in time back then. I still think it's relatively difficult (especially for the build related tasks where the product is not running), but as we're less under pressure with an upcoming move to a new JRE soon and thanks JustJ allowing to decently make everything work, the timing is good to try implementing this better approach which should fix things more sustainably.
Comment 8 Simeon Andreev CLA 2021-01-05 02:51:36 EST
(In reply to Mickael Istria from comment #7)
> Could it be related to
> https://bugs.eclipse.org/bugs/show_bug.cgi?id=567504#c15 ? Is CloudFoundry
> Tools still there?

I don't see CloudFoundry Tools installed when I install the XML editor feature from WebTools (which is sufficient to reproduce the problem). I also don't find anything about com.sun.jdi in the list of required namespace packages of the update site (I also don't find matches in the 2 WebTools git repositories I have checked out, common and sourceediting).

I've tried comparing required namespace packages from the update site, to the diff between the JustJ IU provided packages and the JDK 11 "fake" IU provided packages; I don't find any package that would be in the JustJ IU, but not in the JDK 11 IU.

I did the comparison with those IUs, maybe they are the wrong ones?

https://download.eclipse.org/oomph/archive/reports/download.eclipse.org/releases/2020-12/index/a.jre.javase_11.0.0.html

https://download.eclipse.org/oomph/archive/reports/download.eclipse.org/releases/2020-12/index/org.eclipse.justj.openjdk.hotspot.jre.full.linux.x86_64_15.0.1.v20201027-0507.html
Comment 9 Mickael Istria CLA 2021-01-05 03:02:59 EST
(In reply to Simeon Andreev from comment #8)
> I've tried comparing required namespace packages from the update site, to
> the diff between the JustJ IU provided packages and the JDK 11 "fake" IU
> provided packages; I don't find any package that would be in the JustJ IU,
> but not in the JDK 11 IU.

I'm not sure comparing IUs is enough to find the cause here.
What I did back then to identify CloudFoundry was causing the transitive requirement was that I did a dichotomy on the proposed installed artifacts. However, that case of CFT was during an update with the regular update wizard; I don't know whether "Install Software Items from File" has the same feedback flow that would help in leveraging ths approach.
Comment 10 Simeon Andreev CLA 2021-01-05 08:20:31 EST
I cannot reproduce the problem right now. Is some server "down"? In particular I've made a minimal update site that has a plug-in using "com.sun.jdi.connect"; now this doesn't result in installing JustJ (along with WebTools).

Anyway, we would like to disable this p2 action in our product: org.eclipse.equinox.internal.p2.touchpoint.eclipse.actions.SetJvmAction

In particular the JustJ IU will run this touch point after its installed. Its not great that the plug-in is pulled along with what we actually want to install, but we don't want to use this JDK at all. Especially not without our knowledge and explicit "approval".

Is there already available functionality for this?

Background:

We have several Java applications in our applications and cannot afford to have our Eclipse based products to "randomly" run with a JDK on which we have no coverage (and which we don't plan to "maintain" or use). We don't want that our products use a different JDK after users installed some harmless-looking plug-ins,
Comment 11 Mickael Istria CLA 2021-01-05 08:24:04 EST
If you don't want JustJ installed in your product, you can tweak the p2.inf to add a "negative requirement" on JustJ. This can be achieved similarly to adding a regular requirement but setting the min & max cardinality to 0.
Comment 12 Andrey Loskutov CLA 2021-01-05 09:55:36 EST
(In reply to Mickael Istria from comment #11)
> If you don't want JustJ installed in your product, you can tweak the p2.inf
> to add a "negative requirement" on JustJ. This can be achieved similarly to
> adding a regular requirement but setting the min & max cardinality to 0.

Mickael, what should *I* do if I don't want justj to be installed for SDK by default with steps from this bug description? I do this procedure every work day, and for sure I can't submit p2.inf for SDK itelf, but also I don't want to download JustJ every day (and also don't want to use it either).

Should I add a plugin to the installation that has "negative requirement" on JustJ in p2.inf? Would that work - and who will win - my plugin or plugin that wants to install JustJ (we still didn't identified why it is pulled by p2 for webtools)?
Comment 13 Mickael Istria CLA 2021-01-05 10:00:49 EST
(In reply to Andrey Loskutov from comment #12)
> Should I add a plugin to the installation that has "negative requirement" on
> JustJ in p2.inf? Would that work

Yes, that should work.
Note that it can be any p2 installable unit (eg a feature).

> and who will win - my plugin or plugin
> that wants to install JustJ (we still didn't identified why it is pulled by
> p2 for webtools)?

If your "say-no-to-JustJ" plugin is installed explicitly, as an installation root and not as a transitive requirement; it should always win and never be uninstalled without your explicit request or approval (in case of remdiation), and thus JustJ should never be installed without at least a remediation page review.
Comment 14 Simeon Andreev CLA 2021-01-06 06:56:10 EST
OK I tried installing a minimal plug-in and a feature for it, the feature packages a p2.inf file with the following lines:

requires.0.namespace = org.eclipse.equinox.p2.iu
requires.0.name = org.eclipse.justj.openjdk.hotspot.jre.full
requires.0.min = 0
requires.0.max = 0
requires.1.namespace = org.eclipse.equinox.p2.iu
requires.1.name = org.eclipse.justj.openjdk.hotspot.jre.full.linux.x86_64
requires.1.min = 0
requires.1.max = 0

When trying to install my mini-reproducer (Import-Package with a package from JDK 15, that is available in JustJ IU but not in the "fake" JDK 15 IU), I now see a prompt that asks me to remove the feature with the p2.inf file. This is good.

I can however install WebTools (https://download.eclipse.org/webtools/downloads/drops/R3.20.0/I-latest/repository/) without a problem; JustJ is not included. Though I've noticed JustJ seems to be added *sporadically*, not always (???) so I'm not 100% sure about this.

Is it possible that JustJ is installed *optionally* along with WebTools, without anything actually needing it? I tried debugging p2 code and landed in org.eclipse.equinox.internal.p2.director.Projector (that delegates to a SAT solver without any sources attached, not that they would help). There I see some of these tracing outputs (though I assume they are "normal"/"expected"):

Optional dependency: AbstractVariable: 38142950->[org.eclipse.justj.openjdk.hotspot.jre.full.linux.x86_64 15.0.1.v20201027-0507]

(the line that adds this implication is marked with a FIXME and was added here: https://git.eclipse.org/c/equinox/rt.equinox.p2.git/commit/?id=c46e91eb86)
Comment 15 Andrey Loskutov CLA 2021-01-07 05:06:48 EST
(In reply to Simeon Andreev from comment #14)
> OK I tried installing a minimal plug-in and a feature for it, the feature
> packages a p2.inf file with the following lines:
> 
> requires.0.namespace = org.eclipse.equinox.p2.iu
> requires.0.name = org.eclipse.justj.openjdk.hotspot.jre.full
> requires.0.min = 0
> requires.0.max = 0
> requires.1.namespace = org.eclipse.equinox.p2.iu
> requires.1.name = org.eclipse.justj.openjdk.hotspot.jre.full.linux.x86_64
> requires.1.min = 0
> requires.1.max = 0

I've added this to my plugindependencies feature, put it to update site but I still can reproduce steps from comment 0. 

P2 is happily downloads JustJ to my workstation despite the fact *both* my feature and XML editors were shown as selected and proposed to be installed, and we know that XML feature is what triggers JustJ install. 

After installation, *both* justj and my plugin were installed (& Eclipse happily crashed at startup as expected).

Going to test with the p2.inf in the plugin.
Comment 16 Andrey Loskutov CLA 2021-01-07 05:47:48 EST
(In reply to Andrey Loskutov from comment #15)
> I've added this to my plugindependencies feature, put it to update site but
> I still can reproduce steps from comment 0. 
> 
> P2 is happily downloads JustJ to my workstation despite the fact *both* my
> feature and XML editors were shown as selected and proposed to be installed,
> and we know that XML feature is what triggers JustJ install. 
> 
> After installation, *both* justj and my plugin were installed (& Eclipse
> happily crashed at startup as expected).
> 
> Going to test with the p2.inf in the plugin.

OK, I've realized I never had a pleasure to generate p2 metadata for p2, and obviously time is now to do so. With p2 metadata generated I see no JustJ installed, and all plugins I've requested were installed (except one unrelated, because p2 was not able to figure out where the plugin was, even if site.xml told him so via "<archive path"). No wonder, it is p2, why should it support something that was there for ages...

So I can confirm, the workaround seem to work.
Comment 17 Simeon Andreev CLA 2021-01-19 05:43:27 EST
> Anyway, we would like to disable this p2 action in our product:
> org.eclipse.equinox.internal.p2.touchpoint.eclipse.actions.SetJvmAction
> 
> ...
> 
> Is there already available functionality for this?

What about this? I don't see anything that can be used to disable the touchpoint, at least on first glance. Did I miss something? If not, can we (me or Andrey) add a preference to disable touchpoints (e.g. by ID/class name, or just this specific one)? E.g. throwing an exception when that touchpoint is ran.

See comment 10 for full context.
Comment 18 Mickael Istria CLA 2021-01-19 08:03:24 EST
(In reply to Simeon Andreev from comment #17)
> disable this p2 action in our product:
> org.eclipse.equinox.internal.p2.touchpoint.eclipse.actions.SetJvmAction [...] What about this?

I'm not aware of such a way to disable touchpoints

> If not, can we
> (me or Andrey) add a preference to disable touchpoints (e.g. by ID/class
> name, or just this specific one)? E.g. throwing an exception when that
> touchpoint is ran.

It would be worth investigating whether p2 allows to query artifacts according to their touchpoint. If it's possible, then you can just add a bundle with a negative (min=max=0) against artifacts that do define this touchpoint.
Comment 19 Simeon Andreev CLA 2021-01-19 08:27:39 EST
(In reply to Mickael Istria from comment #18)
> It would be worth investigating whether p2 allows to query artifacts
> according to their touchpoint. If it's possible, then you can just add a
> bundle with a negative (min=max=0) against artifacts that do define this
> touchpoint.

Just to be sure, reading https://wiki.eclipse.org/Equinox/p2/Customizing_Metadata, you mean try to use this?


> Negative requirements can be published by setting min and max values on the
> requirement to 0. For example
>
> metaRequirements.<#>.matchExp = {p2QL expression} (note that in this case the namespace, name and range attributes are not used)
> ...
Comment 20 Mickael Istria CLA 2021-01-19 08:28:37 EST
(In reply to Simeon Andreev from comment #19)
> Just to be sure, reading
> https://wiki.eclipse.org/Equinox/p2/Customizing_Metadata, you mean try to
> use this?
> 
> 
> > Negative requirements can be published by setting min and max values on the
> > requirement to 0. For example
> >
> > metaRequirements.<#>.matchExp = {p2QL expression} (note that in this case the namespace, name and range attributes are not used)
> > ...

Yes.
Comment 21 Simeon Andreev CLA 2021-01-19 11:04:29 EST
Looking at org.eclipse.equinox.p2.tests.ql.EvaluatorTest, this seems to work:

diff --git a/bundles/org.eclipse.equinox.p2.tests/src/org/eclipse/equinox/p2/tests/ql/EvaluatorTest.java b/bundles/org.eclipse.equinox.p2.tests/src/org/eclipse/equinox/p2/tests/ql/EvaluatorTest.java
index 147e2ddd7..8f061fc5f 100644
--- a/bundles/org.eclipse.equinox.p2.tests/src/org/eclipse/equinox/p2/tests/ql/EvaluatorTest.java
+++ b/bundles/org.eclipse.equinox.p2.tests/src/org/eclipse/equinox/p2/tests/ql/EvaluatorTest.java
@@ -217,6 +217,15 @@ public class EvaluatorTest extends AbstractProvisioningTest {
                assertEquals(queryResultSize(result), 616);
        }
 
+       public void testQueryTouchpointSetJvm() throws Exception {
+               IMetadataRepository repo = getMDR("/testData/bug569871");
+               IQueryResult<IInstallableUnit> result = repo.query(QueryUtil.createMatchQuery(//
+                               "touchpointType != null && touchpointType.id == 'org.eclipse.equinox.p2.osgi' && touchpointData.collect(t | t.instructions).flatten().exists(t | t.key == 'install')"),
+                               new NullProgressMonitor());
+               assertEquals("Found wrong number of touchpoints that specify action for setting JVM", 1,
+                               queryResultSize(result));
+       }
+
        public void testTraverse() throws Exception {
                IMetadataRepository repo = getMDR("/testData/galileoM7");
                IQueryResult<IInstallableUnit> result = repo.query(QueryUtil.createQuery(//
diff --git a/bundles/org.eclipse.equinox.p2.tests/testData/bug569871/content.xml b/bundles/org.eclipse.equinox.p2.tests/testData/bug569871/content.xml
new file mode 100644
index 000000000..7540d9b98
--- /dev/null
+++ b/bundles/org.eclipse.equinox.p2.tests/testData/bug569871/content.xml
@@ -0,0 +1,58 @@
+<?xml version='1.0' encoding='UTF-8'?>
+<?metadataRepository class='org.eclipse.equinox.internal.p2.metadata.repository.LocalMetadataRepository' version='1.0.0'?>
+<repository name='Good Test Repository' type='org.eclipse.equinox.internal.p2.metadata.repository.LocalMetadataRepository' version='1' description='Good test repository description'>
+  <properties size='3'>
+    <property name='p2.system' value='true'/>
+    <property name='p2.timestamp' value='1221680367875'/>
+    <property name='site.checksum' value='2404093275'/>
+  </properties>
+  <units size='3'>
+    <unit id='test.bundle' version='1.0.0' singleton='false'>
+      <update id='test.bundle' range='[0.0.0,1.0.0)' severity='0'/>
+      <properties size='2'>
+        <property name='org.eclipse.equinox.p2.partial.iu' value='true'/>
+        <property name='org.eclipse.equinox.p2.type.group' value='true'/>
+      </properties>
+      <provides size='3'>
+        <provided namespace='org.eclipse.equinox.p2.iu' name='test.bundle' version='1.0.0'/>
+        <provided namespace='osgi.bundle' name='test.bundle' version='1.0.0'/>
+        <provided namespace='org.eclipse.equinox.p2.eclipse.type' name='bundle' version='1.0.0'/>
+      </provides>
+      <artifacts size='1'>
+        <artifact classifier='osgi.bundle' id='test.bundle' version='1.0.0'/>
+      </artifacts>
+      <touchpoint id='org.eclipse.equinox.p2.osgi' version='1.0.0'/>
+      <touchpointData size='1'>
+        <instructions size='1'>
+          <instruction key='manifest'>
+          </instruction>
+        </instructions>
+      </touchpointData>
+    </unit>
+
+    <unit id='test.bundle' version='2.1.0' singleton='false'>
+      <update id='test.bundle' range='[0.0.0,1.0.0)' severity='0'/>
+      <properties size='2'>
+        <property name='org.eclipse.equinox.p2.partial.iu' value='true'/>
+        <property name='org.eclipse.equinox.p2.type.group' value='true'/>
+      </properties>
+      <provides size='3'>
+        <provided namespace='org.eclipse.equinox.p2.iu' name='test.bundle' version='1.0.0'/>
+        <provided namespace='osgi.bundle' name='test.bundle' version='1.0.0'/>
+        <provided namespace='org.eclipse.equinox.p2.eclipse.type' name='bundle' version='1.0.0'/>
+      </provides>
+      <artifacts size='1'>
+        <artifact classifier='osgi.bundle' id='test.bundle' version='1.0.0'/>
+      </artifacts>
+      <touchpoint id='org.eclipse.equinox.p2.osgi' version='1.0.0'/>
+      <touchpointData size='1'>
+        <instructions size='2'>
+       <instruction key="uninstall">
+         org.eclipse.equinox.p2.touchpoint.eclipse.setJvm(jvm:null);       </instruction>
+       <instruction key="install">
+         org.eclipse.equinox.p2.touchpoint.eclipse.setJvm(jvm:${artifact.location}/jre/bin);org.eclipse.equinox.p2.touchpoint.eclipse.chmod(targetDir:${artifact.location},targetFile:jre/bin,permissions:+x,options:-R);org.eclipse.equinox.p2.touchpoint.eclipse.chmod(targetDir:${artifact.location},targetFile:jre/lib/jexec,permissions:+x);org.eclipse.equinox.p2.touchpoint.eclipse.chmod(targetDir:${artifact.location},targetFile:jre/lib/jspawnhelper,permissions:+x);       </instruction>
+        </instructions>
+      </touchpointData>
+    </unit>
+  </units>
+</repository>

I'll try the query soon, though I assume it needs at least some sort of adjustments (I'm not 100% sure what the test does to create a "match" query, I've no experience with p2 / p2ql).
Comment 22 Simeon Andreev CLA 2021-01-20 02:44:23 EST
> +               IQueryResult<IInstallableUnit> result =
> repo.query(QueryUtil.createMatchQuery(//
> +                               "touchpointType != null && touchpointType.id
> == 'org.eclipse.equinox.p2.osgi' && touchpointData.collect(t |
> t.instructions).flatten().exists(t | t.key == 'install')"),

Looks like I removed the wrong code when cleaning up my test code. The query was meant to be as follows:

IQueryResult<IInstallableUnit> result = repo.query(QueryUtil.createMatchQuery(//
				"touchpointType != null && touchpointType.id == 'org.eclipse.equinox.p2.osgi' && touchpointData.exists(t | t.instructions.exists(entry | entry.value.body ~= /*org.eclipse.equinox.p2.touchpoint.eclipse.setJvm*/))"),
				new NullProgressMonitor());
Comment 23 Simeon Andreev CLA 2021-01-20 06:00:05 EST
I created an update site, with a feature that contains a p2.inf file with contents:

metaRequirements.0.matchExp = everything.select(x | x.touchpointType != null && x.touchpointType.id == 'org.eclipse.equinox.p2.osgi' && x.touchpointData.exists(t | t.instructions.exists(entry | entry.value.body ~= /*org.eclipse.equinox.p2.touchpoint.eclipse.setJvm*/
metaRequirements.0.min = 0
metaRequirements.0.max = 0
requires.0.min = 0
requires.0.max = 0

The feature also has a plug-in that does a noop on start-up, its MANIFEST.MF contents are:

Manifest-Version: 1.0
Bundle-ManifestVersion: 2
Bundle-Name: P2InfTest
Bundle-SymbolicName: org.example.p2inftest;singleton:=true
Bundle-Version: 1.0.0.qualifier
Automatic-Module-Name: StatusLineExample
Bundle-RequiredExecutionEnvironment: JavaSE-11
Require-Bundle: org.eclipse.swt,
 org.eclipse.ui,
 org.eclipse.equinox.common,
 org.eclipse.core.runtime

When I install this feature from the update site, I *sporadically* get JustJ installed. Sometimes its installed, sometimes it isn't. Can *someone* look into *why* JustJ is being installed in the first place, *but only sometimes*? The p2 behaviour regarding JustJ looks broken.

Aside from the p2 sporadic behaviour, the query seems to work to find an IU from a repository, if the IU defines the SetJvm action in a touchpoint. So the above p2.inf content fails to prevent installing JustJ, but I don't know why. Am I defining the "negative requirements" wrong? I don't quite understand the info for this here: https://wiki.eclipse.org/Equinox/p2/Customizing_Metadata

I assume the query works fine due to running:

	public void testQueryTouchpointSetJvm() throws Exception {
		IMetadataRepository repo = getMDR("/testData/bug569871");
		{
			IQueryResult<IInstallableUnit> result = repo.query(QueryUtil.createMatchQuery(//
					"touchpointType != null && touchpointType.id == 'org.eclipse.equinox.p2.osgi' && touchpointData.exists(t | t.instructions.exists(entry | entry.value.body ~= /*org.eclipse.equinox.p2.touchpoint.eclipse.setJvm*/))"),
					new NullProgressMonitor());
			assertEquals("Found wrong number of touchpoints with SetJVM action", 1, queryResultSize(result));
			Iterator<?> iterator = result.iterator();
			while (iterator.hasNext()) {
				Object r = iterator.next();
				System.out.println(r.getClass() + " : " + r);
			}
		}
		{
			IQueryResult<IInstallableUnit> result = repo.query(QueryUtil.createQuery(//
					"everything.select(x | x.touchpointType != null && x.touchpointType.id == 'org.eclipse.equinox.p2.osgi' && x.touchpointData.exists(t | t.instructions.exists(entry | entry.value.body ~= /*org.eclipse.equinox.p2.touchpoint.eclipse.setJvm*/)))"),
					new NullProgressMonitor());
			assertEquals("Found wrong number of touchpoints with SetJVM action", 1, queryResultSize(result));
			Iterator<?> iterator = result.iterator();
			while (iterator.hasNext()) {
				Object r = iterator.next();
				System.out.println(r.getClass() + " : " + r);
			}
		}
	}

(see comment 21 for the repository xml and test location)

This passes and the output is:

class org.eclipse.equinox.internal.p2.metadata.InstallableUnit : test.bundle 2.1.0
class org.eclipse.equinox.internal.p2.metadata.InstallableUnit : test.bundle 2.1.0
Comment 24 Simeon Andreev CLA 2021-01-20 06:12:06 EST
Created attachment 285326 [details]
Update site that results in sporadic JustJ installation.
Comment 25 Simeon Andreev CLA 2021-01-29 04:33:11 EST
(In reply to Mickael Istria from comment #18)
> (In reply to Simeon Andreev from comment #17)
> > disable this p2 action in our product:
> > org.eclipse.equinox.internal.p2.touchpoint.eclipse.actions.SetJvmAction [...] What about this?
> 
> I'm not aware of such a way to disable touchpoints
> 
> > If not, can we
> > (me or Andrey) add a preference to disable touchpoints (e.g. by ID/class
> > name, or just this specific one)? E.g. throwing an exception when that
> > touchpoint is ran.
> 
> It would be worth investigating whether p2 allows to query artifacts
> according to their touchpoint. If it's possible, then you can just add a
> bundle with a negative (min=max=0) against artifacts that do define this
> touchpoint.

I've checked, if its possible I've not been able to achieve this (see my previous comments).

So can we add *explicit* code that disables SetJvmAction if some new preference is set?
Comment 26 Mickael Istria CLA 2021-01-29 04:44:39 EST
(In reply to Simeon Andreev from comment #25)
> I've checked, if its possible I've not been able to achieve this (see my
> previous comments).
> 
> So can we add *explicit* code that disables SetJvmAction if some new
> preference is set?

The issue with sporadic JustJ installation despite a query that seems valid to exclude the setJVM action still needs to have a clear answer before we consider adding workarounds. As far as I know, this strategy (excluding based on query finding setJVM) is supposed to work; if it's not it's a bug, and then effort should be placed on fixing the bug where it is instead of adding extra code to dodge it.
Comment 27 Simeon Andreev CLA 2021-01-29 04:49:21 EST
(In reply to Mickael Istria from comment #26)
> 
> The issue with sporadic JustJ installation despite a query that seems valid
> to exclude the setJVM action still needs to have a clear answer before we
> consider adding workarounds. As far as I know, this strategy (excluding
> based on query finding setJVM) is supposed to work; if it's not it's a bug,
> and then effort should be placed on fixing the bug where it is instead of
> adding extra code to dodge it.

Which code do I debug then, to find out why the p2.inf file with the query and negative dependency doesn't work?
Comment 28 Mickael Istria CLA 2021-01-29 05:03:28 EST
(In reply to Simeon Andreev from comment #27)
> Which code do I debug then, to find out why the p2.inf file with the query
> and negative dependency doesn't work?

First verify the negative requirement appears in the generated metadata of your artifact.
Then, with p2 code available in your dev-time workspace, try to configure an "Eclipse application" launch configuration that runs p2.director application and reproduce the issue of installing JustJ while requirements should prevent it. Minimize the case as much as possible to ease further analysis and debug.
Once you have a deterministic reproducer for this issue, run you launch config in debug mode and put breakpoint on SimplePlanner#getProvisioningPlan() -which is poorly named as it's far from being a simple piece of code!- and 1. verify the units do contain the expected metadata, that the query is properly defined and do work against this context and if everything seems fine from metadata POV, then 2. drill down in the execution to find out when things go wrong.
Comment 29 Ed Merks CLA 2021-01-29 10:30:33 EST
One alternative (less general) trick might be an explicit requirement on a.jre.javase (unrestricted version).   All the JustJ JREs have a negative requirement on this and normally, without a JustJ JRE, that's how the package and ee requirements (package import and BREE) are satisfied.  So by explicitly requiring this I think you can the JustJ JRE out because they can't both be installed at once...
Comment 30 Andrey Loskutov CLA 2021-01-29 11:00:37 EST
(In reply to Ed Merks from comment #29)
> One alternative (less general) trick might be an explicit requirement on
> a.jre.javase (unrestricted version).   All the JustJ JREs have a negative
> requirement on this and normally

Thanks Ed, but we look for a *general* solution that disallows JRE change for *any* bundle. 

We honestly don't care if it is JustJ or BetterJ or whoever else that replaces default JRE with something else. 

Therefore I personally don't think it is solvable with p2 queries only / negative requirements, because who knows which other code will try to install custom JRE. 

What I personally wish (as product owner) is that my product uses one and only JRE we ship on our workstations and the code in p2 that is responsible to be called on the "change now JRE to something else" has a simple block that does that:

allowedToExchangeJRE = readPreferenceOrSystemProperty()
if(! allowedToExchangeJRE) report error & return;

This change would be trivial & easy to provide and should not harm.
Comment 31 Ed Merks CLA 2021-01-29 11:05:15 EST
I see.

I wonder when happens if you install an IU that removes the -vm argument, i.e., what happens in the case of conflict?
Comment 32 Simeon Andreev CLA 2021-01-29 11:12:57 EST
(In reply to Mickael Istria from comment #28)
> First verify the negative requirement appears in the generated metadata of
> your artifact.

I don't see the query in the file in content.jar, so that must be the (first) problem. I've also not been able to write a p2.inf file that lands the query in that file. E.g. this doesn't do it:

requires.0.namespace = org.eclipse.equinox.p2.iu
requires.0.name = unused
requires.0.matchExp = touchpointType != null && touchpointType.id == 'org.eclipse.equinox.p2.osgi' && touchpointData.exists(t | t.instructions.exists(entry | entry.value.body ~= /*org.eclipse.equinox.p2.touchpoint.eclipse.setJvm*/))
requires.0.min = 0
requires.0.max = 0

I'm also getting "nice" errors when building the update site, presumably due to bad   p2.inf contents:

/home/sandreev/temp_workspaces/132132132131qweeqwe/.metadata/.plugins/org.eclipse.pde.core/temp/org.eclipse.pde.container.feature/assemble.org.eclipse.pde.container.feature.group.group.group.xml:88: The following error occurred while executing this line:
/home/sandreev/temp_workspaces/132132132131qweeqwe/.metadata/.plugins/org.eclipse.pde.core/temp/org.eclipse.pde.container.feature/assemble.org.eclipse.pde.container.feature.group.group.group.xml:169: Unable to find: Installable Unit [ id=org.example.ptwoinftest.feature.feature.group version=1.0.0.202101291709 ] 
The following error occurred while executing this line:
/home/sandreev/temp_workspaces/132132132131qweeqwe/.metadata/.plugins/org.eclipse.pde.core/temp/org.eclipse.pde.container.feature/assemble.org.eclipse.pde.container.feature.group.group.group.xml:169: Unable to find: Installable Unit [ id=org.example.ptwoinftest.feature.feature.group version=1.0.0.202101291709 ] 

The errors persist even when changing the p2.inf content (e.g. fully clearing it). Building the update site from Eclipse also seems to result in "outdated" contents in content.jar with regards to the p2.inf file. E.g. I would add a "negative dependency" to JustJ, build the site, clear the contents of p2.inf, build the site again (after  manually removing the site artefacts in Eclipse) and I would still see the negative dependency for JustJ in the *new* content.jar.
Comment 33 Ed Merks CLA 2021-01-29 11:39:36 EST
Sorry, if this is not relevant because you've already done that, but please be sure that the p2.inf is in the build.properties's bin.includes otherwise it's ignored...
Comment 34 Simeon Andreev CLA 2021-01-29 11:44:42 EST
(In reply to Ed Merks from comment #33)
> Sorry, if this is not relevant because you've already done that, but please
> be sure that the p2.inf is in the build.properties's bin.includes otherwise
> it's ignored...

Its included.
Comment 35 Simeon Andreev CLA 2021-02-04 05:27:12 EST
We can prevent JustJ from installing in our product, by installing a feature with p2.inf contents:

requires.0.namespace = org.eclipse.equinox.p2.iu
requires.0.name = org.eclipse.justj.openjdk.hotspot.jre.full
requires.0.min = 0
requires.0.max = 0
requires.1.namespace = org.eclipse.equinox.p2.iu
requires.1.name = org.eclipse.justj.openjdk.hotspot.jre.full.linux.x86_64
requires.1.min = 0
requires.1.max = 0

Still have to look into a query that prevents *any* SetJVM action from running; I've not been able to specify a query with negative requirements, that I then see in the update site content.xml file (see comment 32).

I'm not sure we'll have time for this though, as either there is no feedback from Eclipse about *what* goes wrong during the site build (for the query to not be added to content.xml), or the errors are too vague to help understanding the problem.
Comment 36 Mickael Istria CLA 2021-02-04 06:24:14 EST
Do I get it right that the issue happen in an RCP product (not EPP) and wouldn't happen if SimRel weren't containing reference to EPP then JustJ ?
Comment 37 Andrey Loskutov CLA 2021-02-04 06:30:54 EST
(In reply to Mickael Istria from comment #36)
> Do I get it right that the issue happen in an RCP product (not EPP) and
> wouldn't happen if SimRel weren't containing reference to EPP then JustJ ?

Mickael, it is not about EPP or JustJ or RCP. 

It is about a generic p2 possibility to replace a running JVM with something else, *doesn't matter what is it and from where that thing is coming*.

We have full featured IDE that allows installation of 3rd party tooling because extensibility is considered a good thing and because there are lot of tools that could be useful that aren't included by default because they are coming from 3rd party companies. 

We simply do not want that *anyone* who wants to exchange JRE via p2 can succeed in our IDE.
Comment 38 Simeon Andreev CLA 2021-02-05 06:14:30 EST
Maybe bug 396136 helps with the p2.inf query. See the change for it: https://git.eclipse.org/c/equinox/rt.equinox.p2.git/commit?id=3d04353d8947710873181679046fa3ab8fa2fc98

And bug 396136 comment 3:

> The entry pertaining to update descriptor would be:
> 
> update.matchExp=providedCapabilities.exists(pc | pc.namespace \=\=
> 'org.eclipse.equinox.p2.iu' && (pc.name \=\=
> 'org.maven.ide.eclipse.wtp.feature.feature.group' || pc.name \=\=
> 'org.eclipse.m2e.wtp.feature.feature.group' && pc.version < '$version$'))
> 
> This is basically the same thing than what you have in the XML with two
> notable differences:
> 1) the string contains some escaped characters since p2.inf is a property
> file
> 2) $version$ is used in order to get the version be taken from the IU.
Comment 39 Simeon Andreev CLA 2021-02-08 03:05:19 EST
In an Eclipse SDK, I can install a feature with the following p2.inf contents:

requires.0.namespace = org.eclipse.equinox.p2.iu
requires.0.matchExp = touchpointData.exists(t | t.instructions.exists(entry | entry.value.body ~= /*org.eclipse.equinox.p2.touchpoint.eclipse.setJvm*/))
requires.0.min = 0
requires.0.max = 0

Then, I can no longer install a bundle that has this in its MANIFEST.MF: "Require-Bundle: org.eclipse.justj.openjdk.hotspot.jre.full"

The error message is:

Cannot complete the install because some dependencies are not satisfiable
  Software being installed: Feature 1.0.0.202102031506 (test.justj.feature.feature.group 1.0.0.202102031506)
  Software currently installed: ... p2 configuration 1.0.0.qualifier (.....p2inf.feature.feature.group 1.0.0.qualifier)
  Cannot satisfy dependency:
    From: ... p2 configuration 1.0.0.qualifier (....p2inf.feature.feature.group 1.0.0.qualifier)
    To: touchpointData.exists(t | t.instructions.exists(entry | entry.value.body ~= /*org.eclipse.equinox.p2.touchpoint.eclipse.setJvm*/))

I'll try this in our product soon, but so far results look promising.
Comment 40 Simeon Andreev CLA 2021-02-10 08:28:36 EST
(In reply to Simeon Andreev from comment #39)
> In an Eclipse SDK, I can install a feature with the following p2.inf
> contents:
> ...
> I'll try this in our product soon, but so far results look promising.

Seems to work also in our product.

Left in this ticket is to understand *why* JustJ is being installed in the first place. Adding the p2.inf file (that stops JustJ from installing) doesn't prevent WebTools 3.20 from installing. So WebTools doesn't *need* JustJ.

Though I doubt me or Andrey will have time to look into this. It might make sense to close this as "wont do" or similar.
Comment 41 Mickael Istria CLA 2021-02-10 08:32:53 EST
Thanks for the series of experiments and conclusion Simeon, it's great to have a as output of this ticket a description of how-to to prevent from installing JustJ or similar in RCP products. That may be quite helpful to some others!

Let's close the ticket as you suggested; but anyone please feel free to reopen if you're willing to investigate all that further!
Comment 42 Jonah Graham CLA 2021-03-03 12:09:05 EST
With Bug 570822 resolved the JustJ no longer appears as part of the SimRel, so that should be another solution to this problem starting in 2021-03.

If you want to test it, you need to simulate the release repo by referencing the RC1 repos directly, and not point at the composite repos. RC1 should be release on Friday

e.g. use:

https://download.eclipse.org/releases/2021-03/202102261000/ and https://download.eclipse.org/technology/epp/packages/2021-03/202102251200/ instead of https://download.eclipse.org/releases/2021-03 directly. Updating the timestamp directories for the RC1 build when it is published.