[MNG-7754] Improvement and extension of plugin validation #1079

cstamas · 2023-04-03T16:25:14Z

This is general rework of current Maven 3.9.x line how it handles plugin and mojo validations.

Changes:

added plugin validations for dependencies
introduced pluginValidationManager that gathers violations
manager creates a report at build end, with dense and non repeating data
this is in spirit to lessen already too verbose logging, as current solution would report violations as many times plugin is used in reactor, and that can be many (ie. a plugin from parent for each module)

Example report of Maven 3.9.x build: https://gist.github.com/cstamas/b62fdcd53eaf316123cf183f5a24e6a5

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

As it seems is confusing to some users --- https://issues.apache.org/jira/browse/MNG-7754

…essages. They are now reported at end.

Fixed ITs to not fail: - assert only for content, neglect and possible indentation - do not assert what is NOT warn logged, as later we may log more (IT would be not future proof) Tested with 3.9.1 and 3,9,2-SNAPSHOT w/ apache/maven#1079 Both passes OK

cstamas · 2023-04-04T13:28:06Z

IT fix apache/maven-integration-testing#256
Tested with 3.9.1 and this PR, both pass OK

michael-o

Looking at the output:

[WARNING]    * 70 : 15, org.apache.maven:maven-settings-builder:3.9.2-SNAPSHOT /home/cstamas/Worx/apache-maven/maven/maven-settings-builder/pom.xml
[WARNING]    * 131 : 15, org.apache.maven:maven-resolver-provider:3.9.2-SNAPSHOT /home/cstamas/Worx/apache-maven/maven/maven-resolver-provider/pom.xml
[WARNING]    * 192 : 15, org.apache.maven:maven-core:3.9.2-SNAPSHOT /home/cstamas/Worx/apache-maven/maven/maven-core/pom.xml

I don't understand how to read those numbers and to what they relate.

michael-o · 2023-04-04T18:43:03Z

...src/main/java/org/apache/maven/plugin/internal/AbstractMavenPluginDependenciesValidator.java

