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

Please use proper version ranges when generating poms to be deployed to maven central #119

Closed
laeubi opened this issue Sep 29, 2022 · 16 comments
Assignees

Comments

@laeubi
Copy link

laeubi commented Sep 29, 2022

As already noticed in the recent IP issue, Platform currently produces (from maven point of view) very bad meta-data by using way to open version ranges. This can easily break consumers of such artifacts and threaten the stability of builds!

One example is:
https://mvnrepository.com/artifact/org.eclipse.platform/org.eclipse.equinox.p2.repository/2.6.200 which even use completely open ranges. That means maven will always resolve to the latest version on central, what obviously will only be a matter of time to break consumers.

This already broke Tycho here:

and even on a frozen branch here:

because the mentioned artifact now pulls in https://repo1.maven.org/maven2/org/bouncycastle/bcpg-jdk18on/1.72/ that was just release a few hours ago, while it was as far as I know compiled against https://mvnrepository.com/artifact/org.bouncycastle/bcpg-jdk18on/1.71

So if it is not know that a certain dependency can work (and compile!) with a range of version, the version used to produce that artifact should be used instead, what still will allow maven to use never version but prefer the given one if no one requests to do so see:
https://cwiki.apache.org/confluence/display/MAVENOLD/Dependency+Mediation+and+Conflict+Resolution#DependencyMediationandConflictResolution-DependencyVersionRanges

@merks
Copy link
Contributor

merks commented Sep 29, 2022

This will be challenging. It's already new to the most recent release that package requirements are mapped to maven dependencies in the first place, by the last rule here:

image

To implement that, we must resolve the package import to a bundle to get the coordinates of the dependency. But there is no information specified everywhere else about what might be an appropriate version range on that bundle. In in the case you're mentioning, you appear to want to exclude the next minor version, but it's far more common to only exclude the next major version expecting APIs to remain compatible. We'll need to add additional rules to covers special cases, but @mickaelistria has been trying to eliminate rules...

@kwin
Copy link

kwin commented Oct 4, 2022

I am working currently on https://issues.apache.org/jira/browse/MENFORCER-427 which allows to easily ban version which require resolving (version ranges and placeholders LATEST, RELEASE)

@kwin
Copy link

kwin commented Oct 4, 2022

Maybe https://www.mojohaus.org/versions-maven-plugin/resolve-ranges-mojo.html can be added to the Maven build to resolve the versions only once during build.

@mickaelistria
Copy link
Contributor

FWIW, we don't need extra plugins to achieve that; we do already know the compatible versions of dependencies when generating the pom files. The pom generator should just be changed to use those versions directly instead of generating a range from the requirement.

@laeubi
Copy link
Author

laeubi commented Oct 5, 2022

@merks I just wanted to let you know that Tycho now includes a new extension component org.eclipse.tycho.packaging.reverseresolve.ArtifactCoordinateResolver that plugins can implement and provide mappings from Dependency > Maven GAV, so your "mappingtable" could maybe be integrated in some kind of cbi-plugin that then could be used here as an alternative source, just in case someone likes to follow the path further...

@merks
Copy link
Contributor

merks commented Oct 5, 2022

I have a feeling that there is something that needs to be clearly stated and properly understood here. For this specific example this IU has the following package requirements:

image

