Re: Understanding MNG-6209 (was: [VOTE] Release Apache Maven 3.5.1)

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
7 messages Options
Reply | Threaded
Open this post in threaded view
|

Re: Understanding MNG-6209 (was: [VOTE] Release Apache Maven 3.5.1)

Igor Fedorenko-3
Real-world scm or wagon <extensions> won't trigger maven2-compat code
path [1]. To avoid that obscure code path we can either make the test
more elaborate (i.e. add dependencies to extjar1/extjar2) or we can use
extensions <plugin>. Either way I don't think we should spend time on
the code path unlikely to be used in real life.

[1]
https://github.com/apache/maven/blob/maven-3.5.1/maven-core/src/main/java/org/apache/maven/project/DefaultProjectBuildingHelper.java#L210-L219

--
Regards,
Igor

On Wed, Sep 20, 2017, at 03:29 AM, Robert Scholte wrote:

> On Wed, 20 Sep 2017 09:12:47 +0200, Stephen Connolly  
> <[hidden email]> wrote:
>
> > On Wed 20 Sep 2017 at 01:29, Igor Fedorenko <[hidden email]> wrote:
> >
> >> In that case, can I suggest couple of changes to the test project
> >>
> >> * I thinks it makes more sense to configure extjar1 and extjar2 as
> >> extensions <plugin> elements in probleN pom.xml files. First, there is
> >> no meaningful order between <extensions> and <plugins> elements. More
> >> importantly, though, simple <extensions> are treated in special
> >> maven2-compat mode and are not representative of likely real-world
> >> extensions.
> >
>
> Not sure I agree with this. I think there are jars worth sharing across  
> multiple plugins, but where making the plugin an extension is a bit
> weird.
> I'm thinking of scm and wagon in this case.
>
> >
> > That sounds like we need documentation updated then. None of that is
> > obvious to me.
> >
> >
> >>
> >> * I think we should introduce META-INF/maven/extension.xml to the test
> >> extensions. This metadata what introduced to configure classpath
> >> visibility, so lets use it.
> >
> >
> > Again, not obvious to me, if that file allows control of classpath
> > visibility then it may be that the only issue *with* 3.5.1 is the lack of
> > documentation... now previous versions would have been adding breaking
> > changes from my PoV but that is the past and should not affect the 3.5.1
> > release.
> >
> > PRs for the probe project welcome. I am happy to try and write docs once  
> > I
> > have an understanding of what the expected behaviours are
> >
> >
> >>
> >> --
> >> Regards,
> >> Igor
> >>
> >> On Tue, Sep 19, 2017, at 05:12 PM, Stephen Connolly wrote:
> >> > Yes, the expectations are key. Depending on what they are we may  
> >> either
> >> > drop 3.5.1 or go ahead as it depends on whether this is more correct  
> >> than
> >> > 3.5.0 or swapping one fix for a bug
> >> >
> >> > On Tue 19 Sep 2017 at 21:39, Igor Fedorenko <[hidden email]>  
> >> wrote:
> >> >
> >> > > Just to confirm I understand what we are trying to establish here.  
> >> We
> >> > > want to decide the expected/desired component injection behaviour  
> >> and
> >> > > classpath visibility in the absence of package and artifact export
> >> > > configuration (i.e. META-INF/maven/extension.xml file). Did I get  
> >> this
> >> > > right?
> >> > >
> >> > > --
> >> > > Regards,
> >> > > Igor
> >> > >
> >> > > On Tue, Sep 19, 2017, at 03:52 PM, Robert Scholte wrote:
> >> > > > Let's do it like this:
> >> > > >
> >> > >
> >> https://cwiki.apache.org/confluence/download/attachments/2329841/classrealms.pdf?api=v2
> >> > > >
> >> > > > Robert
> >> > > >
> >> > > > On Tue, 19 Sep 2017 21:08:39 +0200, Stephen Connolly
> >> > > > <[hidden email]> wrote:
> >> > > >
> >> > > > > I think you will need a link to the PDF as attachments are  
> >> stripped
> >> > > from
> >> > > > > the ML
> >> > > > >
> >> > > > > On Tue 19 Sep 2017 at 19:57, Robert Scholte  
> >> <[hidden email]>
> >> > > wrote:
> >> > > > >
> >> > > > >> Attached a single page overview.
> >> > > > >>
> >> > > > >> Per block you'll see in the upper left corner the executed  
> >> plugin
> >> > > > >> The left column contains the extensions and plugin in orderas
> >> > > specified
> >> > > > >> in
> >> > > > >> the pom.xml
> >> > > > >> In every classloadercolumn you'll see numbers which represent  
> >> the
> >> > > order.
> >> > > > >>
> >> > > > >> I hope I didn't make any mistakes.
> >> > > > >> Tomorrow I have enough time to see if I understand what's
> >> happening
> >> > > > >> here.
> >> > > > >>
> >> > > > >> I will come back with my conclusions.
> >> > > > >>
> >> > > > >> Robert
> >> > > > >>
> >> > > > >> On Tue, 19 Sep 2017 06:55:08 +0200, Igor Fedorenko <
> >> > > [hidden email]>
> >> > > > >> wrote:
> >> > > > >>
> >> > > > >> > TL;DR your test project exposed two existing bugs, one  
> >> change in
> >> > > > >> > behaviour and one quirk I can't explain
> >> > > > >> >
> >> > > > >> > * Build `<extensions>` are loaded by two classloaders, which  
> >> is
> >> a
> >> > > bug
> >> > > > >> in
> >> > > > >> > DefaultProjectBuildingHelper#createProjectRealm and explains
> >> why you
> >> > > > >> see
> >> > > > >> > extjar1/extjar2 in the output
> >> > > > >> > * ClassRealm does not allow same foreign-import from multiple
> >> > > > >> > classloaders, which is a bug and explains why it is not
> >> possible to
> >> > > > >> load
> >> > > > >> > same resource from multiple plugins/extensions
> >> > > > >> > * TCCL does not have access to private (i.e. not exported)
> >> resources
> >> > > > >> of
> >> > > > >> > this extensions plugin, which is a change of behaviour
> >> introduced by
> >> > > > >> > mng-6209 fix
> >> > > > >> > * Also, component injection order appears to be backwards,  
> >> but
> >> maybe
> >> > > > >> > Stuart can explain why.
> >> > > > >> >
> >> > > > >> >
> >> > > > >> > Below is more detailed explanation of expected and observed
> >> > > behaviour
> >> > > > >> >
> >> > > > >> >
> >> > > > >> > ## Component injection depends on the currently running  
> >> plugin
> >> and
> >> > > the
> >> > > > >> > injection site
> >> > > > >> >
> >> > > > >> > Currently running plugins have access to the following  
> >> component
> >> > > > >> > implementations:
> >> > > > >> >
> >> > > > >> > * Regular plugin has access to components implemented by the
> >> plugin,
> >> > > > >> > project build extensions, if any (via project class realm
> >> foreign
> >> > > > >> > import) and Maven Core.
> >> > > > >> > * Extension plugin has access to components implemented by  
> >> the
> >> > > project
> >> > > > >> > build extensions and Maven Core.
> >> > > > >> > * Without a running plugin (e.g., during project dependency
> >> > > > >> resolution),
> >> > > > >> > components implemented by the project build extensions and  
> >> Maven
> >> > > Core
> >> > > > >> > are accessible.
> >> > > > >> >
> >> > > > >> > Different injection sites have access to the following  
> >> component
> >> > > > >> > interfaces:
> >> > > > >> >
> >> > > > >> > * Maven Core has access to component interfaces defined by  
> >> the
> >> core
> >> > > > >> > itself (obviously)
> >> > > > >> > * Project build extensions have access to **public**  
> >> component
> >> > > > >> > interfaces defined by Maven Core and component interfaces
> >> defined by
> >> > > > >> the
> >> > > > >> > build extension itself (there is no way to access component
> >> > > interfaces
> >> > > > >> > defined in other extensions)
> >> > > > >> > * Regular plugins have access to **public** component  
> >> interfaces
> >> > > > >> defined
> >> > > > >> > by Maven Core, component interfaces **exported** by build
> >> extensions
> >> > > > >> and
> >> > > > >> > component interfaces defined in the plugin itself
> >> > > > >> >
> >> > > > >> > For injection to work, injection site has to have access to  
> >> the
> >> > > > >> > component interface and the component implementation must be
> >> > > > >> accessible
> >> > > > >> > through the current context.
> >> > > > >> >
> >> > > > >> > From what I can tell, in your example all plugins have access
> >> to the
> >> > > > >> > right components when using current 3.5.2-SNAPSHOT. The
> >> injection
> >> > > > >> order
> >> > > > >> > does appear to be backwards from what I expected, however.
> >> > > > >> >
> >> > > > >> >
> >> > > > >> > ## Resources lookup fully depends on classpath visibility,
> >> > > > >> specifically
> >> > > > >> >
> >> > > > >> > * Regular plugin class realm has access to resources from the
> >> plugin
> >> > > > >> > itself, from **exported** packages of the project build
> >> extensions
> >> > > and
> >> > > > >> > **public** Maven Core packages
> >> > > > >> > * Extensions plugin class realm has access to the resources
> >> from the
> >> > > > >> > extensions plugin itself and from **public** Maven Core  
> >> packages
> >> > > > >> > * Project class realm has access to classes and resources
> >> > > **exported**
> >> > > > >> > by project build extensions and **public** Maven Core  
> >> packages
> >> > > > >> >
> >> > > > >> > I see three problems here
> >> > > > >> >
> >> > > > >> > * Maven adds build single-jar `<extensions>` elements  
> >> directly
> >> to
> >> > > > >> > project class realm **and** creates separate extensions class
> >> realms
> >> > > > >> for
> >> > > > >> > them. Which results in duplicate classes/resources loaded by  
> >> two
> >> > > > >> > classloaders and explains why you see extjar1/extjar2 output
> >> (which
> >> > > > >> you
> >> > > > >> > shouldn't according to the explanation above)
> >> > > > >> > * ClassRealm does not allow foreign-import of the same  
> >> package
> >> from
> >> > > > >> > multiple classloaders. This makes it impossible to load the  
> >> same
> >> > > > >> > resource from multiple plugins/extensions.
> >> > > > >> > * Extensions plugins cannot access their own private (i.e.  
> >> not
> >> > > > >> exported)
> >> > > > >> > resources via TCCL, this is change in behaviour introduced by
> >> > > mng-6209
> >> > > > >> > fix
> >> > > > >> >
> >> > > > >> > Hope this helps
> >> > > > >>
> >> > > > >>
> >> ---------------------------------------------------------------------
> >> > > > >> To unsubscribe, e-mail: [hidden email]
> >> > > > >> For additional commands, e-mail: [hidden email]
> >> > > >
> >> > > >  
> >> ---------------------------------------------------------------------
> >> > > > To unsubscribe, e-mail: [hidden email]
> >> > > > For additional commands, e-mail: [hidden email]
> >> > > >
> >> > >
> >> > >  
> >> ---------------------------------------------------------------------
> >> > > To unsubscribe, e-mail: [hidden email]
> >> > > For additional commands, e-mail: [hidden email]
> >> > >
> >> > > --
> >> > Sent from my phone
> >>
> >> ---------------------------------------------------------------------
> >> To unsubscribe, e-mail: [hidden email]
> >> For additional commands, e-mail: [hidden email]
> >>
> >> --
> > Sent from my phone
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: [hidden email]
> For additional commands, e-mail: [hidden email]
>

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: Understanding MNG-6209 (was: [VOTE] Release Apache Maven 3.5.1)

stephenconnolly
https://maven.apache.org/guides/mini/guide-maven-classloading.html says:

> When a build plugin is executed, the thread's context classloader is set
to the plugin classloader.

So we'll need to fix something somewhere...

https://svn.apache.org/repos/infra/websites/production/maven/content/reference/maven-classloading.html
is unaccessible from the website due to a rewrite rule...

Things that seem to be missing:

* What is the desired classloading for a plugin that is marked as an
extension? Can a plugin have a META-INF/maven/extension.xml to allow
exporting classes and artifacts when used as an extension? How should the
classloading look for such a strange beast.

* How does one access the plugin classloader if we want TCCL to be other
than that, is it a Dependency Injection or something else?

* What differentiates a Core extension from a Build extension (is it that a
build extension lacks a META-INF/maven/extension.xml and was only declared
in the pom.xml, while a core extension either has a
META-INF/maven/extension.xml - if declared in the pom - or is an extension
declared in .mvn/extensions.xml)

At this point in time I think we are nearing the point where I may have to
declare 3.5.1 abandoned as I think the classloading in that is a symptom of
too many cooks all changing things in different directions. We need a
consistent vision of where we want things to go and - while we need not get
there in one go - the path presented for others to see.

Things I think we should consider:

1. Do we want to formally deprecate Build Extensions and the
/project/build/extensions element (start logging warnings, etc)?
2. Do we want to formally deprecate plugins as extensions and start logging
warnings for
/project/build/(pluginManagement|.)/plugins/plugin/extensions[text()==true]
3. What is the difference in classloading for a /project/build/extensions
which has a META-INF/maven/extension.xml and one that doesn't?

I'm keeping the 3.5.1 release in staging until we get a clear vision for
how we want to have classloading so that I can assess whether the 3.5.1
actuality is only moving nearer to the vision (ok to release) or has moved
nearer in some ways but further in others (not ok to release)

On 20 September 2017 at 12:44, Igor Fedorenko <[hidden email]> wrote:

> Real-world scm or wagon <extensions> won't trigger maven2-compat code
> path [1]. To avoid that obscure code path we can either make the test
> more elaborate (i.e. add dependencies to extjar1/extjar2) or we can use
> extensions <plugin>. Either way I don't think we should spend time on
> the code path unlikely to be used in real life.
>
> [1]
> https://github.com/apache/maven/blob/maven-3.5.1/maven-
> core/src/main/java/org/apache/maven/project/DefaultProjectBuildingHelper.
> java#L210-L219
>
> --
> Regards,
> Igor
>
> On Wed, Sep 20, 2017, at 03:29 AM, Robert Scholte wrote:
> > On Wed, 20 Sep 2017 09:12:47 +0200, Stephen Connolly
> > <[hidden email]> wrote:
> >
> > > On Wed 20 Sep 2017 at 01:29, Igor Fedorenko <[hidden email]>
> wrote:
> > >
> > >> In that case, can I suggest couple of changes to the test project
> > >>
> > >> * I thinks it makes more sense to configure extjar1 and extjar2 as
> > >> extensions <plugin> elements in probleN pom.xml files. First, there is
> > >> no meaningful order between <extensions> and <plugins> elements. More
> > >> importantly, though, simple <extensions> are treated in special
> > >> maven2-compat mode and are not representative of likely real-world
> > >> extensions.
> > >
> >
> > Not sure I agree with this. I think there are jars worth sharing across
> > multiple plugins, but where making the plugin an extension is a bit
> > weird.
> > I'm thinking of scm and wagon in this case.
> >
> > >
> > > That sounds like we need documentation updated then. None of that is
> > > obvious to me.
> > >
> > >
> > >>
> > >> * I think we should introduce META-INF/maven/extension.xml to the test
> > >> extensions. This metadata what introduced to configure classpath
> > >> visibility, so lets use it.
> > >
> > >
> > > Again, not obvious to me, if that file allows control of classpath
> > > visibility then it may be that the only issue *with* 3.5.1 is the lack
> of
> > > documentation... now previous versions would have been adding breaking
> > > changes from my PoV but that is the past and should not affect the
> 3.5.1
> > > release.
> > >
> > > PRs for the probe project welcome. I am happy to try and write docs
> once
> > > I
> > > have an understanding of what the expected behaviours are
> > >
> > >
> > >>
> > >> --
> > >> Regards,
> > >> Igor
> > >>
> > >> On Tue, Sep 19, 2017, at 05:12 PM, Stephen Connolly wrote:
> > >> > Yes, the expectations are key. Depending on what they are we may
> > >> either
> > >> > drop 3.5.1 or go ahead as it depends on whether this is more correct
> > >> than
> > >> > 3.5.0 or swapping one fix for a bug
> > >> >
> > >> > On Tue 19 Sep 2017 at 21:39, Igor Fedorenko <[hidden email]>
> > >> wrote:
> > >> >
> > >> > > Just to confirm I understand what we are trying to establish here.
> > >> We
> > >> > > want to decide the expected/desired component injection behaviour
> > >> and
> > >> > > classpath visibility in the absence of package and artifact export
> > >> > > configuration (i.e. META-INF/maven/extension.xml file). Did I get
> > >> this
> > >> > > right?
> > >> > >
> > >> > > --
> > >> > > Regards,
> > >> > > Igor
> > >> > >
> > >> > > On Tue, Sep 19, 2017, at 03:52 PM, Robert Scholte wrote:
> > >> > > > Let's do it like this:
> > >> > > >
> > >> > >
> > >> https://cwiki.apache.org/confluence/download/attachments/2329841/
> classrealms.pdf?api=v2
> > >> > > >
> > >> > > > Robert
> > >> > > >
> > >> > > > On Tue, 19 Sep 2017 21:08:39 +0200, Stephen Connolly
> > >> > > > <[hidden email]> wrote:
> > >> > > >
> > >> > > > > I think you will need a link to the PDF as attachments are
> > >> stripped
> > >> > > from
> > >> > > > > the ML
> > >> > > > >
> > >> > > > > On Tue 19 Sep 2017 at 19:57, Robert Scholte
> > >> <[hidden email]>
> > >> > > wrote:
> > >> > > > >
> > >> > > > >> Attached a single page overview.
> > >> > > > >>
> > >> > > > >> Per block you'll see in the upper left corner the executed
> > >> plugin
> > >> > > > >> The left column contains the extensions and plugin in orderas
> > >> > > specified
> > >> > > > >> in
> > >> > > > >> the pom.xml
> > >> > > > >> In every classloadercolumn you'll see numbers which represent
> > >> the
> > >> > > order.
> > >> > > > >>
> > >> > > > >> I hope I didn't make any mistakes.
> > >> > > > >> Tomorrow I have enough time to see if I understand what's
> > >> happening
> > >> > > > >> here.
> > >> > > > >>
> > >> > > > >> I will come back with my conclusions.
> > >> > > > >>
> > >> > > > >> Robert
> > >> > > > >>
> > >> > > > >> On Tue, 19 Sep 2017 06:55:08 +0200, Igor Fedorenko <
> > >> > > [hidden email]>
> > >> > > > >> wrote:
> > >> > > > >>
> > >> > > > >> > TL;DR your test project exposed two existing bugs, one
> > >> change in
> > >> > > > >> > behaviour and one quirk I can't explain
> > >> > > > >> >
> > >> > > > >> > * Build `<extensions>` are loaded by two classloaders,
> which
> > >> is
> > >> a
> > >> > > bug
> > >> > > > >> in
> > >> > > > >> > DefaultProjectBuildingHelper#createProjectRealm and
> explains
> > >> why you
> > >> > > > >> see
> > >> > > > >> > extjar1/extjar2 in the output
> > >> > > > >> > * ClassRealm does not allow same foreign-import from
> multiple
> > >> > > > >> > classloaders, which is a bug and explains why it is not
> > >> possible to
> > >> > > > >> load
> > >> > > > >> > same resource from multiple plugins/extensions
> > >> > > > >> > * TCCL does not have access to private (i.e. not exported)
> > >> resources
> > >> > > > >> of
> > >> > > > >> > this extensions plugin, which is a change of behaviour
> > >> introduced by
> > >> > > > >> > mng-6209 fix
> > >> > > > >> > * Also, component injection order appears to be backwards,
> > >> but
> > >> maybe
> > >> > > > >> > Stuart can explain why.
> > >> > > > >> >
> > >> > > > >> >
> > >> > > > >> > Below is more detailed explanation of expected and observed
> > >> > > behaviour
> > >> > > > >> >
> > >> > > > >> >
> > >> > > > >> > ## Component injection depends on the currently running
> > >> plugin
> > >> and
> > >> > > the
> > >> > > > >> > injection site
> > >> > > > >> >
> > >> > > > >> > Currently running plugins have access to the following
> > >> component
> > >> > > > >> > implementations:
> > >> > > > >> >
> > >> > > > >> > * Regular plugin has access to components implemented by
> the
> > >> plugin,
> > >> > > > >> > project build extensions, if any (via project class realm
> > >> foreign
> > >> > > > >> > import) and Maven Core.
> > >> > > > >> > * Extension plugin has access to components implemented by
> > >> the
> > >> > > project
> > >> > > > >> > build extensions and Maven Core.
> > >> > > > >> > * Without a running plugin (e.g., during project dependency
> > >> > > > >> resolution),
> > >> > > > >> > components implemented by the project build extensions and
> > >> Maven
> > >> > > Core
> > >> > > > >> > are accessible.
> > >> > > > >> >
> > >> > > > >> > Different injection sites have access to the following
> > >> component
> > >> > > > >> > interfaces:
> > >> > > > >> >
> > >> > > > >> > * Maven Core has access to component interfaces defined by
> > >> the
> > >> core
> > >> > > > >> > itself (obviously)
> > >> > > > >> > * Project build extensions have access to **public**
> > >> component
> > >> > > > >> > interfaces defined by Maven Core and component interfaces
> > >> defined by
> > >> > > > >> the
> > >> > > > >> > build extension itself (there is no way to access component
> > >> > > interfaces
> > >> > > > >> > defined in other extensions)
> > >> > > > >> > * Regular plugins have access to **public** component
> > >> interfaces
> > >> > > > >> defined
> > >> > > > >> > by Maven Core, component interfaces **exported** by build
> > >> extensions
> > >> > > > >> and
> > >> > > > >> > component interfaces defined in the plugin itself
> > >> > > > >> >
> > >> > > > >> > For injection to work, injection site has to have access to
> > >> the
> > >> > > > >> > component interface and the component implementation must
> be
> > >> > > > >> accessible
> > >> > > > >> > through the current context.
> > >> > > > >> >
> > >> > > > >> > From what I can tell, in your example all plugins have
> access
> > >> to the
> > >> > > > >> > right components when using current 3.5.2-SNAPSHOT. The
> > >> injection
> > >> > > > >> order
> > >> > > > >> > does appear to be backwards from what I expected, however.
> > >> > > > >> >
> > >> > > > >> >
> > >> > > > >> > ## Resources lookup fully depends on classpath visibility,
> > >> > > > >> specifically
> > >> > > > >> >
> > >> > > > >> > * Regular plugin class realm has access to resources from
> the
> > >> plugin
> > >> > > > >> > itself, from **exported** packages of the project build
> > >> extensions
> > >> > > and
> > >> > > > >> > **public** Maven Core packages
> > >> > > > >> > * Extensions plugin class realm has access to the resources
> > >> from the
> > >> > > > >> > extensions plugin itself and from **public** Maven Core
> > >> packages
> > >> > > > >> > * Project class realm has access to classes and resources
> > >> > > **exported**
> > >> > > > >> > by project build extensions and **public** Maven Core
> > >> packages
> > >> > > > >> >
> > >> > > > >> > I see three problems here
> > >> > > > >> >
> > >> > > > >> > * Maven adds build single-jar `<extensions>` elements
> > >> directly
> > >> to
> > >> > > > >> > project class realm **and** creates separate extensions
> class
> > >> realms
> > >> > > > >> for
> > >> > > > >> > them. Which results in duplicate classes/resources loaded
> by
> > >> two
> > >> > > > >> > classloaders and explains why you see extjar1/extjar2
> output
> > >> (which
> > >> > > > >> you
> > >> > > > >> > shouldn't according to the explanation above)
> > >> > > > >> > * ClassRealm does not allow foreign-import of the same
> > >> package
> > >> from
> > >> > > > >> > multiple classloaders. This makes it impossible to load the
> > >> same
> > >> > > > >> > resource from multiple plugins/extensions.
> > >> > > > >> > * Extensions plugins cannot access their own private (i.e.
> > >> not
> > >> > > > >> exported)
> > >> > > > >> > resources via TCCL, this is change in behaviour introduced
> by
> > >> > > mng-6209
> > >> > > > >> > fix
> > >> > > > >> >
> > >> > > > >> > Hope this helps
> > >> > > > >>
> > >> > > > >>
> > >> ---------------------------------------------------------------------
> > >> > > > >> To unsubscribe, e-mail: [hidden email]
> > >> > > > >> For additional commands, e-mail: [hidden email]
> > >> > > >
> > >> > > >
> > >> ---------------------------------------------------------------------
> > >> > > > To unsubscribe, e-mail: [hidden email]
> > >> > > > For additional commands, e-mail: [hidden email]
> > >> > > >
> > >> > >
> > >> > >
> > >> ---------------------------------------------------------------------
> > >> > > To unsubscribe, e-mail: [hidden email]
> > >> > > For additional commands, e-mail: [hidden email]
> > >> > >
> > >> > > --
> > >> > Sent from my phone
> > >>
> > >> ---------------------------------------------------------------------
> > >> To unsubscribe, e-mail: [hidden email]
> > >> For additional commands, e-mail: [hidden email]
> > >>
> > >> --
> > > Sent from my phone
> >
> > ---------------------------------------------------------------------
> > To unsubscribe, e-mail: [hidden email]
> > For additional commands, e-mail: [hidden email]
> >
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: [hidden email]
> For additional commands, e-mail: [hidden email]
>
>
Reply | Threaded
Open this post in threaded view
|

Re: Understanding MNG-6209 (was: [VOTE] Release Apache Maven 3.5.1)

rfscholte
On Sun, 24 Sep 2017 19:36:15 +0200, Stephen Connolly  
<[hidden email]> wrote:

> On Sun 24 Sep 2017 at 17:44, Robert Scholte <[hidden email]> wrote:
>
>> Are these questions we can/should answer within a couple of days?
>> I'm not aware of somebody hitting the regression as caused by MNG-5742
>> other than Igor.
>> However, we've seen a couple of changed behavior clearly caused by
>> MNG-6209 (not answering if this intended or not)
>>
>> We could also make the decision to leave MNG-6209 out, do a new release
>> and complete our investigation directly afterwards.
>> This change is way too tricky to do additional changes under pressure.
>
>
> If we drop 3.5.1 i’d be in favour of reverting both MNG-6209 and MNG-6275
> (the TCCL one) as I have found documentation stating that TCCL is the
> plugin classloader, which makes me wary of changing that in a patch  
> release
> (but I can be convinced otherwise)
>

Sure, I can live with that.

>
>>
>> thanks,
>> Robert
>>
>>
>> On Sun, 24 Sep 2017 18:07:39 +0200, Stephen Connolly
>> <[hidden email]> wrote:
>>
>> > https://maven.apache.org/guides/mini/guide-maven-classloading.html 
>> says:
>> >
>> >> When a build plugin is executed, the thread's context classloader is  
>> set
>> > to the plugin classloader.
>> >
>> > So we'll need to fix something somewhere...
>> >
>> >
>> https://svn.apache.org/repos/infra/websites/production/maven/content/reference/maven-classloading.html
>> > is unaccessible from the website due to a rewrite rule...
>> >
>> > Things that seem to be missing:
>> >
>> > * What is the desired classloading for a plugin that is marked as an
>> > extension? Can a plugin have a META-INF/maven/extension.xml to allow
>> > exporting classes and artifacts when used as an extension? How should  
>> the
>> > classloading look for such a strange beast.
>> >
>> > * How does one access the plugin classloader if we want TCCL to be  
>> other
>> > than that, is it a Dependency Injection or something else?
>> >
>> > * What differentiates a Core extension from a Build extension (is it
>> > that a
>> > build extension lacks a META-INF/maven/extension.xml and was only
>> > declared
>> > in the pom.xml, while a core extension either has a
>> > META-INF/maven/extension.xml - if declared in the pom - or is an
>> > extension
>> > declared in .mvn/extensions.xml)
>> >
>> > At this point in time I think we are nearing the point where I may  
>> have
>> > to
>> > declare 3.5.1 abandoned as I think the classloading in that is a  
>> symptom
>> > of
>> > too many cooks all changing things in different directions. We need a
>> > consistent vision of where we want things to go and - while we need  
>> not
>> > get
>> > there in one go - the path presented for others to see.
>> >
>> > Things I think we should consider:
>> >
>> > 1. Do we want to formally deprecate Build Extensions and the
>> > /project/build/extensions element (start logging warnings, etc)?
>> > 2. Do we want to formally deprecate plugins as extensions and start
>> > logging
>> > warnings for
>> >
>> /project/build/(pluginManagement|.)/plugins/plugin/extensions[text()==true]
>> > 3. What is the difference in classloading for a  
>> /project/build/extensions
>> > which has a META-INF/maven/extension.xml and one that doesn't?
>> >
>> > I'm keeping the 3.5.1 release in staging until we get a clear vision  
>> for
>> > how we want to have classloading so that I can assess whether the  
>> 3.5.1
>> > actuality is only moving nearer to the vision (ok to release) or has
>> > moved
>> > nearer in some ways but further in others (not ok to release)
>> >
>> > On 20 September 2017 at 12:44, Igor Fedorenko <[hidden email]>
>> > wrote:
>> >
>> >> Real-world scm or wagon <extensions> won't trigger maven2-compat code
>> >> path [1]. To avoid that obscure code path we can either make the test
>> >> more elaborate (i.e. add dependencies to extjar1/extjar2) or we can  
>> use
>> >> extensions <plugin>. Either way I don't think we should spend time on
>> >> the code path unlikely to be used in real life.
>> >>
>> >> [1]
>> >> https://github.com/apache/maven/blob/maven-3.5.1/maven-
>> >>
>> core/src/main/java/org/apache/maven/project/DefaultProjectBuildingHelper.
>> >> java#L210-L219
>> >>
>> >> --
>> >> Regards,
>> >> Igor
>> >>
>> >> On Wed, Sep 20, 2017, at 03:29 AM, Robert Scholte wrote:
>> >> > On Wed, 20 Sep 2017 09:12:47 +0200, Stephen Connolly
>> >> > <[hidden email]> wrote:
>> >> >
>> >> > > On Wed 20 Sep 2017 at 01:29, Igor Fedorenko <[hidden email]>
>> >> wrote:
>> >> > >
>> >> > >> In that case, can I suggest couple of changes to the test  
>> project
>> >> > >>
>> >> > >> * I thinks it makes more sense to configure extjar1 and extjar2  
>> as
>> >> > >> extensions <plugin> elements in probleN pom.xml files. First,
>> >> there is
>> >> > >> no meaningful order between <extensions> and <plugins> elements.
>> >> More
>> >> > >> importantly, though, simple <extensions> are treated in special
>> >> > >> maven2-compat mode and are not representative of likely  
>> real-world
>> >> > >> extensions.
>> >> > >
>> >> >
>> >> > Not sure I agree with this. I think there are jars worth sharing
>> >> across
>> >> > multiple plugins, but where making the plugin an extension is a bit
>> >> > weird.
>> >> > I'm thinking of scm and wagon in this case.
>> >> >
>> >> > >
>> >> > > That sounds like we need documentation updated then. None of  
>> that is
>> >> > > obvious to me.
>> >> > >
>> >> > >
>> >> > >>
>> >> > >> * I think we should introduce META-INF/maven/extension.xml to  
>> the
>> >> test
>> >> > >> extensions. This metadata what introduced to configure classpath
>> >> > >> visibility, so lets use it.
>> >> > >
>> >> > >
>> >> > > Again, not obvious to me, if that file allows control of  
>> classpath
>> >> > > visibility then it may be that the only issue *with* 3.5.1 is the
>> >> lack
>> >> of
>> >> > > documentation... now previous versions would have been adding
>> >> breaking
>> >> > > changes from my PoV but that is the past and should not affect  
>> the
>> >> 3.5.1
>> >> > > release.
>> >> > >
>> >> > > PRs for the probe project welcome. I am happy to try and write  
>> docs
>> >> once
>> >> > > I
>> >> > > have an understanding of what the expected behaviours are
>> >> > >
>> >> > >
>> >> > >>
>> >> > >> --
>> >> > >> Regards,
>> >> > >> Igor
>> >> > >>
>> >> > >> On Tue, Sep 19, 2017, at 05:12 PM, Stephen Connolly wrote:
>> >> > >> > Yes, the expectations are key. Depending on what they are we  
>> may
>> >> > >> either
>> >> > >> > drop 3.5.1 or go ahead as it depends on whether this is more
>> >> correct
>> >> > >> than
>> >> > >> > 3.5.0 or swapping one fix for a bug
>> >> > >> >
>> >> > >> > On Tue 19 Sep 2017 at 21:39, Igor Fedorenko  
>> <[hidden email]
>> >
>> >> > >> wrote:
>> >> > >> >
>> >> > >> > > Just to confirm I understand what we are trying to establish
>> >> here.
>> >> > >> We
>> >> > >> > > want to decide the expected/desired component injection
>> >> behaviour
>> >> > >> and
>> >> > >> > > classpath visibility in the absence of package and artifact
>> >> export
>> >> > >> > > configuration (i.e. META-INF/maven/extension.xml file). Did  
>> I
>> >> get
>> >> > >> this
>> >> > >> > > right?
>> >> > >> > >
>> >> > >> > > --
>> >> > >> > > Regards,
>> >> > >> > > Igor
>> >> > >> > >
>> >> > >> > > On Tue, Sep 19, 2017, at 03:52 PM, Robert Scholte wrote:
>> >> > >> > > > Let's do it like this:
>> >> > >> > > >
>> >> > >> > >
>> >> > >>  
>> https://cwiki.apache.org/confluence/download/attachments/2329841/
>> >> classrealms.pdf?api=v2
>> >> > >> > > >
>> >> > >> > > > Robert
>> >> > >> > > >
>> >> > >> > > > On Tue, 19 Sep 2017 21:08:39 +0200, Stephen Connolly
>> >> > >> > > > <[hidden email]> wrote:
>> >> > >> > > >
>> >> > >> > > > > I think you will need a link to the PDF as attachments  
>> are
>> >> > >> stripped
>> >> > >> > > from
>> >> > >> > > > > the ML
>> >> > >> > > > >
>> >> > >> > > > > On Tue 19 Sep 2017 at 19:57, Robert Scholte
>> >> > >> <[hidden email]>
>> >> > >> > > wrote:
>> >> > >> > > > >
>> >> > >> > > > >> Attached a single page overview.
>> >> > >> > > > >>
>> >> > >> > > > >> Per block you'll see in the upper left corner the  
>> executed
>> >> > >> plugin
>> >> > >> > > > >> The left column contains the extensions and plugin in
>> >> orderas
>> >> > >> > > specified
>> >> > >> > > > >> in
>> >> > >> > > > >> the pom.xml
>> >> > >> > > > >> In every classloadercolumn you'll see numbers which
>> >> represent
>> >> > >> the
>> >> > >> > > order.
>> >> > >> > > > >>
>> >> > >> > > > >> I hope I didn't make any mistakes.
>> >> > >> > > > >> Tomorrow I have enough time to see if I understand  
>> what's
>> >> > >> happening
>> >> > >> > > > >> here.
>> >> > >> > > > >>
>> >> > >> > > > >> I will come back with my conclusions.
>> >> > >> > > > >>
>> >> > >> > > > >> Robert
>> >> > >> > > > >>
>> >> > >> > > > >> On Tue, 19 Sep 2017 06:55:08 +0200, Igor Fedorenko <
>> >> > >> > > [hidden email]>
>> >> > >> > > > >> wrote:
>> >> > >> > > > >>
>> >> > >> > > > >> > TL;DR your test project exposed two existing bugs,  
>> one
>> >> > >> change in
>> >> > >> > > > >> > behaviour and one quirk I can't explain
>> >> > >> > > > >> >
>> >> > >> > > > >> > * Build `<extensions>` are loaded by two  
>> classloaders,
>> >> which
>> >> > >> is
>> >> > >> a
>> >> > >> > > bug
>> >> > >> > > > >> in
>> >> > >> > > > >> > DefaultProjectBuildingHelper#createProjectRealm and
>> >> explains
>> >> > >> why you
>> >> > >> > > > >> see
>> >> > >> > > > >> > extjar1/extjar2 in the output
>> >> > >> > > > >> > * ClassRealm does not allow same foreign-import from
>> >> multiple
>> >> > >> > > > >> > classloaders, which is a bug and explains why it is  
>> not
>> >> > >> possible to
>> >> > >> > > > >> load
>> >> > >> > > > >> > same resource from multiple plugins/extensions
>> >> > >> > > > >> > * TCCL does not have access to private (i.e. not
>> >> exported)
>> >> > >> resources
>> >> > >> > > > >> of
>> >> > >> > > > >> > this extensions plugin, which is a change of  
>> behaviour
>> >> > >> introduced by
>> >> > >> > > > >> > mng-6209 fix
>> >> > >> > > > >> > * Also, component injection order appears to be
>> >> backwards,
>> >> > >> but
>> >> > >> maybe
>> >> > >> > > > >> > Stuart can explain why.
>> >> > >> > > > >> >
>> >> > >> > > > >> >
>> >> > >> > > > >> > Below is more detailed explanation of expected and
>> >> observed
>> >> > >> > > behaviour
>> >> > >> > > > >> >
>> >> > >> > > > >> >
>> >> > >> > > > >> > ## Component injection depends on the currently  
>> running
>> >> > >> plugin
>> >> > >> and
>> >> > >> > > the
>> >> > >> > > > >> > injection site
>> >> > >> > > > >> >
>> >> > >> > > > >> > Currently running plugins have access to the  
>> following
>> >> > >> component
>> >> > >> > > > >> > implementations:
>> >> > >> > > > >> >
>> >> > >> > > > >> > * Regular plugin has access to components  
>> implemented by
>> >> the
>> >> > >> plugin,
>> >> > >> > > > >> > project build extensions, if any (via project class
>> >> realm
>> >> > >> foreign
>> >> > >> > > > >> > import) and Maven Core.
>> >> > >> > > > >> > * Extension plugin has access to components  
>> implemented
>> >> by
>> >> > >> the
>> >> > >> > > project
>> >> > >> > > > >> > build extensions and Maven Core.
>> >> > >> > > > >> > * Without a running plugin (e.g., during project
>> >> dependency
>> >> > >> > > > >> resolution),
>> >> > >> > > > >> > components implemented by the project build  
>> extensions
>> >> and
>> >> > >> Maven
>> >> > >> > > Core
>> >> > >> > > > >> > are accessible.
>> >> > >> > > > >> >
>> >> > >> > > > >> > Different injection sites have access to the  
>> following
>> >> > >> component
>> >> > >> > > > >> > interfaces:
>> >> > >> > > > >> >
>> >> > >> > > > >> > * Maven Core has access to component interfaces  
>> defined
>> >> by
>> >> > >> the
>> >> > >> core
>> >> > >> > > > >> > itself (obviously)
>> >> > >> > > > >> > * Project build extensions have access to **public**
>> >> > >> component
>> >> > >> > > > >> > interfaces defined by Maven Core and component
>> >> interfaces
>> >> > >> defined by
>> >> > >> > > > >> the
>> >> > >> > > > >> > build extension itself (there is no way to access
>> >> component
>> >> > >> > > interfaces
>> >> > >> > > > >> > defined in other extensions)
>> >> > >> > > > >> > * Regular plugins have access to **public** component
>> >> > >> interfaces
>> >> > >> > > > >> defined
>> >> > >> > > > >> > by Maven Core, component interfaces **exported** by
>> >> build
>> >> > >> extensions
>> >> > >> > > > >> and
>> >> > >> > > > >> > component interfaces defined in the plugin itself
>> >> > >> > > > >> >
>> >> > >> > > > >> > For injection to work, injection site has to have
>> >> access to
>> >> > >> the
>> >> > >> > > > >> > component interface and the component implementation
>> >> must
>> >> be
>> >> > >> > > > >> accessible
>> >> > >> > > > >> > through the current context.
>> >> > >> > > > >> >
>> >> > >> > > > >> > From what I can tell, in your example all plugins  
>> have
>> >> access
>> >> > >> to the
>> >> > >> > > > >> > right components when using current 3.5.2-SNAPSHOT.  
>> The
>> >> > >> injection
>> >> > >> > > > >> order
>> >> > >> > > > >> > does appear to be backwards from what I expected,
>> >> however.
>> >> > >> > > > >> >
>> >> > >> > > > >> >
>> >> > >> > > > >> > ## Resources lookup fully depends on classpath
>> >> visibility,
>> >> > >> > > > >> specifically
>> >> > >> > > > >> >
>> >> > >> > > > >> > * Regular plugin class realm has access to resources
>> >> from
>> >> the
>> >> > >> plugin
>> >> > >> > > > >> > itself, from **exported** packages of the project  
>> build
>> >> > >> extensions
>> >> > >> > > and
>> >> > >> > > > >> > **public** Maven Core packages
>> >> > >> > > > >> > * Extensions plugin class realm has access to the
>> >> resources
>> >> > >> from the
>> >> > >> > > > >> > extensions plugin itself and from **public** Maven  
>> Core
>> >> > >> packages
>> >> > >> > > > >> > * Project class realm has access to classes and
>> >> resources
>> >> > >> > > **exported**
>> >> > >> > > > >> > by project build extensions and **public** Maven Core
>> >> > >> packages
>> >> > >> > > > >> >
>> >> > >> > > > >> > I see three problems here
>> >> > >> > > > >> >
>> >> > >> > > > >> > * Maven adds build single-jar `<extensions>` elements
>> >> > >> directly
>> >> > >> to
>> >> > >> > > > >> > project class realm **and** creates separate  
>> extensions
>> >> class
>> >> > >> realms
>> >> > >> > > > >> for
>> >> > >> > > > >> > them. Which results in duplicate classes/resources
>> >> loaded
>> >> by
>> >> > >> two
>> >> > >> > > > >> > classloaders and explains why you see extjar1/extjar2
>> >> output
>> >> > >> (which
>> >> > >> > > > >> you
>> >> > >> > > > >> > shouldn't according to the explanation above)
>> >> > >> > > > >> > * ClassRealm does not allow foreign-import of the  
>> same
>> >> > >> package
>> >> > >> from
>> >> > >> > > > >> > multiple classloaders. This makes it impossible to  
>> load
>> >> the
>> >> > >> same
>> >> > >> > > > >> > resource from multiple plugins/extensions.
>> >> > >> > > > >> > * Extensions plugins cannot access their own private
>> >> (i.e.
>> >> > >> not
>> >> > >> > > > >> exported)
>> >> > >> > > > >> > resources via TCCL, this is change in behaviour
>> >> introduced
>> >> by
>> >> > >> > > mng-6209
>> >> > >> > > > >> > fix
>> >> > >> > > > >> >
>> >> > >> > > > >> > Hope this helps
>> >> > >> > > > >>
>> >> > >> > > > >>
>> >> > >>
>> >> ---------------------------------------------------------------------
>> >> > >> > > > >> To unsubscribe, e-mail:  
>> [hidden email]
>> >> > >> > > > >> For additional commands, e-mail:
>> [hidden email]
>> >> > >> > > >
>> >> > >> > > >
>> >> > >>
>> >> ---------------------------------------------------------------------
>> >> > >> > > > To unsubscribe, e-mail: [hidden email]
>> >> > >> > > > For additional commands, e-mail: [hidden email]
>> >> > >> > > >
>> >> > >> > >
>> >> > >> > >
>> >> > >>
>> >> ---------------------------------------------------------------------
>> >> > >> > > To unsubscribe, e-mail: [hidden email]
>> >> > >> > > For additional commands, e-mail: [hidden email]
>> >> > >> > >
>> >> > >> > > --
>> >> > >> > Sent from my phone
>> >> > >>
>> >> > >>
>> >> ---------------------------------------------------------------------
>> >> > >> To unsubscribe, e-mail: [hidden email]
>> >> > >> For additional commands, e-mail: [hidden email]
>> >> > >>
>> >> > >> --
>> >> > > Sent from my phone
>> >> >
>> >> >  
>> ---------------------------------------------------------------------
>> >> > To unsubscribe, e-mail: [hidden email]
>> >> > For additional commands, e-mail: [hidden email]
>> >> >
>> >>
>> >> ---------------------------------------------------------------------
>> >> To unsubscribe, e-mail: [hidden email]
>> >> For additional commands, e-mail: [hidden email]
>> >>
>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: [hidden email]
>> For additional commands, e-mail: [hidden email]
>>
>> --
> Sent from my phone

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: Understanding MNG-6209 (was: [VOTE] Release Apache Maven 3.5.1)

stephenconnolly
In reply to this post by stephenconnolly
I wonder should we do a hangout to decide what you do?

What times on Monday work best?

I can maybe do 8:30-9:30pm Irish time

https://www.timeanddate.com/worldclock/meetingdetails.html?year=2017&month=9&day=25&hour=19&min=30&sec=0&p1=78&p2=37&p3=179

But we’d need to decide who we need and an actual agenda.

If Monday is too soon I can see if I have a window later this week

On Sun 24 Sep 2017 at 18:58, Igor Fedorenko <[hidden email]> wrote:

> See my answers/comments inline
>
>
> On Sun, Sep 24, 2017, at 12:07 PM, Stephen Connolly wrote:
> > https://maven.apache.org/guides/mini/guide-maven-classloading.html says:
> >
> > > When a build plugin is executed, the thread's context classloader is
> set
> > to the plugin classloader.
> >
> > So we'll need to fix something somewhere...
> >
> >
> https://svn.apache.org/repos/infra/websites/production/maven/content/reference/maven-classloading.html
> > is unaccessible from the website due to a rewrite rule...
> >
> > Things that seem to be missing:
> >
> > * What is the desired classloading for a plugin that is marked as an
> > extension? Can a plugin have a META-INF/maven/extension.xml to allow
> > exporting classes and artifacts when used as an extension? How should the
> > classloading look for such a strange beast.
>
> To me, the key requirement is that @Singleton components and class
> static members are singletons when injected in Maven core or in @Mojos.
> This implies that there should be single classloader representing an
> extensions plugins (MNG-5742).
>
> META-INF/maven/extension.xml declares what packages of the extension
> plugin are visible to other (non extension) plugins.
> META-INF/maven/extension.xml does not affect classloading of the
> extension plugin nor it affects the "shape" of other classloaders.
>
> > * How does one access the plugin classloader if we want TCCL to be other
> > than that, is it a Dependency Injection or something else?
>
> this.getClass().getClassLoader() is the most direct way to access plugin
> classloader. Why do you think we need anything more elaborate?
>
>
> > * What differentiates a Core extension from a Build extension (is it that
> > a
> > build extension lacks a META-INF/maven/extension.xml and was only
> > declared
> > in the pom.xml, while a core extension either has a
> > META-INF/maven/extension.xml - if declared in the pom - or is an
> > extension
> > declared in .mvn/extensions.xml)
>
> Core extensions are loaded *before* build starts, so they can contribute
> AbstractMavenLifecycleParticipant#afterSessionStart, for example. They
> can also export packages visible to all build plugins, including
> extensions=true. On the flip side, each core extension is effectively
> singleton, you can't have two different versions of the same Core
> extension. Core extensions also have direct access to Maven core classes
> and can do more interesting things there (for better or worse).
>
> Build extensions are part of the project build and as such are limited
> what components they can contribute to the Core and what core classes
> they have access to.
>
> I tried to capture this in the diagram I drew for
> http://takari.io/book/91-maven-classloading.html.
>
> > At this point in time I think we are nearing the point where I may have
> > to
> > declare 3.5.1 abandoned as I think the classloading in that is a symptom
> > of
> > too many cooks all changing things in different directions. We need a
> > consistent vision of where we want things to go and - while we need not
> > get
> > there in one go - the path presented for others to see.
>
> There were two classloading changes in 3.5.1, namely extensions=true
> plugins now have project realm as TCCL and all realms now use
> application classloader as the parent. Apart from lacking documentation,
> what practical problems have been caused by these two changes?
>
> >
> > Things I think we should consider:
> >
> > 1. Do we want to formally deprecate Build Extensions and the
> > /project/build/extensions element (start logging warnings, etc)?
> > 2. Do we want to formally deprecate plugins as extensions and start
> > logging
> > warnings for
> >
> /project/build/(pluginManagement|.)/plugins/plugin/extensions[text()==true]
>
> I'd keep them both, and maybe fix/remove maven2-compat codepath. If I
> had to choose between the two, however, I'd choose <plugin> with
> extensions=true. Think of a custom packaging type with mojos the user
> wants to configure in pom.xml, it'd be more tedious to configure if I
> had to add build/extension and build/plugin.
>
> > 3. What is the difference in classloading for a /project/build/extensions
> > which has a META-INF/maven/extension.xml and one that doesn't?
>
> I think extensions with META-INF/maven/extension.xml should not go
> through maven2-compat codepath. In other words, we need to change the
> current behaviour.
>
> Extensions without META-INF/maven/extension.xml... I am not sure.
> Probably safer to keep the current maven2-compat behaviour.
>
> > I'm keeping the 3.5.1 release in staging until we get a clear vision for
> > how we want to have classloading so that I can assess whether the 3.5.1
> > actuality is only moving nearer to the vision (ok to release) or has
> > moved
> > nearer in some ways but further in others (not ok to release)
> >
>
>
> --
> Regards,
> Igor
>
>
>
> > On 20 September 2017 at 12:44, Igor Fedorenko <[hidden email]>
> > wrote:
> >
> > > Real-world scm or wagon <extensions> won't trigger maven2-compat code
> > > path [1]. To avoid that obscure code path we can either make the test
> > > more elaborate (i.e. add dependencies to extjar1/extjar2) or we can use
> > > extensions <plugin>. Either way I don't think we should spend time on
> > > the code path unlikely to be used in real life.
> > >
> > > [1]
> > > https://github.com/apache/maven/blob/maven-3.5.1/maven-
> > >
> core/src/main/java/org/apache/maven/project/DefaultProjectBuildingHelper.
> > > java#L210-L219
> > >
> > > --
> > > Regards,
> > > Igor
> > >
> > > On Wed, Sep 20, 2017, at 03:29 AM, Robert Scholte wrote:
> > > > On Wed, 20 Sep 2017 09:12:47 +0200, Stephen Connolly
> > > > <[hidden email]> wrote:
> > > >
> > > > > On Wed 20 Sep 2017 at 01:29, Igor Fedorenko <[hidden email]>
> > > wrote:
> > > > >
> > > > >> In that case, can I suggest couple of changes to the test project
> > > > >>
> > > > >> * I thinks it makes more sense to configure extjar1 and extjar2 as
> > > > >> extensions <plugin> elements in probleN pom.xml files. First,
> there is
> > > > >> no meaningful order between <extensions> and <plugins> elements.
> More
> > > > >> importantly, though, simple <extensions> are treated in special
> > > > >> maven2-compat mode and are not representative of likely real-world
> > > > >> extensions.
> > > > >
> > > >
> > > > Not sure I agree with this. I think there are jars worth sharing
> across
> > > > multiple plugins, but where making the plugin an extension is a bit
> > > > weird.
> > > > I'm thinking of scm and wagon in this case.
> > > >
> > > > >
> > > > > That sounds like we need documentation updated then. None of that
> is
> > > > > obvious to me.
> > > > >
> > > > >
> > > > >>
> > > > >> * I think we should introduce META-INF/maven/extension.xml to the
> test
> > > > >> extensions. This metadata what introduced to configure classpath
> > > > >> visibility, so lets use it.
> > > > >
> > > > >
> > > > > Again, not obvious to me, if that file allows control of classpath
> > > > > visibility then it may be that the only issue *with* 3.5.1 is the
> lack
> > > of
> > > > > documentation... now previous versions would have been adding
> breaking
> > > > > changes from my PoV but that is the past and should not affect the
> > > 3.5.1
> > > > > release.
> > > > >
> > > > > PRs for the probe project welcome. I am happy to try and write docs
> > > once
> > > > > I
> > > > > have an understanding of what the expected behaviours are
> > > > >
> > > > >
> > > > >>
> > > > >> --
> > > > >> Regards,
> > > > >> Igor
> > > > >>
> > > > >> On Tue, Sep 19, 2017, at 05:12 PM, Stephen Connolly wrote:
> > > > >> > Yes, the expectations are key. Depending on what they are we may
> > > > >> either
> > > > >> > drop 3.5.1 or go ahead as it depends on whether this is more
> correct
> > > > >> than
> > > > >> > 3.5.0 or swapping one fix for a bug
> > > > >> >
> > > > >> > On Tue 19 Sep 2017 at 21:39, Igor Fedorenko <
> [hidden email]>
> > > > >> wrote:
> > > > >> >
> > > > >> > > Just to confirm I understand what we are trying to establish
> here.
> > > > >> We
> > > > >> > > want to decide the expected/desired component injection
> behaviour
> > > > >> and
> > > > >> > > classpath visibility in the absence of package and artifact
> export
> > > > >> > > configuration (i.e. META-INF/maven/extension.xml file). Did I
> get
> > > > >> this
> > > > >> > > right?
> > > > >> > >
> > > > >> > > --
> > > > >> > > Regards,
> > > > >> > > Igor
> > > > >> > >
> > > > >> > > On Tue, Sep 19, 2017, at 03:52 PM, Robert Scholte wrote:
> > > > >> > > > Let's do it like this:
> > > > >> > > >
> > > > >> > >
> > > > >> https://cwiki.apache.org/confluence/download/attachments/2329841/
> > > classrealms.pdf?api=v2
> > > > >> > > >
> > > > >> > > > Robert
> > > > >> > > >
> > > > >> > > > On Tue, 19 Sep 2017 21:08:39 +0200, Stephen Connolly
> > > > >> > > > <[hidden email]> wrote:
> > > > >> > > >
> > > > >> > > > > I think you will need a link to the PDF as attachments are
> > > > >> stripped
> > > > >> > > from
> > > > >> > > > > the ML
> > > > >> > > > >
> > > > >> > > > > On Tue 19 Sep 2017 at 19:57, Robert Scholte
> > > > >> <[hidden email]>
> > > > >> > > wrote:
> > > > >> > > > >
> > > > >> > > > >> Attached a single page overview.
> > > > >> > > > >>
> > > > >> > > > >> Per block you'll see in the upper left corner the
> executed
> > > > >> plugin
> > > > >> > > > >> The left column contains the extensions and plugin in
> orderas
> > > > >> > > specified
> > > > >> > > > >> in
> > > > >> > > > >> the pom.xml
> > > > >> > > > >> In every classloadercolumn you'll see numbers which
> represent
> > > > >> the
> > > > >> > > order.
> > > > >> > > > >>
> > > > >> > > > >> I hope I didn't make any mistakes.
> > > > >> > > > >> Tomorrow I have enough time to see if I understand what's
> > > > >> happening
> > > > >> > > > >> here.
> > > > >> > > > >>
> > > > >> > > > >> I will come back with my conclusions.
> > > > >> > > > >>
> > > > >> > > > >> Robert
> > > > >> > > > >>
> > > > >> > > > >> On Tue, 19 Sep 2017 06:55:08 +0200, Igor Fedorenko <
> > > > >> > > [hidden email]>
> > > > >> > > > >> wrote:
> > > > >> > > > >>
> > > > >> > > > >> > TL;DR your test project exposed two existing bugs, one
> > > > >> change in
> > > > >> > > > >> > behaviour and one quirk I can't explain
> > > > >> > > > >> >
> > > > >> > > > >> > * Build `<extensions>` are loaded by two classloaders,
> > > which
> > > > >> is
> > > > >> a
> > > > >> > > bug
> > > > >> > > > >> in
> > > > >> > > > >> > DefaultProjectBuildingHelper#createProjectRealm and
> > > explains
> > > > >> why you
> > > > >> > > > >> see
> > > > >> > > > >> > extjar1/extjar2 in the output
> > > > >> > > > >> > * ClassRealm does not allow same foreign-import from
> > > multiple
> > > > >> > > > >> > classloaders, which is a bug and explains why it is not
> > > > >> possible to
> > > > >> > > > >> load
> > > > >> > > > >> > same resource from multiple plugins/extensions
> > > > >> > > > >> > * TCCL does not have access to private (i.e. not
> exported)
> > > > >> resources
> > > > >> > > > >> of
> > > > >> > > > >> > this extensions plugin, which is a change of behaviour
> > > > >> introduced by
> > > > >> > > > >> > mng-6209 fix
> > > > >> > > > >> > * Also, component injection order appears to be
> backwards,
> > > > >> but
> > > > >> maybe
> > > > >> > > > >> > Stuart can explain why.
> > > > >> > > > >> >
> > > > >> > > > >> >
> > > > >> > > > >> > Below is more detailed explanation of expected and
> observed
> > > > >> > > behaviour
> > > > >> > > > >> >
> > > > >> > > > >> >
> > > > >> > > > >> > ## Component injection depends on the currently running
> > > > >> plugin
> > > > >> and
> > > > >> > > the
> > > > >> > > > >> > injection site
> > > > >> > > > >> >
> > > > >> > > > >> > Currently running plugins have access to the following
> > > > >> component
> > > > >> > > > >> > implementations:
> > > > >> > > > >> >
> > > > >> > > > >> > * Regular plugin has access to components implemented
> by
> > > the
> > > > >> plugin,
> > > > >> > > > >> > project build extensions, if any (via project class
> realm
> > > > >> foreign
> > > > >> > > > >> > import) and Maven Core.
> > > > >> > > > >> > * Extension plugin has access to components
> implemented by
> > > > >> the
> > > > >> > > project
> > > > >> > > > >> > build extensions and Maven Core.
> > > > >> > > > >> > * Without a running plugin (e.g., during project
> dependency
> > > > >> > > > >> resolution),
> > > > >> > > > >> > components implemented by the project build extensions
> and
> > > > >> Maven
> > > > >> > > Core
> > > > >> > > > >> > are accessible.
> > > > >> > > > >> >
> > > > >> > > > >> > Different injection sites have access to the following
> > > > >> component
> > > > >> > > > >> > interfaces:
> > > > >> > > > >> >
> > > > >> > > > >> > * Maven Core has access to component interfaces
> defined by
> > > > >> the
> > > > >> core
> > > > >> > > > >> > itself (obviously)
> > > > >> > > > >> > * Project build extensions have access to **public**
> > > > >> component
> > > > >> > > > >> > interfaces defined by Maven Core and component
> interfaces
> > > > >> defined by
> > > > >> > > > >> the
> > > > >> > > > >> > build extension itself (there is no way to access
> component
> > > > >> > > interfaces
> > > > >> > > > >> > defined in other extensions)
> > > > >> > > > >> > * Regular plugins have access to **public** component
> > > > >> interfaces
> > > > >> > > > >> defined
> > > > >> > > > >> > by Maven Core, component interfaces **exported** by
> build
> > > > >> extensions
> > > > >> > > > >> and
> > > > >> > > > >> > component interfaces defined in the plugin itself
> > > > >> > > > >> >
> > > > >> > > > >> > For injection to work, injection site has to have
> access to
> > > > >> the
> > > > >> > > > >> > component interface and the component implementation
> must
> > > be
> > > > >> > > > >> accessible
> > > > >> > > > >> > through the current context.
> > > > >> > > > >> >
> > > > >> > > > >> > From what I can tell, in your example all plugins have
> > > access
> > > > >> to the
> > > > >> > > > >> > right components when using current 3.5.2-SNAPSHOT. The
> > > > >> injection
> > > > >> > > > >> order
> > > > >> > > > >> > does appear to be backwards from what I expected,
> however.
> > > > >> > > > >> >
> > > > >> > > > >> >
> > > > >> > > > >> > ## Resources lookup fully depends on classpath
> visibility,
> > > > >> > > > >> specifically
> > > > >> > > > >> >
> > > > >> > > > >> > * Regular plugin class realm has access to resources
> from
> > > the
> > > > >> plugin
> > > > >> > > > >> > itself, from **exported** packages of the project build
> > > > >> extensions
> > > > >> > > and
> > > > >> > > > >> > **public** Maven Core packages
> > > > >> > > > >> > * Extensions plugin class realm has access to the
> resources
> > > > >> from the
> > > > >> > > > >> > extensions plugin itself and from **public** Maven Core
> > > > >> packages
> > > > >> > > > >> > * Project class realm has access to classes and
> resources
> > > > >> > > **exported**
> > > > >> > > > >> > by project build extensions and **public** Maven Core
> > > > >> packages
> > > > >> > > > >> >
> > > > >> > > > >> > I see three problems here
> > > > >> > > > >> >
> > > > >> > > > >> > * Maven adds build single-jar `<extensions>` elements
> > > > >> directly
> > > > >> to
> > > > >> > > > >> > project class realm **and** creates separate extensions
> > > class
> > > > >> realms
> > > > >> > > > >> for
> > > > >> > > > >> > them. Which results in duplicate classes/resources
> loaded
> > > by
> > > > >> two
> > > > >> > > > >> > classloaders and explains why you see extjar1/extjar2
> > > output
> > > > >> (which
> > > > >> > > > >> you
> > > > >> > > > >> > shouldn't according to the explanation above)
> > > > >> > > > >> > * ClassRealm does not allow foreign-import of the same
> > > > >> package
> > > > >> from
> > > > >> > > > >> > multiple classloaders. This makes it impossible to
> load the
> > > > >> same
> > > > >> > > > >> > resource from multiple plugins/extensions.
> > > > >> > > > >> > * Extensions plugins cannot access their own private
> (i.e.
> > > > >> not
> > > > >> > > > >> exported)
> > > > >> > > > >> > resources via TCCL, this is change in behaviour
> introduced
> > > by
> > > > >> > > mng-6209
> > > > >> > > > >> > fix
> > > > >> > > > >> >
> > > > >> > > > >> > Hope this helps
> > > > >> > > > >>
> > > > >> > > > >>
> > > > >>
> ---------------------------------------------------------------------
> > > > >> > > > >> To unsubscribe, e-mail: [hidden email]
> > > > >> > > > >> For additional commands, e-mail:
> [hidden email]
> > > > >> > > >
> > > > >> > > >
> > > > >>
> ---------------------------------------------------------------------
> > > > >> > > > To unsubscribe, e-mail: [hidden email]
> > > > >> > > > For additional commands, e-mail: [hidden email]
> > > > >> > > >
> > > > >> > >
> > > > >> > >
> > > > >>
> ---------------------------------------------------------------------
> > > > >> > > To unsubscribe, e-mail: [hidden email]
> > > > >> > > For additional commands, e-mail: [hidden email]
> > > > >> > >
> > > > >> > > --
> > > > >> > Sent from my phone
> > > > >>
> > > > >>
> ---------------------------------------------------------------------
> > > > >> To unsubscribe, e-mail: [hidden email]
> > > > >> For additional commands, e-mail: [hidden email]
> > > > >>
> > > > >> --
> > > > > Sent from my phone
> > > >
> > > > ---------------------------------------------------------------------
> > > > To unsubscribe, e-mail: [hidden email]
> > > > For additional commands, e-mail: [hidden email]
> > > >
> > >
> > > ---------------------------------------------------------------------
> > > To unsubscribe, e-mail: [hidden email]
> > > For additional commands, e-mail: [hidden email]
> > >
> > >
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: [hidden email]
> For additional commands, e-mail: [hidden email]
>
> --
Sent from my phone
Reply | Threaded
Open this post in threaded view
|

Re: Understanding MNG-6209 (was: [VOTE] Release Apache Maven 3.5.1)

Anders Hammar
To add to the topic I think that if we fix/change the class loading, we
shouldn't do that in a bug fix release. I'd rather see a 3.6.0 with it
thought through, fixed and documented (could be in ITs). The arguing being
that a bug fix release shouldn't cause new issues in plugins etc.

/Anders

On Sun, Sep 24, 2017 at 8:28 PM, Stephen Connolly <
[hidden email]> wrote:

> I wonder should we do a hangout to decide what you do?
>
> What times on Monday work best?
>
> I can maybe do 8:30-9:30pm Irish time
>
> https://www.timeanddate.com/worldclock/meetingdetails.
> html?year=2017&month=9&day=25&hour=19&min=30&sec=0&p1=78&p2=37&p3=179
>
> But we’d need to decide who we need and an actual agenda.
>
> If Monday is too soon I can see if I have a window later this week
>
> On Sun 24 Sep 2017 at 18:58, Igor Fedorenko <[hidden email]> wrote:
>
> > See my answers/comments inline
> >
> >
> > On Sun, Sep 24, 2017, at 12:07 PM, Stephen Connolly wrote:
> > > https://maven.apache.org/guides/mini/guide-maven-classloading.html
> says:
> > >
> > > > When a build plugin is executed, the thread's context classloader is
> > set
> > > to the plugin classloader.
> > >
> > > So we'll need to fix something somewhere...
> > >
> > >
> > https://svn.apache.org/repos/infra/websites/production/
> maven/content/reference/maven-classloading.html
> > > is unaccessible from the website due to a rewrite rule...
> > >
> > > Things that seem to be missing:
> > >
> > > * What is the desired classloading for a plugin that is marked as an
> > > extension? Can a plugin have a META-INF/maven/extension.xml to allow
> > > exporting classes and artifacts when used as an extension? How should
> the
> > > classloading look for such a strange beast.
> >
> > To me, the key requirement is that @Singleton components and class
> > static members are singletons when injected in Maven core or in @Mojos.
> > This implies that there should be single classloader representing an
> > extensions plugins (MNG-5742).
> >
> > META-INF/maven/extension.xml declares what packages of the extension
> > plugin are visible to other (non extension) plugins.
> > META-INF/maven/extension.xml does not affect classloading of the
> > extension plugin nor it affects the "shape" of other classloaders.
> >
> > > * How does one access the plugin classloader if we want TCCL to be
> other
> > > than that, is it a Dependency Injection or something else?
> >
> > this.getClass().getClassLoader() is the most direct way to access plugin
> > classloader. Why do you think we need anything more elaborate?
> >
> >
> > > * What differentiates a Core extension from a Build extension (is it
> that
> > > a
> > > build extension lacks a META-INF/maven/extension.xml and was only
> > > declared
> > > in the pom.xml, while a core extension either has a
> > > META-INF/maven/extension.xml - if declared in the pom - or is an
> > > extension
> > > declared in .mvn/extensions.xml)
> >
> > Core extensions are loaded *before* build starts, so they can contribute
> > AbstractMavenLifecycleParticipant#afterSessionStart, for example. They
> > can also export packages visible to all build plugins, including
> > extensions=true. On the flip side, each core extension is effectively
> > singleton, you can't have two different versions of the same Core
> > extension. Core extensions also have direct access to Maven core classes
> > and can do more interesting things there (for better or worse).
> >
> > Build extensions are part of the project build and as such are limited
> > what components they can contribute to the Core and what core classes
> > they have access to.
> >
> > I tried to capture this in the diagram I drew for
> > http://takari.io/book/91-maven-classloading.html.
> >
> > > At this point in time I think we are nearing the point where I may have
> > > to
> > > declare 3.5.1 abandoned as I think the classloading in that is a
> symptom
> > > of
> > > too many cooks all changing things in different directions. We need a
> > > consistent vision of where we want things to go and - while we need not
> > > get
> > > there in one go - the path presented for others to see.
> >
> > There were two classloading changes in 3.5.1, namely extensions=true
> > plugins now have project realm as TCCL and all realms now use
> > application classloader as the parent. Apart from lacking documentation,
> > what practical problems have been caused by these two changes?
> >
> > >
> > > Things I think we should consider:
> > >
> > > 1. Do we want to formally deprecate Build Extensions and the
> > > /project/build/extensions element (start logging warnings, etc)?
> > > 2. Do we want to formally deprecate plugins as extensions and start
> > > logging
> > > warnings for
> > >
> > /project/build/(pluginManagement|.)/plugins/plugin/extensions[text()==
> true]
> >
> > I'd keep them both, and maybe fix/remove maven2-compat codepath. If I
> > had to choose between the two, however, I'd choose <plugin> with
> > extensions=true. Think of a custom packaging type with mojos the user
> > wants to configure in pom.xml, it'd be more tedious to configure if I
> > had to add build/extension and build/plugin.
> >
> > > 3. What is the difference in classloading for a
> /project/build/extensions
> > > which has a META-INF/maven/extension.xml and one that doesn't?
> >
> > I think extensions with META-INF/maven/extension.xml should not go
> > through maven2-compat codepath. In other words, we need to change the
> > current behaviour.
> >
> > Extensions without META-INF/maven/extension.xml... I am not sure.
> > Probably safer to keep the current maven2-compat behaviour.
> >
> > > I'm keeping the 3.5.1 release in staging until we get a clear vision
> for
> > > how we want to have classloading so that I can assess whether the 3.5.1
> > > actuality is only moving nearer to the vision (ok to release) or has
> > > moved
> > > nearer in some ways but further in others (not ok to release)
> > >
> >
> >
> > --
> > Regards,
> > Igor
> >
> >
> >
> > > On 20 September 2017 at 12:44, Igor Fedorenko <[hidden email]>
> > > wrote:
> > >
> > > > Real-world scm or wagon <extensions> won't trigger maven2-compat code
> > > > path [1]. To avoid that obscure code path we can either make the test
> > > > more elaborate (i.e. add dependencies to extjar1/extjar2) or we can
> use
> > > > extensions <plugin>. Either way I don't think we should spend time on
> > > > the code path unlikely to be used in real life.
> > > >
> > > > [1]
> > > > https://github.com/apache/maven/blob/maven-3.5.1/maven-
> > > >
> > core/src/main/java/org/apache/maven/project/
> DefaultProjectBuildingHelper.
> > > > java#L210-L219
> > > >
> > > > --
> > > > Regards,
> > > > Igor
> > > >
> > > > On Wed, Sep 20, 2017, at 03:29 AM, Robert Scholte wrote:
> > > > > On Wed, 20 Sep 2017 09:12:47 +0200, Stephen Connolly
> > > > > <[hidden email]> wrote:
> > > > >
> > > > > > On Wed 20 Sep 2017 at 01:29, Igor Fedorenko <[hidden email]
> >
> > > > wrote:
> > > > > >
> > > > > >> In that case, can I suggest couple of changes to the test
> project
> > > > > >>
> > > > > >> * I thinks it makes more sense to configure extjar1 and extjar2
> as
> > > > > >> extensions <plugin> elements in probleN pom.xml files. First,
> > there is
> > > > > >> no meaningful order between <extensions> and <plugins> elements.
> > More
> > > > > >> importantly, though, simple <extensions> are treated in special
> > > > > >> maven2-compat mode and are not representative of likely
> real-world
> > > > > >> extensions.
> > > > > >
> > > > >
> > > > > Not sure I agree with this. I think there are jars worth sharing
> > across
> > > > > multiple plugins, but where making the plugin an extension is a bit
> > > > > weird.
> > > > > I'm thinking of scm and wagon in this case.
> > > > >
> > > > > >
> > > > > > That sounds like we need documentation updated then. None of that
> > is
> > > > > > obvious to me.
> > > > > >
> > > > > >
> > > > > >>
> > > > > >> * I think we should introduce META-INF/maven/extension.xml to
> the
> > test
> > > > > >> extensions. This metadata what introduced to configure classpath
> > > > > >> visibility, so lets use it.
> > > > > >
> > > > > >
> > > > > > Again, not obvious to me, if that file allows control of
> classpath
> > > > > > visibility then it may be that the only issue *with* 3.5.1 is the
> > lack
> > > > of
> > > > > > documentation... now previous versions would have been adding
> > breaking
> > > > > > changes from my PoV but that is the past and should not affect
> the
> > > > 3.5.1
> > > > > > release.
> > > > > >
> > > > > > PRs for the probe project welcome. I am happy to try and write
> docs
> > > > once
> > > > > > I
> > > > > > have an understanding of what the expected behaviours are
> > > > > >
> > > > > >
> > > > > >>
> > > > > >> --
> > > > > >> Regards,
> > > > > >> Igor
> > > > > >>
> > > > > >> On Tue, Sep 19, 2017, at 05:12 PM, Stephen Connolly wrote:
> > > > > >> > Yes, the expectations are key. Depending on what they are we
> may
> > > > > >> either
> > > > > >> > drop 3.5.1 or go ahead as it depends on whether this is more
> > correct
> > > > > >> than
> > > > > >> > 3.5.0 or swapping one fix for a bug
> > > > > >> >
> > > > > >> > On Tue 19 Sep 2017 at 21:39, Igor Fedorenko <
> > [hidden email]>
> > > > > >> wrote:
> > > > > >> >
> > > > > >> > > Just to confirm I understand what we are trying to establish
> > here.
> > > > > >> We
> > > > > >> > > want to decide the expected/desired component injection
> > behaviour
> > > > > >> and
> > > > > >> > > classpath visibility in the absence of package and artifact
> > export
> > > > > >> > > configuration (i.e. META-INF/maven/extension.xml file). Did
> I
> > get
> > > > > >> this
> > > > > >> > > right?
> > > > > >> > >
> > > > > >> > > --
> > > > > >> > > Regards,
> > > > > >> > > Igor
> > > > > >> > >
> > > > > >> > > On Tue, Sep 19, 2017, at 03:52 PM, Robert Scholte wrote:
> > > > > >> > > > Let's do it like this:
> > > > > >> > > >
> > > > > >> > >
> > > > > >> https://cwiki.apache.org/confluence/download/
> attachments/2329841/
> > > > classrealms.pdf?api=v2
> > > > > >> > > >
> > > > > >> > > > Robert
> > > > > >> > > >
> > > > > >> > > > On Tue, 19 Sep 2017 21:08:39 +0200, Stephen Connolly
> > > > > >> > > > <[hidden email]> wrote:
> > > > > >> > > >
> > > > > >> > > > > I think you will need a link to the PDF as attachments
> are
> > > > > >> stripped
> > > > > >> > > from
> > > > > >> > > > > the ML
> > > > > >> > > > >
> > > > > >> > > > > On Tue 19 Sep 2017 at 19:57, Robert Scholte
> > > > > >> <[hidden email]>
> > > > > >> > > wrote:
> > > > > >> > > > >
> > > > > >> > > > >> Attached a single page overview.
> > > > > >> > > > >>
> > > > > >> > > > >> Per block you'll see in the upper left corner the
> > executed
> > > > > >> plugin
> > > > > >> > > > >> The left column contains the extensions and plugin in
> > orderas
> > > > > >> > > specified
> > > > > >> > > > >> in
> > > > > >> > > > >> the pom.xml
> > > > > >> > > > >> In every classloadercolumn you'll see numbers which
> > represent
> > > > > >> the
> > > > > >> > > order.
> > > > > >> > > > >>
> > > > > >> > > > >> I hope I didn't make any mistakes.
> > > > > >> > > > >> Tomorrow I have enough time to see if I understand
> what's
> > > > > >> happening
> > > > > >> > > > >> here.
> > > > > >> > > > >>
> > > > > >> > > > >> I will come back with my conclusions.
> > > > > >> > > > >>
> > > > > >> > > > >> Robert
> > > > > >> > > > >>
> > > > > >> > > > >> On Tue, 19 Sep 2017 06:55:08 +0200, Igor Fedorenko <
> > > > > >> > > [hidden email]>
> > > > > >> > > > >> wrote:
> > > > > >> > > > >>
> > > > > >> > > > >> > TL;DR your test project exposed two existing bugs,
> one
> > > > > >> change in
> > > > > >> > > > >> > behaviour and one quirk I can't explain
> > > > > >> > > > >> >
> > > > > >> > > > >> > * Build `<extensions>` are loaded by two
> classloaders,
> > > > which
> > > > > >> is
> > > > > >> a
> > > > > >> > > bug
> > > > > >> > > > >> in
> > > > > >> > > > >> > DefaultProjectBuildingHelper#createProjectRealm and
> > > > explains
> > > > > >> why you
> > > > > >> > > > >> see
> > > > > >> > > > >> > extjar1/extjar2 in the output
> > > > > >> > > > >> > * ClassRealm does not allow same foreign-import from
> > > > multiple
> > > > > >> > > > >> > classloaders, which is a bug and explains why it is
> not
> > > > > >> possible to
> > > > > >> > > > >> load
> > > > > >> > > > >> > same resource from multiple plugins/extensions
> > > > > >> > > > >> > * TCCL does not have access to private (i.e. not
> > exported)
> > > > > >> resources
> > > > > >> > > > >> of
> > > > > >> > > > >> > this extensions plugin, which is a change of
> behaviour
> > > > > >> introduced by
> > > > > >> > > > >> > mng-6209 fix
> > > > > >> > > > >> > * Also, component injection order appears to be
> > backwards,
> > > > > >> but
> > > > > >> maybe
> > > > > >> > > > >> > Stuart can explain why.
> > > > > >> > > > >> >
> > > > > >> > > > >> >
> > > > > >> > > > >> > Below is more detailed explanation of expected and
> > observed
> > > > > >> > > behaviour
> > > > > >> > > > >> >
> > > > > >> > > > >> >
> > > > > >> > > > >> > ## Component injection depends on the currently
> running
> > > > > >> plugin
> > > > > >> and
> > > > > >> > > the
> > > > > >> > > > >> > injection site
> > > > > >> > > > >> >
> > > > > >> > > > >> > Currently running plugins have access to the
> following
> > > > > >> component
> > > > > >> > > > >> > implementations:
> > > > > >> > > > >> >
> > > > > >> > > > >> > * Regular plugin has access to components implemented
> > by
> > > > the
> > > > > >> plugin,
> > > > > >> > > > >> > project build extensions, if any (via project class
> > realm
> > > > > >> foreign
> > > > > >> > > > >> > import) and Maven Core.
> > > > > >> > > > >> > * Extension plugin has access to components
> > implemented by
> > > > > >> the
> > > > > >> > > project
> > > > > >> > > > >> > build extensions and Maven Core.
> > > > > >> > > > >> > * Without a running plugin (e.g., during project
> > dependency
> > > > > >> > > > >> resolution),
> > > > > >> > > > >> > components implemented by the project build
> extensions
> > and
> > > > > >> Maven
> > > > > >> > > Core
> > > > > >> > > > >> > are accessible.
> > > > > >> > > > >> >
> > > > > >> > > > >> > Different injection sites have access to the
> following
> > > > > >> component
> > > > > >> > > > >> > interfaces:
> > > > > >> > > > >> >
> > > > > >> > > > >> > * Maven Core has access to component interfaces
> > defined by
> > > > > >> the
> > > > > >> core
> > > > > >> > > > >> > itself (obviously)
> > > > > >> > > > >> > * Project build extensions have access to **public**
> > > > > >> component
> > > > > >> > > > >> > interfaces defined by Maven Core and component
> > interfaces
> > > > > >> defined by
> > > > > >> > > > >> the
> > > > > >> > > > >> > build extension itself (there is no way to access
> > component
> > > > > >> > > interfaces
> > > > > >> > > > >> > defined in other extensions)
> > > > > >> > > > >> > * Regular plugins have access to **public** component
> > > > > >> interfaces
> > > > > >> > > > >> defined
> > > > > >> > > > >> > by Maven Core, component interfaces **exported** by
> > build
> > > > > >> extensions
> > > > > >> > > > >> and
> > > > > >> > > > >> > component interfaces defined in the plugin itself
> > > > > >> > > > >> >
> > > > > >> > > > >> > For injection to work, injection site has to have
> > access to
> > > > > >> the
> > > > > >> > > > >> > component interface and the component implementation
> > must
> > > > be
> > > > > >> > > > >> accessible
> > > > > >> > > > >> > through the current context.
> > > > > >> > > > >> >
> > > > > >> > > > >> > From what I can tell, in your example all plugins
> have
> > > > access
> > > > > >> to the
> > > > > >> > > > >> > right components when using current 3.5.2-SNAPSHOT.
> The
> > > > > >> injection
> > > > > >> > > > >> order
> > > > > >> > > > >> > does appear to be backwards from what I expected,
> > however.
> > > > > >> > > > >> >
> > > > > >> > > > >> >
> > > > > >> > > > >> > ## Resources lookup fully depends on classpath
> > visibility,
> > > > > >> > > > >> specifically
> > > > > >> > > > >> >
> > > > > >> > > > >> > * Regular plugin class realm has access to resources
> > from
> > > > the
> > > > > >> plugin
> > > > > >> > > > >> > itself, from **exported** packages of the project
> build
> > > > > >> extensions
> > > > > >> > > and
> > > > > >> > > > >> > **public** Maven Core packages
> > > > > >> > > > >> > * Extensions plugin class realm has access to the
> > resources
> > > > > >> from the
> > > > > >> > > > >> > extensions plugin itself and from **public** Maven
> Core
> > > > > >> packages
> > > > > >> > > > >> > * Project class realm has access to classes and
> > resources
> > > > > >> > > **exported**
> > > > > >> > > > >> > by project build extensions and **public** Maven Core
> > > > > >> packages
> > > > > >> > > > >> >
> > > > > >> > > > >> > I see three problems here
> > > > > >> > > > >> >
> > > > > >> > > > >> > * Maven adds build single-jar `<extensions>` elements
> > > > > >> directly
> > > > > >> to
> > > > > >> > > > >> > project class realm **and** creates separate
> extensions
> > > > class
> > > > > >> realms
> > > > > >> > > > >> for
> > > > > >> > > > >> > them. Which results in duplicate classes/resources
> > loaded
> > > > by
> > > > > >> two
> > > > > >> > > > >> > classloaders and explains why you see extjar1/extjar2
> > > > output
> > > > > >> (which
> > > > > >> > > > >> you
> > > > > >> > > > >> > shouldn't according to the explanation above)
> > > > > >> > > > >> > * ClassRealm does not allow foreign-import of the
> same
> > > > > >> package
> > > > > >> from
> > > > > >> > > > >> > multiple classloaders. This makes it impossible to
> > load the
> > > > > >> same
> > > > > >> > > > >> > resource from multiple plugins/extensions.
> > > > > >> > > > >> > * Extensions plugins cannot access their own private
> > (i.e.
> > > > > >> not
> > > > > >> > > > >> exported)
> > > > > >> > > > >> > resources via TCCL, this is change in behaviour
> > introduced
> > > > by
> > > > > >> > > mng-6209
> > > > > >> > > > >> > fix
> > > > > >> > > > >> >
> > > > > >> > > > >> > Hope this helps
> > > > > >> > > > >>
> > > > > >> > > > >>
> > > > > >>
> > ---------------------------------------------------------------------
> > > > > >> > > > >> To unsubscribe, e-mail: [hidden email].
> org
> > > > > >> > > > >> For additional commands, e-mail:
> > [hidden email]
> > > > > >> > > >
> > > > > >> > > >
> > > > > >>
> > ---------------------------------------------------------------------
> > > > > >> > > > To unsubscribe, e-mail: [hidden email]
> > > > > >> > > > For additional commands, e-mail:
> [hidden email]
> > > > > >> > > >
> > > > > >> > >
> > > > > >> > >
> > > > > >>
> > ---------------------------------------------------------------------
> > > > > >> > > To unsubscribe, e-mail: [hidden email]
> > > > > >> > > For additional commands, e-mail: [hidden email]
> > > > > >> > >
> > > > > >> > > --
> > > > > >> > Sent from my phone
> > > > > >>
> > > > > >>
> > ---------------------------------------------------------------------
> > > > > >> To unsubscribe, e-mail: [hidden email]
> > > > > >> For additional commands, e-mail: [hidden email]
> > > > > >>
> > > > > >> --
> > > > > > Sent from my phone
> > > > >
> > > > > ------------------------------------------------------------
> ---------
> > > > > To unsubscribe, e-mail: [hidden email]
> > > > > For additional commands, e-mail: [hidden email]
> > > > >
> > > >
> > > > ------------------------------------------------------------
> ---------
> > > > To unsubscribe, e-mail: [hidden email]
> > > > For additional commands, e-mail: [hidden email]
> > > >
> > > >
> >
> > ---------------------------------------------------------------------
> > To unsubscribe, e-mail: [hidden email]
> > For additional commands, e-mail: [hidden email]
> >
> > --
> Sent from my phone
>
Reply | Threaded
Open this post in threaded view
|

Re: Understanding MNG-6209 (was: [VOTE] Release Apache Maven 3.5.1)

Igor Fedorenko-3
In reply to this post by stephenconnolly
Lets decide the agenda first, then who you need to attend (assuming you
are driving this discussion/decision), then pick the time that works.

From my side, I still don't understand the problems we are trying to
solve. If this is the lacking documentation and general "uncomfort" to
mess with classloading in bug fix release, then maybe do what Anders
suggests (I think), bump the version to 3.6.0, document the behaviour we
have on master and move on.

--
Regards,
Igor

On Sun, Sep 24, 2017, at 02:28 PM, Stephen Connolly wrote:

> I wonder should we do a hangout to decide what you do?
>
> What times on Monday work best?
>
> I can maybe do 8:30-9:30pm Irish time
>
> https://www.timeanddate.com/worldclock/meetingdetails.html?year=2017&month=9&day=25&hour=19&min=30&sec=0&p1=78&p2=37&p3=179
>
> But we’d need to decide who we need and an actual agenda.
>
> If Monday is too soon I can see if I have a window later this week
>
> On Sun 24 Sep 2017 at 18:58, Igor Fedorenko <[hidden email]> wrote:
>
> > See my answers/comments inline
> >
> >
> > On Sun, Sep 24, 2017, at 12:07 PM, Stephen Connolly wrote:
> > > https://maven.apache.org/guides/mini/guide-maven-classloading.html says:
> > >
> > > > When a build plugin is executed, the thread's context classloader is
> > set
> > > to the plugin classloader.
> > >
> > > So we'll need to fix something somewhere...
> > >
> > >
> > https://svn.apache.org/repos/infra/websites/production/maven/content/reference/maven-classloading.html
> > > is unaccessible from the website due to a rewrite rule...
> > >
> > > Things that seem to be missing:
> > >
> > > * What is the desired classloading for a plugin that is marked as an
> > > extension? Can a plugin have a META-INF/maven/extension.xml to allow
> > > exporting classes and artifacts when used as an extension? How should the
> > > classloading look for such a strange beast.
> >
> > To me, the key requirement is that @Singleton components and class
> > static members are singletons when injected in Maven core or in @Mojos.
> > This implies that there should be single classloader representing an
> > extensions plugins (MNG-5742).
> >
> > META-INF/maven/extension.xml declares what packages of the extension
> > plugin are visible to other (non extension) plugins.
> > META-INF/maven/extension.xml does not affect classloading of the
> > extension plugin nor it affects the "shape" of other classloaders.
> >
> > > * How does one access the plugin classloader if we want TCCL to be other
> > > than that, is it a Dependency Injection or something else?
> >
> > this.getClass().getClassLoader() is the most direct way to access plugin
> > classloader. Why do you think we need anything more elaborate?
> >
> >
> > > * What differentiates a Core extension from a Build extension (is it that
> > > a
> > > build extension lacks a META-INF/maven/extension.xml and was only
> > > declared
> > > in the pom.xml, while a core extension either has a
> > > META-INF/maven/extension.xml - if declared in the pom - or is an
> > > extension
> > > declared in .mvn/extensions.xml)
> >
> > Core extensions are loaded *before* build starts, so they can contribute
> > AbstractMavenLifecycleParticipant#afterSessionStart, for example. They
> > can also export packages visible to all build plugins, including
> > extensions=true. On the flip side, each core extension is effectively
> > singleton, you can't have two different versions of the same Core
> > extension. Core extensions also have direct access to Maven core classes
> > and can do more interesting things there (for better or worse).
> >
> > Build extensions are part of the project build and as such are limited
> > what components they can contribute to the Core and what core classes
> > they have access to.
> >
> > I tried to capture this in the diagram I drew for
> > http://takari.io/book/91-maven-classloading.html.
> >
> > > At this point in time I think we are nearing the point where I may have
> > > to
> > > declare 3.5.1 abandoned as I think the classloading in that is a symptom
> > > of
> > > too many cooks all changing things in different directions. We need a
> > > consistent vision of where we want things to go and - while we need not
> > > get
> > > there in one go - the path presented for others to see.
> >
> > There were two classloading changes in 3.5.1, namely extensions=true
> > plugins now have project realm as TCCL and all realms now use
> > application classloader as the parent. Apart from lacking documentation,
> > what practical problems have been caused by these two changes?
> >
> > >
> > > Things I think we should consider:
> > >
> > > 1. Do we want to formally deprecate Build Extensions and the
> > > /project/build/extensions element (start logging warnings, etc)?
> > > 2. Do we want to formally deprecate plugins as extensions and start
> > > logging
> > > warnings for
> > >
> > /project/build/(pluginManagement|.)/plugins/plugin/extensions[text()==true]
> >
> > I'd keep them both, and maybe fix/remove maven2-compat codepath. If I
> > had to choose between the two, however, I'd choose <plugin> with
> > extensions=true. Think of a custom packaging type with mojos the user
> > wants to configure in pom.xml, it'd be more tedious to configure if I
> > had to add build/extension and build/plugin.
> >
> > > 3. What is the difference in classloading for a /project/build/extensions
> > > which has a META-INF/maven/extension.xml and one that doesn't?
> >
> > I think extensions with META-INF/maven/extension.xml should not go
> > through maven2-compat codepath. In other words, we need to change the
> > current behaviour.
> >
> > Extensions without META-INF/maven/extension.xml... I am not sure.
> > Probably safer to keep the current maven2-compat behaviour.
> >
> > > I'm keeping the 3.5.1 release in staging until we get a clear vision for
> > > how we want to have classloading so that I can assess whether the 3.5.1
> > > actuality is only moving nearer to the vision (ok to release) or has
> > > moved
> > > nearer in some ways but further in others (not ok to release)
> > >
> >
> >
> > --
> > Regards,
> > Igor
> >
> >
> >
> > > On 20 September 2017 at 12:44, Igor Fedorenko <[hidden email]>
> > > wrote:
> > >
> > > > Real-world scm or wagon <extensions> won't trigger maven2-compat code
> > > > path [1]. To avoid that obscure code path we can either make the test
> > > > more elaborate (i.e. add dependencies to extjar1/extjar2) or we can use
> > > > extensions <plugin>. Either way I don't think we should spend time on
> > > > the code path unlikely to be used in real life.
> > > >
> > > > [1]
> > > > https://github.com/apache/maven/blob/maven-3.5.1/maven-
> > > >
> > core/src/main/java/org/apache/maven/project/DefaultProjectBuildingHelper.
> > > > java#L210-L219
> > > >
> > > > --
> > > > Regards,
> > > > Igor
> > > >
> > > > On Wed, Sep 20, 2017, at 03:29 AM, Robert Scholte wrote:
> > > > > On Wed, 20 Sep 2017 09:12:47 +0200, Stephen Connolly
> > > > > <[hidden email]> wrote:
> > > > >
> > > > > > On Wed 20 Sep 2017 at 01:29, Igor Fedorenko <[hidden email]>
> > > > wrote:
> > > > > >
> > > > > >> In that case, can I suggest couple of changes to the test project
> > > > > >>
> > > > > >> * I thinks it makes more sense to configure extjar1 and extjar2 as
> > > > > >> extensions <plugin> elements in probleN pom.xml files. First,
> > there is
> > > > > >> no meaningful order between <extensions> and <plugins> elements.
> > More
> > > > > >> importantly, though, simple <extensions> are treated in special
> > > > > >> maven2-compat mode and are not representative of likely real-world
> > > > > >> extensions.
> > > > > >
> > > > >
> > > > > Not sure I agree with this. I think there are jars worth sharing
> > across
> > > > > multiple plugins, but where making the plugin an extension is a bit
> > > > > weird.
> > > > > I'm thinking of scm and wagon in this case.
> > > > >
> > > > > >
> > > > > > That sounds like we need documentation updated then. None of that
> > is
> > > > > > obvious to me.
> > > > > >
> > > > > >
> > > > > >>
> > > > > >> * I think we should introduce META-INF/maven/extension.xml to the
> > test
> > > > > >> extensions. This metadata what introduced to configure classpath
> > > > > >> visibility, so lets use it.
> > > > > >
> > > > > >
> > > > > > Again, not obvious to me, if that file allows control of classpath
> > > > > > visibility then it may be that the only issue *with* 3.5.1 is the
> > lack
> > > > of
> > > > > > documentation... now previous versions would have been adding
> > breaking
> > > > > > changes from my PoV but that is the past and should not affect the
> > > > 3.5.1
> > > > > > release.
> > > > > >
> > > > > > PRs for the probe project welcome. I am happy to try and write docs
> > > > once
> > > > > > I
> > > > > > have an understanding of what the expected behaviours are
> > > > > >
> > > > > >
> > > > > >>
> > > > > >> --
> > > > > >> Regards,
> > > > > >> Igor
> > > > > >>
> > > > > >> On Tue, Sep 19, 2017, at 05:12 PM, Stephen Connolly wrote:
> > > > > >> > Yes, the expectations are key. Depending on what they are we may
> > > > > >> either
> > > > > >> > drop 3.5.1 or go ahead as it depends on whether this is more
> > correct
> > > > > >> than
> > > > > >> > 3.5.0 or swapping one fix for a bug
> > > > > >> >
> > > > > >> > On Tue 19 Sep 2017 at 21:39, Igor Fedorenko <
> > [hidden email]>
> > > > > >> wrote:
> > > > > >> >
> > > > > >> > > Just to confirm I understand what we are trying to establish
> > here.
> > > > > >> We
> > > > > >> > > want to decide the expected/desired component injection
> > behaviour
> > > > > >> and
> > > > > >> > > classpath visibility in the absence of package and artifact
> > export
> > > > > >> > > configuration (i.e. META-INF/maven/extension.xml file). Did I
> > get
> > > > > >> this
> > > > > >> > > right?
> > > > > >> > >
> > > > > >> > > --
> > > > > >> > > Regards,
> > > > > >> > > Igor
> > > > > >> > >
> > > > > >> > > On Tue, Sep 19, 2017, at 03:52 PM, Robert Scholte wrote:
> > > > > >> > > > Let's do it like this:
> > > > > >> > > >
> > > > > >> > >
> > > > > >> https://cwiki.apache.org/confluence/download/attachments/2329841/
> > > > classrealms.pdf?api=v2
> > > > > >> > > >
> > > > > >> > > > Robert
> > > > > >> > > >
> > > > > >> > > > On Tue, 19 Sep 2017 21:08:39 +0200, Stephen Connolly
> > > > > >> > > > <[hidden email]> wrote:
> > > > > >> > > >
> > > > > >> > > > > I think you will need a link to the PDF as attachments are
> > > > > >> stripped
> > > > > >> > > from
> > > > > >> > > > > the ML
> > > > > >> > > > >
> > > > > >> > > > > On Tue 19 Sep 2017 at 19:57, Robert Scholte
> > > > > >> <[hidden email]>
> > > > > >> > > wrote:
> > > > > >> > > > >
> > > > > >> > > > >> Attached a single page overview.
> > > > > >> > > > >>
> > > > > >> > > > >> Per block you'll see in the upper left corner the
> > executed
> > > > > >> plugin
> > > > > >> > > > >> The left column contains the extensions and plugin in
> > orderas
> > > > > >> > > specified
> > > > > >> > > > >> in
> > > > > >> > > > >> the pom.xml
> > > > > >> > > > >> In every classloadercolumn you'll see numbers which
> > represent
> > > > > >> the
> > > > > >> > > order.
> > > > > >> > > > >>
> > > > > >> > > > >> I hope I didn't make any mistakes.
> > > > > >> > > > >> Tomorrow I have enough time to see if I understand what's
> > > > > >> happening
> > > > > >> > > > >> here.
> > > > > >> > > > >>
> > > > > >> > > > >> I will come back with my conclusions.
> > > > > >> > > > >>
> > > > > >> > > > >> Robert
> > > > > >> > > > >>
> > > > > >> > > > >> On Tue, 19 Sep 2017 06:55:08 +0200, Igor Fedorenko <
> > > > > >> > > [hidden email]>
> > > > > >> > > > >> wrote:
> > > > > >> > > > >>
> > > > > >> > > > >> > TL;DR your test project exposed two existing bugs, one
> > > > > >> change in
> > > > > >> > > > >> > behaviour and one quirk I can't explain
> > > > > >> > > > >> >
> > > > > >> > > > >> > * Build `<extensions>` are loaded by two classloaders,
> > > > which
> > > > > >> is
> > > > > >> a
> > > > > >> > > bug
> > > > > >> > > > >> in
> > > > > >> > > > >> > DefaultProjectBuildingHelper#createProjectRealm and
> > > > explains
> > > > > >> why you
> > > > > >> > > > >> see
> > > > > >> > > > >> > extjar1/extjar2 in the output
> > > > > >> > > > >> > * ClassRealm does not allow same foreign-import from
> > > > multiple
> > > > > >> > > > >> > classloaders, which is a bug and explains why it is not
> > > > > >> possible to
> > > > > >> > > > >> load
> > > > > >> > > > >> > same resource from multiple plugins/extensions
> > > > > >> > > > >> > * TCCL does not have access to private (i.e. not
> > exported)
> > > > > >> resources
> > > > > >> > > > >> of
> > > > > >> > > > >> > this extensions plugin, which is a change of behaviour
> > > > > >> introduced by
> > > > > >> > > > >> > mng-6209 fix
> > > > > >> > > > >> > * Also, component injection order appears to be
> > backwards,
> > > > > >> but
> > > > > >> maybe
> > > > > >> > > > >> > Stuart can explain why.
> > > > > >> > > > >> >
> > > > > >> > > > >> >
> > > > > >> > > > >> > Below is more detailed explanation of expected and
> > observed
> > > > > >> > > behaviour
> > > > > >> > > > >> >
> > > > > >> > > > >> >
> > > > > >> > > > >> > ## Component injection depends on the currently running
> > > > > >> plugin
> > > > > >> and
> > > > > >> > > the
> > > > > >> > > > >> > injection site
> > > > > >> > > > >> >
> > > > > >> > > > >> > Currently running plugins have access to the following
> > > > > >> component
> > > > > >> > > > >> > implementations:
> > > > > >> > > > >> >
> > > > > >> > > > >> > * Regular plugin has access to components implemented
> > by
> > > > the
> > > > > >> plugin,
> > > > > >> > > > >> > project build extensions, if any (via project class
> > realm
> > > > > >> foreign
> > > > > >> > > > >> > import) and Maven Core.
> > > > > >> > > > >> > * Extension plugin has access to components
> > implemented by
> > > > > >> the
> > > > > >> > > project
> > > > > >> > > > >> > build extensions and Maven Core.
> > > > > >> > > > >> > * Without a running plugin (e.g., during project
> > dependency
> > > > > >> > > > >> resolution),
> > > > > >> > > > >> > components implemented by the project build extensions
> > and
> > > > > >> Maven
> > > > > >> > > Core
> > > > > >> > > > >> > are accessible.
> > > > > >> > > > >> >
> > > > > >> > > > >> > Different injection sites have access to the following
> > > > > >> component
> > > > > >> > > > >> > interfaces:
> > > > > >> > > > >> >
> > > > > >> > > > >> > * Maven Core has access to component interfaces
> > defined by
> > > > > >> the
> > > > > >> core
> > > > > >> > > > >> > itself (obviously)
> > > > > >> > > > >> > * Project build extensions have access to **public**
> > > > > >> component
> > > > > >> > > > >> > interfaces defined by Maven Core and component
> > interfaces
> > > > > >> defined by
> > > > > >> > > > >> the
> > > > > >> > > > >> > build extension itself (there is no way to access
> > component
> > > > > >> > > interfaces
> > > > > >> > > > >> > defined in other extensions)
> > > > > >> > > > >> > * Regular plugins have access to **public** component
> > > > > >> interfaces
> > > > > >> > > > >> defined
> > > > > >> > > > >> > by Maven Core, component interfaces **exported** by
> > build
> > > > > >> extensions
> > > > > >> > > > >> and
> > > > > >> > > > >> > component interfaces defined in the plugin itself
> > > > > >> > > > >> >
> > > > > >> > > > >> > For injection to work, injection site has to have
> > access to
> > > > > >> the
> > > > > >> > > > >> > component interface and the component implementation
> > must
> > > > be
> > > > > >> > > > >> accessible
> > > > > >> > > > >> > through the current context.
> > > > > >> > > > >> >
> > > > > >> > > > >> > From what I can tell, in your example all plugins have
> > > > access
> > > > > >> to the
> > > > > >> > > > >> > right components when using current 3.5.2-SNAPSHOT. The
> > > > > >> injection
> > > > > >> > > > >> order
> > > > > >> > > > >> > does appear to be backwards from what I expected,
> > however.
> > > > > >> > > > >> >
> > > > > >> > > > >> >
> > > > > >> > > > >> > ## Resources lookup fully depends on classpath
> > visibility,
> > > > > >> > > > >> specifically
> > > > > >> > > > >> >
> > > > > >> > > > >> > * Regular plugin class realm has access to resources
> > from
> > > > the
> > > > > >> plugin
> > > > > >> > > > >> > itself, from **exported** packages of the project build
> > > > > >> extensions
> > > > > >> > > and
> > > > > >> > > > >> > **public** Maven Core packages
> > > > > >> > > > >> > * Extensions plugin class realm has access to the
> > resources
> > > > > >> from the
> > > > > >> > > > >> > extensions plugin itself and from **public** Maven Core
> > > > > >> packages
> > > > > >> > > > >> > * Project class realm has access to classes and
> > resources
> > > > > >> > > **exported**
> > > > > >> > > > >> > by project build extensions and **public** Maven Core
> > > > > >> packages
> > > > > >> > > > >> >
> > > > > >> > > > >> > I see three problems here
> > > > > >> > > > >> >
> > > > > >> > > > >> > * Maven adds build single-jar `<extensions>` elements
> > > > > >> directly
> > > > > >> to
> > > > > >> > > > >> > project class realm **and** creates separate extensions
> > > > class
> > > > > >> realms
> > > > > >> > > > >> for
> > > > > >> > > > >> > them. Which results in duplicate classes/resources
> > loaded
> > > > by
> > > > > >> two
> > > > > >> > > > >> > classloaders and explains why you see extjar1/extjar2
> > > > output
> > > > > >> (which
> > > > > >> > > > >> you
> > > > > >> > > > >> > shouldn't according to the explanation above)
> > > > > >> > > > >> > * ClassRealm does not allow foreign-import of the same
> > > > > >> package
> > > > > >> from
> > > > > >> > > > >> > multiple classloaders. This makes it impossible to
> > load the
> > > > > >> same
> > > > > >> > > > >> > resource from multiple plugins/extensions.
> > > > > >> > > > >> > * Extensions plugins cannot access their own private
> > (i.e.
> > > > > >> not
> > > > > >> > > > >> exported)
> > > > > >> > > > >> > resources via TCCL, this is change in behaviour
> > introduced
> > > > by
> > > > > >> > > mng-6209
> > > > > >> > > > >> > fix
> > > > > >> > > > >> >
> > > > > >> > > > >> > Hope this helps
> > > > > >> > > > >>
> > > > > >> > > > >>
> > > > > >>
> > ---------------------------------------------------------------------
> > > > > >> > > > >> To unsubscribe, e-mail: [hidden email]
> > > > > >> > > > >> For additional commands, e-mail:
> > [hidden email]
> > > > > >> > > >
> > > > > >> > > >
> > > > > >>
> > ---------------------------------------------------------------------
> > > > > >> > > > To unsubscribe, e-mail: [hidden email]
> > > > > >> > > > For additional commands, e-mail: [hidden email]
> > > > > >> > > >
> > > > > >> > >
> > > > > >> > >
> > > > > >>
> > ---------------------------------------------------------------------
> > > > > >> > > To unsubscribe, e-mail: [hidden email]
> > > > > >> > > For additional commands, e-mail: [hidden email]
> > > > > >> > >
> > > > > >> > > --
> > > > > >> > Sent from my phone
> > > > > >>
> > > > > >>
> > ---------------------------------------------------------------------
> > > > > >> To unsubscribe, e-mail: [hidden email]
> > > > > >> For additional commands, e-mail: [hidden email]
> > > > > >>
> > > > > >> --
> > > > > > Sent from my phone
> > > > >
> > > > > ---------------------------------------------------------------------
> > > > > To unsubscribe, e-mail: [hidden email]
> > > > > For additional commands, e-mail: [hidden email]
> > > > >
> > > >
> > > > ---------------------------------------------------------------------
> > > > To unsubscribe, e-mail: [hidden email]
> > > > For additional commands, e-mail: [hidden email]
> > > >
> > > >
> >
> > ---------------------------------------------------------------------
> > To unsubscribe, e-mail: [hidden email]
> > For additional commands, e-mail: [hidden email]
> >
> > --
> Sent from my phone

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: Understanding MNG-6209 (was: [VOTE] Release Apache Maven 3.5.1)

stephenconnolly
Right now I have a successful vote to release 3.5.1

I also have two issues changing classloader behaviours in ways that may
surprise people.

I am not against releasing what we have as a bug fix release - if these are
actual bugs.

I am not against fixing inconsistencies with the site documentation - if
the documentation is in error.

But right now what I do not see is a clear expression of how we envision
Maven classloading *should* work.

I expect master is not aligned with what we would want, and 3.5.0 is also
not aligned... but if we say this is an N variable problem and the ideal is
(x,y,z,v,w,...,q) and 3.5.0 is (x-5,y,z-2,v,w,...,q) and 3.5.1 is
(x+3,y-1,z-2,v,w,...,q) which is “closer” to the ideal but has overshot one
factor and actually degraded another... well I don’t want to release that

Otoh if 3.5.1 is (x-4,y,z-1,v,w,...,q) which is moving some aspects closer
but those aspects are still not perfect... i’m fine - as 3.5.1 release
manager - with closing the vote and pushing the release.

Now I see 6209 changing the classloader for plugins that are also build
extensions... the question here is two fold:

1. Is the new behaviour *correct* or just *less wrong*?
2. If “less wrong”, is it less wrong on the same side of correct as the old
behaviour, or is it less wrong on the other side of correct?

The other one is 6275 changing the TCCL. We have site documentation
*stating* that TCCL will be the plugin classloader, and we are changing now
so that TCCL is not.

3. Which do we need to fix: site or code?
4. Are we sure we can guarantee that the plugin classloader is always the
classloader that loaded the plugin class: what if I have plugin A
dependends on Plugin B (not what i’d recommend, but users do crazy things)
so we have the mojos in Plugin B coming from a jar dependency of Plugin
A... so could we then we have layered classloaders in which case when I
invoke A:mojo-from-b will it be loaded by A’s classloader or a parent of A
that hold the B jar?

Or what if I were to use an extension to provide the mojo but advertising
the extension’s mojo class via a plugin?

These are all *really* stupid things in my opinion, but we haven’t said
“thou shalt not expose mojos from other jar files” so someone *could* have
done it... how are they to get the plugin classloader now that 6275 is
landing? (I think a component of type classloader with a role-hint of
“plugin” would make sense to me)

Alternatively, we document “thou shalt not” and be done with it...

But these are the kinds of things we need to resolve before I feel I can
close the 3.5.1 vote one way or another.


On Sun 24 Sep 2017 at 20:06, Igor Fedorenko <[hidden email]> wrote:

> Lets decide the agenda first, then who you need to attend (assuming you
> are driving this discussion/decision), then pick the time that works.
>
> From my side, I still don't understand the problems we are trying to
> solve. If this is the lacking documentation and general "uncomfort" to
> mess with classloading in bug fix release, then maybe do what Anders
> suggests (I think), bump the version to 3.6.0, document the behaviour we
> have on master and move on.
>
> --
> Regards,
> Igor
>
> On Sun, Sep 24, 2017, at 02:28 PM, Stephen Connolly wrote:
> > I wonder should we do a hangout to decide what you do?
> >
> > What times on Monday work best?
> >
> > I can maybe do 8:30-9:30pm Irish time
> >
> >
> https://www.timeanddate.com/worldclock/meetingdetails.html?year=2017&month=9&day=25&hour=19&min=30&sec=0&p1=78&p2=37&p3=179
> >
> > But we’d need to decide who we need and an actual agenda.
> >
> > If Monday is too soon I can see if I have a window later this week
> >
> > On Sun 24 Sep 2017 at 18:58, Igor Fedorenko <[hidden email]> wrote:
> >
> > > See my answers/comments inline
> > >
> > >
> > > On Sun, Sep 24, 2017, at 12:07 PM, Stephen Connolly wrote:
> > > > https://maven.apache.org/guides/mini/guide-maven-classloading.html
> says:
> > > >
> > > > > When a build plugin is executed, the thread's context classloader
> is
> > > set
> > > > to the plugin classloader.
> > > >
> > > > So we'll need to fix something somewhere...
> > > >
> > > >
> > >
> https://svn.apache.org/repos/infra/websites/production/maven/content/reference/maven-classloading.html
> > > > is unaccessible from the website due to a rewrite rule...
> > > >
> > > > Things that seem to be missing:
> > > >
> > > > * What is the desired classloading for a plugin that is marked as an
> > > > extension? Can a plugin have a META-INF/maven/extension.xml to allow
> > > > exporting classes and artifacts when used as an extension? How
> should the
> > > > classloading look for such a strange beast.
> > >
> > > To me, the key requirement is that @Singleton components and class
> > > static members are singletons when injected in Maven core or in @Mojos.
> > > This implies that there should be single classloader representing an
> > > extensions plugins (MNG-5742).
> > >
> > > META-INF/maven/extension.xml declares what packages of the extension
> > > plugin are visible to other (non extension) plugins.
> > > META-INF/maven/extension.xml does not affect classloading of the
> > > extension plugin nor it affects the "shape" of other classloaders.
> > >
> > > > * How does one access the plugin classloader if we want TCCL to be
> other
> > > > than that, is it a Dependency Injection or something else?
> > >
> > > this.getClass().getClassLoader() is the most direct way to access
> plugin
> > > classloader. Why do you think we need anything more elaborate?
> > >
> > >
> > > > * What differentiates a Core extension from a Build extension (is it
> that
> > > > a
> > > > build extension lacks a META-INF/maven/extension.xml and was only
> > > > declared
> > > > in the pom.xml, while a core extension either has a
> > > > META-INF/maven/extension.xml - if declared in the pom - or is an
> > > > extension
> > > > declared in .mvn/extensions.xml)
> > >
> > > Core extensions are loaded *before* build starts, so they can
> contribute
> > > AbstractMavenLifecycleParticipant#afterSessionStart, for example. They
> > > can also export packages visible to all build plugins, including
> > > extensions=true. On the flip side, each core extension is effectively
> > > singleton, you can't have two different versions of the same Core
> > > extension. Core extensions also have direct access to Maven core
> classes
> > > and can do more interesting things there (for better or worse).
> > >
> > > Build extensions are part of the project build and as such are limited
> > > what components they can contribute to the Core and what core classes
> > > they have access to.
> > >
> > > I tried to capture this in the diagram I drew for
> > > http://takari.io/book/91-maven-classloading.html.
> > >
> > > > At this point in time I think we are nearing the point where I may
> have
> > > > to
> > > > declare 3.5.1 abandoned as I think the classloading in that is a
> symptom
> > > > of
> > > > too many cooks all changing things in different directions. We need a
> > > > consistent vision of where we want things to go and - while we need
> not
> > > > get
> > > > there in one go - the path presented for others to see.
> > >
> > > There were two classloading changes in 3.5.1, namely extensions=true
> > > plugins now have project realm as TCCL and all realms now use
> > > application classloader as the parent. Apart from lacking
> documentation,
> > > what practical problems have been caused by these two changes?
> > >
> > > >
> > > > Things I think we should consider:
> > > >
> > > > 1. Do we want to formally deprecate Build Extensions and the
> > > > /project/build/extensions element (start logging warnings, etc)?
> > > > 2. Do we want to formally deprecate plugins as extensions and start
> > > > logging
> > > > warnings for
> > > >
> > >
> /project/build/(pluginManagement|.)/plugins/plugin/extensions[text()==true]
> > >
> > > I'd keep them both, and maybe fix/remove maven2-compat codepath. If I
> > > had to choose between the two, however, I'd choose <plugin> with
> > > extensions=true. Think of a custom packaging type with mojos the user
> > > wants to configure in pom.xml, it'd be more tedious to configure if I
> > > had to add build/extension and build/plugin.
> > >
> > > > 3. What is the difference in classloading for a
> /project/build/extensions
> > > > which has a META-INF/maven/extension.xml and one that doesn't?
> > >
> > > I think extensions with META-INF/maven/extension.xml should not go
> > > through maven2-compat codepath. In other words, we need to change the
> > > current behaviour.
> > >
> > > Extensions without META-INF/maven/extension.xml... I am not sure.
> > > Probably safer to keep the current maven2-compat behaviour.
> > >
> > > > I'm keeping the 3.5.1 release in staging until we get a clear vision
> for
> > > > how we want to have classloading so that I can assess whether the
> 3.5.1
> > > > actuality is only moving nearer to the vision (ok to release) or has
> > > > moved
> > > > nearer in some ways but further in others (not ok to release)
> > > >
> > >
> > >
> > > --
> > > Regards,
> > > Igor
> > >
> > >
> > >
> > > > On 20 September 2017 at 12:44, Igor Fedorenko <[hidden email]>
> > > > wrote:
> > > >
> > > > > Real-world scm or wagon <extensions> won't trigger maven2-compat
> code
> > > > > path [1]. To avoid that obscure code path we can either make the
> test
> > > > > more elaborate (i.e. add dependencies to extjar1/extjar2) or we
> can use
> > > > > extensions <plugin>. Either way I don't think we should spend time
> on
> > > > > the code path unlikely to be used in real life.
> > > > >
> > > > > [1]
> > > > > https://github.com/apache/maven/blob/maven-3.5.1/maven-
> > > > >
> > >
> core/src/main/java/org/apache/maven/project/DefaultProjectBuildingHelper.
> > > > > java#L210-L219
> > > > >
> > > > > --
> > > > > Regards,
> > > > > Igor
> > > > >
> > > > > On Wed, Sep 20, 2017, at 03:29 AM, Robert Scholte wrote:
> > > > > > On Wed, 20 Sep 2017 09:12:47 +0200, Stephen Connolly
> > > > > > <[hidden email]> wrote:
> > > > > >
> > > > > > > On Wed 20 Sep 2017 at 01:29, Igor Fedorenko <
> [hidden email]>
> > > > > wrote:
> > > > > > >
> > > > > > >> In that case, can I suggest couple of changes to the test
> project
> > > > > > >>
> > > > > > >> * I thinks it makes more sense to configure extjar1 and
> extjar2 as
> > > > > > >> extensions <plugin> elements in probleN pom.xml files. First,
> > > there is
> > > > > > >> no meaningful order between <extensions> and <plugins>
> elements.
> > > More
> > > > > > >> importantly, though, simple <extensions> are treated in
> special
> > > > > > >> maven2-compat mode and are not representative of likely
> real-world
> > > > > > >> extensions.
> > > > > > >
> > > > > >
> > > > > > Not sure I agree with this. I think there are jars worth sharing
> > > across
> > > > > > multiple plugins, but where making the plugin an extension is a
> bit
> > > > > > weird.
> > > > > > I'm thinking of scm and wagon in this case.
> > > > > >
> > > > > > >
> > > > > > > That sounds like we need documentation updated then. None of
> that
> > > is
> > > > > > > obvious to me.
> > > > > > >
> > > > > > >
> > > > > > >>
> > > > > > >> * I think we should introduce META-INF/maven/extension.xml to
> the
> > > test
> > > > > > >> extensions. This metadata what introduced to configure
> classpath
> > > > > > >> visibility, so lets use it.
> > > > > > >
> > > > > > >
> > > > > > > Again, not obvious to me, if that file allows control of
> classpath
> > > > > > > visibility then it may be that the only issue *with* 3.5.1 is
> the
> > > lack
> > > > > of
> > > > > > > documentation... now previous versions would have been adding
> > > breaking
> > > > > > > changes from my PoV but that is the past and should not affect
> the
> > > > > 3.5.1
> > > > > > > release.
> > > > > > >
> > > > > > > PRs for the probe project welcome. I am happy to try and write
> docs
> > > > > once
> > > > > > > I
> > > > > > > have an understanding of what the expected behaviours are
> > > > > > >
> > > > > > >
> > > > > > >>
> > > > > > >> --
> > > > > > >> Regards,
> > > > > > >> Igor
> > > > > > >>
> > > > > > >> On Tue, Sep 19, 2017, at 05:12 PM, Stephen Connolly wrote:
> > > > > > >> > Yes, the expectations are key. Depending on what they are
> we may
> > > > > > >> either
> > > > > > >> > drop 3.5.1 or go ahead as it depends on whether this is more
> > > correct
> > > > > > >> than
> > > > > > >> > 3.5.0 or swapping one fix for a bug
> > > > > > >> >
> > > > > > >> > On Tue 19 Sep 2017 at 21:39, Igor Fedorenko <
> > > [hidden email]>
> > > > > > >> wrote:
> > > > > > >> >
> > > > > > >> > > Just to confirm I understand what we are trying to
> establish
> > > here.
> > > > > > >> We
> > > > > > >> > > want to decide the expected/desired component injection
> > > behaviour
> > > > > > >> and
> > > > > > >> > > classpath visibility in the absence of package and
> artifact
> > > export
> > > > > > >> > > configuration (i.e. META-INF/maven/extension.xml file).
> Did I
> > > get
> > > > > > >> this
> > > > > > >> > > right?
> > > > > > >> > >
> > > > > > >> > > --
> > > > > > >> > > Regards,
> > > > > > >> > > Igor
> > > > > > >> > >
> > > > > > >> > > On Tue, Sep 19, 2017, at 03:52 PM, Robert Scholte wrote:
> > > > > > >> > > > Let's do it like this:
> > > > > > >> > > >
> > > > > > >> > >
> > > > > > >>
> https://cwiki.apache.org/confluence/download/attachments/2329841/
> > > > > classrealms.pdf?api=v2
> > > > > > >> > > >
> > > > > > >> > > > Robert
> > > > > > >> > > >
> > > > > > >> > > > On Tue, 19 Sep 2017 21:08:39 +0200, Stephen Connolly
> > > > > > >> > > > <[hidden email]> wrote:
> > > > > > >> > > >
> > > > > > >> > > > > I think you will need a link to the PDF as
> attachments are
> > > > > > >> stripped
> > > > > > >> > > from
> > > > > > >> > > > > the ML
> > > > > > >> > > > >
> > > > > > >> > > > > On Tue 19 Sep 2017 at 19:57, Robert Scholte
> > > > > > >> <[hidden email]>
> > > > > > >> > > wrote:
> > > > > > >> > > > >
> > > > > > >> > > > >> Attached a single page overview.
> > > > > > >> > > > >>
> > > > > > >> > > > >> Per block you'll see in the upper left corner the
> > > executed
> > > > > > >> plugin
> > > > > > >> > > > >> The left column contains the extensions and plugin in
> > > orderas
> > > > > > >> > > specified
> > > > > > >> > > > >> in
> > > > > > >> > > > >> the pom.xml
> > > > > > >> > > > >> In every classloadercolumn you'll see numbers which
> > > represent
> > > > > > >> the
> > > > > > >> > > order.
> > > > > > >> > > > >>
> > > > > > >> > > > >> I hope I didn't make any mistakes.
> > > > > > >> > > > >> Tomorrow I have enough time to see if I understand
> what's
> > > > > > >> happening
> > > > > > >> > > > >> here.
> > > > > > >> > > > >>
> > > > > > >> > > > >> I will come back with my conclusions.
> > > > > > >> > > > >>
> > > > > > >> > > > >> Robert
> > > > > > >> > > > >>
> > > > > > >> > > > >> On Tue, 19 Sep 2017 06:55:08 +0200, Igor Fedorenko <
> > > > > > >> > > [hidden email]>
> > > > > > >> > > > >> wrote:
> > > > > > >> > > > >>
> > > > > > >> > > > >> > TL;DR your test project exposed two existing bugs,
> one
> > > > > > >> change in
> > > > > > >> > > > >> > behaviour and one quirk I can't explain
> > > > > > >> > > > >> >
> > > > > > >> > > > >> > * Build `<extensions>` are loaded by two
> classloaders,
> > > > > which
> > > > > > >> is
> > > > > > >> a
> > > > > > >> > > bug
> > > > > > >> > > > >> in
> > > > > > >> > > > >> > DefaultProjectBuildingHelper#createProjectRealm and
> > > > > explains
> > > > > > >> why you
> > > > > > >> > > > >> see
> > > > > > >> > > > >> > extjar1/extjar2 in the output
> > > > > > >> > > > >> > * ClassRealm does not allow same foreign-import
> from
> > > > > multiple
> > > > > > >> > > > >> > classloaders, which is a bug and explains why it
> is not
> > > > > > >> possible to
> > > > > > >> > > > >> load
> > > > > > >> > > > >> > same resource from multiple plugins/extensions
> > > > > > >> > > > >> > * TCCL does not have access to private (i.e. not
> > > exported)
> > > > > > >> resources
> > > > > > >> > > > >> of
> > > > > > >> > > > >> > this extensions plugin, which is a change of
> behaviour
> > > > > > >> introduced by
> > > > > > >> > > > >> > mng-6209 fix
> > > > > > >> > > > >> > * Also, component injection order appears to be
> > > backwards,
> > > > > > >> but
> > > > > > >> maybe
> > > > > > >> > > > >> > Stuart can explain why.
> > > > > > >> > > > >> >
> > > > > > >> > > > >> >
> > > > > > >> > > > >> > Below is more detailed explanation of expected and
> > > observed
> > > > > > >> > > behaviour
> > > > > > >> > > > >> >
> > > > > > >> > > > >> >
> > > > > > >> > > > >> > ## Component injection depends on the currently
> running
> > > > > > >> plugin
> > > > > > >> and
> > > > > > >> > > the
> > > > > > >> > > > >> > injection site
> > > > > > >> > > > >> >
> > > > > > >> > > > >> > Currently running plugins have access to the
> following
> > > > > > >> component
> > > > > > >> > > > >> > implementations:
> > > > > > >> > > > >> >
> > > > > > >> > > > >> > * Regular plugin has access to components
> implemented
> > > by
> > > > > the
> > > > > > >> plugin,
> > > > > > >> > > > >> > project build extensions, if any (via project class
> > > realm
> > > > > > >> foreign
> > > > > > >> > > > >> > import) and Maven Core.
> > > > > > >> > > > >> > * Extension plugin has access to components
> > > implemented by
> > > > > > >> the
> > > > > > >> > > project
> > > > > > >> > > > >> > build extensions and Maven Core.
> > > > > > >> > > > >> > * Without a running plugin (e.g., during project
> > > dependency
> > > > > > >> > > > >> resolution),
> > > > > > >> > > > >> > components implemented by the project build
> extensions
> > > and
> > > > > > >> Maven
> > > > > > >> > > Core
> > > > > > >> > > > >> > are accessible.
> > > > > > >> > > > >> >
> > > > > > >> > > > >> > Different injection sites have access to the
> following
> > > > > > >> component
> > > > > > >> > > > >> > interfaces:
> > > > > > >> > > > >> >
> > > > > > >> > > > >> > * Maven Core has access to component interfaces
> > > defined by
> > > > > > >> the
> > > > > > >> core
> > > > > > >> > > > >> > itself (obviously)
> > > > > > >> > > > >> > * Project build extensions have access to
> **public**
> > > > > > >> component
> > > > > > >> > > > >> > interfaces defined by Maven Core and component
> > > interfaces
> > > > > > >> defined by
> > > > > > >> > > > >> the
> > > > > > >> > > > >> > build extension itself (there is no way to access
> > > component
> > > > > > >> > > interfaces
> > > > > > >> > > > >> > defined in other extensions)
> > > > > > >> > > > >> > * Regular plugins have access to **public**
> component
> > > > > > >> interfaces
> > > > > > >> > > > >> defined
> > > > > > >> > > > >> > by Maven Core, component interfaces **exported** by
> > > build
> > > > > > >> extensions
> > > > > > >> > > > >> and
> > > > > > >> > > > >> > component interfaces defined in the plugin itself
> > > > > > >> > > > >> >
> > > > > > >> > > > >> > For injection to work, injection site has to have
> > > access to
> > > > > > >> the
> > > > > > >> > > > >> > component interface and the component
> implementation
> > > must
> > > > > be
> > > > > > >> > > > >> accessible
> > > > > > >> > > > >> > through the current context.
> > > > > > >> > > > >> >
> > > > > > >> > > > >> > From what I can tell, in your example all plugins
> have
> > > > > access
> > > > > > >> to the
> > > > > > >> > > > >> > right components when using current
> 3.5.2-SNAPSHOT. The
> > > > > > >> injection
> > > > > > >> > > > >> order
> > > > > > >> > > > >> > does appear to be backwards from what I expected,
> > > however.
> > > > > > >> > > > >> >
> > > > > > >> > > > >> >
> > > > > > >> > > > >> > ## Resources lookup fully depends on classpath
> > > visibility,
> > > > > > >> > > > >> specifically
> > > > > > >> > > > >> >
> > > > > > >> > > > >> > * Regular plugin class realm has access to
> resources
> > > from
> > > > > the
> > > > > > >> plugin
> > > > > > >> > > > >> > itself, from **exported** packages of the project
> build
> > > > > > >> extensions
> > > > > > >> > > and
> > > > > > >> > > > >> > **public** Maven Core packages
> > > > > > >> > > > >> > * Extensions plugin class realm has access to the
> > > resources
> > > > > > >> from the
> > > > > > >> > > > >> > extensions plugin itself and from **public** Maven
> Core
> > > > > > >> packages
> > > > > > >> > > > >> > * Project class realm has access to classes and
> > > resources
> > > > > > >> > > **exported**
> > > > > > >> > > > >> > by project build extensions and **public** Maven
> Core
> > > > > > >> packages
> > > > > > >> > > > >> >
> > > > > > >> > > > >> > I see three problems here
> > > > > > >> > > > >> >
> > > > > > >> > > > >> > * Maven adds build single-jar `<extensions>`
> elements
> > > > > > >> directly
> > > > > > >> to
> > > > > > >> > > > >> > project class realm **and** creates separate
> extensions
> > > > > class
> > > > > > >> realms
> > > > > > >> > > > >> for
> > > > > > >> > > > >> > them. Which results in duplicate classes/resources
> > > loaded
> > > > > by
> > > > > > >> two
> > > > > > >> > > > >> > classloaders and explains why you see
> extjar1/extjar2
> > > > > output
> > > > > > >> (which
> > > > > > >> > > > >> you
> > > > > > >> > > > >> > shouldn't according to the explanation above)
> > > > > > >> > > > >> > * ClassRealm does not allow foreign-import of the
> same
> > > > > > >> package
> > > > > > >> from
> > > > > > >> > > > >> > multiple classloaders. This makes it impossible to
> > > load the
> > > > > > >> same
> > > > > > >> > > > >> > resource from multiple plugins/extensions.
> > > > > > >> > > > >> > * Extensions plugins cannot access their own
> private
> > > (i.e.
> > > > > > >> not
> > > > > > >> > > > >> exported)
> > > > > > >> > > > >> > resources via TCCL, this is change in behaviour
> > > introduced
> > > > > by
> > > > > > >> > > mng-6209
> > > > > > >> > > > >> > fix
> > > > > > >> > > > >> >
> > > > > > >> > > > >> > Hope this helps
> > > > > > >> > > > >>
> > > > > > >> > > > >>
> > > > > > >>
> > > ---------------------------------------------------------------------
> > > > > > >> > > > >> To unsubscribe, e-mail:
> [hidden email]
> > > > > > >> > > > >> For additional commands, e-mail:
> > > [hidden email]
> > > > > > >> > > >
> > > > > > >> > > >
> > > > > > >>
> > > ---------------------------------------------------------------------
> > > > > > >> > > > To unsubscribe, e-mail:
> [hidden email]
> > > > > > >> > > > For additional commands, e-mail:
> [hidden email]
> > > > > > >> > > >
> > > > > > >> > >
> > > > > > >> > >
> > > > > > >>
> > > ---------------------------------------------------------------------
> > > > > > >> > > To unsubscribe, e-mail: [hidden email]
> > > > > > >> > > For additional commands, e-mail:
> [hidden email]
> > > > > > >> > >
> > > > > > >> > > --
> > > > > > >> > Sent from my phone
> > > > > > >>
> > > > > > >>
> > > ---------------------------------------------------------------------
> > > > > > >> To unsubscribe, e-mail: [hidden email]
> > > > > > >> For additional commands, e-mail: [hidden email]
> > > > > > >>
> > > > > > >> --
> > > > > > > Sent from my phone
> > > > > >
> > > > > >
> ---------------------------------------------------------------------
> > > > > > To unsubscribe, e-mail: [hidden email]
> > > > > > For additional commands, e-mail: [hidden email]
> > > > > >
> > > > >
> > > > >
> ---------------------------------------------------------------------
> > > > > To unsubscribe, e-mail: [hidden email]
> > > > > For additional commands, e-mail: [hidden email]
> > > > >
> > > > >
> > >
> > > ---------------------------------------------------------------------
> > > To unsubscribe, e-mail: [hidden email]
> > > For additional commands, e-mail: [hidden email]
> > >
> > > --
> > Sent from my phone
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: [hidden email]
> For additional commands, e-mail: [hidden email]
>
> --
Sent from my phone