Dan, are you saying we do or do not need --add-opens for reflection? The opens directive is for open up classes in its own modules for reflection. I don't think we can use that to open up jdk's packages, can we?
On Fri, Nov 2, 2018 at 3:52 PM Dan Smith <dsm...@pivotal.io> wrote: > I guess I'm ok with documenting the --add-opens workaround in the short > term, so that users can start playing with Geode on JDK 11. Maybe we should > indicate the JDK 11 support is experimental. I guess in addition to that, > we are going to document that users should put Geode on the classpath and > not the module path? Has anyone tested putting Geode on the module path? > > There are actually two ways to reserve a module name - you can create the > module-info.java, OR you can just add an Automatic-Module-Name header to > the your jar manifest. I think that would probably be the minimum we should > do before we start telling users to put geode on the module path. > > Regarding reflection - I don't think we don't need--add-opens for that. > Users just need to use the opens directive to mark their packages as > available for reflection. > > -Dan > > > On Fri, Nov 2, 2018 at 8:49 AM Jinmei Liao <jil...@pivotal.io> wrote: > > > Galen, you are right, --add-opens is necessary for accessing the private > > fields and methods when doing deep reflection which is used a lot in our > > serialization code. > > > > Let me step back and explain the scope of what we are trying to do here. > > We all know to be fully java11 compliant, we need to: > > 1. remove all the java internal API dependencies in geode itself > > 2. upgrade all the 3rd party libraries that was using java internal API > as > > well. > > 3. properly modularize geode and include module-info in the manifest. > > > > The bad news is: we are NOT doing any of them YET, and even if we > achieved > > all the above, from what I read, these "--add-opens" are still necessary > if > > we need to allow our code to do deep reflection at runtime. > > > > What we are trying to achieve here is: > > 1. get a green jdk11 pipeline up and running first. We need to be able to > > use jdk11 to run all our tests first, so that we can begin working on > the 3 > > things in the above list. > > 2. our users can download our code and starting running in jdk11 (with > some > > additional configuration of course), this way, we can get the community > to > > experiment with geode in jdk11 and improve upon it. > > > > We are only trying to discuss how to achieve the bottom 2 goals first > here. > > > > Cheers. > > > > On Thu, Nov 1, 2018 at 11:16 PM Galen O'Sullivan <gosulli...@pivotal.io> > > wrote: > > > > > I did a little more reading, and it sounds like we should create an > > > module-info.java to reserve the proper name for those customers who are > > > using Java 9+. See this article[1] for a description of what can go > wrong > > > if people start using the automatic package without us having declared > a > > > name. I think a module-info should be necessary for *any* level of Java > > 9+ > > > support. > > > > > > Jinmei, please correct me if I'm wrong here, but I believe --add-opens > is > > > necessary for the reflection that we use in PDX auto-serialization, and > > > probably elsewhere as well. This would make it necessary for any Java > > > program communicating with Geode that uses automatic serialization to > > have > > > --add-opens. I don't understand all that well what level of reflection > is > > > available in Java 11, but it will probably take quite a bit of time to > > do a > > > complete fix. > > > > > > +1 to this approach, provided we create a module-info.java > > > > > > [1]: > https://blog.joda.org/2017/05/java-se-9-jpms-automatic-modules.html > > > > > > > > > On Thu, Nov 1, 2018 at 10:57 AM Jinmei Liao <jil...@pivotal.io> wrote: > > > > > > > And one disclaimer I have to add is that these "--add-opens" are the > > ones > > > > uncovered by our current set of tests, there might be more needed in > > > areas > > > > that are not covered by our tests. So to say the most, our current > > jdk11 > > > > support is still in beta mode. > > > > > > > > On Thu, Nov 1, 2018 at 10:33 AM Jinmei Liao <jil...@pivotal.io> > wrote: > > > > > > > > > 1) yes, gfsh script will need to be updated to add these > > > configurations. > > > > > 2) yes, these ad-opens are required to run geode clients as well. > We > > > will > > > > > need to document them. > > > > > > > > > > On Thu, Nov 1, 2018 at 10:31 AM Dan Smith <dsm...@pivotal.io> > wrote: > > > > > > > > > >> A couple of questions: > > > > >> > > > > >> 1) Are you proposing changing gfsh start server to automatically > add > > > > these > > > > >> add-opens, or are you suggesting users will have to do that? > > > > >> 2) Are these add-opens required for running a geode client? > > > > >> > > > > >> -Dan > > > > >> > > > > >> On Thu, Nov 1, 2018 at 9:48 AM Patrick Rhomberg < > > prhomb...@apache.org > > > > > > > > >> wrote: > > > > >> > > > > >> > In case anyone else's email broke the thread, below is a link to > > the > > > > >> > previous thread in the mail archive for context. > > > > >> > > > > > >> > https://markmail.org/thread/xt224pvavxu3d54p > > > > >> > > > > > >> > On Thu, Nov 1, 2018 at 9:35 AM, Jinmei Liao <jil...@pivotal.io> > > > > wrote: > > > > >> > > > > > >> > > We will need to wrap up this discussion with a decision. Looks > > > like > > > > we > > > > >> > are > > > > >> > > skeptical about #4, and it's proven to work with #3 since our > > > > current > > > > >> > jdk11 > > > > >> > > pipeline is green with this approach. > > > > >> > > > > > > >> > > Can I propose we do #3 and document the extra configuration > > needed > > > > for > > > > >> > > jdk11 for now and then work towards #1 and #2? > > > > >> > > > > > > >> > > Here is the extra configuration to the jvm that are need to > run > > > > geode > > > > >> > under > > > > >> > > jdk11: > > > > >> > > > > > > >> > > > > > --add-opens=jdk.management/com.sun.management.internal=ALL-UNNAMED" > > > > >> > > --add-opens=java.xml/jdk.xml.internal=ALL-UNNAMED" > > > > >> > > --add-opens=java.base/jdk.internal.module=ALL-UNNAMED" > > > > >> > > --add-opens=java.base/java.lang.module=ALL-UNNAMED" > > > > >> > > > > > > >> > > comments? votes? > > > > >> > > > > > > >> > > Thanks! > > > > >> > > > > > > >> > > On Thu, Oct 11, 2018 at 10:20 AM Sai Boorlagadda < > > > > >> > > sai.boorlaga...@gmail.com> > > > > >> > > wrote: > > > > >> > > > > > > >> > > > >Do we know what third party libraries are using java > > internals > > > > that > > > > >> > >we > > > > >> > > > might > > > > >> > > > have problems with? #2 isn't going to work for those > > >libraries, > > > > >> unless > > > > >> > > > they also add a module-info.class. So maybe we >will need to > > do > > > #3 > > > > >> for > > > > >> > > > third-party libraries? > > > > >> > > > > > > > >> > > > Adding these third-party libs on module path[1] rather than > > > class > > > > >> path > > > > >> > > > seems to address this issue. > > > > >> > > > > > > > >> > > > [1] http://openjdk.java.net/projects/jigsaw/spec/sotms/# > > > > >> > > automatic-modules > > > > >> > > > > > > > >> > > > > > > >> > > > > > > >> > > -- > > > > >> > > Cheers > > > > >> > > > > > > >> > > Jinmei > > > > >> > > > > > > >> > > > > > >> > > > > > > > > > > > > > > > -- > > > > > Cheers > > > > > > > > > > Jinmei > > > > > > > > > > > > > > > > > -- > > > > Cheers > > > > > > > > Jinmei > > > > > > > > > > > > > -- > > Cheers > > > > Jinmei > > > -- Cheers Jinmei
