Use Maven targets for OSGi compliant artifacts from Maven-Central

[HannesWell]

In order to simplify consuming third-party artifacts from Maven-Central this PR aims to use a Maven-type location in the Target-Platform for all artifacts from Maven-Central that are already OSGi compliant by default.
The version of the artifacts is not updated, this is left for a follow up PR.
Furthermore Guice is not yet included because it has non-OSGi-compliant dependencies (see also merks/simrel-maven#4).

The gpg KEYRING is already set up in the deploy workflow
https://github.com/eclipse/xtext/blob/7ec6b4f188d35ca44d1ba2ea758e9afc61d611e2/jenkins/nightly-deploy/Jenkinsfile#L36-L46
and therefore only the gpg-sgining for the p2 repo has to be activated.

The only thing that is currently missing in this PR is the inclusion of the maven-target IUs in the Oomph-setup respectively in the workspace target location set with it.
At the moment my idea is to move all locations shared between all selectable target-platforms into a xtext-shared.target, which is included in all other xtext-targets as nested target and included into the Oomph targlet as composed-targets.

Additionally the Targlet-Task in the setup could be changed to conditionally include the target-file of the target-platform selected in the workspace (see https://wiki.eclipse.org/Eclipse_Oomph_Authoring#Conditional_Tasks). Actually a task for each target-file would be created, but filtered for the corresponding value of the eclipse.target.platform variable.
This would allow to further reduce the number of URLs defined in the setup (eventually maybe even to zero) and only specify the repo-locations once in a target-file.

This would avoid the need to declare many URLs multiple times and prevents divergence of definitions (I had the impression that in the target-files sometimes a different repository URL is used than in the Oomph-Targlet for the same project).

@cdietrich, @LorenzoBettini @szarnekow what do you think?

[cdietrich]

@HannesWell the current plan is to consume these from the new aggregated maven and orbit thing ed is currently working on

merks/simrel-maven#3 (comment)

So that from our perspective it is transparent if a 3rd party comes from orbit or from maven central
And so also no issue in oomph

regarding the junit5 part I have currentlz this wip branch

https://github.com/eclipse/xtext/tree/cd_junit592intarget

but here the setup with mixing junit 5 and 4 still makes problems to have it nice in eclipse and maven
https://github.com/eclipse/xtext/pull/2208/files

[cdietrich]

i asume you did not stumbe over the junit5 container problem as you migrated latest only

[LorenzoBettini]

@HannesWell @cdietrich, I did not follow the discussion about the new p2 repo replacing orbit. If that repo solves the problem of consuming Maven artifacts, I'd stay with that solution, which looks simpler. Or is using Maven artifacts in the target platform providing some benefits?

Concerning the other parts:

• It would be great to have target platform file inclusions if that can be handled in Eclipse, Tycho, and Oomph.
• Having Oomph using target files would also be great since it would allow us to reuse the same target platform dependency configurations in Oomph and the Tycho build.

[HannesWell]

@HannesWell the current plan is to consume these from the new aggregated maven and orbit thing ed is currently working on

May I ask you why you made that descision?
From a technical point of view it looks possible to me (maybe I oversee something?) and it would allow xtext to update dependencies directly and at the same time test with older versions reliably (e.g. a older version of a lib in the older targets).

And so also no issue in oomph

The suggested change in the setup would probably help even without using Maven Targets to keep the workspace and CI in sync without the need to duplicate/maintain the required IUs and the repo URLs. If you are interested I can do that in a separat PR first.

I have not yet looked into or encountered the junit container issue but I can look into it this evening.

[cdietrich]

cause in the past i made bad performance experience with the maven feature
and alos the setup, doing other n repos, was to complex for me
also oomph does not support it.
also newer versions e.g. asm have different constants
so using old version may not be an option.

=> using maven orbit works as it worked with old orbit with no extra effort

[szarnekow]

Thank you for the initiative Hannes.

In order to simplify consuming third-party artifacts

I wonder how a change +153 -26 can be considered a simplification, though? As you already pointed out, it works only for maven artifacts that are already OSGI bundles wheres the joint orbit repo provides a one-stop-shopping experience to us.

Can you help me understand why it's better to list the maven coordinates in our target file?

[HannesWell]

@HannesWell @cdietrich, I did not follow the discussion about the new p2 repo replacing orbit. If that repo solves the problem of consuming Maven artifacts, I'd stay with that solution, which looks simpler. Or is using Maven artifacts in the target platform providing some benefits?

Can you help me understand why it's better to list the maven coordinates in our target file?

The main advantage of Maven-Targets is that you can consume artifacts from Maven-Central directly.
If you want to add a new artifact you can copy the project's groupd-artifact-version (GAV) in the usual Maven-syntax, Add or Edit a Maven Target location and just hit Add and Save and you have it in your TP after a reload.
If you want to have a new version you can just bump the artifact's version in the Editor or the Target source, reload the TP and you are done.

All of that without the need to do any change in any other repo. Even if that would just be a simple version bump like it is for many deps with the service Ed just created, you still have to go to anther repo, create a PR there, wait until it is reviewed, merged and deployed to a P2 repo.
If you want to do that in Orbit it is much more work, I have never done any change in the Orbit repo but Christion you did and can probably tell about your experience? And for example for the Guice update the first and almost only question was how to do the update Guice in Orbit and since then nothing happened: #2056 (comment)
With Maven-Targets that would have been a single line change in the target-file.

For some artifacts you may be lucky and somebody else updates it, but for some you are not. For example Guice is still at the old version although the update was requested a year ago.

Another advantage of using maven targets is that you can try out snapshot version of libraries by simply setting the version to that snapshot. Consequently you can for example try it out in a draft PR, make necessary code adjustments and report issues early to the library maintainers.

So that from our perspective it is transparent if a 3rd party comes from orbit or from maven central
And so also no issue in oomph

Maven-Targets are not directly supported by Oomph, that is correct, but indirectly if you include a target-file in a Targlet or set it directly using a TargetPlatformTask, in which you use a Maven-location. So with with #2213 the Maven-targets are support indirectly and that works. We use it in M2E for a while, I use that at work and I'm not aware of any significant issue that exists in a recent version.

cause in the past i made bad performance experience with the maven feature and alos the setup, doing other n repos, was to complex for me also oomph does not support it.

IIRC there was a bug in m2e.pde a while ago that wrapped artifacts were downloaded again on each reload or restart, but that is fixed for a while and I'm not aware of any other performance issue.
And from my experience Maven targets are quite fast (of course the first time a artifact must be downloaded) and very robust. Often more robust than PDE's Installable Unit location and therefore also faster (because you don't have to reload again and again).

I wonder how a change +153 -26 can be considered a simplification, though?

The simplification was not regarding the lines of code, which is often a valid measure, but in this case regarding the procedure how to update to a new version or add a completely new artifact. And if we consider #2213 as at least motivated by this change you still have a net save of ~200 lines. :)

As you already pointed out, it works only for maven artifacts that are already OSGI bundles wheres the joint orbit repo provides a one-stop-shopping experience to us.

Sorry if my remark was missleading, but that statement is not correct. Maven-targets work for every artifact available in an accessible Maven repository.
But you can use already OSGi compliant artifacts without any worry because everyone using the same GAV will use the exact bit-wise identical jar file.
If a artifact is not a OSGi bundle you can choose to let the Maven target location generate a corresponding MANIFEST.MF using bnd-lib. If you choose to generate the OSGi MANIFEST.MF, you can also edit the BND instructions, when clicking on Edit instructions, in case the defaults are not sufficient:

The only 'problem' with generating the OSGi metadata is that it can happen that different projects generate different Metadata for the same Maven artifact and for example contribute the same artifact under different Bundle-SymbolicNames to the SimRel.
At the same time the problem is not fundamental if you only use Import-Package to reference such artifacts and not Require-Bundle. The generated Export-Package names are very likely to be same in all cases since they are based on the java package names and the artifact's version. And if such artifacts are only pulled in as transitive dependency (i.e. not contained in a feature or contributed to SimRel directly) only one version will end up in the SimRel repo, if that can serve all requirerers. The resolver will just choose any one. The only drawback is that we then have to modify the jar and cannot take it as it is. But that was the case for Orbit in such scenarios and will be the case when one uses Maven targets, either in each project or in a central service.

Nevertheless, for the sake of having the same metadata generated it would probably be good to have them generated by a 'central' service like Orbit or Ed's SimRel Orbit.

[HannesWell]

Additionally I just added a license compliance check workflow based on the reusable GitHub
workflow provided by eclipse/dash-licenses:

• https://github.com/eclipse/dash-licenses/tree/master#reusable-github-workflow-for-automatic-license-check-and-ip-team-review-requests
• https://github.com/eclipse/dash-licenses/blob/master/.github/workflows/mavenLicenseCheck.yml

This checks that the license of all dependencies are vetted and if some are not, allows to request a license review by the Eclipse IP team using a simple comment on a PR. This only requires a EclipseFDN Gitlab API token:
https://github.com/eclipse/dash-licenses/tree/master#automatic-ip-team-review-requests
If we eventually agree to use Maven targets I can create a Gitlab issue to ask the infra team to add it.

[cdietrich]

@HannesWell we need a repo to consume in oomph from anyway.

what i have understood is

• there is the new simrel oomph maven + orbit thing
• this one aggregates old orbit
• and a new repo the uses the GAV Maven feature

so for bumps i would bump gav version there and can consume it from target platform and oomph

what is unclear to me: how to solve the oomph part of the problem
without introducing a separate "Xtext Orbit"?
or is the proposal dont use the targlet feature in oomph at all but just redirect it to the target platforms?
we would have to try out if this properly works.

[cdietrich]

its not possbile to leverage bom here i assume

[HannesWell]

Not yet, but there is a proposal to support properties in target-definitions, which could be used to only define the version once: eclipse-pde/eclipse.pde#204

[HannesWell]

Thinking about it a second time, Maven targets actually also support pom typed artifacts, that work as bom.
So it is probably already working. I'll check it and report back.

[HannesWell]

OK, using boms is not yet possible, but it should not be too complicated to support it:
eclipse-m2e/m2e-core#625

But it then will probably only be available in Tycho 4.

[HannesWell]

@HannesWell we need a repo to consume in oomph from anyway.

what i have understood is

* there is the new simrel oomph maven + orbit thing

* this one aggregates old orbit

* and a new repo the uses the GAV Maven feature

so for bumps i would bump gav version there and can consume it from target platform and oomph

That's right or you can just bump the version in this repo target and don't have to go to any other repo, wait for any build to complete or don't have to monitor any other repo. :)

what is unclear to me: how to solve the oomph part of the problem without introducing a separate "Xtext Orbit"? or is the proposal dont use the targlet feature in oomph at all but just redirect it to the target platforms? we would have to try out if this properly works.

Yes that was the proposal and with #2213, I think we can consider this problem solved. :)

[HannesWell]

Another advantage of using maven targets is that you can try out snapshot version of libraries by simply setting the version to that snapshot.

[HannesWell]

@cdietrich, @LorenzoBettini, @szarnekow How should we proceed here?

[cdietrich]

i propose to consume from eds site for now

[LorenzoBettini]

i propose to consume from eds site for now

I agree. We can leave this PR open anyway.

[HannesWell]

i propose to consume from eds site for now

I agree. We can leave this PR open anyway.

Can you please elaborate, what you think is missing at the moment in order to complete this?
From the previous question I believe you are waiting for bom support as suggested in eclipse-m2e/m2e-core#625?

[cdietrich]

my main concerns:

• when i was using this in a project with more than 2 deps we saw terrible resolution performance (10 mins instead of 1 min)
• i fear if every project does this on its own the version chaos will become much worse. thus i would perfer to use eds simrel-maven as orbit replacement
• minor concern: have zero clue how the signing thing works, but i assume once it is setup it never will need change

[HannesWell]

my main concerns:

* when i was using this in a project with more than 2 deps we saw terrible resolution performance (10 mins instead of 1 min)

That's unexpected and I don't have the same experience. Maybe this was a bug in a an old version. Looking at the runtime now they are relatively similar like the mater branch.

* i fear if every project does this on its own the version chaos will become much worse. thus i would perfer to use eds simrel-maven as orbit replacement

In other discussions about this topic, the conclusion was to use simply the latest version, unless there is a specific reason not to use the latest version (e.g. for the jakarta.inject:jakarta.inject-api:1.0.5 artifact where the package namespace is changed in version two).

If you configure your metadata properly, i.e. specify suitable version range it does not harm if somebody else contributes a more recent version to SimRel. The resolve then tries to minimize the number of artifacts. But in order to be able to minimize the number of artifacts you should not include third-party deps in features because they are always bound to a specific version.

* minor concern: have zero clue how the signing thing works, but i assume once it is setup it never will need change

This is already done in this PR and the PGP keyring is already set up in the deploy Jenkins pipelines. So everthing is in place. :)

