tycho icon indicating copy to clipboard operation
tycho copied to clipboard

Enable new resolver to see what tests fail

Open laeubi opened this issue 3 years ago • 12 comments
trafficstars

laeubi avatar Aug 16 '22 06:08 laeubi

Test Results

360 files  360 suites   2h 15m 36s :stopwatch: 329 tests 319 :heavy_check_mark:   9 :zzz: 0 :x: 1 :fire: 658 runs  638 :heavy_check_mark: 19 :zzz: 0 :x: 1 :fire:

For more details on these errors, see this check.

Results for commit f6be5a58.

:recycle: This comment has been updated with latest results.

github-actions[bot] avatar Aug 16 '22 08:08 github-actions[bot]

This sounds like smth for 3.0, no?

akurtakov avatar Sep 05 '22 15:09 akurtakov

Yes, If I find the time to fix the platform build, it seems there is also one issue with the integration test.

laeubi avatar Sep 05 '22 16:09 laeubi

What needs fixing in platform build?

akurtakov avatar Sep 06 '22 06:09 akurtakov

What needs fixing in platform build?

If you look here: https://github.com/eclipse-tycho/tycho/runs/7852194553?check_suite_focus=true you see that an artifact is not found with the new resolver so something is missing here, so I need to find out what is special about the org.eclipse.platform:org.eclipse.platform.doc.isv so it misses the dependency when using the new resolver.

laeubi avatar Sep 06 '22 06:09 laeubi

What needs fixing in platform build?

If you look here: https://github.com/eclipse-tycho/tycho/runs/7852194553?check_suite_focus=true you see that an artifact is not found with the new resolver so something is missing here, so I need to find out what is special about the org.eclipse.platform:org.eclipse.platform.doc.isv so it misses the dependency when using the new resolver.

So there are 2 things target-platform-configuration extraRequirements added and also tycho-eclipserun-plugin with extra dependencies. I guess one of them fails.

akurtakov avatar Sep 06 '22 13:09 akurtakov

target-platform-configuration extraRequirements

Yep I think that could be the cause here as the extraRequirements reference reactor projects, eclipse-run is not executed at this phase ... Actually TargetPlatformConfigurationInstallableUnitProvider should handle this.

But as org.eclipse.equinox:org.eclipse.equinox.concurrent:jar:1.2.100-SNAPSHOT is not directly referenced it is somehow ignored because maybe only required indirectly by a package constraint that could also be fulfilled otherwise...

laeubi avatar Sep 07 '22 15:09 laeubi

Maybe it is also an issue with custom-bundle-plugin, there are no failing tests, but there are also not really an integration test using that mojo ...

Also there is one test failing: https://github.com/eclipse-tycho/tycho/runs/7853305177 for a very strange use-case where two bundles with the same id but different versions are used to "simulate" additional requirements...

laeubi avatar Sep 07 '22 15:09 laeubi

Also there is one test failing

I think I found (and fixed) that one, and the platform build is getting a bit further ...

laeubi avatar Sep 07 '22 18:09 laeubi

Also there is one test failing

I think I found (and fixed) that one, and the platform build is getting a bit further ...

All Tycho integration tests are passing now!

laeubi avatar Sep 08 '22 05:09 laeubi

@akurtakov I think I found the issue here the org.eclipse.platform.doc.isv requires the complained equinox.concurrent only through a target platform dependency:

 +- target-platform-extra-requirements-dfcbcb91-e2fb-4757-aefa-d44fdf02841b (0.0.0.1662704954206)
    +- org.eclipse.ecf (3.10.0.v20210925-0032) satisfies osgi.bundle; org.eclipse.ecf 0.0.0
       +- org.eclipse.equinox.concurrent (1.2.100.qualifier) satisfies java.package; org.eclipse.equinox.concurrent.future [1.0.0,2.0.0) --> [org.eclipse.equinox:org.eclipse.equinox.concurrent:eclipse-plugin:1.2.100-SNAPSHOT]

as the new resolver only takes (direct) reactor projects into account when computing inter-project dependencies (because otherwise it would need to fully resolve the target platform what would contradict the idea of the new resolver).

So at this point I don't have a good idea how to solve this without further changes and without resolving the full target platform :-\

laeubi avatar Sep 09 '22 06:09 laeubi

I have now started a discussion at the maven mailing-list about dynamic reordering of project builds, what would solve the issue here as we dynamically discovering a new dependency here, and as we can still inject it into the model, we need a way to trigger/wait for the dynamic discovered reactor dependency to be build first.

laeubi avatar Sep 11 '22 05:09 laeubi

For the ISV stuff I was now able to emit an error that shows the problem:

  • https://github.com/eclipse-tycho/tycho/pull/1744

as every automatic mitigation of such a construct requires fundamental work both in Tycho and Maven (even though very interesting!) I think this would be sufficient for the user whats going on. One can then for example add a pom or target platform dependency to fix the problem.

laeubi avatar Nov 27 '22 12:11 laeubi

ISV now contains extra requirment see

  • https://github.com/eclipse-platform/eclipse.platform.common/pull/102

laeubi avatar Dec 12 '22 16:12 laeubi

Eclipse Platform build runs successful now :+1:

laeubi avatar Dec 14 '22 05:12 laeubi

