You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@maven.apache.org by Robert Scholte <rf...@apache.org> on 2019/10/18 18:04:53 UTC
Second MNG-6765 ([Regression] tycho pom-less builds fails with 3.6.2)
Hi,
with the help from Stuart McCulloch we've been able to provide a patch for
MNG-6765[1]
Please review and test.
thanks,
Robert
[1] https://issues.apache.org/jira/browse/MNG-6765
[2]
https://github.com/apache/maven/commit/24e6c0ec0a87b6682513287a23c36db6996b874c
[3]
https://github.com/apache/maven/commit/53a70bc8543124569ee787725b2004bc92a681b6
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
For additional commands, e-mail: dev-help@maven.apache.org
Re: Second MNG-6765 ([Regression] tycho pom-less builds fails with 3.6.2)
Posted by Tibor Digana <ti...@apache.org>.
I am trying to explain to you in second thread that removing Guice will
remove all related issues.
On Sat, Oct 19, 2019 at 5:09 PM Robert Scholte <rf...@apache.org> wrote:
> Tibor,
>
> you're doing it again. The title of this topic is "Second MNG-6765
> ([Regression] tycho pom-less builds fails with 3.6.2)" and none of your
> comments helped on that, i.e. trying to move it forward.
> Based on your reply you should a new discussion on the mailinglist.
>
> Robert
>
> ps. I won't make it to ApacheCon this year.
>
>
> On Sat, 19 Oct 2019 15:48:36 +0200, Tibor Digana <ti...@apache.org>
> wrote:
>
> Robert we suffer from bad design in Maven. There is Jira issue where Maven
> accepted these annotation maybe because they are so famous and I do not
> remember if we had any Vote for this significant change. If there was a
> Vote I would vote -1 and I would explain why.
> We can talk about it in the conference the next week but gaian I am saying
> that this feature with these annotations crerated conflicts between Maven
> and plugins.
> I doubt that Google Guice is certified by TCK and the Oracle which means
> that these annotations are a fan of developers but not a container I can
> trust.
> Additionally, joining Maven domain with EE domain is like joining Applet
> with Weld container of EE, the same mess.
>
> On Sat, Oct 19, 2019 at 3:41 PM Robert Scholte <rf...@apache.org>
> wrote:
>
>> Tibor, please lower your voice.
>> You're turning this topic into an unnecessary fight.
>> We're solving a regression introduced in 3.6.2.
>> Stuart was able to identify the problem, provide the fix and clearly
>> explain what happened.
>> Code has been review, fix is confirmed, so we're good to go.
>>
>> So don't hijack this thread and complain by sharing your strong opinions.
>> They might be related to the code we're touching, but it doesn't help
>> fixing the issue.
>> If you want to discuss that, please just start a new topic on the
>> dev-list.
>>
>> thanks,
>> Robert
>>
>> On Sat, 19 Oct 2019 15:20:23 +0200, Tibor Digana <ti...@apache.org>
>>
>> wrote:
>>
>> > Stuart, you are wrong.
>> > It is no more Java SE
>> > It is JakartaEE ans if you want to obey wrong design of Java SE go for
>> it
>> > but then Maven would become a mess of Java EE annotations in non-EE
>> > container.
>> > Do you understand that @Name represents a key in the container? And
>> how
>> > can
>> > you create a string like "core-default"? What's that? I do not know.
>> > It would be worth to have commercial experiences with Java EE and then
>> > you
>> > will understand that this is a big mistake and mess in Maven.
>> >
>> > On Sat, Oct 19, 2019 at 2:31 PM Stuart McCulloch <mc...@gmail.com>
>> > wrote:
>> >
>> >> @Named is not specific to Java EE, it's from JSR 330 which actually
>> >> targets
>> >> Java SE (and there are many SE based containers that support it)
>> >>
>> >> All @Named does is give a component an identity in the container (think
>> >> of @Named like the hint in Plexus [1]) so it can be referenced by
>> that
>> >> name
>> >> elsewhere. It also means you can inject lists or maps, where the key
>> is
>> >> the
>> >> name, of all component implementations for a given type. This lets
>> >> plugins
>> >> and extensions contribute implementations of a core type and core can
>> >> then
>> >> see them and choose the right one for the job (such as "file" for a
>> file
>> >> specific implementation.)
>> >>
>> >> Maven historically also supports overriding of components by plugins
>> and
>> >> extensions, where if you have a component with the same interface
>> (role)
>> >> and name (hint) then it can override the core component while that
>> >> plugin
>> >> is active. This lets plugins customize certain core behaviour in a
>> >> controlled manner. If we didn't allow overriding then a lot of plugins
>> >> would fail and Maven would be a lot less flexible.
>> >>
>> >> [1] https://wiki.eclipse.org/Sisu/PlexusMigration
>> >>
>> >> On Sat, 19 Oct 2019 at 11:58, Tibor Digana <ti...@apache.org>
>> >> wrote:
>> >>
>> >> > Are you talking about
>> >> > @Named( “not-default” )
>> >> > @Named( “coreAllowingOverride” ) or @Named( “coreExtensionPoint” )?
>> >> >
>> >> > In Java EE (and these annotations are from Java EE application
>> >> servers)
>> >> > mean the name of the bean which is unique - it is not a group of
>> >> beans.
>> >> > Please notice that beans container is Map<String, Bean> simply
>> >> speaking.
>> >> > and therefore here it would mean :
>> >> >
>> >> > "core-default" -> singleton instance (DefaultModelProcessor@1234567)
>> >> >
>> >> > so this means a conflict because you cannot create:
>> >> >
>> >> > "core-default" -> singleton instance (DefaultModelProcessor@1234567)
>> >> > "core-default" -> singleton instance (DefaultResolver@1234567)
>> >> > "core-default" -> singleton instance (AnotherBeanType@1234567)
>> >> >
>> >> > logical would be to have Expression API from Java EE and:
>> >> >
>> >> > "core-default-modelprocessor" -> singleton instance
>> >> > (DefaultModelProcessor@1234567)
>> >> > "core-default-resolver" -> singleton instance
>> >> (DefaultResolver@1234567)
>> >> > "core-default-xxx" -> singleton instance (AnotherBeanType@1234567)
>> >> >
>> >> >
>> >> > On Sat, Oct 19, 2019 at 12:03 PM Hervé BOUTEMY <
>> herve.boutemy@free.fr>
>> >> > wrote:
>> >> >
>> >> > > +1
>> >> > > just added a comment on a typo
>> >> > >
>> >> > > for the name of the component, perhaps "core-default", but I won't
>> >> > > complain
>> >> > > about any choice: the javadoc is what was really needed
>> >> > >
>> >> > > notice: perhaps we have other component that should have the same
>> >> > > improvement
>> >> > > to permit overriding in the future
>> >> > >
>> >> > > Regards,
>> >> > >
>> >> > > Hervé
>> >> > >
>> >> > > Le vendredi 18 octobre 2019, 20:04:53 CEST Robert Scholte a écrit :
>> >> > > > Hi,
>> >> > > >
>> >> > > > with the help from Stuart McCulloch we've been able to provide a
>> >> patch
>> >> > > for
>> >> > > > MNG-6765[1]
>> >> > > > Please review and test.
>> >> > > >
>> >> > > > thanks,
>> >> > > > Robert
>> >> > > >
>> >> > > > [1] https://issues.apache.org/jira/browse/MNG-6765
>> >> > > > [2]
>> >> > > >
>> >> > >
>> >> >
>> >>
>> https://github.com/apache/maven/commit/24e6c0ec0a87b6682513287a23c36db6996b8
>> >> > > > 74c [3]
>> >> > > >
>> >> > >
>> >> >
>> >>
>> https://github.com/apache/maven/commit/53a70bc8543124569ee787725b2004bc92a68
>> >> > > > 1b6
>> >> > > >
>> >> > > >
>> >> ---------------------------------------------------------------------
>> >> > > > To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
>> >> > > > For additional commands, e-mail: dev-help@maven.apache.org
>> >> > >
>> >> > >
>> >> > >
>> >> > >
>> >> > >
>> >> > >
>> >> ---------------------------------------------------------------------
>> >> > > To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
>> >> > > For additional commands, e-mail: dev-help@maven.apache.org
>> >> > >
>> >> > >
>> >> >
>>
>
>
>
>
Re: Second MNG-6765 ([Regression] tycho pom-less builds fails with
3.6.2)
Posted by Robert Scholte <rf...@apache.org>.
Tibor,
you're doing it again. The title of this topic is "Second MNG-6765
([Regression] tycho pom-less builds fails with 3.6.2)" and none of your
comments helped on that, i.e. trying to move it forward.
Based on your reply you should a new discussion on the mailinglist.
Robert
ps. I won't make it to ApacheCon this year.
On Sat, 19 Oct 2019 15:48:36 +0200, Tibor Digana <ti...@apache.org>
wrote:
> Robert we suffer from bad design in Maven. There is Jira issue where
> Maven accepted these annotation maybe because they are so famous and I
> >do not remember if we had any Vote for this significant change. If
> there was a Vote I would vote -1 and I would explain why.
> We can talk about it in the conference the next week but gaian I am
> saying that this feature with these annotations crerated conflicts
> between >Maven and plugins.
> I doubt that Google Guice is certified by TCK and the Oracle which means
> that these annotations are a fan of developers but not a container I can
> >trust.
> Additionally, joining Maven domain with EE domain is like joining Applet
> with Weld container of EE, the same mess.
>
> On Sat, Oct 19, 2019 at 3:41 PM Robert Scholte <rf...@apache.org>
> wrote:
>> Tibor, please lower your voice.
>> You're turning this topic into an unnecessary fight.
>> We're solving a regression introduced in 3.6.2.
>> Stuart was able to identify the problem, provide the fix and clearly
>> explain what happened.
>> Code has been review, fix is confirmed, so we're good to go.
>>
>> So don't hijack this thread and complain by sharing your strong
>> opinions.
>> They might be related to the code we're touching, but it doesn't help
>> fixing the issue.
>> If you want to discuss that, please just start a new topic on the
>> dev-list.
>>
>> thanks,
>> Robert
>>
>> On Sat, 19 Oct 2019 15:20:23 +0200, Tibor Digana
>> <ti...@apache.org> wrote:
>>
>>> Stuart, you are wrong.
>>> It is no more Java SE
>>> It is JakartaEE ans if you want to obey wrong design of Java SE go for
>>> it
>>> but then Maven would become a mess of Java EE annotations in non-EE
>>> container.
>>> Do you understand that @Name represents a key in the container? And
>>> how can
>>> you create a string like "core-default"? What's that? I do not know.
>>> It would be worth to have commercial experiences with Java EE and then
>>> you
>>> will understand that this is a big mistake and mess in Maven.
>>>
>>> On Sat, Oct 19, 2019 at 2:31 PM Stuart McCulloch <mc...@gmail.com>
>>> wrote:
>>>
>>>> @Named is not specific to Java EE, it's from JSR 330 which actually
>>>> targets
>>>> Java SE (and there are many SE based containers that support it)
>>>>
>>>> All @Named does is give a component an identity in the container
>>>> (think
>>>> of @Named like the hint in Plexus [1]) so it can be referenced by
>>>> that name
>>>> elsewhere. It also means you can inject lists or maps, where the key
>>>> is the
>>>> name, of all component implementations for a given type. This lets
>>>> plugins
>>>> and extensions contribute implementations of a core type and core can
>>>> then
>>>> see them and choose the right one for the job (such as "file" for a
>>>> file
>>>> specific implementation.)
>>>>
>>>> Maven historically also supports overriding of components by plugins
>>>> and
>>>> extensions, where if you have a component with the same interface
>>>> (role)
>>>> and name (hint) then it can override the core component while that
>>>> plugin
>>>> is active. This lets plugins customize certain core behaviour in a
>>>> controlled manner. If we didn't allow overriding then a lot of plugins
>>>> would fail and Maven would be a lot less flexible.
>>>>
>>>> [1] https://wiki.eclipse.org/Sisu/PlexusMigration
>>>>
>>>> On Sat, 19 Oct 2019 at 11:58, Tibor Digana <ti...@apache.org>
>>>> wrote:
>>>>
>>>> > Are you talking about
>>>> > @Named( “not-default” )
>>>> > @Named( “coreAllowingOverride” ) or @Named( “coreExtensionPoint” )?
>>>> >
>>>> > In Java EE (and these annotations are from Java EE application
>>>> servers)
>>>> > mean the name of the bean which is unique - it is not a group of
>>>> beans.
>>>> > Please notice that beans container is Map<String, Bean> simply
>>>> speaking.
>>>> > and therefore here it would mean :
>>>> >
>>>> > "core-default" -> singleton instance (DefaultModelProcessor@1234567)
>>>> >
>>>> > so this means a conflict because you cannot create:
>>>> >
>>>> > "core-default" -> singleton instance (DefaultModelProcessor@1234567)
>>>> > "core-default" -> singleton instance (DefaultResolver@1234567)
>>>> > "core-default" -> singleton instance (AnotherBeanType@1234567)
>>>> >
>>>> > logical would be to have Expression API from Java EE and:
>>>> >
>>>> > "core-default-modelprocessor" -> singleton instance
>>>> > (DefaultModelProcessor@1234567)
>>>> > "core-default-resolver" -> singleton instance
>>>> (DefaultResolver@1234567)
>>>> > "core-default-xxx" -> singleton instance (AnotherBeanType@1234567)
>>>> >
>>>> >
>>>> > On Sat, Oct 19, 2019 at 12:03 PM Hervé BOUTEMY
>>>> <he...@free.fr>
>>>> > wrote:
>>>> >
>>>> > > +1
>>>> > > just added a comment on a typo
>>>> > >
>>>> > > for the name of the component, perhaps "core-default", but I won't
>>>> > > complain
>>>> > > about any choice: the javadoc is what was really needed
>>>> > >
>>>> > > notice: perhaps we have other component that should have the same
>>>> > > improvement
>>>> > > to permit overriding in the future
>>>> > >
>>>> > > Regards,
>>>> > >
>>>> > > Hervé
>>>> > >
>>>> > > Le vendredi 18 octobre 2019, 20:04:53 CEST Robert Scholte a écrit
>>>> :
>>>> > > > Hi,
>>>> > > >
>>>> > > > with the help from Stuart McCulloch we've been able to provide a
>>>> patch
>>>> > > for
>>>> > > > MNG-6765[1]
>>>> > > > Please review and test.
>>>> > > >
>>>> > > > thanks,
>>>> > > > Robert
>>>> > > >
>>>> > > > [1] https://issues.apache.org/jira/browse/MNG-6765
>>>> > > > [2]
>>>> > > >
>>>> > >
>>>> >
>>>> https://github.com/apache/maven/commit/24e6c0ec0a87b6682513287a23c36db6996b8
>>>> > > > 74c [3]
>>>> > > >
>>>> > >
>>>> >
>>>> https://github.com/apache/maven/commit/53a70bc8543124569ee787725b2004bc92a68
>>>> > > > 1b6
>>>> > > >
>>>> > > >
>>>> ---------------------------------------------------------------------
>>>> > > > To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
>>>> > > > For additional commands, e-mail: dev-help@maven.apache.org
>>>> > >
>>>> > >
>>>> > >
>>>> > >
>>>> > >
>>>> > >
>>>> ---------------------------------------------------------------------
>>>> > > To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
>>>> > > For additional commands, e-mail: dev-help@maven.apache.org
>>>> > >
>>>> > >
>>>> >
Re: Second MNG-6765 ([Regression] tycho pom-less builds fails with 3.6.2)
Posted by Tibor Digana <ti...@apache.org>.
Robert we suffer from bad design in Maven. There is Jira issue where Maven
accepted these annotation maybe because they are so famous and I do not
remember if we had any Vote for this significant change. If there was a
Vote I would vote -1 and I would explain why.
We can talk about it in the conference the next week but gaian I am saying
that this feature with these annotations crerated conflicts between Maven
and plugins.
I doubt that Google Guice is certified by TCK and the Oracle which means
that these annotations are a fan of developers but not a container I can
trust.
Additionally, joining Maven domain with EE domain is like joining Applet
with Weld container of EE, the same mess.
On Sat, Oct 19, 2019 at 3:41 PM Robert Scholte <rf...@apache.org> wrote:
> Tibor, please lower your voice.
> You're turning this topic into an unnecessary fight.
> We're solving a regression introduced in 3.6.2.
> Stuart was able to identify the problem, provide the fix and clearly
> explain what happened.
> Code has been review, fix is confirmed, so we're good to go.
>
> So don't hijack this thread and complain by sharing your strong opinions.
> They might be related to the code we're touching, but it doesn't help
> fixing the issue.
> If you want to discuss that, please just start a new topic on the dev-list.
>
> thanks,
> Robert
>
> On Sat, 19 Oct 2019 15:20:23 +0200, Tibor Digana <ti...@apache.org>
>
> wrote:
>
> > Stuart, you are wrong.
> > It is no more Java SE
> > It is JakartaEE ans if you want to obey wrong design of Java SE go for it
> > but then Maven would become a mess of Java EE annotations in non-EE
> > container.
> > Do you understand that @Name represents a key in the container? And how
> > can
> > you create a string like "core-default"? What's that? I do not know.
> > It would be worth to have commercial experiences with Java EE and then
> > you
> > will understand that this is a big mistake and mess in Maven.
> >
> > On Sat, Oct 19, 2019 at 2:31 PM Stuart McCulloch <mc...@gmail.com>
> > wrote:
> >
> >> @Named is not specific to Java EE, it's from JSR 330 which actually
> >> targets
> >> Java SE (and there are many SE based containers that support it)
> >>
> >> All @Named does is give a component an identity in the container (think
> >> of @Named like the hint in Plexus [1]) so it can be referenced by that
> >> name
> >> elsewhere. It also means you can inject lists or maps, where the key
> is
> >> the
> >> name, of all component implementations for a given type. This lets
> >> plugins
> >> and extensions contribute implementations of a core type and core can
> >> then
> >> see them and choose the right one for the job (such as "file" for a file
> >> specific implementation.)
> >>
> >> Maven historically also supports overriding of components by plugins and
> >> extensions, where if you have a component with the same interface (role)
> >> and name (hint) then it can override the core component while that
> >> plugin
> >> is active. This lets plugins customize certain core behaviour in a
> >> controlled manner. If we didn't allow overriding then a lot of plugins
> >> would fail and Maven would be a lot less flexible.
> >>
> >> [1] https://wiki.eclipse.org/Sisu/PlexusMigration
> >>
> >> On Sat, 19 Oct 2019 at 11:58, Tibor Digana <ti...@apache.org>
> >> wrote:
> >>
> >> > Are you talking about
> >> > @Named( “not-default” )
> >> > @Named( “coreAllowingOverride” ) or @Named( “coreExtensionPoint” )?
> >> >
> >> > In Java EE (and these annotations are from Java EE application
> >> servers)
> >> > mean the name of the bean which is unique - it is not a group of
> >> beans.
> >> > Please notice that beans container is Map<String, Bean> simply
> >> speaking.
> >> > and therefore here it would mean :
> >> >
> >> > "core-default" -> singleton instance (DefaultModelProcessor@1234567)
> >> >
> >> > so this means a conflict because you cannot create:
> >> >
> >> > "core-default" -> singleton instance (DefaultModelProcessor@1234567)
> >> > "core-default" -> singleton instance (DefaultResolver@1234567)
> >> > "core-default" -> singleton instance (AnotherBeanType@1234567)
> >> >
> >> > logical would be to have Expression API from Java EE and:
> >> >
> >> > "core-default-modelprocessor" -> singleton instance
> >> > (DefaultModelProcessor@1234567)
> >> > "core-default-resolver" -> singleton instance
> >> (DefaultResolver@1234567)
> >> > "core-default-xxx" -> singleton instance (AnotherBeanType@1234567)
> >> >
> >> >
> >> > On Sat, Oct 19, 2019 at 12:03 PM Hervé BOUTEMY <herve.boutemy@free.fr
> >
> >> > wrote:
> >> >
> >> > > +1
> >> > > just added a comment on a typo
> >> > >
> >> > > for the name of the component, perhaps "core-default", but I won't
> >> > > complain
> >> > > about any choice: the javadoc is what was really needed
> >> > >
> >> > > notice: perhaps we have other component that should have the same
> >> > > improvement
> >> > > to permit overriding in the future
> >> > >
> >> > > Regards,
> >> > >
> >> > > Hervé
> >> > >
> >> > > Le vendredi 18 octobre 2019, 20:04:53 CEST Robert Scholte a écrit :
> >> > > > Hi,
> >> > > >
> >> > > > with the help from Stuart McCulloch we've been able to provide a
> >> patch
> >> > > for
> >> > > > MNG-6765[1]
> >> > > > Please review and test.
> >> > > >
> >> > > > thanks,
> >> > > > Robert
> >> > > >
> >> > > > [1] https://issues.apache.org/jira/browse/MNG-6765
> >> > > > [2]
> >> > > >
> >> > >
> >> >
> >>
> https://github.com/apache/maven/commit/24e6c0ec0a87b6682513287a23c36db6996b8
> >> > > > 74c [3]
> >> > > >
> >> > >
> >> >
> >>
> https://github.com/apache/maven/commit/53a70bc8543124569ee787725b2004bc92a68
> >> > > > 1b6
> >> > > >
> >> > > >
> >> ---------------------------------------------------------------------
> >> > > > To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
> >> > > > For additional commands, e-mail: dev-help@maven.apache.org
> >> > >
> >> > >
> >> > >
> >> > >
> >> > >
> >> > >
> >> ---------------------------------------------------------------------
> >> > > To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
> >> > > For additional commands, e-mail: dev-help@maven.apache.org
> >> > >
> >> > >
> >> >
>
Re: Second MNG-6765 ([Regression] tycho pom-less builds fails with
3.6.2)
Posted by Robert Scholte <rf...@apache.org>.
Tibor, please lower your voice.
You're turning this topic into an unnecessary fight.
We're solving a regression introduced in 3.6.2.
Stuart was able to identify the problem, provide the fix and clearly
explain what happened.
Code has been review, fix is confirmed, so we're good to go.
So don't hijack this thread and complain by sharing your strong opinions.
They might be related to the code we're touching, but it doesn't help
fixing the issue.
If you want to discuss that, please just start a new topic on the dev-list.
thanks,
Robert
On Sat, 19 Oct 2019 15:20:23 +0200, Tibor Digana <ti...@apache.org>
wrote:
> Stuart, you are wrong.
> It is no more Java SE
> It is JakartaEE ans if you want to obey wrong design of Java SE go for it
> but then Maven would become a mess of Java EE annotations in non-EE
> container.
> Do you understand that @Name represents a key in the container? And how
> can
> you create a string like "core-default"? What's that? I do not know.
> It would be worth to have commercial experiences with Java EE and then
> you
> will understand that this is a big mistake and mess in Maven.
>
> On Sat, Oct 19, 2019 at 2:31 PM Stuart McCulloch <mc...@gmail.com>
> wrote:
>
>> @Named is not specific to Java EE, it's from JSR 330 which actually
>> targets
>> Java SE (and there are many SE based containers that support it)
>>
>> All @Named does is give a component an identity in the container (think
>> of @Named like the hint in Plexus [1]) so it can be referenced by that
>> name
>> elsewhere. It also means you can inject lists or maps, where the key is
>> the
>> name, of all component implementations for a given type. This lets
>> plugins
>> and extensions contribute implementations of a core type and core can
>> then
>> see them and choose the right one for the job (such as "file" for a file
>> specific implementation.)
>>
>> Maven historically also supports overriding of components by plugins and
>> extensions, where if you have a component with the same interface (role)
>> and name (hint) then it can override the core component while that
>> plugin
>> is active. This lets plugins customize certain core behaviour in a
>> controlled manner. If we didn't allow overriding then a lot of plugins
>> would fail and Maven would be a lot less flexible.
>>
>> [1] https://wiki.eclipse.org/Sisu/PlexusMigration
>>
>> On Sat, 19 Oct 2019 at 11:58, Tibor Digana <ti...@apache.org>
>> wrote:
>>
>> > Are you talking about
>> > @Named( “not-default” )
>> > @Named( “coreAllowingOverride” ) or @Named( “coreExtensionPoint” )?
>> >
>> > In Java EE (and these annotations are from Java EE application
>> servers)
>> > mean the name of the bean which is unique - it is not a group of
>> beans.
>> > Please notice that beans container is Map<String, Bean> simply
>> speaking.
>> > and therefore here it would mean :
>> >
>> > "core-default" -> singleton instance (DefaultModelProcessor@1234567)
>> >
>> > so this means a conflict because you cannot create:
>> >
>> > "core-default" -> singleton instance (DefaultModelProcessor@1234567)
>> > "core-default" -> singleton instance (DefaultResolver@1234567)
>> > "core-default" -> singleton instance (AnotherBeanType@1234567)
>> >
>> > logical would be to have Expression API from Java EE and:
>> >
>> > "core-default-modelprocessor" -> singleton instance
>> > (DefaultModelProcessor@1234567)
>> > "core-default-resolver" -> singleton instance
>> (DefaultResolver@1234567)
>> > "core-default-xxx" -> singleton instance (AnotherBeanType@1234567)
>> >
>> >
>> > On Sat, Oct 19, 2019 at 12:03 PM Hervé BOUTEMY <he...@free.fr>
>> > wrote:
>> >
>> > > +1
>> > > just added a comment on a typo
>> > >
>> > > for the name of the component, perhaps "core-default", but I won't
>> > > complain
>> > > about any choice: the javadoc is what was really needed
>> > >
>> > > notice: perhaps we have other component that should have the same
>> > > improvement
>> > > to permit overriding in the future
>> > >
>> > > Regards,
>> > >
>> > > Hervé
>> > >
>> > > Le vendredi 18 octobre 2019, 20:04:53 CEST Robert Scholte a écrit :
>> > > > Hi,
>> > > >
>> > > > with the help from Stuart McCulloch we've been able to provide a
>> patch
>> > > for
>> > > > MNG-6765[1]
>> > > > Please review and test.
>> > > >
>> > > > thanks,
>> > > > Robert
>> > > >
>> > > > [1] https://issues.apache.org/jira/browse/MNG-6765
>> > > > [2]
>> > > >
>> > >
>> >
>> https://github.com/apache/maven/commit/24e6c0ec0a87b6682513287a23c36db6996b8
>> > > > 74c [3]
>> > > >
>> > >
>> >
>> https://github.com/apache/maven/commit/53a70bc8543124569ee787725b2004bc92a68
>> > > > 1b6
>> > > >
>> > > >
>> ---------------------------------------------------------------------
>> > > > To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
>> > > > For additional commands, e-mail: dev-help@maven.apache.org
>> > >
>> > >
>> > >
>> > >
>> > >
>> > >
>> ---------------------------------------------------------------------
>> > > To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
>> > > For additional commands, e-mail: dev-help@maven.apache.org
>> > >
>> > >
>> >
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
For additional commands, e-mail: dev-help@maven.apache.org
Re: Second MNG-6765 ([Regression] tycho pom-less builds fails with 3.6.2)
Posted by Tibor Digana <ti...@apache.org>.
Stuart, you are wrong.
It is no more Java SE
It is JakartaEE ans if you want to obey wrong design of Java SE go for it
but then Maven would become a mess of Java EE annotations in non-EE
container.
Do you understand that @Name represents a key in the container? And how can
you create a string like "core-default"? What's that? I do not know.
It would be worth to have commercial experiences with Java EE and then you
will understand that this is a big mistake and mess in Maven.
On Sat, Oct 19, 2019 at 2:31 PM Stuart McCulloch <mc...@gmail.com> wrote:
> @Named is not specific to Java EE, it's from JSR 330 which actually targets
> Java SE (and there are many SE based containers that support it)
>
> All @Named does is give a component an identity in the container (think
> of @Named like the hint in Plexus [1]) so it can be referenced by that name
> elsewhere. It also means you can inject lists or maps, where the key is the
> name, of all component implementations for a given type. This lets plugins
> and extensions contribute implementations of a core type and core can then
> see them and choose the right one for the job (such as "file" for a file
> specific implementation.)
>
> Maven historically also supports overriding of components by plugins and
> extensions, where if you have a component with the same interface (role)
> and name (hint) then it can override the core component while that plugin
> is active. This lets plugins customize certain core behaviour in a
> controlled manner. If we didn't allow overriding then a lot of plugins
> would fail and Maven would be a lot less flexible.
>
> [1] https://wiki.eclipse.org/Sisu/PlexusMigration
>
> On Sat, 19 Oct 2019 at 11:58, Tibor Digana <ti...@apache.org> wrote:
>
> > Are you talking about
> > @Named( “not-default” )
> > @Named( “coreAllowingOverride” ) or @Named( “coreExtensionPoint” )?
> >
> > In Java EE (and these annotations are from Java EE application servers)
> > mean the name of the bean which is unique - it is not a group of beans.
> > Please notice that beans container is Map<String, Bean> simply speaking.
> > and therefore here it would mean :
> >
> > "core-default" -> singleton instance (DefaultModelProcessor@1234567)
> >
> > so this means a conflict because you cannot create:
> >
> > "core-default" -> singleton instance (DefaultModelProcessor@1234567)
> > "core-default" -> singleton instance (DefaultResolver@1234567)
> > "core-default" -> singleton instance (AnotherBeanType@1234567)
> >
> > logical would be to have Expression API from Java EE and:
> >
> > "core-default-modelprocessor" -> singleton instance
> > (DefaultModelProcessor@1234567)
> > "core-default-resolver" -> singleton instance (DefaultResolver@1234567)
> > "core-default-xxx" -> singleton instance (AnotherBeanType@1234567)
> >
> >
> > On Sat, Oct 19, 2019 at 12:03 PM Hervé BOUTEMY <he...@free.fr>
> > wrote:
> >
> > > +1
> > > just added a comment on a typo
> > >
> > > for the name of the component, perhaps "core-default", but I won't
> > > complain
> > > about any choice: the javadoc is what was really needed
> > >
> > > notice: perhaps we have other component that should have the same
> > > improvement
> > > to permit overriding in the future
> > >
> > > Regards,
> > >
> > > Hervé
> > >
> > > Le vendredi 18 octobre 2019, 20:04:53 CEST Robert Scholte a écrit :
> > > > Hi,
> > > >
> > > > with the help from Stuart McCulloch we've been able to provide a
> patch
> > > for
> > > > MNG-6765[1]
> > > > Please review and test.
> > > >
> > > > thanks,
> > > > Robert
> > > >
> > > > [1] https://issues.apache.org/jira/browse/MNG-6765
> > > > [2]
> > > >
> > >
> >
> https://github.com/apache/maven/commit/24e6c0ec0a87b6682513287a23c36db6996b8
> > > > 74c [3]
> > > >
> > >
> >
> https://github.com/apache/maven/commit/53a70bc8543124569ee787725b2004bc92a68
> > > > 1b6
> > > >
> > > > ---------------------------------------------------------------------
> > > > To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
> > > > For additional commands, e-mail: dev-help@maven.apache.org
> > >
> > >
> > >
> > >
> > >
> > > ---------------------------------------------------------------------
> > > To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
> > > For additional commands, e-mail: dev-help@maven.apache.org
> > >
> > >
> >
>
Re: Second MNG-6765 ([Regression] tycho pom-less builds fails with 3.6.2)
Posted by Stuart McCulloch <mc...@gmail.com>.
@Named is not specific to Java EE, it's from JSR 330 which actually targets
Java SE (and there are many SE based containers that support it)
All @Named does is give a component an identity in the container (think
of @Named like the hint in Plexus [1]) so it can be referenced by that name
elsewhere. It also means you can inject lists or maps, where the key is the
name, of all component implementations for a given type. This lets plugins
and extensions contribute implementations of a core type and core can then
see them and choose the right one for the job (such as "file" for a file
specific implementation.)
Maven historically also supports overriding of components by plugins and
extensions, where if you have a component with the same interface (role)
and name (hint) then it can override the core component while that plugin
is active. This lets plugins customize certain core behaviour in a
controlled manner. If we didn't allow overriding then a lot of plugins
would fail and Maven would be a lot less flexible.
[1] https://wiki.eclipse.org/Sisu/PlexusMigration
On Sat, 19 Oct 2019 at 11:58, Tibor Digana <ti...@apache.org> wrote:
> Are you talking about
> @Named( “not-default” )
> @Named( “coreAllowingOverride” ) or @Named( “coreExtensionPoint” )?
>
> In Java EE (and these annotations are from Java EE application servers)
> mean the name of the bean which is unique - it is not a group of beans.
> Please notice that beans container is Map<String, Bean> simply speaking.
> and therefore here it would mean :
>
> "core-default" -> singleton instance (DefaultModelProcessor@1234567)
>
> so this means a conflict because you cannot create:
>
> "core-default" -> singleton instance (DefaultModelProcessor@1234567)
> "core-default" -> singleton instance (DefaultResolver@1234567)
> "core-default" -> singleton instance (AnotherBeanType@1234567)
>
> logical would be to have Expression API from Java EE and:
>
> "core-default-modelprocessor" -> singleton instance
> (DefaultModelProcessor@1234567)
> "core-default-resolver" -> singleton instance (DefaultResolver@1234567)
> "core-default-xxx" -> singleton instance (AnotherBeanType@1234567)
>
>
> On Sat, Oct 19, 2019 at 12:03 PM Hervé BOUTEMY <he...@free.fr>
> wrote:
>
> > +1
> > just added a comment on a typo
> >
> > for the name of the component, perhaps "core-default", but I won't
> > complain
> > about any choice: the javadoc is what was really needed
> >
> > notice: perhaps we have other component that should have the same
> > improvement
> > to permit overriding in the future
> >
> > Regards,
> >
> > Hervé
> >
> > Le vendredi 18 octobre 2019, 20:04:53 CEST Robert Scholte a écrit :
> > > Hi,
> > >
> > > with the help from Stuart McCulloch we've been able to provide a patch
> > for
> > > MNG-6765[1]
> > > Please review and test.
> > >
> > > thanks,
> > > Robert
> > >
> > > [1] https://issues.apache.org/jira/browse/MNG-6765
> > > [2]
> > >
> >
> https://github.com/apache/maven/commit/24e6c0ec0a87b6682513287a23c36db6996b8
> > > 74c [3]
> > >
> >
> https://github.com/apache/maven/commit/53a70bc8543124569ee787725b2004bc92a68
> > > 1b6
> > >
> > > ---------------------------------------------------------------------
> > > To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
> > > For additional commands, e-mail: dev-help@maven.apache.org
> >
> >
> >
> >
> >
> > ---------------------------------------------------------------------
> > To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
> > For additional commands, e-mail: dev-help@maven.apache.org
> >
> >
>
Re: Second MNG-6765 ([Regression] tycho pom-less builds fails with 3.6.2)
Posted by Tibor Digana <ti...@apache.org>.
Are you talking about
@Named( “not-default” )
@Named( “coreAllowingOverride” ) or @Named( “coreExtensionPoint” )?
In Java EE (and these annotations are from Java EE application servers)
mean the name of the bean which is unique - it is not a group of beans.
Please notice that beans container is Map<String, Bean> simply speaking.
and therefore here it would mean :
"core-default" -> singleton instance (DefaultModelProcessor@1234567)
so this means a conflict because you cannot create:
"core-default" -> singleton instance (DefaultModelProcessor@1234567)
"core-default" -> singleton instance (DefaultResolver@1234567)
"core-default" -> singleton instance (AnotherBeanType@1234567)
logical would be to have Expression API from Java EE and:
"core-default-modelprocessor" -> singleton instance
(DefaultModelProcessor@1234567)
"core-default-resolver" -> singleton instance (DefaultResolver@1234567)
"core-default-xxx" -> singleton instance (AnotherBeanType@1234567)
On Sat, Oct 19, 2019 at 12:03 PM Hervé BOUTEMY <he...@free.fr>
wrote:
> +1
> just added a comment on a typo
>
> for the name of the component, perhaps "core-default", but I won't
> complain
> about any choice: the javadoc is what was really needed
>
> notice: perhaps we have other component that should have the same
> improvement
> to permit overriding in the future
>
> Regards,
>
> Hervé
>
> Le vendredi 18 octobre 2019, 20:04:53 CEST Robert Scholte a écrit :
> > Hi,
> >
> > with the help from Stuart McCulloch we've been able to provide a patch
> for
> > MNG-6765[1]
> > Please review and test.
> >
> > thanks,
> > Robert
> >
> > [1] https://issues.apache.org/jira/browse/MNG-6765
> > [2]
> >
> https://github.com/apache/maven/commit/24e6c0ec0a87b6682513287a23c36db6996b8
> > 74c [3]
> >
> https://github.com/apache/maven/commit/53a70bc8543124569ee787725b2004bc92a68
> > 1b6
> >
> > ---------------------------------------------------------------------
> > To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
> > For additional commands, e-mail: dev-help@maven.apache.org
>
>
>
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
> For additional commands, e-mail: dev-help@maven.apache.org
>
>
Re: Second MNG-6765 ([Regression] tycho pom-less builds fails with 3.6.2)
Posted by Hervé BOUTEMY <he...@free.fr>.
+1
just added a comment on a typo
for the name of the component, perhaps "core-default", but I won't complain
about any choice: the javadoc is what was really needed
notice: perhaps we have other component that should have the same improvement
to permit overriding in the future
Regards,
Hervé
Le vendredi 18 octobre 2019, 20:04:53 CEST Robert Scholte a écrit :
> Hi,
>
> with the help from Stuart McCulloch we've been able to provide a patch for
> MNG-6765[1]
> Please review and test.
>
> thanks,
> Robert
>
> [1] https://issues.apache.org/jira/browse/MNG-6765
> [2]
> https://github.com/apache/maven/commit/24e6c0ec0a87b6682513287a23c36db6996b8
> 74c [3]
> https://github.com/apache/maven/commit/53a70bc8543124569ee787725b2004bc92a68
> 1b6
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
> For additional commands, e-mail: dev-help@maven.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
For additional commands, e-mail: dev-help@maven.apache.org
Re: Second MNG-6765 ([Regression] tycho pom-less builds fails with
3.6.2)
Posted by Robert Scholte <rf...@apache.org>.
Hi Joseph,
I'm pretty sure this would have happened to anybody that migrated the code.
I agree that we don't need an additional IT since it doesn't break Maven itself, plugins/extensions that rely on this mechanism should validate it.
And Eclipse already added a test for it, so we should be good.
thanks,
Robert
On 21-10-2019 05:55:31, Joseph Walton <jw...@atlassian.com> wrote:
Robert and Stuart; thank you for fixing the bug introduced by my change,
with such a clear explanation and without a major reversion.
From your description of this as an edge case, it doesn't seem like this
quite deserves a new test in maven-integration-testing, but it may be worth
testing with the Polyglot plugin (any others?) when finishing off MNG-5577.
On Sat, 19 Oct 2019 at 23:19, Stuart McCulloch wrote:
> ...
> which is why
> I added a detailed description in the original patch (and not just the
> annotation change)
>
Re: Second MNG-6765 ([Regression] tycho pom-less builds fails with 3.6.2)
Posted by Joseph Walton <jw...@atlassian.com>.
Robert and Stuart; thank you for fixing the bug introduced by my change,
with such a clear explanation and without a major reversion.
From your description of this as an edge case, it doesn't seem like this
quite deserves a new test in maven-integration-testing, but it may be worth
testing with the Polyglot plugin (any others?) when finishing off MNG-5577.
On Sat, 19 Oct 2019 at 23:19, Stuart McCulloch <mc...@gmail.com> wrote:
> ...
> which is why
> I added a detailed description in the original patch (and not just the
> annotation change)
>
Re: Second MNG-6765 ([Regression] tycho pom-less builds fails with 3.6.2)
Posted by Stuart McCulloch <mc...@gmail.com>.
OK, so this rabbit-hole is exactly what I wanted to avoid and which is why
I added a detailed description in the original patch (and not just the
annotation change)
To clarify: plugins and extensions can already override any component,
because their baseline priority is always higher than maven core (this
mimics the same behaviour as Plexus as configured by Maven, but the
underlying approach can support different models and also allows individual
priorities per-component.)
So the patch was _not_ about allowing overriding in the general case, but
instead to fix a small corner-case where a default component uses @Typed
(which requests that it is directly bound to the given type) and another
component just @Injects that type without any qualification. I've pasted
the full explanation at the end of this email just to increase its
visibility.
In other words... if you don't use @Typed (which is almost every case) then
you can continue to use "default" or a blank name for default components
and any other name for non-default components.
The _only_ time you need to be careful is when you:
1) start off with a default component which either uses @Named("default")
or uses @Named and the class is DefaultXYZ
and... 2) decide to add @Typed to restrict what types a component is
visible as (ie. what it can be injected as)
and... 3) want to allow overriding of that component
when all 3 of those are true then you need to use another name such
as @Named("typed-default") to stop the container from taking the
short-circuit.
If anyone needs further details I'm happy to supply them, I just want to
make clear that this is a corner-case which shouldn't affect the current
recommendations around @Named when moving from Plexus (which is to just use
the hint) because the advice here really is only necessary when the above 3
conditions apply which is very rare.
--------8<-------
* Note: uses @Typed to limit the types it is available for injection to
just ModelProcessor.
*
* This is because the ModelProcessor interface extends ModelLocator and
ModelReader. If we
* made this component available under all its interfaces then it could end
up being injected
* into itself leading to a stack overflow.
*
* A side-effect of using @Typed is that it translates to explicit bindings
in the container.
* So instead of binding the component under a 'wildcard' key it is now
bound with an explicit
* key. Since this is a default component this will be a plain binding of
ModelProcessor to
* this implementation type, ie. no hint/name.
*
* This leads to a second side-effect in that any @Inject request for just
ModelProcessor in
* the same injector is immediately matched to this explicit binding, which
means extensions
* cannot override this binding. This is because the lookup is always
short-circuited in this
* specific situation (plain @Inject request, and plain explicit binding
for the same type.)
*
* The simplest solution is to use a custom @Named here so it isn't bound
under the plain key.
* This is only necessary for default components using @Typed that want to
support overriding.
*
* As a non-default component this now gets a -ve priority relative to
other implementations
* of the same interface. Since we want to allow overriding this doesn't
matter in this case.
* (if it did we could add @Priority of 0 to match the priority given to
default components.)
On Sat, 19 Oct 2019 at 10:18, Robert Scholte <rf...@apache.org> wrote:
> On Sat, 19 Oct 2019 02:25:47 +0200, Jason van Zyl <ja...@vanzyl.ca> wrote:
>
> > Inline:
> >
> > On Oct 18, 2019, at 5:15 PM, Stuart McCulloch <mc...@gmail.com>
> wrote:
> >>
> >> Any string apart from "default" or the empty string will work here - I
> >> happened to chose "core" because I viewed it as a core implementation.
> >>
> >> Also a quick reminder that this is only needed when a component uses
> >> @Typed
> >> and wants to allow overriding, it's not necessary in any other
> >> situation -
> >> so in that sense "allowDefaultOverride" wouldn't be quite accurate. (See
> >> the javadoc in the patch for more detail.)
> >>
> >
> > Stuart,
> >
> > There is an idiom like this used in the ReactorReader and the
> > GraphBuilder where there are implementations of them in extensions out
> > in the wild, and while those are not @Typed they use a @Named(
> > “not-default” ) key pattern. If these components are intended to allow
> > for custom implementations then a common way of doing this would be
> > easier to understand. Something like @Named( “coreAllowingOverride” )
> or
> > @Named( “coreExtensionPoint” ) or whatever most appropriate. Right now
> > there’s a bit of fiddly magic that works in Plexus with a lookup and
> > Plexus with its annotations, and slightly different way with Sisu with
> > @Inject annotations. So, sure, this particular case of requiring
> @Named(
> > “core” ) to fix the case where you want to override a component with
> the
> > implicit “default” key is required, but maybe something we should avoid
> > and if it’s meant to be extended just have an explicit common pattern.
> I
> > realize with DI you can override anything, but I don’t think being a
> > little more explicit would hurt. The nuance of how the bindings work
> > maybe you and Igor know/remember how it works, I had to use a debugger
> > this afternoon :-)
>
> I guess the best value would have been "default", but that won't work.
> Since all components can be overridden, I suggest to change it to simply
> "DefaultModelProcessor".
>
> Some people already want to add module descriptors to their
> plugins/extensions. In order to support that we already need to
> restructure interfaces and make a clear separatation of SPIs and APIs.
> That's likely better than trying to do more magic with the @Named
> annotation that's already around for quite some time.
>
> >
> > Robert,
> >
> > What’s overridden in polyglot is to be injected into Maven’s core is
> the
> > ModelProcessor, not the ModelBuilder:
> >
> >
> https://github.com/takari/polyglot-maven/blob/master/polyglot-common/src/main/java/org/sonatype/maven/polyglot/TeslaModelProcessor.java
> >
> > The ModelReaders are injected into a manager within the
> > TeslaModelProcessor, sure, but it’s the ModelProcessor being overridden
> > which makes the magic happen in polyglot. When the ModelProcessor
> > polyglot requires doesn't get injected it’s trying to use the XML-based
> > model reader to read Kotlin which doesn’t work out so well. Trying to
> > build a polyglot project without the extensions loaded in 3.6.1 yields
> > the same result as trying to use 3.6.2 with a polyglot project: the
> same
> > error which is a result of the right ModelProcessor not being found.
> > You’ll see the the DefaultModelProcessor being used instead of the
> > TeslaModelProcessor in the stack trace.
> >
>
> Right, so the stacktrace fooled me. ModelProcessor it is.
>
> >> PS. the reason DefaultModelProcessor needs to use @Typed is because it
> >> has
> >> an "interesting" interface hierarchy where ModelProcessor extends
> >> both ModelLocator and ModelReader, so it can act as both and delegate
> >> accordingly - but then it also asks to have a ModelLocator and
> >> ModelReader
> >> injected, which is where things get messy. If it had a cleaner hierarchy
> >> (ie. it wasn't asking to inject something that it also implements)
> then
> >> it
> >> wouldn't need @Typed and wouldn't then need the custom name.
> >>
> >
> > Agreed. Looks fixable and there are probably only two consumers in the
> > world. Polyglot and
> > https://github.com/qoomon/maven-git-versioning-extension where it was
> > attempt to fix the problem in 3.6.2.
> >
> > jvz
> >
> >>
> >> On Fri, 18 Oct 2019 at 20:35, Robert Scholte <rf...@apache.org>
> >> wrote:
> >>
> >>> Hi,
> >>>
> >>> the adjusted @Named annotation is on DefaultModelProcessor, not
> >>> DefaultModelBuilder.
> >>> They both implement the ModelBuilder interface, but the one that
> >>> extensions like to overwrite is the implementation of
> >>> DefaultModelBuilder.
> >>> So I'd prefer to stick to "core" as proposed my Stuart.
> >>>
> >>> thanks for the confirmation that this works,
> >>> Robert
> >>>
> >>> On Fri, 18 Oct 2019 21:10:18 +0200, Jason van Zyl <jv...@apache.org>
> >>> wrote:
> >>>
> >>>> Hi,
> >>>>
> >>>> As noted in Slack I think it would be more clear if we used something
> >>>> like
> >>>>
> >>>> @Named( “allowDefaultOverride” )
> >>>>
> >>>> vs
> >>>>
> >>>> @Named( “core” )
> >>>>
> >>>> As that expresses the intent and can be used anywhere it's allowed for
> >>> a
> >>>> client to override the default component.
> >>>>
> >>>> The tests in polyglot all pass with this change, and I’m able to run
> >>>> polyglot example projects again, so I assume the Tycho POM-less are
> >>>> happy again as well but hopefully someone can verify.
> >>>>
> >>>> Jason
> >>>>
> >>>>> On Oct 18, 2019, at 2:04 PM, Robert Scholte <rf...@apache.org>
> >>>>> wrote:
> >>>>>
> >>>>> Hi,
> >>>>>
> >>>>> with the help from Stuart McCulloch we've been able to provide a
> >>>>> patch
> >>>>> for MNG-6765[1]
> >>>>> Please review and test.
> >>>>>
> >>>>> thanks,
> >>>>> Robert
> >>>>>
> >>>>> [1] https://issues.apache.org/jira/browse/MNG-67
> >>>>> [2]
> >>>>>
> >>>
> https://github.com/apache/maven/commit/24e6c0ec0a87b6682513287a23c36db6996b874c
> >>>>> [3]
> >>>>>
> >>>
> https://github.com/apache/maven/commit/53a70bc8543124569ee787725b2004bc92a681b6
> >>>>>
> >>>>> ---------------------------------------------------------------------
> >>>>> To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
> >>>>> For additional commands, e-mail: dev-help@maven.apache.org
> >>>>>
> >>>>
> >>>>
> >>>> ---------------------------------------------------------------------
> >>>> To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
> >>>> For additional commands, e-mail: dev-help@maven.apache.org
> >>>
> >>> ---------------------------------------------------------------------
> >>> To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
> >>> For additional commands, e-mail: dev-help@maven.apache.org
> >>>
> >>>
> >
> >
> >
> > ---------------------------------------------------------------------
> > To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
> > For additional commands, e-mail: dev-help@maven.apache.org
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
> For additional commands, e-mail: dev-help@maven.apache.org
>
>
Re: Second MNG-6765 ([Regression] tycho pom-less builds fails with
3.6.2)
Posted by Robert Scholte <rf...@apache.org>.
On Sat, 19 Oct 2019 02:25:47 +0200, Jason van Zyl <ja...@vanzyl.ca> wrote:
> Inline:
>
> On Oct 18, 2019, at 5:15 PM, Stuart McCulloch <mc...@gmail.com> wrote:
>>
>> Any string apart from "default" or the empty string will work here - I
>> happened to chose "core" because I viewed it as a core implementation.
>>
>> Also a quick reminder that this is only needed when a component uses
>> @Typed
>> and wants to allow overriding, it's not necessary in any other
>> situation -
>> so in that sense "allowDefaultOverride" wouldn't be quite accurate. (See
>> the javadoc in the patch for more detail.)
>>
>
> Stuart,
>
> There is an idiom like this used in the ReactorReader and the
> GraphBuilder where there are implementations of them in extensions out
> in the wild, and while those are not @Typed they use a @Named(
> “not-default” ) key pattern. If these components are intended to allow
> for custom implementations then a common way of doing this would be
> easier to understand. Something like @Named( “coreAllowingOverride” ) or
> @Named( “coreExtensionPoint” ) or whatever most appropriate. Right now
> there’s a bit of fiddly magic that works in Plexus with a lookup and
> Plexus with its annotations, and slightly different way with Sisu with
> @Inject annotations. So, sure, this particular case of requiring @Named(
> “core” ) to fix the case where you want to override a component with the
> implicit “default” key is required, but maybe something we should avoid
> and if it’s meant to be extended just have an explicit common pattern. I
> realize with DI you can override anything, but I don’t think being a
> little more explicit would hurt. The nuance of how the bindings work
> maybe you and Igor know/remember how it works, I had to use a debugger
> this afternoon :-)
I guess the best value would have been "default", but that won't work.
Since all components can be overridden, I suggest to change it to simply
"DefaultModelProcessor".
Some people already want to add module descriptors to their
plugins/extensions. In order to support that we already need to
restructure interfaces and make a clear separatation of SPIs and APIs.
That's likely better than trying to do more magic with the @Named
annotation that's already around for quite some time.
>
> Robert,
>
> What’s overridden in polyglot is to be injected into Maven’s core is the
> ModelProcessor, not the ModelBuilder:
>
> https://github.com/takari/polyglot-maven/blob/master/polyglot-common/src/main/java/org/sonatype/maven/polyglot/TeslaModelProcessor.java
>
> The ModelReaders are injected into a manager within the
> TeslaModelProcessor, sure, but it’s the ModelProcessor being overridden
> which makes the magic happen in polyglot. When the ModelProcessor
> polyglot requires doesn't get injected it’s trying to use the XML-based
> model reader to read Kotlin which doesn’t work out so well. Trying to
> build a polyglot project without the extensions loaded in 3.6.1 yields
> the same result as trying to use 3.6.2 with a polyglot project: the same
> error which is a result of the right ModelProcessor not being found.
> You’ll see the the DefaultModelProcessor being used instead of the
> TeslaModelProcessor in the stack trace.
>
Right, so the stacktrace fooled me. ModelProcessor it is.
>> PS. the reason DefaultModelProcessor needs to use @Typed is because it
>> has
>> an "interesting" interface hierarchy where ModelProcessor extends
>> both ModelLocator and ModelReader, so it can act as both and delegate
>> accordingly - but then it also asks to have a ModelLocator and
>> ModelReader
>> injected, which is where things get messy. If it had a cleaner hierarchy
>> (ie. it wasn't asking to inject something that it also implements) then
>> it
>> wouldn't need @Typed and wouldn't then need the custom name.
>>
>
> Agreed. Looks fixable and there are probably only two consumers in the
> world. Polyglot and
> https://github.com/qoomon/maven-git-versioning-extension where it was
> attempt to fix the problem in 3.6.2.
>
> jvz
>
>>
>> On Fri, 18 Oct 2019 at 20:35, Robert Scholte <rf...@apache.org>
>> wrote:
>>
>>> Hi,
>>>
>>> the adjusted @Named annotation is on DefaultModelProcessor, not
>>> DefaultModelBuilder.
>>> They both implement the ModelBuilder interface, but the one that
>>> extensions like to overwrite is the implementation of
>>> DefaultModelBuilder.
>>> So I'd prefer to stick to "core" as proposed my Stuart.
>>>
>>> thanks for the confirmation that this works,
>>> Robert
>>>
>>> On Fri, 18 Oct 2019 21:10:18 +0200, Jason van Zyl <jv...@apache.org>
>>> wrote:
>>>
>>>> Hi,
>>>>
>>>> As noted in Slack I think it would be more clear if we used something
>>>> like
>>>>
>>>> @Named( “allowDefaultOverride” )
>>>>
>>>> vs
>>>>
>>>> @Named( “core” )
>>>>
>>>> As that expresses the intent and can be used anywhere it's allowed for
>>> a
>>>> client to override the default component.
>>>>
>>>> The tests in polyglot all pass with this change, and I’m able to run
>>>> polyglot example projects again, so I assume the Tycho POM-less are
>>>> happy again as well but hopefully someone can verify.
>>>>
>>>> Jason
>>>>
>>>>> On Oct 18, 2019, at 2:04 PM, Robert Scholte <rf...@apache.org>
>>>>> wrote:
>>>>>
>>>>> Hi,
>>>>>
>>>>> with the help from Stuart McCulloch we've been able to provide a
>>>>> patch
>>>>> for MNG-6765[1]
>>>>> Please review and test.
>>>>>
>>>>> thanks,
>>>>> Robert
>>>>>
>>>>> [1] https://issues.apache.org/jira/browse/MNG-67
>>>>> [2]
>>>>>
>>> https://github.com/apache/maven/commit/24e6c0ec0a87b6682513287a23c36db6996b874c
>>>>> [3]
>>>>>
>>> https://github.com/apache/maven/commit/53a70bc8543124569ee787725b2004bc92a681b6
>>>>>
>>>>> ---------------------------------------------------------------------
>>>>> To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
>>>>> For additional commands, e-mail: dev-help@maven.apache.org
>>>>>
>>>>
>>>>
>>>> ---------------------------------------------------------------------
>>>> To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
>>>> For additional commands, e-mail: dev-help@maven.apache.org
>>>
>>> ---------------------------------------------------------------------
>>> To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
>>> For additional commands, e-mail: dev-help@maven.apache.org
>>>
>>>
>
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
> For additional commands, e-mail: dev-help@maven.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
For additional commands, e-mail: dev-help@maven.apache.org
Re: Second MNG-6765 ([Regression] tycho pom-less builds fails with
3.6.2)
Posted by Jason van Zyl <ja...@vanzyl.ca>.
Inline:
On Oct 18, 2019, at 5:15 PM, Stuart McCulloch <mc...@gmail.com> wrote:
>
> Any string apart from "default" or the empty string will work here - I
> happened to chose "core" because I viewed it as a core implementation.
>
> Also a quick reminder that this is only needed when a component uses @Typed
> and wants to allow overriding, it's not necessary in any other situation -
> so in that sense "allowDefaultOverride" wouldn't be quite accurate. (See
> the javadoc in the patch for more detail.)
>
Stuart,
There is an idiom like this used in the ReactorReader and the GraphBuilder where there are implementations of them in extensions out in the wild, and while those are not @Typed they use a @Named( “not-default” ) key pattern. If these components are intended to allow for custom implementations then a common way of doing this would be easier to understand. Something like @Named( “coreAllowingOverride” ) or @Named( “coreExtensionPoint” ) or whatever most appropriate. Right now there’s a bit of fiddly magic that works in Plexus with a lookup and Plexus with its annotations, and slightly different way with Sisu with @Inject annotations. So, sure, this particular case of requiring @Named( “core” ) to fix the case where you want to override a component with the implicit “default” key is required, but maybe something we should avoid and if it’s meant to be extended just have an explicit common pattern. I realize with DI you can override anything, but I don’t think being a little more explicit would hurt. The nuance of how the bindings work maybe you and Igor know/remember how it works, I had to use a debugger this afternoon :-)
Robert,
What’s overridden in polyglot is to be injected into Maven’s core is the ModelProcessor, not the ModelBuilder:
https://github.com/takari/polyglot-maven/blob/master/polyglot-common/src/main/java/org/sonatype/maven/polyglot/TeslaModelProcessor.java
The ModelReaders are injected into a manager within the TeslaModelProcessor, sure, but it’s the ModelProcessor being overridden which makes the magic happen in polyglot. When the ModelProcessor polyglot requires doesn't get injected it’s trying to use the XML-based model reader to read Kotlin which doesn’t work out so well. Trying to build a polyglot project without the extensions loaded in 3.6.1 yields the same result as trying to use 3.6.2 with a polyglot project: the same error which is a result of the right ModelProcessor not being found. You’ll see the the DefaultModelProcessor being used instead of the TeslaModelProcessor in the stack trace.
> PS. the reason DefaultModelProcessor needs to use @Typed is because it has
> an "interesting" interface hierarchy where ModelProcessor extends
> both ModelLocator and ModelReader, so it can act as both and delegate
> accordingly - but then it also asks to have a ModelLocator and ModelReader
> injected, which is where things get messy. If it had a cleaner hierarchy
> (ie. it wasn't asking to inject something that it also implements) then it
> wouldn't need @Typed and wouldn't then need the custom name.
>
Agreed. Looks fixable and there are probably only two consumers in the world. Polyglot and https://github.com/qoomon/maven-git-versioning-extension where it was attempt to fix the problem in 3.6.2.
jvz
>
> On Fri, 18 Oct 2019 at 20:35, Robert Scholte <rf...@apache.org> wrote:
>
>> Hi,
>>
>> the adjusted @Named annotation is on DefaultModelProcessor, not
>> DefaultModelBuilder.
>> They both implement the ModelBuilder interface, but the one that
>> extensions like to overwrite is the implementation of DefaultModelBuilder.
>> So I'd prefer to stick to "core" as proposed my Stuart.
>>
>> thanks for the confirmation that this works,
>> Robert
>>
>> On Fri, 18 Oct 2019 21:10:18 +0200, Jason van Zyl <jv...@apache.org>
>> wrote:
>>
>>> Hi,
>>>
>>> As noted in Slack I think it would be more clear if we used something
>>> like
>>>
>>> @Named( “allowDefaultOverride” )
>>>
>>> vs
>>>
>>> @Named( “core” )
>>>
>>> As that expresses the intent and can be used anywhere it's allowed for
>> a
>>> client to override the default component.
>>>
>>> The tests in polyglot all pass with this change, and I’m able to run
>>> polyglot example projects again, so I assume the Tycho POM-less are
>>> happy again as well but hopefully someone can verify.
>>>
>>> Jason
>>>
>>>> On Oct 18, 2019, at 2:04 PM, Robert Scholte <rf...@apache.org>
>>>> wrote:
>>>>
>>>> Hi,
>>>>
>>>> with the help from Stuart McCulloch we've been able to provide a patch
>>>> for MNG-6765[1]
>>>> Please review and test.
>>>>
>>>> thanks,
>>>> Robert
>>>>
>>>> [1] https://issues.apache.org/jira/browse/MNG-67
>>>> [2]
>>>>
>> https://github.com/apache/maven/commit/24e6c0ec0a87b6682513287a23c36db6996b874c
>>>> [3]
>>>>
>> https://github.com/apache/maven/commit/53a70bc8543124569ee787725b2004bc92a681b6
>>>>
>>>> ---------------------------------------------------------------------
>>>> To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
>>>> For additional commands, e-mail: dev-help@maven.apache.org
>>>>
>>>
>>>
>>> ---------------------------------------------------------------------
>>> To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
>>> For additional commands, e-mail: dev-help@maven.apache.org
>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
>> For additional commands, e-mail: dev-help@maven.apache.org
>>
>>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
For additional commands, e-mail: dev-help@maven.apache.org
Re: Second MNG-6765 ([Regression] tycho pom-less builds fails with 3.6.2)
Posted by Stuart McCulloch <mc...@gmail.com>.
Any string apart from "default" or the empty string will work here - I
happened to chose "core" because I viewed it as a core implementation.
Also a quick reminder that this is only needed when a component uses @Typed
and wants to allow overriding, it's not necessary in any other situation -
so in that sense "allowDefaultOverride" wouldn't be quite accurate. (See
the javadoc in the patch for more detail.)
PS. the reason DefaultModelProcessor needs to use @Typed is because it has
an "interesting" interface hierarchy where ModelProcessor extends
both ModelLocator and ModelReader, so it can act as both and delegate
accordingly - but then it also asks to have a ModelLocator and ModelReader
injected, which is where things get messy. If it had a cleaner hierarchy
(ie. it wasn't asking to inject something that it also implements) then it
wouldn't need @Typed and wouldn't then need the custom name.
On Fri, 18 Oct 2019 at 20:35, Robert Scholte <rf...@apache.org> wrote:
> Hi,
>
> the adjusted @Named annotation is on DefaultModelProcessor, not
> DefaultModelBuilder.
> They both implement the ModelBuilder interface, but the one that
> extensions like to overwrite is the implementation of DefaultModelBuilder.
> So I'd prefer to stick to "core" as proposed my Stuart.
>
> thanks for the confirmation that this works,
> Robert
>
> On Fri, 18 Oct 2019 21:10:18 +0200, Jason van Zyl <jv...@apache.org>
> wrote:
>
> > Hi,
> >
> > As noted in Slack I think it would be more clear if we used something
> > like
> >
> > @Named( “allowDefaultOverride” )
> >
> > vs
> >
> > @Named( “core” )
> >
> > As that expresses the intent and can be used anywhere it's allowed for
> a
> > client to override the default component.
> >
> > The tests in polyglot all pass with this change, and I’m able to run
> > polyglot example projects again, so I assume the Tycho POM-less are
> > happy again as well but hopefully someone can verify.
> >
> > Jason
> >
> >> On Oct 18, 2019, at 2:04 PM, Robert Scholte <rf...@apache.org>
> >> wrote:
> >>
> >> Hi,
> >>
> >> with the help from Stuart McCulloch we've been able to provide a patch
> >> for MNG-6765[1]
> >> Please review and test.
> >>
> >> thanks,
> >> Robert
> >>
> >> [1] https://issues.apache.org/jira/browse/MNG-67
> >> [2]
> >>
> https://github.com/apache/maven/commit/24e6c0ec0a87b6682513287a23c36db6996b874c
> >> [3]
> >>
> https://github.com/apache/maven/commit/53a70bc8543124569ee787725b2004bc92a681b6
> >>
> >> ---------------------------------------------------------------------
> >> To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
> >> For additional commands, e-mail: dev-help@maven.apache.org
> >>
> >
> >
> > ---------------------------------------------------------------------
> > To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
> > For additional commands, e-mail: dev-help@maven.apache.org
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
> For additional commands, e-mail: dev-help@maven.apache.org
>
>
Re: Second MNG-6765 ([Regression] tycho pom-less builds fails with
3.6.2)
Posted by Robert Scholte <rf...@apache.org>.
Hi,
the adjusted @Named annotation is on DefaultModelProcessor, not
DefaultModelBuilder.
They both implement the ModelBuilder interface, but the one that
extensions like to overwrite is the implementation of DefaultModelBuilder.
So I'd prefer to stick to "core" as proposed my Stuart.
thanks for the confirmation that this works,
Robert
On Fri, 18 Oct 2019 21:10:18 +0200, Jason van Zyl <jv...@apache.org>
wrote:
> Hi,
>
> As noted in Slack I think it would be more clear if we used something
> like
>
> @Named( “allowDefaultOverride” )
>
> vs
>
> @Named( “core” )
>
> As that expresses the intent and can be used anywhere it's allowed for a
> client to override the default component.
>
> The tests in polyglot all pass with this change, and I’m able to run
> polyglot example projects again, so I assume the Tycho POM-less are
> happy again as well but hopefully someone can verify.
>
> Jason
>
>> On Oct 18, 2019, at 2:04 PM, Robert Scholte <rf...@apache.org>
>> wrote:
>>
>> Hi,
>>
>> with the help from Stuart McCulloch we've been able to provide a patch
>> for MNG-6765[1]
>> Please review and test.
>>
>> thanks,
>> Robert
>>
>> [1] https://issues.apache.org/jira/browse/MNG-67
>> [2]
>> https://github.com/apache/maven/commit/24e6c0ec0a87b6682513287a23c36db6996b874c
>> [3]
>> https://github.com/apache/maven/commit/53a70bc8543124569ee787725b2004bc92a681b6
>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
>> For additional commands, e-mail: dev-help@maven.apache.org
>>
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
> For additional commands, e-mail: dev-help@maven.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
For additional commands, e-mail: dev-help@maven.apache.org
Re: Second MNG-6765 ([Regression] tycho pom-less builds fails with
3.6.2)
Posted by Jason van Zyl <jv...@apache.org>.
Hi,
As noted in Slack I think it would be more clear if we used something like
@Named( “allowDefaultOverride” )
vs
@Named( “core” )
As that expresses the intent and can be used anywhere it's allowed for a client to override the default component.
The tests in polyglot all pass with this change, and I’m able to run polyglot example projects again, so I assume the Tycho POM-less are happy again as well but hopefully someone can verify.
Jason
> On Oct 18, 2019, at 2:04 PM, Robert Scholte <rf...@apache.org> wrote:
>
> Hi,
>
> with the help from Stuart McCulloch we've been able to provide a patch for MNG-6765[1]
> Please review and test.
>
> thanks,
> Robert
>
> [1] https://issues.apache.org/jira/browse/MNG-67
> [2] https://github.com/apache/maven/commit/24e6c0ec0a87b6682513287a23c36db6996b874c
> [3] https://github.com/apache/maven/commit/53a70bc8543124569ee787725b2004bc92a681b6
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
> For additional commands, e-mail: dev-help@maven.apache.org
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
For additional commands, e-mail: dev-help@maven.apache.org
Re: Second MNG-6765 ([Regression] tycho pom-less builds fails with 3.6.2)
Posted by Enrico Olivelli <eo...@gmail.com>.
+1
Thank you Stuart
Maybe it would be better to add a test case so that we want break this
so-called corner-case in the future.
side note: if we create pull requests it is easier to review changes
Enrico
Il giorno ven 18 ott 2019 alle ore 20:04 Robert Scholte <
rfscholte@apache.org> ha scritto:
> Hi,
>
> with the help from Stuart McCulloch we've been able to provide a patch
> for
> MNG-6765[1]
> Please review and test.
>
> thanks,
> Robert
>
> [1] https://issues.apache.org/jira/browse/MNG-6765
> [2]
>
> https://github.com/apache/maven/commit/24e6c0ec0a87b6682513287a23c36db6996b874c
> [3]
>
> https://github.com/apache/maven/commit/53a70bc8543124569ee787725b2004bc92a681b6
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
> For additional commands, e-mail: dev-help@maven.apache.org
>
>