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 >
