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

Support specifying the baseline artifact version range #4317

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,17 @@ public static enum ReportBehavior {
@Parameter(property = "onIllegalVersion", defaultValue = "fail", alias = "tycho.p2.baseline.onIllegalVersion")
private ReportBehavior onIllegalVersion;

/**
* The version range of the reference artifact in the baseline.
* <p>
* This can be used to select the desired baseline artifact if multiple versions of an artifact
* are available and to compare. By default a local artifact is compared to the latest version
* in the baseline.
* </p>
*/
@Parameter(defaultValue = "0.0.0", alias = "tycho.p2.baseline.versionRange")
private String baselineVersionRange = "0.0.0";
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems hardly usable, as one needs to explicitly configure the version per project (also 0.0.0 is not really a version range).

In PDE features we have the concept of a "match rule" that might be more sufficient here as we then can compute a version range based on the current project version.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes that's what I meant in my initial comment with:

Instead of specifying the version range explicitly we could also define an enum similar to ReportBehavior to specify 'named version rules' for the baseline artifact, like same-major and latest.

So I think the main question is what rules to support?
We could start simple and just have something like same-major and latest. Using the exact set of rules from PDE doesn't look suitable to me, because Perfect and Greater or Equal doesn't make sense in this context, because this mojo also checks for backward steps.
So besides same-major and latest actually only same-minor might be of interest.
But the names of the rules could be more speaking.

For the implementation we maybe could re-use PDE's new:
https://github.com/eclipse-pde/eclipse.pde/blob/3f973e87fd7ede513388e8ed870d1b65de438947/ui/org.eclipse.pde.core/src/org/eclipse/pde/core/plugin/VersionMatchRule.java

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So I think the main question is what rules to support?

We can just support what is useful today and add more if required, I think thats how its already done on some other places in Tycho

We could start simple and just have something like same-major and latest.

I would prefer to just use the same naming as for features as this is a P2 concept and we should not mix to much naming.

Using the exact set of rules from PDE doesn't look suitable to me, because Perfect and Greater or Equal doesn't make sense in this context,

Greater or equal is actually what is the default today, the only one not so useful might be "Perfect" that actually would never match anything.

For the implementation we maybe could re-use PDE's new:

Tycho has already org.eclipse.tycho.model.Feature.getVersionRange(ImportRef) we can simply generalize that method, so I don't think relying on PDE-UI in Tycho just to reuse an enum is very useful, if we ever do

it would of course be good to reuse as much as possible.


@Component()
P2ResolverFactory resolverFactory;

Expand Down Expand Up @@ -161,7 +172,7 @@ public void execute() throws MojoExecutionException, MojoFailureException {
try {
String id = item.getId();
Version version = new Version(item.getVersion().toString());
P2ResolutionResult res = resolver.resolveInstallableUnit(baselineTP, id, "0.0.0");
P2ResolutionResult res = resolver.resolveInstallableUnit(baselineTP, id, baselineVersionRange);

for (Entry foundInBaseline : res.getArtifacts()) {
Version baselineVersion = new Version(foundInBaseline.getVersion());
Expand Down
Loading