DependencyComputerTest.testFragmentSplitPackage:235 fails maybe because from the test description only has an optional requirements and is now evaluated "too early" ...

but

[ERROR]   RemoteAgentMavenMirrorsTest.testLoadFromMirroredLocation:77->loadRepositories:119 » Provision
[ERROR]   RemoteAgentMavenMirrorsTest.testLoadFromMirroredLocationWithFallbackId:96->loadRepositories:119 » Provision

is a bit a mystery here...

laeubi avatar Dec 14 '22 06:12 laeubi

Seems some might be because tests still use the legacy resolver stuff here:

  • https://github.com/eclipse-tycho/tycho/pull/1879

laeubi avatar Dec 14 '22 07:12 laeubi

Only these three unit-tests fail as shown by the commits that temporarily disables them...

laeubi avatar Dec 14 '22 11:12 laeubi

@akurtakov @mickaelistria I plan to merge this after the 2022-03 release so we finally get this running (even with the two disabled tests) and have enough room for improvements or fixes without disturbing platform too much.

laeubi avatar Jan 25 '23 05:01 laeubi

It showed up that these failures are not related to the new resolver, so a good start might be right after 2022-03-M2 ...

laeubi avatar Jan 25 '23 15:01 laeubi

@laeubi currently, if I'm not mistaken, Tycho resolves dependencies in AbstractMavenLifecycleParticipant#afterProjectsRead.
Is this changed in the new resolver? Asking because if this is changed I need to revisit IntelliJ IDE compatibility.

lppedd avatar Jan 27 '23 21:01 lppedd

Yeah, just tested it out. Target Platform dependencies are not resolved anymore.
So one has to go back to the classic resolver when importing in the IDE (as the IDE does not build to import).

Hopefully you plan on keeping the classic resolver for a while. Or at least to keep a way of resolving dependencies immediately.

I was thinking on how this could work with the new resolver and TBH... no way.

lppedd avatar Jan 27 '23 21:01 lppedd

Yeah, just tested it out. Target Platform dependencies are not resolved anymore.

They are resolved in the project execution phase, only dependencies between projects are evaluated in the early stage

So one has to go back to the classic resolver when importing in the IDE (as the IDE does not build to import).

Maybe in IntelliJ as it does not resolve the target itself, but as noted elsewhere Tycho builds the stuff from the IDE (Eclipse) not the other way round, if you want it that way you should declare the dependencies in the pom (what IntelliJ will understand)

Hopefully you plan on keeping the classic resolver for a while.

It obviously will be part of Tycho 4, but most likely will vanish in Tycho 5 as Tycho itself has no real need for this and we can't maintain unused stuff for ever.

I was thinking on how this could work with the new resolver and TBH... no way.

m2eclipse has a way for plugins to supply a connector that adds additional stuff for a maven-plugin/mojo to the IDE don't know if IntelliJ support that as well.

laeubi avatar Jan 28 '23 05:01 laeubi

@laeubi maybe the immediate resolution can be offloaded to a third party. When I setup the Maven container in IJ I can override the Tycho lifecycle participant, if it has a name. The key is keeping methods to resolve stuff in a "single method call" in the core.

Regarding m2e, does Tycho have a connector? Tho it's really Eclipse centric and I doubt it can be adapted for IJ.

lppedd avatar Jan 28 '23 09:01 lppedd

can override the Tycho lifecycle participant

Why not register an own one then?

Regarding m2e, does Tycho have a connector?

Tycho has some mappings for mojos but in general a connector is not required as PDE is supported in Eclipse out of the box.

The key is keeping methods to resolve stuff in a "single method call" in the core.

Its very unlikely that Tycho will "standarize" such thing as the whole code of Tycho is considered internal API, but one can see org.eclipse.tycho.resolver.TychoResolver as quite stable internal API that can be used to resolve a project with Tycho, but keep in mind that this is a very costly and long running operation and there is no gurantee about concurrency or reentrance so when you are using it you have to take care that no other code is currently running that method for the same project.

laeubi avatar Jan 28 '23 09:01 laeubi

@laeubi

Why not register an own one then?

Keep in mind this is only for importing, the actual build process won't be touched.

this is a very costly and long running operation and there is no gurantee about concurrency or reentrance so when you are using it you have to take care that no other code is currently running that method for the same project.

I mean I've used as it is now for quite a long time and in my case the importing phase takes about 2/3 minutes (80~ projects), not a big deal.

It's something you do infrequently so a linear import is ok.

What I meant was, if you remove the classic resolver from the lifecycle participant, evaluate if it's necessary to remove all the methods chain or if there is something that can be kept for third party reuse.

lppedd avatar Jan 28 '23 10:01 lppedd

Why not register an own one then?

Keep in mind this is only for importing, the actual build process won't be touched.

But if you can replace a component, then your certainly can simply register an own :-)

What I meant was, if you remove the classic resolver from the lifecycle participant, evaluate if it's necessary to remove all the methods chain or if there is something that can be kept for third party reuse.

Actually there will always be a resolver and it will always do its job (classic or not) what changes is only when it is called (today: before projects are build) but it will always be called during a build. So if anyone wants to perform the full resolve in a context where no build happens, one can call TychoResolver#resolveProject(MavenSession, MavenProject), this is done by some Tycho unit tests already.

laeubi avatar Jan 28 '23 10:01 laeubi