hi Harold, The changes look fine. I have a couple of questions though:
#1 Would the "test/runtime/modules/ModuleOptionsTest.java” be more robust if we included a case with a non-error return value that takes more than one module? Ex: “java --add-modules=java.base --add-modules=jdk.internal.reflect=ALL-UNNAMED -version” #2 Looking at the changes for jdk it appears we now also allow duplicate "--add-exports” and "--add-reads"? Does it also mean we allow duplicate "--add-modules”, Ex: “java --add-modules=java.base --add-modules=java.base -version”? cheers > On Sep 8, 2016, at 8:23 AM, harold seigel <harold.sei...@oracle.com> wrote: > > Hi, > > Please review this fix for JDK-8165634. The fix changes the --add-modules > option from being a 'last one wins' option to a cumulative one. With this > change, if multiple --add-modules options are specified, the VM accumulates > all the options' values, instead of ignoring all but the last option's value. > The --add-modules values are reported back to the JDK as properties using > the Arguments::create_numbered_property() function. > > JBS Bug: https://bugs.openjdk.java.net/browse/JDK-8165634 > > Open webrevs: > > http://cr.openjdk.java.net/~hseigel/bug_8165634.hs/ > > http://cr.openjdk.java.net/~hseigel/bug_8165634.jdk/ > > The fix was tested with the JCK Lang and VM tests, the hotpot, and java/lang, > java/util and other JTreg tests, the RBT tier2 tests, and the NSK > non-colocated quick tests. > > The JDK changes were done by Mandy Chung (mchung). > > Thanks, Harold > >