[GitHub] [maven] michael-o commented on pull request #645: [MNG-7375] prevent potential NPE in Metadata.merge(...)
michael-o commented on pull request #645: URL: https://github.com/apache/maven/pull/645#issuecomment-1053634921 @cstamas @gnodet @hboutemy Any objections in general? -- 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-unsubscr...@maven.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [maven] michael-o commented on pull request #645: [MNG-7375] prevent potential NPE in Metadata.merge(...)
michael-o commented on pull request #645: URL: https://github.com/apache/maven/pull/645#issuecomment-1053610312 Do you see this for 3.8.x 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-unsubscr...@maven.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [maven] michael-o commented on pull request #645: [MNG-7375] prevent potential NPE in Metadata.merge(...)
michael-o commented on pull request #645: URL: https://github.com/apache/maven/pull/645#issuecomment-1053578715 Alright. Will check later. -- 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-unsubscr...@maven.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [maven] michael-o commented on pull request #645: [MNG-7375] prevent potential NPE in Metadata.merge(...)
michael-o commented on pull request #645: URL: https://github.com/apache/maven/pull/645#issuecomment-1053532716 Is this one still relevant regarding the changes done on MPLUGIN? -- 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-unsubscr...@maven.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [maven] michael-o commented on pull request #645: [MNG-7375] prevent potential NPE in Metadata.merge(...)
michael-o commented on pull request #645: URL: https://github.com/apache/maven/pull/645#issuecomment-1041411056 No, not yet. Ping me in a week. -- 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-unsubscr...@maven.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [maven] michael-o commented on pull request #645: [MNG-7375] prevent potential NPE in Metadata.merge(...)
michael-o commented on pull request #645: URL: https://github.com/apache/maven/pull/645#issuecomment-1005505425 > > Introduce a maven-repository-metadata-builder module > > Done in [d39be11](https://github.com/apache/maven/commit/d39be11613d20228a4e826302ef57adf19ba1194). This module currently only contains the validator interface and default implementation. Should we also have a real builder there? Good question, I will come back to this as soon as I have completed my open votes. -- 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-unsubscr...@maven.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [maven] michael-o commented on pull request #645: [MNG-7375] prevent potential NPE in Metadata.merge(...)
michael-o commented on pull request #645: URL: https://github.com/apache/maven/pull/645#issuecomment-1005105026 > Correct, we pointlessly waste cycles for their generation. This applies to a lot of stuff we do. -- 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-unsubscr...@maven.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [maven] michael-o commented on pull request #645: [MNG-7375] prevent potential NPE in Metadata.merge(...)
michael-o commented on pull request #645: URL: https://github.com/apache/maven/pull/645#issuecomment-1004330642 I support @cstamas position. > > So, IMO maven-repository-metadata getting added components is wrong IMHO. > > Any suggestion where to add the default validation module then, because as you say "any validation should be next where model is"? Likely like POM model or settings model. Introduce a `maven-repository-metadata-builder` module. That would make your new code consistent with the rest. Look at the given ones. -- 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-unsubscr...@maven.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [maven] michael-o commented on pull request #645: [MNG-7375] prevent potential NPE in Metadata.merge(...)
michael-o commented on pull request #645: URL: https://github.com/apache/maven/pull/645#issuecomment-1004273279 > Metadata is only partially handled inside Maven (core), mostly it is now handled transparently in [Maven Resolver](https://github.com/apache/maven-resolver/blob/master/maven-resolver-api/src/main/java/org/eclipse/aether/metadata/Metadata.java), so probably the validator should be part of Maven Resolver and only for some places where Maven directly leverages metadata it should be reused inside Maven directly. WDYT? > > Update: As the validation is Maven repository specific it needs to be called from maven-resolver-provider which is in fact part of the Maven main reactor. I added validation calls in all metadata reads from all Maven modules except the legacy "maven-compat". @cstamas What is your opinion on the package of this new solution? I don't have one yet. -- 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-unsubscr...@maven.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [maven] michael-o commented on pull request #645: [MNG-7375] prevent potential NPE in Metadata.merge(...)
michael-o commented on pull request #645: URL: https://github.com/apache/maven/pull/645#issuecomment-1003778248 @kwin Insane! Please open a new JIRA issue for this. I am thinking whether we can introduce this in 3.8.x or must wait at least until 3.9.x. We have a lot of talks about validators with @rfscholte last year. He should be part of this discussion 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-unsubscr...@maven.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [maven] michael-o commented on pull request #645: [MNG-7375] prevent potential NPE in Metadata.merge(...)
michael-o commented on pull request #645: URL: https://github.com/apache/maven/pull/645#issuecomment-1003603066 I don't have access to the NEXUS 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-unsubscr...@maven.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [maven] michael-o commented on pull request #645: [MNG-7375] prevent potential NPE in Metadata.merge(...)
michael-o commented on pull request #645: URL: https://github.com/apache/maven/pull/645#issuecomment-1003602899 > Both issues are correct but are rather follow-ups to [MNG-7375](https://issues.apache.org/jira/browse/MNG-7375). My question is about how to deal with a missing prefix within `Metadata.merge` (this can happen even if the XSD would say it is mandatory, e.g. by a bug like in this case through `staging-maven-plugin`). Would a validator really prevent `null` being passed to `Metadata.merge`? The validator would fail the build with an appropriate error message before anything being merged. I.e., source and target are validated upfront before merge happens. -- 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-unsubscr...@maven.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [maven] michael-o commented on pull request #645: [MNG-7375] prevent potential NPE in Metadata.merge(...)
michael-o commented on pull request #645: URL: https://github.com/apache/maven/pull/645#issuecomment-1003586710 -- 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-unsubscr...@maven.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [maven] michael-o commented on pull request #645: [MNG-7375] prevent potential NPE in Metadata.merge(...)
michael-o commented on pull request #645: URL: https://github.com/apache/maven/pull/645#issuecomment-1001510028 This PR hides the fact that the input data is invalid. Obviously, we need a `MetadataValidator`. -- 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-unsubscr...@maven.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
