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

Fix MavenArtifactResolver issues with maven 3.9+ #519

Merged
merged 5 commits into from
Oct 15, 2024

Conversation

jperedadnr
Copy link
Contributor

Fixes #506, by removing unnecessary and deprecated way of getting an instance of a RepositorySystem, and a RepositorySession, by just getting these injected by Maven itself in the base Mojo.

Many dependencies from org.eclipse.aether/org.apache.maven.resolver are removed in the process, and therefore, the org.eclipse.aether.connector.basic.BasicRepositoryConnectorFactory missing class from maven-core is not needed anymore.

Now, the plugin can run with any version up until 3.9.9 (latest before going to Maven 4, which is still in beta)

@@ -74,14 +77,31 @@ public abstract class NativeBaseMojo extends AbstractMojo {

private static final List<String> ALLOWED_DEPENDENCY_TYPES = Collections.singletonList("jar");

// TODO: Remove this restriction when MavenArtifactResolver works with Maven 3.9.0+
private static final Version MAX_SUPPORTED_MAVEN_VERSION = new Version(3, 8, 8);
private static final Version MAX_SUPPORTED_MAVEN_VERSION = new Version(3, 9, 9);
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we expect this not to work on 4?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I haven't tested it yet, honestly. Many parts are moving and being deprecated/removed for Maven 4, so I'd say we'll need to revisited when people start to use it (beta for now), or it gets finally released.

Copy link
Contributor

Choose a reason for hiding this comment

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

I believe we need to change at least some logic here, because at this point, the plugin won't work starting with maven 3.9.10. We can perhaps invert the logic and say that we support all versions up to 4.0.0 (but not inclusive)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Maven 4.0.0 is not released yet, but I'm trying to test Maven 4.0.0-alpha-13, and new Version() fails to parse it.

Removing the whole version check works (I've tested 3.6.3, 3.8.1, 3.8.8, 3.9.0, 3.9.9, 4.0.0-alpha-13), so we don't need it anymore.

@jperedadnr jperedadnr merged commit a1a45a6 into gluonhq:master Oct 15, 2024
1 check passed
@jperedadnr jperedadnr deleted the 506-mavenresolver branch October 15, 2024 11:21
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

Successfully merging this pull request may close these issues.

Support for Maven 3.9.0 or newer
3 participants