iloveeclipse
commented
1 year ago @merks : thoughts?
Closed iloveeclipse closed 1 year ago
iloveeclipse
commented
1 year ago @merks : thoughts?
laeubi
commented
1 year ago So I guess either we should fix p2 and/or equinox or bump hamcrest to 1.3.0.v20230806-0831 in the SDK?
What exactly do you like to fix here? For Equinox I can't see any error and P2 does not support use-constraints since ever and adding them requires non trivial enhancements, so as usual the problem is better solved at the root cause that is not requiring a strict bundle version and instead use semantic version ranges to allow relaxed resolving and not include third party stuff in features so duplicate content is not pulled in unnecessarily.
iloveeclipse
commented
1 year ago Please check manifests, neither of affected bundles requires strict versions, they all require 1.3.0.
iloveeclipse
commented
1 year ago What exactly do you like to fix here?
I think it is obvious : as a user I want be able to install SWTbot into SDK and use it. Currently installation works but result is not usable.
laeubi
commented
1 year ago Please check manifests, neither of affected bundles requires strict versions, they all require 1.3.0.
Please check the error message (emphasis by me):
org.osgi.framework.BundleException: Could not resolve module: org.eclipse.swtbot.swt.finder [398] Unresolved requirement: Require-Bundle: org.hamcrest.library; bundle-version="1.3.0"
so swtbot finder strictly requires the org.hamcrest.library bundle in version 1.3 or above so the resolver is not allowed to substitute the requirement with org.hamcrest.core providing similar/same packages.
That's why you see the the next error with JUnit, there is simply a conflict in the classspace that could not be satisfied.
I think it is obvious : as a user I want be able to install SWTbot into SDK and use it. Currently installation works but result is not usable.
Because it has to strict constraints (e.g. a feature requiring a specific version of hamcrest with a specific name). So if you ask P2 to install THAT version it can't do anything more than install it and hope the best as it is not forbidden to do so.
iloveeclipse
commented
1 year ago Could it be, the shorter answer would be: yes, let us bump org.hamcrest.core version we ship in SDK as proposed to 1.3.0.v20230806-0831 version, because mixing different versions of 1.3.0 hamcrest bundles is not supported by equinox at runtime (even if we allow to install the mix)?
merks
commented
1 year ago Indeed this sounds related to this issue:
laeubi
commented
1 year ago because mixing different versions of 1.3.0 hamcrest bundles is not supported by equinox at runtime
It is supported by Equinox, you misinterpret the error here it has nothing to do with anything unsupported, it has to do that your installation requires an impossible class-space, from the errors shared here:
org.hamcrest.libraryorg.hamcrest.coreorg.junitorg.hamcrest.core what is a sue constraint violation because java can only load classes of a package from one classloaderso it is not equinox to blame here but bundles using Require-Bundle (and reexports!) instead of import package what would have allowed the resolver to choose one of them to form a consistent class space.
merks
commented
1 year ago @iloveeclipse
FYI, I'll look at the target platform right now, and come back here with concrete steps forward.
merks
commented
1 year ago @iloveeclipse
As you can see, I created a PR to bump the version. It's using a nightly build repo for the time being, which is of course bad practice, but I am personally monitoring the situation, so I will ensure that the I-Build will not break suddenly, and will fix this when I produce a new orbit milestone aggregation, which would ideally be the case after "we" have confirmed that the problem you describe here is fixed. Okay?
@laeubi
You are correct that there is a nasty inconsistency in the java package exports. Of course it's all because of the extremely distasteful historical bundle requirement in org.junit, compounded with the even more unspeakably putrid re-export of the bundle requirement, with the org.hamcrest.core/org.hamcrest.library split package abomination for added spicy flavor...
iloveeclipse
commented
1 year ago @merks : sure. If you need / want, we can trigger IBuild right now after merging your PR.
merks
commented
1 year ago @iloveeclipse
It's ready for testing. (Sorry for the hassle.)
iloveeclipse
commented
1 year ago It's ready for testing.
Ed, not sure if and how I'm supposed to test? Do you mean, we can merge & trigger IBuild?
merks
commented
1 year ago Sorry. Yes, I assume we/you need a full I-Build to test what you showed at the top of the issue.
iloveeclipse
commented
1 year ago OK, I've triggered https://ci.eclipse.org/releng/job/Builds/job/I-build-4.29/92/ to see if the change works & doesn't break anything else.
iloveeclipse
commented
1 year ago Doesn't work.
eclipse.buildId=4.29.0.I20230808-0530
java.version=17.0.6-internal
java.vendor=N/A
BootLoader constants: OS=linux, ARCH=x86_64, WS=gtk, NL=en_US
Command-line arguments: -data file:/tmp/wsp/ -os linux -ws gtk -arch x86_64
org.eclipse.swtbot.swt.finder
Error
Tue Aug 08 13:39:33 CEST 2023
FrameworkEvent ERROR
org.osgi.framework.BundleException: Could not resolve module: org.eclipse.swtbot.swt.finder [398]
Unresolved requirement: Require-Bundle: org.hamcrest.library; bundle-version="1.3.0"
-> Bundle-SymbolicName: org.hamcrest.library; bundle-version="1.3.0.v20230806-0831"
org.hamcrest.library [438]
No resolution report for the bundle. Bundle was not resolved because of a uses constraint violation.
org.apache.felix.resolver.reason.ReasonException: Uses constraint violation. Unable to resolve resource org.eclipse.swtbot.swt.finder [osgi.identity; osgi.identity="org.eclipse.swtbot.swt.finder"; type="osgi.bundle"; version:Version="4.1.0.202307261500"; singleton:="true"] because it is exposed to package 'org.hamcrest' from resources org.hamcrest.core [osgi.identity; type="osgi.bundle"; version:Version="1.3.0.v20230806-0849"; osgi.identity="org.hamcrest.core"] and org.hamcrest [osgi.identity; osgi.identity="org.hamcrest"; type="osgi.bundle"; version:Version="2.2.0"] via two dependency chains.
Chain 1:
org.eclipse.swtbot.swt.finder [osgi.identity; osgi.identity="org.eclipse.swtbot.swt.finder"; type="osgi.bundle"; version:Version="4.1.0.202307261500"; singleton:="true"]
require: (&(osgi.wiring.bundle=org.hamcrest.core)(bundle-version>=1.3.0))
|
provide: osgi.wiring.bundle: org.hamcrest.core
org.hamcrest.core [osgi.identity; type="osgi.bundle"; version:Version="1.3.0.v20230806-0849"; osgi.identity="org.hamcrest.core"]
Chain 2:
org.eclipse.swtbot.swt.finder [osgi.identity; osgi.identity="org.eclipse.swtbot.swt.finder"; type="osgi.bundle"; version:Version="4.1.0.202307261500"; singleton:="true"]
import: (&(osgi.wiring.package=org.junit)(version>=4.12.0))
|
export: osgi.wiring.package=org.junit; uses:=org.hamcrest
org.junit [osgi.identity; type="osgi.bundle"; version:Version="4.13.2.v20230725-0701"; osgi.identity="org.junit"]
import: (&(osgi.wiring.package=org.hamcrest)(version>=1.3.0))
|
export: osgi.wiring.package: org.hamcrest
org.hamcrest [osgi.identity; osgi.identity="org.hamcrest"; type="osgi.bundle"; version:Version="2.2.0"]
at org.eclipse.osgi.container.Module.start(Module.java:463)
at org.eclipse.osgi.container.ModuleContainer$ContainerStartLevel$2.run(ModuleContainer.java:1852)
at org.eclipse.osgi.internal.framework.EquinoxContainerAdaptor$1$1.execute(EquinoxContainerAdaptor.java:136)
at org.eclipse.osgi.container.ModuleContainer$ContainerStartLevel.incStartLevel(ModuleContainer.java:1845)
at org.eclipse.osgi.container.ModuleContainer$ContainerStartLevel.incStartLevel(ModuleContainer.java:1786)
at org.eclipse.osgi.container.ModuleContainer$ContainerStartLevel.doContainerStartLevel(ModuleContainer.java:1750)
at org.eclipse.osgi.container.ModuleContainer$ContainerStartLevel.dispatchEvent(ModuleContainer.java:1672)
at org.eclipse.osgi.container.ModuleContainer$ContainerStartLevel.dispatchEvent(ModuleContainer.java:1)
at org.eclipse.osgi.framework.eventmgr.EventManager.dispatchEvent(EventManager.java:234)
at org.eclipse.osgi.framework.eventmgr.EventManager$EventThread.run(EventManager.java:345)@HannesWell
Is there any way out of this mess? It looks like one use of org.junit (Chain 2) wants to bind to org.hamcrest 2.2.0 while another use of org.junit (Chain 1) wants to bind to org.hamcrest.core 1.3.0, and that seems not workable in principle. So perhaps from this we can conclude that we cannot have a single junit that works with both versions of hamcrest. But then I see no path forward that does not involve everyone switching from one version of hamcrest to the other at once.
One next potential "horrible hack" would be to constrain the require bundle to exclude 2.0.0, because that would appear to break the last segment of the chain above. But I'm getting more and more skeptical that this would work. What if something requires org.junit and explicitly requires at least org.hamcrest 2.2.0 while another uses it and explicitly requires org.hamcrest.core 1.3.0 and both are installed in the same IDE? Can that actually ever work?
Edit....
The above comment is stupid because the bundle require is of org.hamcrest.core which does not exist at version 2.2.0...
laeubi
commented
1 year ago As written above it is org.eclipse.swtbot.swt.finder that requires org.hamcrest.library and imports org.junit (package) and the bundle (org.junit) exporting the package that requires org.hamcrest.core because it reexport org.hamcrest.core packages.
But then I see no path forward that does not involve everyone switching from one version of hamcrest to the other at once.
Is 2.2.x actually binary compatible to 1.3.x for consumers?
Can that actually ever work?
Resolver says no, but it is an NP-hard problem, so there might be a solution but we need a quantum-computer to be sure in linear time :-)
merks
commented
1 year ago Yes, they claim 2.x is is a drop-in replacement.
laeubi
commented
1 year ago By the way I'm not even sure the reexport is useful for anything today, as far as I know JDT already include Hamcrest anyways, PDE has also special handling when junit is present and Tycho as well... so maybe it would be better to drop that reexport, use "nice" import ranges and maybe if required let consumers adjust their imports slightly ... It's better to make a painful break than draw out the agony. There was already a breaking change on JDT side for some releases now and people have already adapted there.
merks
commented
1 year ago As soon as I removed it, the Platform Tycho build failed, We've covered that ground already:
Even the Platform has not made the necessary painful changes yet...
merks
commented
1 year ago BTW, it might well be the case that if SWTBot switched to hamcrest 2.2.0 that the problem would go away...
akurtakov
commented
1 year ago As there is way too much information for me here - What are the changes that Platform would have to do ?
merks
commented
1 year ago @akurtakov
It's not entirely clear. Remove everything that requires/replies-on org.hamcrest.core/.library 1.3.0? Stop relying on org.junit's exported bundle-require of org.hamcrest.core. Do that across Platform, PDE, Equinox and JDT. Do it all in one painful step? It seems like a large hurdle to me.
Let's wait for Hannes to comment...
Personally I'm close to my limit on this problem from hell. 😱 It feels like we've painted ourselves into a corner...
akurtakov
commented
1 year ago At the end it might happen that migrating to JUnit 5 might take the same amount of time :) .
HannesWell
commented
1 year ago You are correct that there is a nasty inconsistency in the java package exports. Of course it's all because of the extremely distasteful historical bundle requirement in org.junit, compounded with the even more unspeakably putrid re-export of the bundle requirement, with the org.hamcrest.core/org.hamcrest.library split package abomination for added spicy flavor...
org.junit plus org.hamcrest.core is really the worst combinations of all bad OSGi practices I know of.
I belive more and more that it would indeed be the best to just remove the reexport and recommend all to use Import-Package and hamcrest 2.2.
This would only require manifest changes. And should Work as long as hamcrest.library is not used. And if it is used, hamcrest-2.2 should be less of an issue.
Of course this should be announced clearly, e.g. on the mailing list for other SimRel projects and in the release-notes for downstream consumers.
If one really needs the reexport it can still be 'added' back again with a Fragment supplying the reexport.
Switching to Import-Package can be done now already and would also ignore the reexport so that can be done in advance and the build stays green all the time. I started that for PDE already, but the discussion became a bit broader. I'll complete the change for PDE tomorrow and shift further discussions to another issue. I can also look at platform and equinox, but help would of course be welcome, especially for JDT.
HannesWell
commented
1 year ago I belive more and more that it would indeed be the best to just remove the reexport and recommend all to use Import-Package and hamcrest 2.2.
Does anybody know a compelling reason that speaks against this, besides that it needs adjustments in the Manifest?
HannesWell
commented
1 year ago At the end it might happen that migrating to JUnit 5 might take the same amount of time :) .
That would be good, but takes probably far more time, since all test classes need to be adjusted. A lot can probably be done Semi-Automatic with search+Release but still. And IIRC at some time expected and actual value in the assert methods was changed, but I'm not Sure If this was done from 3 -> 4 or 4 -> 5.
And can we even use junit-5 in I-builds already? IIRC, a while ago this was not possible due to limitations I did't understood Back then, but I havn't checked it since.
HannesWell
commented
1 year ago And what I did in our product, was to use the empty hamcrest-core 2.2 artifact and let the Maven Target generate a manifest that required the bundle org.hamcrest in Version 2.2.
This way org.junit was wired to it too.
But we don't have swtbot in our TP, so I cannot say with confidence that this would work in a broader case too.
merks
commented
1 year ago I do get the sense that if swtbot used org.hamcrest 2.2 the problem would also go away. I will follow up tomorrow...
HannesWell
commented
1 year ago I do get the sense that if swtbot used org.hamcrest 2.2 the problem would also go away. I will follow up tomorrow...
This will probably help. Maybe it is already enough to make ITS requirements less strict, use Import-Package and not include hamcrest in the feature.
merks
commented
1 year ago I have changed the recipes as described in the commit above. I used the test case provided in https://github.com/eclipse-orbit/orbit-simrel/issues/8 but enhanced it to add an swtbot dependency. Certainly I could reproduce many variations of these constraint violation problems. So I gave up trying to improve the org.junit/org.hamcrest.core/org.hamcrest.library admonition, and restored them to be pretty much exactly like they were before. Also I created org.hamcrest.core/.library 2.2.0 "wrappers" so that org.junit can work with either 1.3 or 2.2; both 2.2 wrappers just require the org.hamcrest 2.2.0 bundle and re-export it.
With this design, I verified/tested that a junit project like the cdt one, with swtbot dependencies, will run with hamcrest 1.3 present, with hamcrest 2.2 present, and with both present.
Now I have to wait for these IP reviews to complete which appear to be taking longer than usual:
https://gitlab.eclipse.org/eclipsefdn/emo-team/iplab/-/issues/9982 https://gitlab.eclipse.org/eclipsefdn/emo-team/iplab/-/issues/9983
Looking at SimRel, only JDT and RAP constrain the org.hamcrest.core/library versions so as to exclude 2.2.0, so the actual migration to 2.2.0 should be much less painful by replicating/preserving the existing horrible design.
HannesWell
commented
1 year ago Sounds good. Thanks a lot Ed, for all your work on this can of worms.
Maybe for the next Release we (as all SimRel projects) could work on migrating to hamcrest 2.2 and maybe we could then reconsider to fix the broken design and remove the reexport (or at least make it optional so the hamcrest.core is not required anymore). PDE should be ready now.
merks
commented
1 year ago FYI, now this is being held up because these got opened when I did a build today, though version 9.7.0 of lucene has been present for quite some time!
https://gitlab.eclipse.org/eclipsefdn/emo-team/iplab/-/issues/10010
And when those are automatically done, I do another build and yet more issues are opened:
So annoying...
iloveeclipse
commented
1 year ago @merks : I'm trying to build SDK locally on tag I20230810-1800 but the build fails due "missing" hamcrest.
Any clue?
[INFO] eclipse-sdk-prereqs 4.29.0-SNAPSHOT ................ SUCCESS [ 29.344 s]
[INFO] org.eclipse.jdt.core.compiler.batch 3.35.0-SNAPSHOT FAILURE [ 3.702 s]
[INFO] ------------------------------------------------------------------------
[INFO] BUILD FAILURE
[INFO] ------------------------------------------------------------------------
[INFO] Total time: 02:55 min
[INFO] Finished at: 2023-08-11T07:56:32+02:00
[INFO] ------------------------------------------------------------------------
[WARNING]
[WARNING] Plugin validation issues were detected in 3 plugin(s)
[WARNING]
[WARNING] * org.apache.maven.plugins:maven-site-plugin:3.12.1
[WARNING] * org.apache.maven.plugins:maven-install-plugin:2.3.1
[WARNING] * org.eclipse.tycho.extras:tycho-p2-extras-plugin:4.0.1
[WARNING]
[WARNING] For more or less details, use 'maven.plugin.validation' property with one of the values (case insensitive): [BRIEF, DEFAULT, VERBOSE]
[WARNING]
[ERROR] Failed to resolve target definition file:/data/4x_platform/eclipse.platform.releng.aggregator/eclipse.platform.releng.prereqs.sdk/eclipse-sdk-prereqs.target: Could not find "org.hamcrest.core/1.3.0.v20230806-0849" in the repositories of the current location
org.eclipse.tycho.BuildFailureException: Failed to resolve target definition file:/data/4x_platform/eclipse.platform.releng.aggregator/eclipse.platform.releng.prereqs.sdk/eclipse-sdk-prereqs.target: Could not find "org.hamcrest.core/1.3.0.v20230806-0849" in the repositories of the current location
at org.eclipse.tycho.p2resolver.TargetDefinitionResolver.resolveContent (TargetDefinitionResolver.java:108)
at org.eclipse.tycho.p2resolver.TargetDefinitionResolverService.resolveFromArguments (TargetDefinitionResolverService.java:93)@iloveeclipse
Sorry, yes, this is the hazard of depending on a nightly build and me changing the versions.
But https://github.com/eclipse-platform/eclipse.platform.releng.aggregator/pull/1249 is completed now, so you can do a pull and the versions in that target platform are available now.
merks
commented
1 year ago @iloveeclipse
It would be good to test the original problem again with an integration build, but I did test that scenario via the test plugin with dependencies on swtbot, so I think the problem is solved this time...
merks
commented
1 year ago @iloveeclipse
I wanted to confirm that you're not stuck right now because soon I'll be unavailable until later in the afternoon...
iloveeclipse
commented
1 year ago But #1249 is completed now, so you can do a pull and the versions in that target platform are available now.
I would prefer to trigger IBuild now, is it OK?
merks
commented
1 year ago @iloveeclipse
I would recommend that yes!
iloveeclipse
commented
1 year ago iloveeclipse
commented
1 year ago Good we've triggered the build. It is unstable, one class that uses hamcrest seem to change bytecode on recompile with new version. https://download.eclipse.org/eclipse/downloads/drops4/I20230811-0350/buildlogs/comparatorlogs/buildtimeComparatorUnanticipated.log.txt I will touch the bundle now & trigger new build.
iloveeclipse
commented
1 year ago iloveeclipse
commented
1 year ago Using I20230811-0600 build I do see some strange issues in JUnit 5 area, I fear something is missing in JDT so JUnit support is probably broken now.
I got NP errors trying to resolve classpath containers in a Java project (unfortunately without stack)
Null entry in container 'org.eclipse.jdt.junit.JUNIT_CONTAINER/5'
laeubi
commented
1 year ago The JUnit containers have hard coded bundle names so if the name of the bundle changed it needs to be adjusted.
iloveeclipse
commented
1 year ago The JUnit containers have hard coded bundle names so if the name of the bundle changed it needs to be adjusted.
Do you know which bundle is changed?
merks
commented
1 year ago I know that @HannesWell has been working on related stuff in PDE:
But I don't think that would impact JDT...
iloveeclipse
commented
1 year ago ... and where JUnit containers are hard coding bundles?
merks
commented
1 year ago I do notice that the latest I-Build installation has only hamcrest 2.2 bundles installed:
I don't know if there are hard-code assumptions in JDT about such bundle versions; I only made these available in the target platform, without changing anything else...
iloveeclipse
commented
1 year ago has been working on related stuff in PDE:
Thats sounds unrelated, that was merged earlier and I didn't saw any issues with the build from yesterday.
org.eclipse.jdt.junit.core/src/org/eclipse/jdt/internal/junit/buildpath/BuildPathSupport.java seem to be the one we need.
Since few days I observe these errors after installing latest SWTbot into SDK (using https://download.eclipse.org/tools/orbit/downloads/latest-I)
I see that SDK has 1.3.0.v20230721-0740 hamcrest version, SWTbot seem to require 1.3.0.v20230806-0831.
If I forcibly update 1.3.0.v20230721-0740 version of org.hamcrest.core into SDK I get another issue that org.junit included into SDK requires older hamcrest core bundle (which is wrong, but see below)!
I believe the problem is caused by the fact that both org.hamcrest.core and org.hamcrest.library bundles are exporting org.hamcrest split package in same version - so it exists in 3 different bundles (old and new hamcrest core + hamcrest library) in the resulting installation.
What is also puzzling, installation works, but at runtime framework reports error at startup like above.
So I guess either we should fix p2 and/or equinox or bump hamcrest to 1.3.0.v20230806-0831 in the SDK?