[MNG-8141] Model Builder report issues during build #1569
Conversation
And ArtifactDescriptorReader should not "toss away" but report WARNings (as in case of ERROR or FATAL MB throws). Part 1: port ArtifactDescriptorReader to not loose WARNs --- https://issues.apache.org/jira/browse/MNG-8141
|
lose, not loose |
|
@gnodet pls take a peek:
|
|
@gnodet also, re caching, I think you are wrong (so resolver does builds same model for same GAV asked for different GACEVs), I mean it DOES cache raw models, but lineage, normalization, profile evaluation etc... all that is repeated (basically only XML parsing from file is spared). EDIT: Am pretty much sure ArtifactDescriptorReader is it. Resolver itself is "agnostic" (no idea about Maven universe), and even have no (and should not have) logic that tells "POM of a Maven GACEV artifact is on coordinates GApomV". Moreover, this indirection is done in ArtifactDescritptorReader as that is Maven specific implementation of ArtifactDescriptorReader resolver component. |
Right, only file and raw models are cached, but even if the effective model is computed twice, no resolver request / disk access should be performed, as it should start from the already loaded raw model. |
| @@ -816,6 +817,15 @@ private Model readEffectiveModel( | |||
| // profile activation | |||
| profileActivationContext.setProjectProperties(modelv4.getProperties()); | |||
|
|
|||
| // profile check | |||
| HashSet<String> profileIds = new HashSet<>(); | |||
There was a problem hiding this comment.
Can't this be done in the ModelValidator ?
There was a problem hiding this comment.
Should a validateProfiles method be added to the ModelValidator ?
There was a problem hiding this comment.
Validator does validate profiles. But for these "far" POMs we use consequently "min" level, that does NOT.
In Mvn3 the idea was to detect this duplication while processing (as profiles do affect build a lot, can add extra properties, dependencies, and failure to properly evaluate them can lead to mysterious failures).
We can move ID validation to Min level, yes, as it currently validate very few things.
There was a problem hiding this comment.
I like these changes (having validation in validator), so am tempted to introduce this change in 3.9 line as well...
Apply changes to related api impl classes as well.
|
Reproducer looks like this: In DEBUG messages are: (which surprised me, as they exactly pinpoint the problems at file line level, unlike 3.9.x) |
No (logic) change, merely moved the new code to proper place (validation) to not piggy back onto processing: this is much cleaner. --- https://issues.apache.org/jira/browse/MNG-8141 Inspired by suggestions in master PR #1569
And ArtifactDescriptorReader should not "toss away" but report WARNings (as in case of ERROR or FATAL MB throws).
Part 1: port ArtifactDescriptorReader to not lose WARNs
Part 2: make model builder report Profile ID duplications as model WARNs
https://issues.apache.org/jira/browse/MNG-8141