You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@maven.apache.org by GitBox <gi...@apache.org> on 2021/11/24 09:00:06 UTC
[GitHub] [maven] gnodet opened a new pull request #618: [MNG-7345] Fix exported packages
gnodet opened a new pull request #618:
URL: https://github.com/apache/maven/pull/618
Packages not exported from maven-core:
* o.a.m.bridge
* o.a.m.eventspy
* o.a.m.execution.infoproviders
* o.a.m.lifecycle.mapping
* o.a.m.feature
Packages not exported from dependencies
* org.eclipse.sisu
the above one is required for @Priority, but there may be others needed, not sure.
Packages that should not be exported
* o.a.m.rtinfo.internal
My commit only fixes `o.a.m.feature`, `org.eclipse.sisu` and `o.a.m.rtinfo.internal`.
--
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] rmannibucau commented on pull request #618: [MNG-7345] Fix exported packages
Posted by GitBox <gi...@apache.org>.
rmannibucau commented on pull request #618:
URL: https://github.com/apache/maven/pull/618#issuecomment-977681026
@gnodet no difference should be the target but guess it can be a build time metadata of sisu instead of a runtime annotation - or we do it by reflection and provide it in the bundle, but by the past we got a lot of troubles leaking internal IoC details so hope we don't go back there - not blaming, I know where you went from, just explaining the reasoning.
--
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] rmannibucau edited a comment on pull request #618: [MNG-7345] Fix exported packages
Posted by GitBox <gi...@apache.org>.
rmannibucau edited a comment on pull request #618:
URL: https://github.com/apache/maven/pull/618#issuecomment-977681026
@gnodet no difference should be the target but guess it can be a build time metadata of sisu instead of a runtime annotation - or we do it by reflection and provide it in the bundle, but by the past we got a lot of troubles leaking internal IoC details so hope we don't go back there - not blaming, I know where you went from, just explaining the reasoning.
edit: Do we have a list of @Priority usages, i didnt see any in core and since it is not exported it is not really a feature so can also lead to just use an API for that (public int priority();)
--
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] gnodet commented on pull request #618: [MNG-7345] Fix exported packages
Posted by GitBox <gi...@apache.org>.
gnodet commented on pull request #618:
URL: https://github.com/apache/maven/pull/618#issuecomment-979027091
> > @gnodet no difference should be the target but guess it can be a build time metadata of sisu instead of a runtime annotation - or we do it by reflection and provide it in the bundle, but by the past we got a lot of troubles leaking internal IoC details so hope we don't go back there - not blaming, I know where you went from, just explaining the reasoning.
>
> No problem with this idea, however anything we may need internally for maven may also be needed by users. I'd rather avoid relying on non supported apis, so maybe we could rely on `javax.annotation.Priority` which seems to be supported by Guice. But we have the same problem and Guice need to see the same class.
Using `java.annotation.Priority` works because this package is already exported by maven-core, thus Sisu/Guice has visibility on this annotation inside extensions.
--
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] gnodet commented on pull request #618: [MNG-7345] Fix exported packages
Posted by GitBox <gi...@apache.org>.
gnodet commented on pull request #618:
URL: https://github.com/apache/maven/pull/618#issuecomment-977678475
> For o.a.m.f it looks ok but I don't think we should export org.eclipse.sisu., it is internal details which shouldn't leak IMHO since it is not an API - and far from being one IMHO, no?
Do you have a workaround for using `@Priority` ? or is that another wanted difference between core / build extensions ?
--
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] gnodet merged pull request #618: [MNG-7345] Fix exported packages
Posted by GitBox <gi...@apache.org>.
gnodet merged pull request #618:
URL: https://github.com/apache/maven/pull/618
--
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] gnodet commented on pull request #618: [MNG-7345] Fix exported packages
Posted by GitBox <gi...@apache.org>.
gnodet commented on pull request #618:
URL: https://github.com/apache/maven/pull/618#issuecomment-979029853
I've removed the export of the sisu package.
--
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] gnodet commented on pull request #618: [MNG-7345] Fix exported packages
Posted by GitBox <gi...@apache.org>.
gnodet commented on pull request #618:
URL: https://github.com/apache/maven/pull/618#issuecomment-977936313
> @gnodet no difference should be the target but guess it can be a build time metadata of sisu instead of a runtime annotation - or we do it by reflection and provide it in the bundle, but by the past we got a lot of troubles leaking internal IoC details so hope we don't go back there - not blaming, I know where you went from, just explaining the reasoning.
No problem with this idea, however anything we may need internally for maven may also be needed by users. I'd rather avoid relying on non supported apis, so maybe we could rely on `javax.annotation.Priority` which seems to be supported by Guice. But we have the same problem and Guice need to see the same class.
I'll investigate to see if I can come up with a solution what could bridge classloaders. A custom `RankingFunction` for sisu should do the trick.
--
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