+
+    @Override
+    public void validate(MavenSession mavenSession, MojoDescriptor mojoDescriptor) {
+        if (mojoDescriptor.getPluginDescriptor() != null


Can they be null?! Weird

...e/src/main/java/org/apache/maven/plugin/internal/AbstractMavenPluginParametersValidator.java

maven-core/src/main/java/org/apache/maven/plugin/internal/DefaultMavenPluginManager.java

...n-core/src/main/java/org/apache/maven/plugin/internal/DefaultPluginDependenciesResolver.java

maven-core/src/main/java/org/apache/maven/plugin/internal/DefaultPluginValidationManager.java

...n-core/src/main/java/org/apache/maven/plugin/internal/DeprecatedCoreExpressionValidator.java

maven-core/src/main/java/org/apache/maven/plugin/internal/Maven2DependenciesValidator.java

michael-o · 2023-04-04T18:52:40Z

maven-core/src/main/java/org/apache/maven/plugin/internal/MavenMixedDependenciesValidator.java

+        if (mavenVersions.size() > 1) {
+            pluginValidationManager.reportPluginValidationIssue(
+                    mavenSession, mojoDescriptor, "Plugin mixes multiple Maven versions: " + mavenVersions);
+        }


How to understand this message?

Unsure how to put it in words, but the point is that "among plugin dependencies there are maven artifacts that has different versions" -- kinda like "dep convergence" but for maven? There are some shared stuff (maybe older filtering) that caused this, especially as they pulled in maven-project, artifact that is gone from maven 3, so even depMgt could not "align" it as it was maven2 artifact.

Now I see, from the message it is hard to derive that.

.../main/java/org/apache/maven/plugin/internal/PlexusContainerDefaultDependenciesValidator.java

slawekjaranowski

I would like to put sections:

Declared at location(s)
Used in module(s)

as optional - default disabled - in projects with hundred modules such message will make more mess than benefit

maybe we can have a option
maven.plugin.validation with values:

dafault
verbose with location and modules
false - disbaled at all

...e/src/main/java/org/apache/maven/plugin/internal/AbstractMavenPluginParametersValidator.java

...n-core/src/main/java/org/apache/maven/plugin/internal/DefaultPluginDependenciesResolver.java

maven-core/src/main/java/org/apache/maven/plugin/internal/Maven2DependenciesValidator.java

maven-core/src/main/java/org/apache/maven/plugin/internal/MavenMixedDependenciesValidator.java

maven-core/src/main/java/org/apache/maven/plugin/internal/MavenScopeDependenciesValidator.java

maven-core/src/main/java/org/apache/maven/plugin/internal/DefaultPluginValidationManager.java

cstamas · 2023-04-05T06:33:55Z

Applied PR comments and proposals

If user set anything, fail if value is junk

Report validation issues (as one liner warning) even if disabled Do not fail just warn when wrong value for property set

michael-o · 2023-04-05T13:03:42Z

I still have a problem to read that this output:

Don't bother, it comes only when you set 'verbose' but that means you have bigger problems as well 😆

Seriously: any proposal how to improve it?

I don't even know what those numbers mean, can you point me to the source where it comes from?

cstamas · 2023-04-05T13:06:44Z

They point to POM sources:
70 : 15, org.apache.maven:maven-settings-builder:3.9.2-SNAPSHOT -> https://github.com/apache/maven/blob/maven-3.9.x/maven-settings-builder/pom.xml#L70
131 : 15, org.apache.maven:maven-resolver-provider:3.9.2-SNAPSHOT -> https://github.com/apache/maven/blob/maven-3.9.x/maven-resolver-provider/pom.xml#L131
192 : 15, org.apache.maven:maven-core:3.9.2-SNAPSHOT -> https://github.com/apache/maven/blob/maven-3.9.x/maven-core/pom.xml#L192

gnodet · 2023-04-05T13:34:14Z

What about:

[WARNING]    * org.apache.maven:maven-settings-builder:3.9.2-SNAPSHOT /home/cstamas/Worx/apache-maven/maven/maven-settings-builder/pom.xml, line 70, column 15
[WARNING]    * org.apache.maven:maven-resolver-provider:3.9.2-SNAPSHOT /home/cstamas/Worx/apache-maven/maven/maven-resolver-provider/pom.xml, line 131, column 15
[WARNING]    * org.apache.maven:maven-core:3.9.2-SNAPSHOT /home/cstamas/Worx/apache-maven/maven/maven-core/pom.xml, line 192, column 15

michael-o · 2023-04-05T13:50:14Z

What about:

[WARNING]    * org.apache.maven:maven-settings-builder:3.9.2-SNAPSHOT /home/cstamas/Worx/apache-maven/maven/maven-settings-builder/pom.xml, line 70, column 15
[WARNING]    * org.apache.maven:maven-resolver-provider:3.9.2-SNAPSHOT /home/cstamas/Worx/apache-maven/maven/maven-resolver-provider/pom.xml, line 131, column 15
[WARNING]    * org.apache.maven:maven-core:3.9.2-SNAPSHOT /home/cstamas/Worx/apache-maven/maven/maven-core/pom.xml, line 192, column 15

this is much better, we can leverage an improvement (#756) from @hboutemy recently:

[WARNING] * org.apache.maven:maven-core:3.9.2-SNAPSHOT from /home/cstamas/Worx/apache-maven/maven/maven-core/pom.xml, line 192, column 15

[WARNING] * org.apache.maven:maven-core:3.9.2-SNAPSHOT from maven-core/pom.xml, line 192, column 15

which might even shorten the output. The rest is from org.apache.maven.building.DefaultProblem.getLocation()

cstamas · 2023-04-05T14:29:46Z

The improvement is already in, in the "Used in module(s)" section, see https://gist.github.com/cstamas/b62fdcd53eaf316123cf183f5a24e6a5#file-gistfile1-txt-L39

But here I have no idea how to reformat location... any example?

michael-o · 2023-04-05T14:31:32Z

The improvement is already in, in the "Used in module(s)" section, see https://gist.github.com/cstamas/b62fdcd53eaf316123cf183f5a24e6a5#file-gistfile1-txt-L39

But here I have no idea how to reformat location... any example?

Right, but not here: https://gist.github.com/cstamas/b62fdcd53eaf316123cf183f5a24e6a5#file-gistfile1-txt-L71-L75

As a user is just causes confusion, not clarification.

cstamas · 2023-04-05T14:53:06Z

Updated gist with new output. Removed prj.name as well to consistently use g:a:v (whether in build module or external artifact), and this also smooth out the output. Also, showing only line number, IMHO no need for column

cstamas · 2023-04-05T15:03:55Z

Updated gist once more: since last, removed "from" repeated string, only path alone is in braces. If within TLP, is relative, otherwise is abs (ie. from local repo).

gnodet · 2023-04-05T15:11:10Z

Updated gist once more: since last, removed "from" repeated string, only path alone is in braces. If within TLP, is relative, otherwise is abs (ie. from local repo).

I wonder if the class: should be removed too... Not sure it bring much.

cstamas · 2023-04-05T15:24:28Z

Agreed, and removed

michael-o · 2023-04-05T15:31:51Z

maven-core/src/main/java/org/apache/maven/plugin/internal/DefaultPluginValidationManager.java

+            stringBuilder.append(inputLocation.getSource().getModelId());
+            String location = inputLocation.getSource().getLocation();
+            if (location != null) {
+                if (location.contains("://")) {


You assume that this can be a URL?

cstamas · 2023-04-05T15:33:47Z

Interesting, invoked mvn help:help and mvn clean (was just wondering what happens), and BOTH produce SAME output:

[WARNING] 
[WARNING] Plugin validation issues were detected in 1 plugin(s)
[WARNING] 
[WARNING] Plugin org.apache.maven.plugins:maven-site-plugin:3.12.1
[WARNING]   Plugin issue(s):
[WARNING]    * Plugin depends on the deprecated Maven 2.x compatibility layer, which may not be supported in Maven 4.x
[WARNING] 
[WARNING] 
[WARNING] To fix these issues, please upgrade above listed plugins, or, notify their maintainers about reported issues.
[WARNING] 
[WARNING] For more or less details, use 'maven.plugin.validation' property with one of the values (case insensitive): [DISABLED, ENABLED, VERBOSE]
[WARNING] 
➜

How does site plugin come in picture for ANY of these?

michael-o · 2023-04-05T15:34:48Z

Interesting, invoked mvn help:help and mvn clean (was just wondering what happens), and BOTH produce SAME output:

[WARNING] 
[WARNING] Plugin validation issues were detected in 1 plugin(s)
[WARNING] 
[WARNING] Plugin org.apache.maven.plugins:maven-site-plugin:3.12.1
[WARNING]   Plugin issue(s):
[WARNING]    * Plugin depends on the deprecated Maven 2.x compatibility layer, which may not be supported in Maven 4.x
[WARNING] 
[WARNING] 
[WARNING] To fix these issues, please upgrade above listed plugins, or, notify their maintainers about reported issues.
[WARNING] 
[WARNING] For more or less details, use 'maven.plugin.validation' property with one of the values (case insensitive): [DISABLED, ENABLED, VERBOSE]
[WARNING] 
➜

How does site plugin come in picture for ANY of these?

Lifecycle binding?

cstamas · 2023-04-05T15:36:15Z

Lifecycle binding?

Huh? for help:help which lifecycle you mean specifically? Also, for clean lifecycle, where is site? https://github.com/apache/maven/blob/maven-3.9.x/maven-core/src/main/resources/META-INF/plexus/components.xml#L67

BTW, the message means site plugin is ~~LOADED up~~ RESOLVED. Why?

michael-o · 2023-04-05T15:41:05Z

Lifecycle binding?

Huh? for help:help which lifecycle you mean specifically? Also, for clean lifecycle, where is site? https://github.com/apache/maven/blob/maven-3.9.x/maven-core/src/main/resources/META-INF/plexus/components.xml#L67

BTW, the message means site plugin is ~~LOADED up~~ RESOLVED. Why?

Eager plugin scanning in core?`Remove maven-site-plugin from local repo and try again. This might prove my theory.

...n-core/src/main/java/org/apache/maven/plugin/internal/DeprecatedCoreExpressionValidator.java

…ecatedCoreExpressionValidator.java Co-authored-by: Konrad Windszus <konrad_w@gmx.de>

This is general rework of current Maven 3.9.x line how it handles plugin and mojo validations. Changes: * added plugin validations for dependencies * introduced pluginValidationManager that gathers violations * manager creates a report at build end, with dense and non repeating data * this is in spirit to lessen already too verbose logging, as current solution would report violations as many times plugin is used in reactor, and that can be many (ie. a plugin from parent for each module) Example report of Maven 3.9.x build: https://gist.github.com/cstamas/b62fdcd53eaf316123cf183f5a24e6a5 --- https://issues.apache.org/jira/browse/MNG-7754

[MNG-7754] Improve Mojo parameter deprecation message

f1181a3

As it seems is confusing to some users --- https://issues.apache.org/jira/browse/MNG-7754

cstamas requested a review from slawekjaranowski April 3, 2023 16:25

cstamas self-assigned this Apr 3, 2023

slawekjaranowski approved these changes Apr 3, 2023

View reviewed changes

rmannibucau approved these changes Apr 4, 2023

View reviewed changes

[MNG-7754] General improvement (and extension) of plugin validation m…

6bfc8df

…essages. They are now reported at end.

cstamas changed the title ~~[MNG-7754] Improve Mojo parameter deprecation message~~ [MNG-7754] Improvement and extension of plugin validation handling Apr 4, 2023

Tidy up

c89b1f2

cstamas requested review from rmannibucau and slawekjaranowski April 4, 2023 11:34

rmannibucau approved these changes Apr 4, 2023

View reviewed changes

cstamas added 3 commits April 4, 2023 14:33

Broaden how violation can be reported

2eeb3f0

Add possibility to disable plugin validation.

321307a

maven-compat detection was flawed, fix it

b804fd8

cstamas changed the title ~~[MNG-7754] Improvement and extension of plugin validation handling~~ [MNG-7754] Improvement and extension of plugin validation Apr 4, 2023

cstamas mentioned this pull request Apr 4, 2023

[MNG-7754] Improve plugin validation apache/maven-integration-testing#256

Merged

cstamas added 2 commits April 4, 2023 15:46

Rephrase message

cea9312

Tidy

42e3a5d

michael-o requested changes Apr 4, 2023

View reviewed changes

slawekjaranowski requested changes Apr 4, 2023

View reviewed changes

cstamas added 2 commits April 5, 2023 08:16

PR comments

2dff59c

Validation levels

8c23603

cstamas added 5 commits April 5, 2023 08:58

Simplify, no need for model, javadoc and typos

1ae3233

Simplify config

084afa1

If user set anything, fail if value is junk

Provide friendlier message, ignore empty

ee066f9

Typo

d21478e

Simplify

f02a150

Report validation issues (as one liner warning) even if disabled Do not fail just warn when wrong value for property set

gnodet approved these changes Apr 5, 2023

View reviewed changes

Improve reporting of occurences and declarations

6bda05f

Remove "class:" as well

4c416c7

michael-o reviewed Apr 5, 2023

View reviewed changes

michael-o approved these changes Apr 5, 2023

View reviewed changes

slawekjaranowski approved these changes Apr 5, 2023

View reviewed changes

kwin reviewed Apr 11, 2023

View reviewed changes

...n-core/src/main/java/org/apache/maven/plugin/internal/DeprecatedCoreExpressionValidator.java Outdated Show resolved Hide resolved

kwin mentioned this pull request Apr 11, 2023

Improve error message for deprecated core expressions #1083

Closed

cstamas and others added 2 commits April 12, 2023 11:11

Update maven-core/src/main/java/org/apache/maven/plugin/internal/Depr…

e0aab8f

…ecatedCoreExpressionValidator.java Co-authored-by: Konrad Windszus <konrad_w@gmx.de>

Reformat

710a2db

cstamas merged commit 36a4e9f into apache:maven-3.9.x Apr 12, 2023

cstamas deleted the maven-3.9.x-MNG-7754-deprecation-message-rephrase branch April 12, 2023 09:42

gnodet mentioned this pull request May 24, 2023

MNG 7587 3.9.x #1120

Closed

gnodet mentioned this pull request May 29, 2024

Maven 3.9.x should use Maven 3.1 validation in strict mode #1541

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[MNG-7754] Improvement and extension of plugin validation #1079

[MNG-7754] Improvement and extension of plugin validation #1079

cstamas commented Apr 3, 2023 •

edited

Loading

cstamas commented Apr 4, 2023

michael-o left a comment

michael-o Apr 4, 2023

michael-o Apr 4, 2023

cstamas Apr 5, 2023

michael-o Apr 5, 2023

slawekjaranowski left a comment

cstamas commented Apr 5, 2023

michael-o commented Apr 5, 2023

cstamas commented Apr 5, 2023 •

edited

Loading

gnodet commented Apr 5, 2023

michael-o commented Apr 5, 2023

cstamas commented Apr 5, 2023

michael-o commented Apr 5, 2023 •

edited

Loading

cstamas commented Apr 5, 2023 •

edited

Loading

cstamas commented Apr 5, 2023 •

edited

Loading

gnodet commented Apr 5, 2023

cstamas commented Apr 5, 2023

michael-o Apr 5, 2023

cstamas commented Apr 5, 2023

michael-o commented Apr 5, 2023

cstamas commented Apr 5, 2023 •

edited

Loading

michael-o commented Apr 5, 2023

[MNG-7754] Improvement and extension of plugin validation #1079

[MNG-7754] Improvement and extension of plugin validation #1079

Conversation

cstamas commented Apr 3, 2023 • edited Loading

cstamas commented Apr 4, 2023

michael-o left a comment

Choose a reason for hiding this comment

michael-o Apr 4, 2023

Choose a reason for hiding this comment

michael-o Apr 4, 2023

Choose a reason for hiding this comment

cstamas Apr 5, 2023

Choose a reason for hiding this comment

michael-o Apr 5, 2023

Choose a reason for hiding this comment

slawekjaranowski left a comment

Choose a reason for hiding this comment

cstamas commented Apr 5, 2023

michael-o commented Apr 5, 2023

cstamas commented Apr 5, 2023 • edited Loading

gnodet commented Apr 5, 2023

michael-o commented Apr 5, 2023

cstamas commented Apr 5, 2023

michael-o commented Apr 5, 2023 • edited Loading

cstamas commented Apr 5, 2023 • edited Loading

cstamas commented Apr 5, 2023 • edited Loading

gnodet commented Apr 5, 2023

cstamas commented Apr 5, 2023

michael-o Apr 5, 2023

Choose a reason for hiding this comment

cstamas commented Apr 5, 2023

michael-o commented Apr 5, 2023

cstamas commented Apr 5, 2023 • edited Loading

michael-o commented Apr 5, 2023

cstamas commented Apr 3, 2023 •

edited

Loading

cstamas commented Apr 5, 2023 •

edited

Loading

michael-o commented Apr 5, 2023 •

edited

Loading

cstamas commented Apr 5, 2023 •

edited

Loading

cstamas commented Apr 5, 2023 •

edited

Loading

cstamas commented Apr 5, 2023 •

edited

Loading