RFR(L) 8136930: Simplify use of module-system options by custom launchers
harold seigel
harold.seigel at oracle.com
Thu Aug 4 12:46:21 UTC 2016
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
>>
>
More information about the jigsaw-dev
mailing list