Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Validation error when launching child eclipse #194

Closed
sravanlakkimsetti opened this issue Jul 1, 2022 · 43 comments · Fixed by #221
Closed

Validation error when launching child eclipse #194

sravanlakkimsetti opened this issue Jul 1, 2022 · 43 comments · Fixed by #221

Comments

@sravanlakkimsetti
Copy link
Contributor

We are getting this error
image

All these are for JDK packages.

Build information:
Eclipse SDK
Version: 2022-06 (4.25)
Build id: I20220630-1800
OS: Windows 10, v.10.0, x86_64 / win32
Java vendor: Eclipse Adoptium
Java runtime version: 17.0.3+7
Java version: 17.0.3

Notifications: @SarikaSinha @akurtakov

@sravanlakkimsetti
Copy link
Contributor Author

sravanlakkimsetti commented Jul 1, 2022

These packages got added in Import-package directive in latest Jetty (version 10.0.11) in 10.0.9 these were not there

java.io,
java.lang,
java.lang.invoke,
java.net,
java.nio,
java.nio.channels,
java.nio.charset,
java.text,
java.util,
java.util.concurrent,
java.util.concurrent.atomic,
java.util.function,
java.util.regex,
java.util.stream,
java.util.zip,

@vik-chand
Copy link
Member

Happens 1st on 28th June build

gitlog of 28th

2022-06-28 Build input for build I20220628-1800 Eclipse Releng Bot
2022-06-28 Build input for build I20220628-1040 Eclipse Releng Bot
2022-06-28 Update to Jetty 10.0.11 Александър Куртаков

@vik-chand
Copy link
Member

image

@vik-chand
Copy link
Member

I removed the import packages of comment#2 (by Sravan) in org.eclipse.jetty.http and that particular no longer is in the list of validation error.

Another issue is why org.eclipse.osgi.internal.resolver.UserState@492d0748 cant resolve import of java.io etc

@vik-chand
Copy link
Member

If I add "Import-Package: java.io" manually in manifest of pde.core, plugin validation give pde.core also as the offending plugin.

@sravanlakkimsetti
Copy link
Contributor Author

Shouldn't we resolve JDK libraries?

@merks
Copy link
Contributor

merks commented Jul 5, 2022

Yes, one really kind of needs the a.jre.javase IU (or equivalent there of) corresponding to the selected JRE of the launcher to verify that the required packages are really in the actual JRE. But that's easier said than done.

@vik-chand
Copy link
Member

