[MNG-7851] Improve error message when modelVersion is 4.0

[candrews]

MNG-7851 Improve error message when modelVersion is 4.0

Improve DefaultModelValidator.compareModelVersions to be able to compare versions which different numbers of segments, allowing it to compare "4", "4.0", and "4.0.0" to each other for example.

---

Following this checklist to help us incorporate your
contribution quickly and easily:

• Make sure there is a JIRA issue filed
  for the change (usually before you start working on it). Trivial changes like typos do not
  require a JIRA issue. Your pull request should address just this issue, without
  pulling in other changes.
• Each commit in the pull request should have a meaningful subject line and body.
• Format the pull request title like [MNG-XXX] SUMMARY,
  where you replace MNG-XXX and SUMMARY with the appropriate JIRA issue.
• Also format the first line of the commit message like [MNG-XXX] SUMMARY.
  Best practice is to use the JIRA issue title in both the pull request title and in the first line of the commit message.
• Write a pull request description that is detailed enough to understand what the pull request does, how, and why.
• Run mvn clean verify to make sure basic checks pass. A more thorough check will
  be performed on your pull request automatically.
• You have run the Core IT successfully.

If your pull request is about ~20 lines of code you don't need to sign an
Individual Contributor License Agreement if you are unsure
please ask on the developers list.

To make clear that you license your contribution under
the Apache License Version 2.0, January 2004
you have to acknowledge this by using the following check-box.

• I hereby declare this contribution to be licenced under the Apache License Version 2.0, January 2004

• In any other case, please file an Apache Individual Contributor License Agreement.

[michael-o]

This will very likely violate the generated XML Schema. Moreover, the validator prints a hard version. I consider this change wrong.

[candrews]

This will very likely violate the generated XML Schema.

How does it violate the schema? Can you point me to something?

The result of this change is only that the error message changes - no other behavior changes.
With <modelVersion>4.0.0</modelVersion>:
Before:
'modelVersion' of '4.0' is newer than the versions supported by this version of Maven: [4.0.0]. Building this project requires a newer version of Maven.
With this PR:
'modelVersion' must be one of [4.0.0] but is '4.0'.

In this PR, I added a unit test that demonstrates this change in error messaging behavior for this situation.

Moreover, the validator prints a hard version. I consider this change wrong.

I'm sorry, I don't understand... Can you please clarify, perhaps with an example?

[michael-o]

Just checked, the schema does not enforce the model version...weird. I'd say that it should be fixed literal 4.0.0. For me, this is similar to an XML namespace, if you change a single char then you change the entire namespace.
@gnodet @cstamas @hboutemy WDYT?

[candrews]

I'd say that it should be fixed literal 4.0.0.

Currently, Maven does require it to be the literal 4.0.0:
https://github.com/apache/maven/blob/maven-3.9.4/maven-model-builder/src/main/java/org/apache/maven/model/validation/DefaultModelValidator.java#L138
https://github.com/apache/maven/blob/maven-3.9.4/maven-model-builder/src/main/java/org/apache/maven/model/validation/DefaultModelValidator.java#L1433

This PR adjusts the error message that is returned when attempts to use non-4.0.0 values are made.

[michael-o]

I take my first comment back. Thanks for this clarification.

[cstamas]

allowing it to compare "4", "4.0", and "4.0.0" to each other for example.

And to do what? I don't see where is the error message adjusted... what do i miss?

[candrews]

And to do what? I don't see where is the error message adjusted... what do i miss?

The new test case at

maven/maven-model-builder/src/test/java/org/apache/maven/model/validation/DefaultModelValidatorTest.java

Line 120 in 78da8ff

public void testModelVersion40() throws Exception {

demonstrates how the message changes. Before this PR, the message in this case is 'modelVersion' of '4.0' is newer than the versions supported by this version of Maven: [4.0.0]. Building this project requires a newer version of Maven. And with this PR, as this test case proves, the error message is 'modelVersion' must be one of [4.0.0] but is '4.0'..

[cstamas]

LGTM

[michael-o]

LGTM

then please approve

[gnodet]

If we're going litteral, shouldn't we simply check that the version in the model is one of the supported version. The validateModelVersion could be simplified to just check the version is supported or thrown an exception without having to check for newer / older versions.

[cstamas]

The "version comparing"-like code is there (and was) only for "meaningful" error message: newer -> you need newer maven. But i agree, to me this looks overly complicated as well (or just use Version class instead?)

[gnodet]

If we're going litteral, shouldn't we simply check that the version in the model is one of the supported version. The validateModelVersion could be simplified to just check the version is supported or thrown an exception without having to check for newer / older versions.

@candrews could you simplify the code for the validateModelVersion method so that it simply checks if the version is in the provided set instead (third branch of the tests) instead of comparing with all other versions ?

[gnodet]

I changed my mind... LGTM