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
> 
> 

Reply via email to