Regarding the not OSGi-ified error-prone dependency, I filed a PR to add OSGi metadata (google/error-prone#3903).
In the meantime, if the PR is not merged and released quickly, I would ask Ed to add a wrapped bundle to the simrel maven that can be consumed from there and is consistent for other potential consumers.
Then you can also simply update to Guice 6/7 once it is released.

[cdietrich]

@HannesWell what, from your perspective, besides turnaround times, speaks against using "eds place"

[HannesWell]

@HannesWell what, from your perspective, besides turnaround times, speaks against using "eds place"

It is not per-se wrong to use 'Eds place', so there are not harmful reasons that speak against using it, besides that it is redundant to build another mini-maven-central-mirror (aka. SimRel-Orbit) for artifacts that are already OSGi complaint.
Ed created that to help other SimRel projects with that because there seems to be demand (I assume because it is similar to how the 'good old Orbit' was), but if everybody would use Maven-Targets for OSGi compliant artifacts, that would not be necessary.

The disadvantages of not using Maven-Targets are simply that you don't get the benefits of them. As you said faster turn-around times, you can experiment with pre-release versions (like you did in #2056, I'm glad you like it for experimenting, but why not use it all the time?), you can update dependencies and required code changes (if any) one by one and don't have to do a bulk update because multiple version changed in Ed's place.

