I'm with Galen on this one. As nice as it would be to be able to say "This is what we'll do," I don't think any of these is going to be the proverbial silver bullet.
I think we should, in priority order, (#1) remove what restricted API calls we can, (#2) adopt the module system and properly declare our dependencies on APIs that we do need, and (#3) update our wrappers where me must, perhaps in cases like Dan has noticed, where it's not even our own consumption of an internal API that would be problematic. On Thu, Oct 11, 2018 at 9:49 AM, Dan Smith <dsm...@pivotal.io> 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? > > For #2, maybe we don't have to go whole hog on adding module-info to all of > our modules right away. We can identify and isolate our use of internal JDK > classes to some small modules with few dependencies. For example, we could > move our use of Unsafe to a geode-off-heap module and just add a > module-info for that. We probably ought to have that code in a separate > module anyway! > > -Dan > > On Wed, Oct 10, 2018 at 2:59 PM Jinmei Liao <jil...@pivotal.io> wrote: > > > #3 first and then work towards #1, then #2. We can document what > > "--add-open" options needs to be added to start client if running with > > jdk11+ in the meantime. > > > > I also have reservations about how #4 works. If it works, it is a good > > alternation for #3. > > > > On Wed, Oct 10, 2018 at 2:23 PM Sai Boorlagadda < > sai.boorlaga...@gmail.com > > > > > wrote: > > > > > +1 to Dan's idea if its possible. > > > > > > There is a maven plugin to invoke javac twice with respective java > > targets. > > > > > > > > https://maven.apache.org/plugins/maven-compiler-plugin/ > examples/module-info.html > > > > > > > > > On Wed, Oct 10, 2018 at 1:52 PM Galen O'Sullivan < > gosulli...@pivotal.io> > > > wrote: > > > > > > > er, lost the end of that first sentence there. I think that reducing > > > > dependencies on Unsafe &c is a good goal regardless. > > > > > > > > On Wed, Oct 10, 2018 at 1:51 PM Galen O'Sullivan < > > gosulli...@pivotal.io> > > > > wrote: > > > > > > > > > #1 sounds awesome but may be unrealistic given our advertised > feature > > > > set. > > > > > I think that reducing dependencies on Unsafe &c > > > > > > > > > > If we can conditionally use Jigsaw modules for Java versions later > > > than 8 > > > > > while maintaining Java 8 compatiblity, that seems like the best > > > solution. > > > > > > > > > > +2 to Dan's idea if it allows this. > > > > > > > > > > On Wed, Oct 10, 2018 at 1:47 PM Jacob Barrett <jbarr...@pivotal.io > > > > > > wrote: > > > > > > > > > >> Here is a discussion from Google Guava project about compiling > > > > >> module-info.java in Java 9+ and including it in a jar with classes > > > > >> compiled > > > > >> for Java 8. > > > > >> > > > > >> https://github.com/google/guava/issues/2970 > > > > >> > > > > >> > > > > >> On Wed, Oct 10, 2018 at 1:39 PM Jacob Barrett < > jbarr...@pivotal.io> > > > > >> wrote: > > > > >> > > > > >> > I like Dan’s idea! I would rather we work towards the correct > > > > solution. > > > > >> > > > > > >> > > On Oct 10, 2018, at 1:22 PM, Dan Smith <dsm...@pivotal.io> > > wrote: > > > > >> > > > > > > >> > > #2 seems like the least hacky way to continue using things > like > > > > >> > > sun.misc.Unsafe. Could we just compile a module-info.java with > > > Java > > > > 9 > > > > >> and > > > > >> > > bundle it? This would also help consumers of geode that want > to > > > use > > > > >> Java > > > > >> > 9 > > > > >> > > modules. > > > > >> > > > > > > >> > > I'm a little bit sceptical of this permit-reflect libary, > seeing > > > as > > > > >> it's > > > > >> > > been around for about 1 month, has 0 tests in the source that > I > > > can > > > > >> see, > > > > >> > > and seems to be tripling down on relying on sun.misc.Unsafe to > > do > > > > >> stuff. > > > > >> > > I'd be inclined to do #3 before this. > > > > >> > > > > > > >> > > -Dan > > > > >> > > > > > > >> > > On Wed, Oct 10, 2018 at 12:20 PM Owen Nichols < > > > onich...@pivotal.io> > > > > >> > wrote: > > > > >> > > > > > > >> > >> Goal: > > > > >> > >> > > > > >> > >> Run Geode on Java 11 (GEODE-3 < > > > > >> > >> https://issues.apache.org/jira/browse/GEODE-3>). > > > > >> > >> > > > > >> > >> > > > > >> > >> Problem: > > > > >> > >> > > > > >> > >> Java 8 allows Geode (and its 3rd party libraries) full access > > to > > > > all > > > > >> > Java > > > > >> > >> APIs, including internal APIs. However, Java 11 restricts > > access > > > > to > > > > >> > many > > > > >> > >> of these APIs by default. > > > > >> > >> > > > > >> > >> > > > > >> > >> Solution #1: > > > > >> > >> > > > > >> > >> Remove all usage of restricted APIs from all Geode code, and > > find > > > > >> > >> replacements for all 3rd party libraries that depend on > > > restricted > > > > >> APIs. > > > > >> > >> > > > > >> > >> Solution #2: > > > > >> > >> > > > > >> > >> Adopt Java 11’s “Jigsaw" Module System and properly declare > > > > >> dependencies > > > > >> > >> on restricted APIs. > > > > >> > >> > > > > >> > >> Solution #3: > > > > >> > >> > > > > >> > >> Update all existing public and personal scripts, wrappers, > IDE > > > > >> > >> configurations, test harnesses, and other java invocations to > > > add a > > > > >> > handful > > > > >> > >> of --add-opens flags to the java commandline to override the > > > > default > > > > >> > Java > > > > >> > >> 11 restrictions. > > > > >> > >> > > > > >> > >> Solution #4: > > > > >> > >> > > > > >> > >> Use the MIT-licensed permit-reflect < > > > > >> > >> https://github.com/nqzero/permit-reflect> library to > > > > >> programmatically > > > > >> > >> override Java 11’s API restrictions. > > > > >> > >> > > > > >> > >> > > > > >> > >> In terms of feasibility: > > > > >> > >> #1 would be extremely difficult. Geode has a large number of > > > > >> > dependencies > > > > >> > >> on internal Java APIs in critical areas, and replacing them > > would > > > > be > > > > >> > >> time-consuming, potentially destabilizing, and very likely to > > > > >> negatively > > > > >> > >> impact performance. > > > > >> > >> #2 is complex because we still need Geode to run on Java 8, > so > > > not > > > > >> using > > > > >> > >> any Java 11 features seems safer than introducing > multi-version > > > > jars, > > > > >> > >> cross-compilation, or separate releases per target Java > > platform. > > > > >> > >> #3 is easy enough to implement in scripts that are under > source > > > > >> control, > > > > >> > >> but users or developers that have their own IDE > configurations > > or > > > > >> test > > > > >> > >> environments may struggle to understand why they are getting > > > errors > > > > >> and > > > > >> > how > > > > >> > >> to fix them. > > > > >> > >> #4 restores full Java8-like permissions with essentially > just a > > > > >> change > > > > >> > to > > > > >> > >> main() method. > > > > >> > >> > > > > >> > >> > > > > >> > >> Which strategy do you prefer? Java 11 test jobs are in the > > > > pipeline > > > > >> < > > > > >> > >> > > > https://concourse.apachegeode-ci.info/teams/main/pipelines/develop > > > > > > > > > >> as > > > > >> > of > > > > >> > >> today — let’s make them green! > > > > >> > >> > > > > >> > >> > > > > >> > >> -Owen > > > > >> > > > > > >> > > > > > >> > > > > > > > > > > > > > > > > > > -- > > Cheers > > > > Jinmei > > >
