RFR(L) 8136930: Simplify use of module-system options by custom launchers

[harold seigel]

Hi Coleen,

Thanks for reviewing this.  I added the following two bugs, as you 
requested:

https://bugs.openjdk.java.net/browse/JDK-8162412

https://bugs.openjdk.java.net/browse/JDK-8162401

Harold


On 7/21/2016 5:40 PM, Coleen Phillimore wrote:
>
>
> On 7/21/16 5:37 PM, Coleen Phillimore wrote:
>>
>>
>> On 7/21/16 1:39 PM, harold seigel wrote:
>>> Hi Coleen,
>>>
>>> Thank you for reviewing my changes!
>>>
>>> Please see comments inline.
>>>
>>>
>>> On 7/20/2016 5:47 PM, Coleen Phillimore wrote:
>>>>
>>>> http://cr.openjdk.java.net/~hseigel/bug_8136930.hs/src/share/vm/runtime/arguments.cpp.udiff.html 
>>>>
>>>>
>>>> To check if is_internal_module_property, the numbered ones go to 
>>>> is_matching_numbered_property, but do you also want to disallow:
>>>>
>>>> jdk.module.addreads.foo   or jdk.module.addreads (with no number?)
>>> We don't ignore properties like these because they do not have 
>>> special meaning to the jdk.  I didn't want to ignore properties just 
>>> because they are very similar to ones that do get ignored.
>>
>> Don't we own the jdk.module namespace and from these patterns it's 
>> clear that it's an error, although we won't flag it, and somebody 
>> might get results that are unexpected.
>>
>> I think you should flag these.
>
> You could make this a follow-on bug.
>
> Coleen
>
>>
>>>>
>>>> Why aren't addmods and limitmods allowed be specified more than 
>>>> once on the command line and handled like the other ones?
>>> Please see Alan's response.  Should I open an RFE for allowing 
>>> multiple --add-modules options?
>>
>> Yes, and you can discuss it there.
>>
>>>>
>>>> + // options. For example: use "--add-modules java.sql", not 
>>>> "-Djdk.module.addmods=java.sql"
>>>>
>>>> Does this comment need an equal sign?
>>> Yes, done.
>>>>
>>>> http://cr.openjdk.java.net/~hseigel/bug_8136930.hs/test/compiler/unsafe/UnsafeGetConstantField.java.udiff.html 
>>>>
>>>>
>>>> Can you fix the indentation?
>>> Done.
>>>>
>>>> We should add an option -XX:+UseUnsafe to be a shortcut for this:
>>>>
>>>> --add-exports=java.base/jdk.internal.misc=ALL-UNNAMED
>>> Please see Alan's response.
>>
>> I was looking at all the tests and was wondering if there's a way for 
>> some other option to imply these -add-exports options, so we wouldn't 
>> need so many.  But the patterns aren't all the same in all the tests, 
>> except the ALL-UNNAMED bit.
>>
>> I've Reviewed this now.
>>
>> Thanks,
>> Coleen
>>
>>
>>>>
>>>>
>>>>
>>>> The rest looks good.   This implementation looks very clean.
>>> Thanks! Harold
>>>>
>>>> Thanks,
>>>> Coleen
>>>>
>>>>
>>>> On 7/17/16 7:05 PM, harold seigel wrote:
>>>>> Hi,
>>>>>
>>>>> Please review these Hotspot VM only changes to process the seven 
>>>>> module-specific options that have been renamed to have gnu-like 
>>>>> names.  JDK changes for this bug will be reviewed separately.
>>>>>
>>>>> Descriptions of these options are here 
>>>>> <http://openjdk.java.net/jeps/293>.  For these six options, 
>>>>> --module-path, --upgrade-module-path, --add-modules, 
>>>>> --limit-modules, --add-reads, and --add-exports, the JVM just sets 
>>>>> a system property.  For the --patch-module option, the JVM sets a 
>>>>> system property and then processes the option in the same way as 
>>>>> when it was named -Xpatch.
>>>>>
>>>>> Additionally, the JVM now checks properties specified on the 
>>>>> command line.  If a property matches one of the properties used by 
>>>>> one of the above options then the JVM ignores the property. This 
>>>>> forces users to use the explicit option when wanting to do things 
>>>>> like add a module or a package export.
>>>>>
>>>>> The RFR contains two new tests.  Also, many existing tests were 
>>>>> changed to use the new option names.
>>>>>
>>>>> JBS Bug: https://bugs.openjdk.java.net/browse/JDK-8136930
>>>>>
>>>>> Webrev: http://cr.openjdk.java.net/~hseigel/bug_8136930.hs/
>>>>>
>>>>> The changes were tested with the JCK lang and VM tests, the JTreg 
>>>>> hotspot tests, and the RBT hotspot nightlies.
>>>>>
>>>>> Thanks, Harold
>>>>
>>>
>>
>