While resolving the bundle org.eclipse.jetty.http in ResolverImpl for org.eclipse.jetty.http

	if (!failed) {
		// Iterate thru imports of 'bundle' trying to find matching exports.
		ResolverImport[] imports = bundle.getImportPackages();
		for (ResolverImport resolverImport : imports) {
			// Only resolve non-dynamic imports here
			if (!resolverImport.isDynamic() && !resolveImport(resolverImport, cycle)) {

For ResolverImport (java.io)

// Resolve the supplied import. Returns true if the import can be resolved, false otherwise
private boolean resolveImport(ResolverImport imp, List<ResolverBundle> cycle) {
	if (DEBUG_IMPORTS)
		ResolverImpl.log("Trying to resolve: " + imp.getBundle() + ", " + imp.getName()); //$NON-NLS-1$ //$NON-NLS-2$
	if (imp.getSelectedSupplier() != null) {

imp.getSelectedSupplier() comes as null

@vik-chand
Copy link
Member

The above is causing the resolve of org.eclipse.jetty.http to fail in OSGI.

@vik-chand
Copy link
Member

@tjwatson Can you provide your views on this?

@laeubi
Copy link
Contributor

laeubi commented Jul 5, 2022

Yes, one really kind of needs the a.jre.javase IU (or equivalent there of) corresponding to the selected JRE of the launcher to verify that the required packages are really in the actual JRE. But that's easier said than done.

This is an ancient (one can't really call it old anymore) bug in PDE, and to clarify this is not an Validation error but a warning, and it does not really require a a.jre.javase IU as this all can be derived from the selected JDK itself (and P2 already do that for the workspace JRE).

The above is causing the resolve of org.eclipse.jetty.http to fail in OSGI.

Can you show the exact error? This should only fail if using OSGi <= 6, at all as it is valid to import java packages since OSGi R7:
https://blog.osgi.org/2018/02/osgi-r7-highlights-java-9-support.html

@laeubi
Copy link
Contributor

laeubi commented Jul 5, 2022

But that's easier said than done.

That's really not that hard, Tycho uses the following to get all system-packages of a JVM:
https://github.com/eclipse/tycho/blob/master/tycho-core/src/main/java/org/eclipse/tycho/core/ee/ListSystemPackages.java

the a.jre.javase IU (or equivalent there of)

and if you prefer a JRE .profile you can generate one with these packages like this
https://github.com/eclipse/tycho/blob/f3bc5084e319a4dd119e67a9af66f58dd8bb5c6e/tycho-core/src/main/java/org/eclipse/tycho/core/ee/ExecutionEnvironmentUtils.java#L236

@merks
Copy link
Contributor

merks commented Jul 5, 2022

Yes, though that requires launching the JRE. And in the end, I believe that JDT has already computed all the packages of each JVM so it's just a matter of creating an IU like the a.jre.javase IU with those package capabilities...

@laeubi
Copy link
Contributor

laeubi commented Jul 5, 2022

Yes, though that requires launching the JRE. And in the end, I believe that JDT has already computed all the packages of each JVM

Probably yes :-)

so it's just a matter of creating an IU like the a.jre.javase IU with those package capabilities...

I thin this is just relevant for P2, the problem is that PDE uses "old" OSGi API and that these packages are superposed to be exported by the system-bundle dynamically, so at least PDE must handle this and hopefully could finally throw away the javax is from JRE assumption so we can get better errors / warnings at the first place.

@merks
Copy link
Contributor

merks commented Jul 5, 2022

I'm just pointing out that "the system bundle information" must be the one that would be specified for the actual JRE of the launcher, not the one of the running IDE itself.

@laeubi
Copy link
Contributor

laeubi commented Jul 5, 2022

I'm just pointing out that "the system bundle information" must be the one that would be specified for the actual JRE of the launcher, not the one of the running IDE itself.

Yep, int this case it is actually the Target-JRE.

@iloveeclipse
Copy link
Member

A note from other perspective: the same error will be shown also if we debug from "old" Eclipse against new target platform containing Jetty. I know we usually say "just update to latest", but if that is impossible, people will be angry too.

So why Jetty is requiring now system packages at all? What''s the point doing that?
Shouldn't we fix Jetty manifest instead? This would allow "older" Eclipse versions to be used with new Jetty too (assuming we will find solution for 4.25).

@vik-chand
Copy link
Member

A note from other perspective: the same error will be shown also if we debug from "old" Eclipse against new target platform containing Jetty. I know we usually say "just update to latest", but if that is impossible, people will be angry too.

So why Jetty is requiring now system packages at all? What''s the point doing that? Shouldn't we fix Jetty manifest instead? This would allow "older" Eclipse versions to be used with new Jetty too (assuming we will find solution for 4.25).

See bug -> jetty/jetty.project#8265

@vik-chand
Copy link
Member

Notification @bjhargrave

@HannesWell
Copy link
Member

For those that are asking if these packages are generated intentionally (by BND), the answer is yes:
bndtools/bnd#4348

@laeubi
Copy link
Contributor

laeubi commented Jul 5, 2022

So why Jetty is requiring now system packages at all? What''s the point doing that?
Shouldn't we fix Jetty manifest instead?

There is nothing to fix see https://blog.osgi.org/2018/02/osgi-r7-highlights-java-9-support.html

@merks
Copy link
Contributor

merks commented Jul 5, 2022

Why should Jetty do this? These days it's quite possible to use jlink to create reduced JREs which may not provide all the packages. By declaring it needs a certain package it's ensuring that there won't just be class not found exceptions but rather that it's clear a certain package is needed and not present...

@bjhargrave
Copy link

In OSGi Core Release 7, it became supported to import java.* packages when the code targets Java 9+. See https://docs.osgi.org/specification/osgi.core/7.0.0/framework.module.html#framework.module-execution.environment.

So Bnd supports this. An OSGi R7 framework when running on Java 9+ will export all the available java.* packages from the system bundle so any bundle importing java.* packages will resolve IF the Java 9+ runtime has been configure with all the modules needed to support the bundle's java.* imports.

It appears here that parts of Eclipse (PDE) do not properly handle this.

Bnd has a -noimportjava: true option which can be used to tell Bnd not to declare imports on java.* packages. See https://bnd.bndtools.org/instructions/noimportjava.html. But Jetty is allowed to import java.* packages. Tooling needs to be updated to handle this.

@HannesWell
Copy link
Member

I can offer to try to fix this false positive validation error in the next days (for the current PDE).

That's really not that hard, Tycho uses the following to get all system-packages of a JVM: https://github.com/eclipse/tycho/blob/master/tycho-core/src/main/java/org/eclipse/tycho/core/ee/ListSystemPackages.java

... with this in mind it should be straight forward.

Anybody that is interested in having this fix in older versions is then free to backport it. :)

Just for curiosity, would this automatically be fixed if PDE migrates off Resolver-State in the o.e.osgi.compatibility.state fragment?

@laeubi
Copy link
Contributor

laeubi commented Jul 5, 2022

Just for curiosity, would this automatically be fixed if PDE migrates off Resolver-State in the o.e.osgi.compatibility.state fragment?

Have not checked this, but I think it is more that the current target platform state need to consider the packages of the selected target-jre instead. and not really related to the o.e.osgi.compatibility.state

@vik-chand
Copy link
Member

I can offer to try to fix this false positive validation error

We should try to fix this in M1 since this is rather annoying validation error ( which kind-of suggests that the launch is not good) . If this is not fixed, we should also consider rollback of jetty update for M1.

@laeubi
Copy link
Contributor

laeubi commented Jul 5, 2022

@vik-chand I honestly won't consider a rollback because PDE is annoying ... that will only lead to this bug never fixed and jetty never upgraded.

which kind-of suggests that the launch is not good

This is not the only case where PDE annoys the user ... I regularly get complains form PDE and the OSGi is fine with that, so this is really not a measure...

@tjwatson
Copy link
Contributor

tjwatson commented Jul 5, 2022

Just for curiosity, would this automatically be fixed if PDE migrates off Resolver-State in the o.e.osgi.compatibility.state fragment?

It comes down to how PDE discovers the packages available from the execution environment. I don't recall the details on how PDE does this now. But it needs to be able to have the java.* packages for each execution environment.

Moving away from the old equinox resolver for this is not going to fix this directly. You would still be left with the same issue of having to configure what the system bundle exports from the execution environment for java.*. The difference being that the old equinox resolver had features it put in just for PDE to try and handle multiple EEs in a single resolver state. That was an error prone and complicated feature that I did not put in the org.eclipse.osgi.container API that uses the OSGi resolver spec to model OSGi dependencies. All that complication would need to be placed in the PDE implementation of a ResolveContext if they want to limit what packages from the JVM can import based on their required execution environment.

@laeubi
Copy link
Contributor

laeubi commented Jul 5, 2022

All that complication would need to be placed in the PDE implementation of a ResolveContext if they want to limit what packages from the JVM can import based on their required execution environment.

I don't think this is needed, actually a Target hat a JVM and that has to be used instead of an individual per bundle.

@laeubi
Copy link
Contributor

laeubi commented Jul 18, 2022

If I use the latest 4.25 I build as a host, there are no errors.

These are not errors, just warnings.

I'm not clear from the previous discussion if there is planned a change that will allow 4.24 to be used as host when targetting 4.25, or whether 4.25 will be required as host?

Simply click Continue button ...

@Phillipus
Copy link
Contributor

These are not errors, just warnings.

Yes, indeed.

Simply click Continue button ...

Right. Or untick the launch configuration option Validate Plug-ins automatically prior to launching

HannesWell added a commit to HannesWell/eclipse.pde that referenced this issue Jul 18, 2022
HannesWell added a commit to HannesWell/eclipse.pde that referenced this issue Jul 18, 2022
The list of packages in the <ee>-systempackages.profile file in
o.e.pde.api.tools is not suitable.

Fixes eclipse-pde#194
HannesWell added a commit to HannesWell/eclipse.pde that referenced this issue Jul 18, 2022
The list of system-packages in the <ee>-systempackages.profile file in
o.e.pde.api.tools is not suitable for Launch-validation.

Fixes eclipse-pde#194
HannesWell added a commit to HannesWell/eclipse.pde that referenced this issue Jul 18, 2022
The list of system-packages in the <EE>-systempackages.profile file in
o.e.pde.api.tools is not suitable for Launch-validation.

Fixes eclipse-pde#194
@HannesWell
Copy link
Member

Created #221 that should fix this issue.
Verified it with the mentioned o.e.jetty.http and a workspace Plug-in with Import-Package: java.io.

HannesWell added a commit to HannesWell/eclipse.pde that referenced this issue Jul 20, 2022
HannesWell added a commit to HannesWell/eclipse.pde that referenced this issue Jul 20, 2022
The list of system-packages in the <EE>-systempackages.profile file in
o.e.pde.api.tools is not suitable for Launch-validation, query them from
the VM instead.

Fixes eclipse-pde#194
HannesWell added a commit that referenced this issue Jul 20, 2022
HannesWell added a commit that referenced this issue Jul 20, 2022
The list of system-packages in the <EE>-systempackages.profile file in
o.e.pde.api.tools is not suitable for Launch-validation, query them from
the VM instead.

Fixes #194
vik-chand pushed a commit that referenced this issue Aug 9, 2022
vik-chand pushed a commit that referenced this issue Aug 9, 2022
The list of system-packages in the <EE>-systempackages.profile file in
o.e.pde.api.tools is not suitable for Launch-validation, query them from
the VM instead.

Fixes #194
HannesWell added a commit to HannesWell/eclipse.pde that referenced this issue Aug 11, 2022
The list of system-packages in the <EE>-systempackages.profile file in
o.e.pde.api.tools is not suitable for Launch-validation, query them from
the VM instead.

Fixes eclipse-pde#194 for
PDE-build
HannesWell added a commit to HannesWell/eclipse.pde that referenced this issue Aug 11, 2022
HannesWell added a commit to HannesWell/eclipse.pde that referenced this issue Aug 12, 2022
HannesWell added a commit that referenced this issue Aug 13, 2022
gireeshpunathil pushed a commit to gireeshpunathil/eclipse.pde.build that referenced this issue Jun 18, 2023
gireeshpunathil pushed a commit to gireeshpunathil/eclipse.pde that referenced this issue Jun 18, 2023
gireeshpunathil pushed a commit to gireeshpunathil/eclipse.pde.build that referenced this issue Jun 18, 2023
gireeshpunathil pushed a commit to eclipse-pde/eclipse.pde.build that referenced this issue Jun 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.