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

[MNG-8180] Fail install/deploy if rogue Maven Plugin metadata found #1611

Merged
merged 4 commits into from
Jul 11, 2024

Conversation

cstamas
Copy link
Member

@cstamas cstamas commented Jul 11, 2024

Resolver handles transparently the repository metadata, and in case of plugins it peeks into META-INF/maven/plugin.xml of given artifact JAR to figure out needed metadata bits (prefix, name, etc).

But, this was done "blindly", while it is expected that GA of JAR artifact without classifier (requirement for maven plugins) and GA in embedded plugin metadata must be same.

Decision here is to fail hard, prevent this being installed and deployed, as this is most probably wrong (unsure what maven-indexer or even Sonatype search would do in this case).


https://issues.apache.org/jira/browse/MNG-8180

Resolver handles transparently the repository metadata, and in case
of plugins it peeks into META-INF/maven/plugin.xml of given
artifact JAR to figure out needed metadata bits (prefix, name, etc).

But, this was done "blindly", while it is expected that GA of JAR
artifact without classifier (requirement for maven plugins) and
GA in embedded plugin metadata must be same.

Decision here is to fail hard, prevent this being installed
and deployed, as this is most probably wrong (unsure what
maven-indexer or even Sonatype search would do in this case).

---

https://issues.apache.org/jira/browse/MNG-8180
@cstamas cstamas self-assigned this Jul 11, 2024
@cstamas cstamas marked this pull request as ready for review July 11, 2024 10:02
@cstamas cstamas added this to the 3.9.9 milestone Jul 11, 2024
}
}
} catch (RuntimeException e) {
Copy link
Member

Choose a reason for hiding this comment

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

Which for example?

Copy link
Member Author

Choose a reason for hiding this comment

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

IAPMEx? But lets have NPE and other fail the build as well, as below it is in comment, IOEx, PlexusXMLEx etc are and were swallowed.

Copy link
Member

Choose a reason for hiding this comment

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

But why catch and rethrow?

Copy link
Member Author

Choose a reason for hiding this comment

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

As other catch below catches all?

Copy link
Member Author

@cstamas cstamas Jul 11, 2024

Choose a reason for hiding this comment

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

I might be missing something, intent is "throw all unexpected ones" (or, swallow all expected ones, like IOEx, XML parsing and Plexus XML ones, these are all checked ones).

Also, the newly introduced IAPMEx is "unexpected" as well. All "unexpected" ones will fail the build, while all the "expected" ones are swallowed, as were since 1.9.0.

Copy link
Member

Choose a reason for hiding this comment

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

Then I would just rethrow InvalidArtifactPluginMetadataException.

Copy link
Member Author

Choose a reason for hiding this comment

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

K, I will merge this PR as is, and later we can refine.

@cstamas cstamas merged commit 8d0e438 into apache:maven-3.9.x Jul 11, 2024
12 checks passed
@cstamas cstamas deleted the maven-3.9.x-MNG-8180 branch July 11, 2024 16:47
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.

3 participants