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

[harold seigel]

Hi,

Please review the latest version of this change.  It is similar to the 
below change except it changes uses of -mp to -p.

http://cr.openjdk.java.net/~hseigel/bug_8136930.hs3/

Thanks! Harold


On 8/4/2016 8:46 AM, harold seigel wrote:
>
> Hi,
>
> Please review this update for this fix.  This webrev only shows the 
> changes since the last webrev.  These changes include:
>
>  1. Fix forJDK-8162415
>     <https://bugs.openjdk.java.net/browse/JDK-8162415> - the JVM now
>     prints the following message when ignoring a property and
>     PrintWarnings is enabled:
>     warning: Ignoring system property options whose names start with
>     '-Djdk.module'.  They are reserved for internal use.
>
>  2. Fix for JDK-8162412
>     <https://bugs.openjdk.java.net/browse/JDK-8162412>
>  3. Fixes a problem where JVMTI was failing two JCK vm/jvmti tests
>
>  4. Incorporates review comments from Alan, Coleen, Dan, and Lois
>  5. Fixes JTReg tests that failed due to the new option syntax.
>
> Revised webrev: http://cr.openjdk.java.net/~hseigel/bug_8136930.hs2/
>
> Thanks, Harold
>
> On 8/2/2016 9:25 AM, harold seigel wrote:
>>
>> Hi Lois,
>>
>> Thanks for the review.  Please see comments in-line.
>>
>> Harold
>>
>>
>> On 8/1/2016 8:40 PM, Lois Foltan wrote:
>>>
>>> On 7/17/2016 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/
>>>
>>> Hi Harold,
>>>
>>> Overall looks good.  A couple of comments:
>>>
>>> src/share/vm/prims/jvmtiEnv.cpp
>>> - line #3428 - The if statement is incorrect.  There are internal 
>>> properties, like jdk.boot.class.path.append, whose value if non-null 
>>> should be returned.
>> This code will be reworked in the next version of these changes 
>> because of multiple issues.
>>>
>>> src/share/vm/runtime/arguments.cpp
>>> - Arguments::append_to_addmods_property was added before the VM 
>>> starting to process --add-modules.  So with this fix, it seems like 
>>> it could be simply changed to:
>>>
>>>     bool Arguments::append_to_addmods_property(const char*
>>>     module_name) {
>>>       PropertyList_unique_add(&_system_properties,
>>>     Arguments::get_property("jdk.module.addmods"),
>>>                                                    module_name,
>>>     AppendProperty, UnwriteableProperty, InternalProperty);
>>>     }
>>>
>>> Please consider making this change since currently it contains a lot 
>>> of duplicated code that is now unnecessary.
>> The one difference is that append_to_addmods_property() returns a 
>> status but PropertyList_unique_add() does not.  I'll look into this a 
>> bit further.
>>>
>>> - line #3171, should the comment be "--add-modules=java.sql" instead 
>>> of "--add-modules java.sql"?
>> yes.
>>
>> The changes suggested by you, Coleen, and Dan will be in the next 
>> version of this webrev.
>>
>> Thanks, Harold
>>>
>>> Thanks,
>>> Lois
>>>
>>>
>>>
>>>
>>>
>>>
>>>
>>>
>>>
>>>
>>>
>>>
>>>
>>>>
>>>> The changes were tested with the JCK lang and VM tests, the JTreg 
>>>> hotspot tests, and the RBT hotspot nightlies.
>>>>
>>>> Thanks, Harold
>>>
>>
>