Re: RFR JDK-8044627: Update JNDI to work with modules

2014-10-14 Thread Pavel Rappo
Thanks a lot! -Pavel On 14 Oct 2014, at 15:33, Chris Hegarty wrote: > META-INF files in the webrev, two of which are in the wrong location. They > are directly under 'META-INF’, where they should all be under > ‘META-INF/services’. This is just a note for Pavel, when he follows up later > wi

Re: RFR JDK-8044627: Update JNDI to work with modules

2014-10-14 Thread Chris Hegarty
On 14 Oct 2014, at 15:15, Daniel Fuchs wrote: > On 14/10/14 16:09, Chris Hegarty wrote: >> On 14 Oct 2014, at 15:03, Pavel Rappo wrote: >> >>> OK, so what I will do for now is I exclude these 4 files and push without >>> them. I'll create a new issue to add them later. >> >> That sounds like

Re: RFR JDK-8044627: Update JNDI to work with modules

2014-10-14 Thread Daniel Fuchs
On 14/10/14 16:09, Chris Hegarty wrote: On 14 Oct 2014, at 15:03, Pavel Rappo wrote: OK, so what I will do for now is I exclude these 4 files and push without them. I'll create a new issue to add them later. That sounds like a fine plan. This issue has already gone on for long enough, and

Re: RFR JDK-8044627: Update JNDI to work with modules

2014-10-14 Thread Chris Hegarty
On 14 Oct 2014, at 15:03, Pavel Rappo wrote: > OK, so what I will do for now is I exclude these 4 files and push without > them. I'll create a new issue to add them later. That sounds like a fine plan. This issue has already gone on for long enough, and I don’t think that the crooks of the cha

Re: RFR JDK-8044627: Update JNDI to work with modules

2014-10-14 Thread Pavel Rappo
OK, so what I will do for now is I exclude these 4 files and push without them. I'll create a new issue to add them later. -Pavel On 14 Oct 2014, at 14:44, Alan Bateman wrote: > On 14/10/2014 14:34, Daniel Fuchs wrote: >> Hi Pavel, >> >> I saw your mail on build-dev. >> I guess the issue will

Re: RFR JDK-8044627: Update JNDI to work with modules

2014-10-14 Thread Pavel Rappo
Here's the updated webrev: http://cr.openjdk.java.net/~prappo/8044627/webrev.01/ -Pavel On 22 Sep 2014, at 09:55, Alan Bateman wrote: > On 16/09/2014 12:12, Pavel Rappo wrote: >> Hi everyone, >> >> Could you please review my change for JDK-8044627? >> > Pavel - are you planning to send an upd

Re: RFR JDK-8044627: Update JNDI to work with modules

2014-09-22 Thread Alan Bateman
On 16/09/2014 12:12, Pavel Rappo wrote: Hi everyone, Could you please review my change for JDK-8044627? Pavel - are you planning to send an updated webrev based on the discussion so far? The other thing that I meant to ask is whether this change will add service configuration files for the

Re: RFR JDK-8044627: Update JNDI to work with modules

2014-09-16 Thread Pavel Rappo
Daniel, > So is it expected that modules (e.g. java.corba) will register > their own service provider for the InitialContextFactory > (I mean - using META-INF/services/)? Alan's already answered this point. TCCL is the way to go. You are right. > The difference however is that in this two case

Re: RFR JDK-8044627: Update JNDI to work with modules

2014-09-16 Thread Daniel Fuchs
On 9/16/14 4:14 PM, Pavel Rappo wrote: Daniel, Given that helper.loadClass uses the context class loader, Should you also simply use ServiceLoader loader = ServiceLoader.load(InitialContextFactory.class); at lines 680-681 ? It needs to be the system class loader to allo

Re: RFR JDK-8044627: Update JNDI to work with modules

2014-09-16 Thread Alan Bateman
On 16/09/2014 15:14, Pavel Rappo wrote: Daniel, Given that helper.loadClass uses the context class loader, Should you also simply use ServiceLoader loader = ServiceLoader.load(InitialContextFactory.class); at lines 680-681 ? It needs to be the system class loader to allo

Re: RFR JDK-8044627: Update JNDI to work with modules

2014-09-16 Thread Pavel Rappo
Daniel, > Given that helper.loadClass uses the context class loader, > Should you also simply use > ServiceLoader loader = > ServiceLoader.load(InitialContextFactory.class); > at lines 680-681 ? It needs to be the system class loader to allow for JNDI providers that might be

Re: RFR JDK-8044627: Update JNDI to work with modules

2014-09-16 Thread Daniel Fuchs
On 9/16/14 1:12 PM, Pavel Rappo wrote: Hi everyone, Could you please review my change for JDK-8044627? http://cr.openjdk.java.net/~prappo/8044627/webrev.00/ -Pavel Hi Pavel, Given that helper.loadClass uses the context class loader, Should you also simply use ServiceLoader loader =

RFR JDK-8044627: Update JNDI to work with modules

2014-09-16 Thread Pavel Rappo
Hi everyone, Could you please review my change for JDK-8044627? http://cr.openjdk.java.net/~prappo/8044627/webrev.00/ -Pavel