You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@maven.apache.org by "cstamas (via GitHub)" <gi...@apache.org> on 2023/04/17 13:15:37 UTC
[GitHub] [maven] cstamas opened a new pull request, #1092: [MNG-7767] Tone down plugin validator
cstamas opened a new pull request, #1092:
URL: https://github.com/apache/maven/pull/1092
This change reduces default output making it more compact, and also renames the "levels" to brief, default and verbose.
---
https://issues.apache.org/jira/browse/MNG-7767
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] michael-o commented on pull request #1092: [MNG-7767] Tone down plugin validator
Posted by "michael-o (via GitHub)" <gi...@apache.org>.
michael-o commented on PR #1092:
URL: https://github.com/apache/maven/pull/1092#issuecomment-1511379349
> > I am really confused why `RepositorySystemSession` and `ConfigUtils` are used here. They feel a bit misplaced in plugin validation, don't they?
>
> Well, this PR does not change anything about that. But, as validation is per session, the session is really the only thing that offers itself to store things in "natural" way. MavenSession does not have any means for this, so it is resolver session data that is used. It is done like it as this guarantees that validator does do what it needs to do in correct way in single-threaded and multi-threaded mvn and also in mvnd.
I understand that, but still looks like because it is not the right storage for this. Maven session contains user and system properties.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] cstamas commented on pull request #1092: [MNG-7767] Tone down plugin validator
Posted by "cstamas (via GitHub)" <gi...@apache.org>.
cstamas commented on PR #1092:
URL: https://github.com/apache/maven/pull/1092#issuecomment-1512616152
Did not, just the code make it happen: validator uses resolver ConfigUtil:
https://github.com/apache/maven/pull/1092/files#diff-9478004903992887e1696cebb44ffb9cb99032f9d25db4447e734c4a26930229L68
that on other hand does this:
https://github.com/apache/maven-resolver/blob/master/maven-resolver-util/src/main/java/org/eclipse/aether/util/ConfigUtils.java#L92
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] michael-o commented on pull request #1092: [MNG-7767] Tone down plugin validator
Posted by "michael-o (via GitHub)" <gi...@apache.org>.
michael-o commented on PR #1092:
URL: https://github.com/apache/maven/pull/1092#issuecomment-1511492333
Done: https://issues.apache.org/jira/browse/MNG-7768
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] michael-o commented on pull request #1092: [MNG-7767] Tone down plugin validator
Posted by "michael-o (via GitHub)" <gi...@apache.org>.
michael-o commented on PR #1092:
URL: https://github.com/apache/maven/pull/1092#issuecomment-1511474459
Question: If the users passes `mvn -DKEY` Maven will pass `true` as value. Do you want to accept this silently or reject?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] cstamas merged pull request #1092: [MNG-7767] Tone down plugin validator
Posted by "cstamas (via GitHub)" <gi...@apache.org>.
cstamas merged PR #1092:
URL: https://github.com/apache/maven/pull/1092
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] michael-o commented on pull request #1092: [MNG-7767] Tone down plugin validator
Posted by "michael-o (via GitHub)" <gi...@apache.org>.
michael-o commented on PR #1092:
URL: https://github.com/apache/maven/pull/1092#issuecomment-1511398762
> Maven session contains user and system properties? Well, nope, it does not, it contains request, and those are `java.util.Properties` that may not contains "any object" and also does not offer any concurrent methods like putIfAbsent...
https://maven.apache.org/ref/3.3.9/maven-core/apidocs/org/apache/maven/execution/MavenSession.html#getUserProperties()
`#getProperty(String)` gives you a string. The abuse of `RSS` is in indicator of missing bits in `MavenSession`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] michael-o commented on pull request #1092: [MNG-7767] Tone down plugin validator
Posted by "michael-o (via GitHub)" <gi...@apache.org>.
michael-o commented on PR #1092:
URL: https://github.com/apache/maven/pull/1092#issuecomment-1512621376
> Did not, just the code make it happen: validator uses resolver ConfigUtil: https://github.com/apache/maven/pull/1092/files#diff-9478004903992887e1696cebb44ffb9cb99032f9d25db4447e734c4a26930229L68 that on other hand does this: https://github.com/apache/maven-resolver/blob/master/maven-resolver-util/src/main/java/org/eclipse/aether/util/ConfigUtils.java#L92
It won't return `boolean`, but `"true"` as a string.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] cstamas commented on pull request #1092: [MNG-7767] Tone down plugin validator
Posted by "cstamas (via GitHub)" <gi...@apache.org>.
cstamas commented on PR #1092:
URL: https://github.com/apache/maven/pull/1092#issuecomment-1511410291
> `#getProperty(String)` gives you a string. The abuse of `RSS` is in indicator of missing bits in `MavenSession`.
Again, I agree completely, but again, this PR is not about "adding missing bits to maven session"
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] cstamas commented on pull request #1092: [MNG-7767] Tone down plugin validator
Posted by "cstamas (via GitHub)" <gi...@apache.org>.
cstamas commented on PR #1092:
URL: https://github.com/apache/maven/pull/1092#issuecomment-1511340493
> I am really confused why `RepositorySystemSession` and `ConfigUtils` are used here. They feel a bit misplaced in plugin validation, don't they?
Well, this PR does not change anything about that. But, as validation is per session, the session is really the only thing that offers itself to store things in "natural" way. MavenSession does not have any means for this, so it is resolver session data that is used. It is done like it as this guarantees that validator does do what it needs to do in correct way in single-threaded and multi-threaded mvn and also in mvnd.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] michael-o commented on pull request #1092: [MNG-7767] Tone down plugin validator
Posted by "michael-o (via GitHub)" <gi...@apache.org>.
michael-o commented on PR #1092:
URL: https://github.com/apache/maven/pull/1092#issuecomment-1511470590
> > `#getProperty(String)` gives you a string. The abuse of `RSS` is in indicator of missing bits in `MavenSession`.
>
> Again, I agree completely, but again, this PR is not about "adding missing bits to maven session"
I know, I don't object this PR, I just noticed this. I will raise an issue.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] michael-o commented on pull request #1092: [MNG-7767] Tone down plugin validator
Posted by "michael-o (via GitHub)" <gi...@apache.org>.
michael-o commented on PR #1092:
URL: https://github.com/apache/maven/pull/1092#issuecomment-1512612516
> > Question: If the users passes `mvn -DKEY` Maven will pass `true` as value. Do you want to accept this silently or reject?
>
> Sure. The existing behavior of validation manager is aligned: it changes output level ONLY if value user passed is ANY of the 2 non-default values, otherwise is default. So to say, "true" would yield "default" as well.
Have you actually tried? I think that `"true"` will be returned by `ConfigUtils` and it will fail with IAE.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] cstamas commented on pull request #1092: [MNG-7767] Tone down plugin validator
Posted by "cstamas (via GitHub)" <gi...@apache.org>.
cstamas commented on PR #1092:
URL: https://github.com/apache/maven/pull/1092#issuecomment-1511387855
Maven session contains user and system properties? Well, nope, it does not, it contains request, and those are `java.util.Properties` that may not contains "any object" and also does not offer any concurrent methods like putIfAbsent...
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] cstamas commented on pull request #1092: [MNG-7767] Tone down plugin validator
Posted by "cstamas (via GitHub)" <gi...@apache.org>.
cstamas commented on PR #1092:
URL: https://github.com/apache/maven/pull/1092#issuecomment-1512500979
> Question: If the users passes `mvn -DKEY` Maven will pass `true` as value. Do you want to accept this silently or reject?
Sure. The existing behavior of validation manager is aligned: it changes output level ONLY if value user passed is ANY of the 2 non-default values, otherwise is default. So to say, "true" would yield "default" as well.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] cstamas commented on pull request #1092: [MNG-7767] Tone down plugin validator
Posted by "cstamas (via GitHub)" <gi...@apache.org>.
cstamas commented on PR #1092:
URL: https://github.com/apache/maven/pull/1092#issuecomment-1523460242
Am merging this PR, my parallel with log levels still stand (as the property is really just like a log level, does not alter anything else, just the amount of validation output)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] cstamas commented on pull request #1092: [MNG-7767] Tone down plugin validator
Posted by "cstamas (via GitHub)" <gi...@apache.org>.
cstamas commented on PR #1092:
URL: https://github.com/apache/maven/pull/1092#issuecomment-1519718672
IMHO this above IS aligned with my statement: "it changes output level ONLY if value user passed is ANY of the 2 non-default values, otherwise is default"
In your case, it remained DEFAULT. Also, this property does not affect Maven functionality (build, plugin or whatever wise), it merely controls the level of output, I see no value of failing for invalid input (as in other case, it may affect output of build, ie. making it incomplete or whatever... but again, this property does not affect anything functionality wise, merely the report detail). Validation happens "as early" as plugins are about to resolve, that is already "late".
I really don't want to spread this all over the place just to achieve some "ideal" state: i'd rather keep it simple and focused. To achieve early validation, we need some early hook that would involve and perform validation, etc... is it all really worth it? Again, this is really just a splat to user face, to update the build. If he mistypes the property value, and instead of "quiet" output "default" output happens, is it the end of the world? Should the build break for it? Am unsure.
@slawekjaranowski @gnodet or anyone else? Any opinion?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org