You often tell that you have difficulties to keep up in Xtext with Eclipse platform's pace for new dependencies. I'm convinced that using Maven-Targets will help you in that regard and will make your life easier. Therefore I created this PR and therefore I helped using Tycho 3 in Xtext.

For Guice 6/7 the error-prone dependency is now optional (in Maven and OSGi, google/guice#1739). In case you accept this PR before the upgrade to Guice 6/7 you just have to bump the guice version and no wrapping will be necessary.

i fear if every project does this on its own the version chaos will become much worse. thus i would perfer to use eds simrel-maven as orbit replacement

Btw. looking at the 2023-03 release repo I see for example three versions of guava, according to the qualifier, two of them come from Orbit.
Looking at the 2023-06 M2 release repo I see even four versions of guava and three of them are from Orbit.
Therefore I'm not convinced that Orbit really helps in reducing the number of versions. I assume that is mainly a question of discipline of the project regardless where the artifacts are coming from.

I also want to say again that OSGi already gives us everything we need to let the tools deal with potential multiple version in the SimRel 'reactor', mainly use Import-Package with suitable version-Ranges for third-party dependencies and don't include them in features to not 'lock their version. Additionally this would also have the benefit that this would also lead to higher quality metadata from which also down-stream users would benefit.
And if one really needs an older version for technical reasons (e.g. guice 5 or slf4j-1), then this can be expressed with a proper version range too.

[cdietrich]

is m2e pde integration still not default in the packages eclipse ships?
so using this in our wizard generated target files wont work ootb?

[LorenzoBettini]

is m2e pde integration still not default in the packages eclipse ships? so using this in our wizard generated target files wont work ootb?

It is in the "Eclipse for Committers": https://www.eclipse.org/downloads/packages/release/2023-03/r/eclipse-ide-eclipse-committers

[cdietrich]

see also problems i see in #2671

[HannesWell]

Can we come to a conclusion here?

I'm currently working on support for BOM dependencies in Maven-targets and hopefully will complete this before Tycho 4 is released end of this month, so that it could then be used here.

see also problems i see in #2671

If you decide to use Maven targets of course I can help if any such usage problems arise, although I assume once everything is settled, most of the time not much will change.

[cdietrich]

i am still unsure how to deal with this in domain model example or wizard where we have target platforms users maybe set as target plaform and how to communicate it to users.

[HannesWell]

i am still unsure how to deal with this in domain model example or wizard where we have target platforms users maybe set as target plaform and how to communicate it to users.

Wouldn't it be possible to just use the SimRel release repo (at https://download.eclipse.org/releases/...) there?
Alternatively the maven dependencies could be included in Xtext's p2-repos.

[HannesWell]

i am still unsure how to deal with this in domain model example or wizard where we have target platforms users maybe set as target plaform and how to communicate it to users.

Wouldn't it be possible to just use the SimRel release repo (at https://download.eclipse.org/releases/...) there? Alternatively the maven dependencies could be included in Xtext's p2-repos.

Looks like this is already the case:

https://github.com/eclipse/xtext/blob/main/org.eclipse.xtext.xtext.ui.examples/projects/domainmodel/org.eclipse.xtext.example.domainmodel.releng/tp/domainmodel.target#LL11C77-L11C77

Therefore I think this would not be a problem, since the third-party deps are contributed to SimRel, either via Xtext's p2-repo or indirectly via the Maven-Target mirrors from SimRel-Maven.