In general, these might or might not include a version range; in this case they have a one with only a lower bound. In any case, we must resolve each requirement to the corresponding capability/capabilities and then we decide that the IU(s) that provides those capabilities are to be treated as dependencies; this is new and was not supported before the most recent release. Keep in mind too that in general there is no relation between the IU's version and the capability's (package's) version so even if the package requirement has a range, that range cannot necessarily be mapped to a range of IUs. Even in this specific example, OSGi suggests that any package version higher than 1.69.0 is fine, but you're saying that's not correct either...

In any case, yes, we could create a range for such a dependency based on the resolved IU available at the time the mapping is built; we already do that work just to get the IU needed for the dependency. But now the question is, is the very narrow range you are suggesting the only "proper" thing to produce? I think different users of the aggregator will have different ideas of what's proper and desired. It is after all extremely common to create ranges that exclude the next major version and we also see that it's problematic to consume 3rd party dependencies that have narrow dependency version ranges because it makes it hard to use other newer things. So I'm more than a little doubtful that "proper" is well defined and that everyone will generally agree it should be done exactly a certain way, e.g, with a very narrow range.

@laeubi
Copy link
Author

laeubi commented Oct 5, 2022

Even in this specific example, OSGi suggests that any package version higher than 1.69.0 is fine, but you're saying that's not correct either...

You map a package import (that might resolve to any version >= 1.69) to an artifact (that is the actual bundle) and this bundles (maven) version should then be used, not a range, because that is the item the aggregator was built with. Given a single version in maven means:

Please use this version, but if (because of conflicts with other dependencies) someone else suggest a higher version this is fine.

and that's the closest match we can get between maven<->osgi here, that open version ranges on packages/bundles are also bad is just another story...

@merks
Copy link
Contributor

merks commented Oct 5, 2022

@laeubi Thanks for the clarification! So the idea is to use the version of the resolved bundle as the dependency's version which is strong hint to use that version but is also effectively like a lower bound...

@kwin
Copy link

kwin commented Oct 5, 2022

The Maven dependencies are just used for building and should use the lower bound of things which are deployed at run time in the OSGi container. They should indicate the minimum version of a dependency they require from an API perspective.

@laeubi
Copy link
Author

laeubi commented Oct 5, 2022

@merks basically yes, but keep in mind that OSGi Version is not always maven version of a given artifact it is just often the case.

merks added a commit to eclipse-cbi/p2repo-aggregator that referenced this issue Oct 9, 2022
In particular, support the ability to specify the version range of the
dependency, including expressions to extract the version components from
the bundle that provides the capability that satisfies the requirement,
e.g., [major.minor.micro,major+1.0.0)

#11

eclipse-platform/eclipse.platform.releng#119
merks added a commit to merks/eclipse.platform.releng that referenced this issue Oct 9, 2022
Use 4.26-I-builds in the SDK4Mvn.aggr.

Eliminate the mapping for bouncycastle.

eclipse-platform#119
merks added a commit that referenced this issue Oct 9, 2022
…122)

Use 4.26-I-builds in the SDK4Mvn.aggr.

Eliminate the mapping for bouncycastle.

#119
@merks
Copy link
Contributor

merks commented Oct 9, 2022

I modified the SDK4Mvn.aggr to use the new support described here:

eclipse-cbi/p2repo-aggregator#11 (comment)

Unfortunately these builds never seem to be fully successful:

https://ci.eclipse.org/releng/view/Publish%20to%20Maven/

@merks merks closed this as completed Oct 9, 2022
@laeubi
Copy link
Author

laeubi commented Oct 9, 2022

@merks I'm not sure if I understand correctly

I modified the SDK4Mvn.aggr to use the new support described here:
Unfortunately these builds never seem to be fully successful:

So was the code changed but this leads to failing builds, or are the failing builds unrelated but currently prevent one from testing it?

@merks
Copy link
Contributor

merks commented Oct 9, 2022

This part, which uses the aggregator, works:

https://ci.eclipse.org/releng/view/Publish to Maven/job/CBIaggregator/lastBuild/console

These downstream triggered jobs fail quite a lot but sometimes succeed:

https://ci.eclipse.org/releng/view/Publish%20to%20Maven/job/PublishPDEToMaven/
https://ci.eclipse.org/releng/job/PublishJDTtoMaven/

This one hasn't worked for several weeks:

https://ci.eclipse.org/releng/job/PublishPlatformToMaven/

There are clues about the failures in files like this:

https://ci.eclipse.org/releng/job/PublishPDEToMaven/lastSuccessfulBuild/artifact/.log/javadoc-upload.txt

Nothing changed in the aggregator since it was successfully used to publish the 4.25 release, until today.

That why I asked on this issue which build is involved in producing the "missing" results:

#121

If it relies on the following build succeeding then one might have to wait forever:

https://ci.eclipse.org/releng/job/PublishPlatformToMaven/

@merks
Copy link
Contributor

merks commented Oct 9, 2022

Actually what I said above I think is not entirely true. I think the aggregator did change after the release. I need to retest how it is handling snapshot versions because that's poorly tested I think...

merks added a commit to eclipse-cbi/p2repo-aggregator that referenced this issue Oct 9, 2022
@merks
Copy link
Contributor

merks commented Oct 9, 2022

In here:

https://ci.eclipse.org/releng/view/Publish%20to%20Maven/job/PublishPlatformToMaven/lastBuild/artifact/.log/artifact-upload.txt

I see an error like this:

[ERROR] Failed to execute goal org.apache.maven.plugins:maven-gpg-plugin:3.0.1:sign-and-deploy-file (default-cli) on project org.eclipse.platform.publish2maven: /home/jenkins/agent/workspace/PublishPlatformToMaven/org/eclipse/platform/org.eclipse.equinox.p2.core/2.9.200-SNAPSHOT/org.eclipse.equinox.p2.core-2.9.200-20220817.120800-0.jar not found. -> [Help 1]

But building the aggregation locally I see the jar exists:

image

@mickaelistria
Copy link
Contributor

mickaelistria commented Oct 21, 2022

Topic continues and evolves as #127

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants