maven-plugin-tools icon indicating copy to clipboard operation
maven-plugin-tools copied to clipboard

[MPLUGIN-522] Prerequisites should be opt-in

Open cstamas opened this issue 1 year ago • 11 comments

But they are not. In fact, if user does not deal with them, they are way too aggressive and in fact, plain wrong.

Prerequisite was opt-in and should have remain opt-in.

This PR:

  • by default does NOT add prerequisite (as before)
  • user can set "auto" (when it is IMHO wrongly determined)
  • or can set any value needed

https://issues.apache.org/jira/browse/MPLUGIN-522

cstamas avatar May 07 '24 07:05 cstamas

and in fact, plain wrong.

I disagree with this statement, and also disagree that they should be opt in. Details in the JIRA: https://issues.apache.org/jira/browse/MPLUGIN-522?focusedCommentId=17844188&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-17844188

kwin avatar May 07 '24 08:05 kwin

Ok, then we have similar situation as with prefix: fail the build.

Plugin should be modified to:

  • if none set, fail the build
  • if "auto" set by user, everything happens as now
  • if some value set by user, use that

We must make users aware of this requirement (hence the build failure), as otherwise they implicitly use "auto" and end result is wrong.

cstamas avatar May 07 '24 08:05 cstamas

I think enforcing prerequisites is a good thing and the default match for 99% of the cases. For other cases just overwrite with explicit values. Not stating prerequisites should no longer be supported because it is frustratring for users to figure out the implicit prerequisites through trial and error...

Putting prerequisites which are greater than the one actually used to build are, at first, very weird. For example, if you project requires jdk 11, you build with jdk 11, test it with jdk 11, but your prerequisites ends up with jdk 22... ? That's really unexpected, and the user has no real knowledge about the value used.

The prerequisites should not be higher than the target JDK.

gnodet avatar May 07 '24 08:05 gnodet

Also, unsure how then this thing works? https://maven.apache.org/plugins/maven-jar-plugin/plugin-info.html#system-requirements-history

cstamas avatar May 07 '24 08:05 cstamas

Putting prerequisites which are greater than the one actually used to build are, at first, very weird.

Let's focus on issue with the automatic detection. Please point me to a concrete example where this is the case and where it is wrong.

kwin avatar May 07 '24 08:05 kwin

I think enforcing and explicitly stating prerequisites is a good thing and the default matches for 99% of the cases. For other cases just overwrite with explicit values. Not stating prerequisites should no longer be supported because it is frustratring for users to figure out the implicit prerequisites through trial and error...

Just to repeat myself, the prerequisites are documented, so no need for user trial-and-error. This table shows Maven and Java prerequisites clearly: https://maven.apache.org/plugins/maven-jar-plugin/plugin-info.html#system-requirements-history

cstamas avatar May 07 '24 09:05 cstamas

This PR should be redone once Maven3 and Maven4 m-p-p are split (into maven-3.x and master) branch. And then:

  • default Java prerequisite should pick up from compiler plugin (in pretty much similar way as report does)
  • default Maven prerequisite should be "3.2.5" on maven-3.x branch and "4.0.0" on master branch.

Originally, the lack of prerequisite meant "it works with Maven3" (no range specified). IMO, using "3.2.5" today is completely okay, it covers 10 years of history. If users wants to go more backward, it can explicitly configure prerequisite to "3.1.1" or whatever.

cstamas avatar May 07 '24 09:05 cstamas

But just for the record: current m-p-p releases produce broken Maven Plugin XML, in a way that plugin works just fine with Maven3, while Maven4 explodes with invalid error messages.

cstamas avatar May 07 '24 09:05 cstamas

For the record: The automatic detection in this case was not wrong it was just that the referenced artifact (JLine3) states it is Java8 compatible but contains Java22 bytecode classes (https://the-asf.slack.com/archives/C7Q9JB404/p1715084058873669?thread_ts=1715069202.024659&cid=C7Q9JB404). I think this edge case justifies that the automatic detection should be explicitly overwritten!

kwin avatar Aug 01 '24 16:08 kwin

Am unsure here... as I do not control is my transitive (2nd level, 3rd level) dependency uses multi-release or not. For me this is magic, and the bad kind of it.

cstamas avatar Aug 01 '24 22:08 cstamas