Re: [PR] Restrict classloader for Maven 4 plugins [maven]
gnodet commented on code in PR #1336: URL: https://github.com/apache/maven/pull/1336#discussion_r1420563752 ## maven-core/src/main/resources/META-INF/maven/extension.xml: ## @@ -187,6 +189,7 @@ under the License. org.apache.maven.resolver:maven-resolver-util org.apache.maven.resolver:maven-resolver-connector-basic +jakarta.inject:jakarta.inject-api Review Comment: I've raised https://issues.apache.org/jira/browse/MNG-7954 -- 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
Re: [PR] Restrict classloader for Maven 4 plugins [maven]
rmannibucau commented on code in PR #1336: URL: https://github.com/apache/maven/pull/1336#discussion_r1418926148 ## maven-core/src/main/resources/META-INF/maven/extension.xml: ## @@ -187,6 +189,7 @@ under the License. org.apache.maven.resolver:maven-resolver-util org.apache.maven.resolver:maven-resolver-connector-basic +jakarta.inject:jakarta.inject-api Review Comment: :thinking: I don't see why: * injections in mojo are already bridged * abstracting injection markers (with a maven inject annotation) is easy in guice * abstracting the injector or a lookup (like we had/have in plexus container) is quite trivial * abstracting a scope is not crazy (wayless than what we already have) So overall, even if I'm not sure which case(s) you want to cover, I don't see a reason to break our original hypotheis to define a clean and maven owned API and export anything jakarta related. -- 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
Re: [PR] Restrict classloader for Maven 4 plugins [maven]
gnodet commented on code in PR #1336: URL: https://github.com/apache/maven/pull/1336#discussion_r1418919151 ## maven-core/src/main/resources/META-INF/maven/extension.xml: ## @@ -187,6 +189,7 @@ under the License. org.apache.maven.resolver:maven-resolver-util org.apache.maven.resolver:maven-resolver-connector-basic +jakarta.inject:jakarta.inject-api Review Comment: The only other solution I could think about is to rewrite the whole DI injection (Sisu + Guice) because they are not pluggable at all. I'm not ready for that... -- 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
Re: [PR] Restrict classloader for Maven 4 plugins [maven]
rmannibucau commented on code in PR #1336: URL: https://github.com/apache/maven/pull/1336#discussion_r1418819764 ## maven-core/src/main/resources/META-INF/maven/extension.xml: ## @@ -187,6 +189,7 @@ under the License. org.apache.maven.resolver:maven-resolver-util org.apache.maven.resolver:maven-resolver-connector-basic +jakarta.inject:jakarta.inject-api Review Comment: not adding at all to anything, will bite us for sure, my understanding was that the guice upgrade was done under javax umbrella and/or we facade any needed api. Ad of today using javax.inject is fine cause code is frozen but anything jakarta can break without notice and can conflict with mojo so goes against maven4 api track for me. -- 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
Re: [PR] Restrict classloader for Maven 4 plugins [maven]
gnodet commented on code in PR #1336: URL: https://github.com/apache/maven/pull/1336#discussion_r1418771130 ## maven-core/src/main/resources/META-INF/maven/extension.xml: ## @@ -187,6 +189,7 @@ under the License. org.apache.maven.resolver:maven-resolver-util org.apache.maven.resolver:maven-resolver-connector-basic +jakarta.inject:jakarta.inject-api Review Comment: Or do you mean not adding it for Maven 3.x plugins ? That would be absolutely fine with me. -- 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
Re: [PR] Restrict classloader for Maven 4 plugins [maven]
gnodet commented on code in PR #1336: URL: https://github.com/apache/maven/pull/1336#discussion_r1418703115 ## maven-core/src/main/resources/META-INF/maven/extension.xml: ## @@ -187,6 +189,7 @@ under the License. org.apache.maven.resolver:maven-resolver-util org.apache.maven.resolver:maven-resolver-connector-basic +jakarta.inject:jakarta.inject-api Review Comment: We can't. We need it in the new api. It's used to define scopes (those can't work without the package) and it's also used to inject various components. See https://github.com/apache/maven/pull/1309 -- 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
Re: [PR] Restrict classloader for Maven 4 plugins [maven]
gnodet commented on code in PR #1336: URL: https://github.com/apache/maven/pull/1336#discussion_r1418703115 ## maven-core/src/main/resources/META-INF/maven/extension.xml: ## @@ -187,6 +189,7 @@ under the License. org.apache.maven.resolver:maven-resolver-util org.apache.maven.resolver:maven-resolver-connector-basic +jakarta.inject:jakarta.inject-api Review Comment: We can't. We need it in the new api. It's used to define scopes (those can't work without the package) and it's also used to inject various components. -- 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
Re: [PR] Restrict classloader for Maven 4 plugins [maven]
rmannibucau commented on code in PR #1336: URL: https://github.com/apache/maven/pull/1336#discussion_r1418572514 ## maven-core/src/main/resources/META-INF/maven/extension.xml: ## @@ -187,6 +189,7 @@ under the License. org.apache.maven.resolver:maven-resolver-util org.apache.maven.resolver:maven-resolver-connector-basic +jakarta.inject:jakarta.inject-api Review Comment: don't think it should land there, ideally we shouldn't have it (we saw that javax was an issue already) but if imposed by some part of the stack (not sure which one since I thought we stayed on javax intentionally) we should just hide it from any consumer IMHO -- 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
[PR] Restrict classloader for Maven 4 plugins [maven]
gnodet opened a new pull request, #1336: URL: https://github.com/apache/maven/pull/1336 (no comment) -- 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
