On Sun 24 Sep 2017 at 17:44, Robert Scholte <rfscho...@apache.org> 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) > > thanks, > Robert > > > On Sun, 24 Sep 2017 18:07:39 +0200, Stephen Connolly > <stephen.alan.conno...@gmail.com> 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 <i...@ifedorenko.com> > > 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 > >> > <stephen.alan.conno...@gmail.com> wrote: > >> > > >> > > On Wed 20 Sep 2017 at 01:29, Igor Fedorenko <i...@ifedorenko.com> > >> 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 <i...@ifedorenko.com > > > >> > >> 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 > >> > >> > > > <stephen.alan.conno...@gmail.com> 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 > >> > >> <rfscho...@apache.org> > >> > >> > > 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 < > >> > >> > > i...@ifedorenko.com> > >> > >> > > > >> 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: dev-unsubscr...@maven.apache.org > >> > >> > > > >> For additional commands, e-mail: > dev-h...@maven.apache.org > >> > >> > > > > >> > >> > > > > >> > >> > >> --------------------------------------------------------------------- > >> > >> > > > To unsubscribe, e-mail: dev-unsubscr...@maven.apache.org > >> > >> > > > For additional commands, e-mail: dev-h...@maven.apache.org > >> > >> > > > > >> > >> > > > >> > >> > > > >> > >> > >> --------------------------------------------------------------------- > >> > >> > > To unsubscribe, e-mail: dev-unsubscr...@maven.apache.org > >> > >> > > For additional commands, e-mail: dev-h...@maven.apache.org > >> > >> > > > >> > >> > > -- > >> > >> > Sent from my phone > >> > >> > >> > >> > >> --------------------------------------------------------------------- > >> > >> To unsubscribe, e-mail: dev-unsubscr...@maven.apache.org > >> > >> For additional commands, e-mail: dev-h...@maven.apache.org > >> > >> > >> > >> -- > >> > > Sent from my phone > >> > > >> > --------------------------------------------------------------------- > >> > To unsubscribe, e-mail: dev-unsubscr...@maven.apache.org > >> > For additional commands, e-mail: dev-h...@maven.apache.org > >> > > >> > >> --------------------------------------------------------------------- > >> To unsubscribe, e-mail: dev-unsubscr...@maven.apache.org > >> For additional commands, e-mail: dev-h...@maven.apache.org > >> > > --------------------------------------------------------------------- > To unsubscribe, e-mail: dev-unsubscr...@maven.apache.org > For additional commands, e-mail: dev-h...@maven.apache.org > > -- Sent from my phone
