Support version ranges and no versions for units in target definitions

[HannesWell]

This is an early draft to add support for version ranges and no versions for unit elements in target-definitions.

This allows users to omit the version attribute in a unit element (which is equivalent to the value 0.0.0) or to specify a range (in the usual OSGi synax) as its value.

For example:

<target name="myTarget">
	<locations>
		<location includeAllPlatforms="false" includeConfigurePhase="true" includeMode="planner" includeSource="false" type="InstallableUnit">
			<repository location="https://download.eclipse.org/releases/2023-12/"/>
			<unit id="org.eclipse.sdk.feature.group" version="0.0.0"/>
			<unit id="org.eclipse.m2e.sdk.feature.feature.group" />
			<unit id="com.google.guava" version="[30,40)"/>
		</location>
	</locations>
</target>

Resolves to

This also contains two commits with code clean-ups in the target handling that I plan to provide as separate PR soon.

There are still a few things to work out regarding removal of IUs and Updating the target and the UI should probably also be adapted, but the most basic functionality is already working.

Fixes #757

[github-actions]

Test Results

   285 files  ± 0     285 suites  ±0   50m 51s ⏱️ + 1m 13s
 3 586 tests + 5   3 508 ✅ + 3   76 💤 ±0  0 ❌ ±0  2 🔥 +2 
10 950 runs  +15  10 717 ✅ +13  231 💤 ±0  0 ❌ ±0  2 🔥 +2 

For more details on these errors, see this check.

Results for commit 12bdc13. ± Comparison against base commit c6325a1.

♻️ This comment has been updated with latest results.

[merks]

I guess support for this in tycho is implemented separately.

[laeubi]

I guess support for this in tycho is implemented separately.

Yes that's the only drawback this will immediately make the target file incompatible to any older Eclipse or Tycho version :-\

Theoretically there is a "target version" processing instruction, so maybe we should try to use that to make at least PDE aware of the incompatibility?

[laeubi]

Just one thing about the UI I think in such a case we should adapt the labelprovider to show the range + the resolved version.

Also then we need some UI to edit the version (range) e.g. in the wizard there could be an additional radio option to use:

1. use exact version
2. use latest version
3. use latest excluding next major

and of course the most challenging thing will be if one edit such a location so it retains the exiting version ranges.

[HannesWell]

Just one thing about the UI I think in such a case we should adapt the labelprovider to show the range + the resolved version.

Also then we need some UI to edit the version (range) e.g. in the wizard there could be an additional radio option to use:

1. use exact version

2. use latest version

3. use latest excluding next major

My plan is to add an extra column labeled Version specification where one can either enter a version-range or latest to get 0.0.0 respectively no version which leads to the latest version:

As a side-note: That items are displayed three times also happens without this change for me and is something I'll look into separately.

I also considered adding a separate column with a check-box latest version or alike or the suggested radio button, but then we have multiple columns and one can only use one of them. Having only a version specification that can be used for both, solves this problem and the artifact version on the left implicitly shows the 'resolved' version.
What should be added is a validation that the artifact's version is within the specified range.

This is probably not a perfect solution, but this dialog is quite difficult so I'm happy if there is a working solution that's no too bad.

[laeubi]

Having only a version specification that can be used for both, solves this problem and the artifact version on the left implicitly shows the 'resolved' version.

I would simply call it either "version" or "version range", btw. the displayed range in the screenshot looks wrong should it be [1, 2)?

[merks]

