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

[Coleen Phillimore]

http://cr.openjdk.java.net/~hseigel/bug_8136930.hs3/src/share/vm/prims/jvmtiEnv.cpp.udiff.html

*!_char **tmp_value = *property_ptr+readable_count++_;*


This looks really painful to me.  Can you add some parentheses?  Or put 
readable_count++ on the next statement.

Is this an incremental webrev?  Can you send a full one?
thanks,
Coleen


On 8/8/16 1:25 PM, harold seigel wrote:
> 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
>>>>
>>>
>>
>