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