From what I recall from a few weeks ago, if there are multiple locations and item will be shown once per location leading to what look like duplicates. (I tried to explain to the user that these aren't really duplicates in the sense of duplicated artifacts; that took some convincing!)

[laeubi]

if there are multiple locations and item will be shown once per location leading to what look like duplicates.

I would argue that they are actual duplicates (in this context) as P2 will only pick up one anyways and one can't specify the location where to load it from.

[HannesWell]

With all the other preceding PRs this is now in a state where I think it's complete.
The only thing that I think is missing are a few tests.

I guess support for this in tycho is implemented separately.

Yes that's the only drawback this will immediately make the target file incompatible to any older Eclipse or Tycho version :-\

Theoretically there is a "target version" processing instruction, so maybe we should try to use that to make at least PDE aware of the incompatibility?

I think that's in the nature of such change. But as long as one does not use the new possibilities, everything continues to work as it used to be. Therefore I'm not sure specifying a new version is necessary, especially since it's not trivial to add handling for a new version and implementing the procedure for the user to switch to the new version respectively stay on the old version. This would require quite a lot of extra code and special-handling.

But I'll provide a PR to support it in Tycho.

Having only a version specification that can be used for both, solves this problem and the artifact version on the left implicitly shows the 'resolved' version.

I would simply call it either "version" or "version range", btw. the displayed range in the screenshot looks wrong should it be [1, 2)?

But there is already a version column and version range does not fit as well if one uses latest. Version specification is IMO suitable for both.
Yes the version is odd, but just because I simply specified it this way for testing the ranges.

if there are multiple locations and item will be shown once per location leading to what look like duplicates.

I would argue that they are actual duplicates (in this context) as P2 will only pick up one anyways and one can't specify the location where to load it from.

I only had one location with multiple repos. I looked a bit into it but this UI in general is difficult.
But anyways, I would like to discuss that topic somewhere else and focus here on the support for version ranges and empty versions.

[laeubi]

I think that's in the nature of such change. But as long as one does not use the new possibilities, everything continues to work as it used to be. Therefore I'm not sure specifying a new version is necessary, especially since it's not trivial to add handling for a new version and implementing the procedure for the user to switch to the new version respectively stay on the old version

I mean this one here

https://github.com/eclipse-platform/eclipse.platform.releng.aggregator/blob/53b4ab2ba38d033775f7f5cc141b15c92b5b29d7/eclipse.platform.releng.prereqs.sdk/eclipse-sdk-prereqs.target#L2

I have never checked how it works but somehow PDE can already handle older target versions and then works slightly different. I just noticed that accidentally in the past where some features didn't work as I somehow created an older target version with some wizard.

This would require quite a lot of extra code and special-handling.

Maybe we than should probably drop support for "versioned" target file format?

But there is already a version column and version range does not fit as well if one uses latest. Version specification is IMO suitable for both.

Maybe we should rename the old one to "Resolved Version" and switch the order, and name it then "Specified Version" or "Provided Version".

[HannesWell]

I think that's in the nature of such change. But as long as one does not use the new possibilities, everything continues to work as it used to be. Therefore I'm not sure specifying a new version is necessary, especially since it's not trivial to add handling for a new version and implementing the procedure for the user to switch to the new version respectively stay on the old version

I mean this one here

https://github.com/eclipse-platform/eclipse.platform.releng.aggregator/blob/53b4ab2ba38d033775f7f5cc141b15c92b5b29d7/eclipse.platform.releng.prereqs.sdk/eclipse-sdk-prereqs.target#L2

I have never checked how it works but somehow PDE can already handle older target versions and then works slightly different. I just noticed that accidentally in the past where some features didn't work as I somehow created an older target version with some wizard.

Yes, but as far as I know this is just for reading and not considered while writing. And this is necessary because with older formats the target definition used a different structure.
But in this case the read is backwards compatible with old target-files that don't use the new features.

I have no further refined the code so that specified empty versions are preserved in order to increase compatibility. With the current implementation this has the effect that if one adds units through the UI the explicit empty version is added back. But I assume if one manages a target through the UI, that person does not really care if it can save typing the empty version.
Just updating a target preserves specified empty version and no specified version as it is.
Since leaving out the version is mainly useful if one edits the target file via the source tab, I think this behavior is fine.

And if some time has passed and support for no versions exists long enough we could change PDE the automatically remove empty versions on the next change.

This would require quite a lot of extra code and special-handling.

Maybe we than should probably drop support for "versioned" target file format?

That's another issue we can think of later. I have to check first since when the latest version exists. But yes dropping support for older versions would allow us to remove quite some code.

But there is already a version column and version range does not fit as well if one uses latest. Version specification is IMO suitable for both.

Maybe we should rename the old one to "Resolved Version" and switch the order, and name it then "Specified Version" or "Provided Version".

Since that UI is mainly inherited from P2 I actually would like to do as few changes as possible. And since the UI actually lists available Unit I find it somehow fitting to keep the name of that column as version (since it the version of the artifact). As said the dialog is actually from P2 to select software to install. It is not too bad to use it in PDE if one specifies exact version, but with version ranges and empty versions that's all not ideal. But as said above from my POV these new features are mainly for working from the source tab and the editor is for me only secondary.
On the long run I would like to make the UI of the Editor obsolete by improving tool support in the Source-tab.

That being said, we could also name the new column Specified Version or Provided Version, but I have to say that I don't find it more suitable. In general I think to substantives are better here. And if you don't like version specification, maybe version definition or version declaration is better?

[HannesWell]

I have now tests for the resolution and serialization of units without version and with version range.

Given these tests pass (🤞🏽) and there are no major objections I'll submit this tomorrow, Wednesday evening.

[HannesWell]

Test failures are unrelated, submitting.

Thanks to everyone who participated in this discussion.

[HannesWell]

Support in Tycho for this is added via

• Support version-ranges and no-version for units in IU target locations eclipse-tycho/tycho#4361

[HannesWell]

N&N entry:

• Add N&N for support of version ranges by the PDETarget Editor eclipse-platform/www.eclipse.org-eclipse#236