RFR: JDK-8203891: Upgrade JOpt Simple to 5.0.4
Hi, I'd like to upgrade the JOpt Simple library we are using to version 5.0.4. Bug: https://bugs.openjdk.java.net/browse/JDK-8203891 Complete webrev: http://cr.openjdk.java.net/~jlahoda/8203891/webrev.00/complete/ Delta webrev only showing (all) JDK changes in JOpt Simple and related changes in tests needed for the upgrade, etc.: http://cr.openjdk.java.net/~jlahoda/8203891/webrev.00/joptsimple.delta/ Probably the biggest issue with this upgrade is that for two subsequent parameters: "--libs=", "/tmp" "/tmp" used to be interpreted as the parameter of "libs", but now the "libs" parameter is empty (as there's nothing behind the '='). See the changes to test/jdk/tools/jmod/JmodTest.java for an example. Hopefully, this is a reasonable change. How does this look? Thanks, Jan
On 31/05/2018 10:11, Jan Lahoda wrote:
Hi,
I'd like to upgrade the JOpt Simple library we are using to version 5.0.4.
Bug: https://bugs.openjdk.java.net/browse/JDK-8203891 Complete webrev: http://cr.openjdk.java.net/~jlahoda/8203891/webrev.00/complete/
Delta webrev only showing (all) JDK changes in JOpt Simple and related changes in tests needed for the upgrade, etc.: http://cr.openjdk.java.net/~jlahoda/8203891/webrev.00/joptsimple.delta/
Probably the biggest issue with this upgrade is that for two subsequent parameters: "--libs=", "/tmp" "/tmp" used to be interpreted as the parameter of "libs", but now the "libs" parameter is empty (as there's nothing behind the '='). See the changes to test/jdk/tools/jmod/JmodTest.java for an example. Hopefully, this is a reasonable change.
How does this look? Surprising to see that jmod needs to be updated too but I think it's okay, the update to the jmods tests too.
One question - does the update to make/CompileJavaModules.gmk mean that we were missing the JOpt Simple properties file from the run-time image? -Alan.
On 4.6.2018 17:00, Alan Bateman wrote:
On 31/05/2018 10:11, Jan Lahoda wrote:
Hi,
I'd like to upgrade the JOpt Simple library we are using to version 5.0.4.
Bug: https://bugs.openjdk.java.net/browse/JDK-8203891 Complete webrev: http://cr.openjdk.java.net/~jlahoda/8203891/webrev.00/complete/
Delta webrev only showing (all) JDK changes in JOpt Simple and related changes in tests needed for the upgrade, etc.: http://cr.openjdk.java.net/~jlahoda/8203891/webrev.00/joptsimple.delta/
Probably the biggest issue with this upgrade is that for two subsequent parameters: "--libs=", "/tmp" "/tmp" used to be interpreted as the parameter of "libs", but now the "libs" parameter is empty (as there's nothing behind the '='). See the changes to test/jdk/tools/jmod/JmodTest.java for an example. Hopefully, this is a reasonable change.
How does this look? Surprising to see that jmod needs to be updated too but I think it's okay, the update to the jmods tests too.
I believe the API has been changed to use List instead of Collection on a few places here: https://github.com/jopt-simple/jopt-simple/commit/b8f859ac95a37dcb9abb084fe2... So the changes to jmod reflect that.
One question - does the update to make/CompileJavaModules.gmk mean that we were missing the JOpt Simple properties file from the run-time image?
I believe the properties files have not been part of 4.6, the messages have been hardcoded in the classfiles. So the need to copy them is new. Thanks, Jan
-Alan.
Hi Jan, On 5/31/18 2:11 AM, Jan Lahoda wrote:
Hi,
I'd like to upgrade the JOpt Simple library we are using to version 5.0.4.
Bug: https://bugs.openjdk.java.net/browse/JDK-8203891 Complete webrev: http://cr.openjdk.java.net/~jlahoda/8203891/webrev.00/complete/
Delta webrev only showing (all) JDK changes in JOpt Simple and related changes in tests needed for the upgrade, etc.: http://cr.openjdk.java.net/~jlahoda/8203891/webrev.00/joptsimple.delta/ > Probably the biggest issue with this upgrade is that for two subsequent parameters: "--libs=", "/tmp" "/tmp" used to be interpreted as the parameter of "libs", but now the "libs" parameter is empty (as there's nothing behind the '=').
This is a reasonable fix and no whitespace is expected following `=`.
See the changes to test/jdk/tools/jmod/JmodTest.java for an example. Hopefully, this is a reasonable change.
114 "--libs=" + libDir.toString(), Alternatively, you can take out `=` and run it as jmod --libs /tmp "--libs", libDir.toString(), Mandy
On 4.6.2018 19:40, mandy chung wrote:
Hi Jan,
On 5/31/18 2:11 AM, Jan Lahoda wrote:
Hi,
I'd like to upgrade the JOpt Simple library we are using to version 5.0.4.
Bug: https://bugs.openjdk.java.net/browse/JDK-8203891 Complete webrev: http://cr.openjdk.java.net/~jlahoda/8203891/webrev.00/complete/
Delta webrev only showing (all) JDK changes in JOpt Simple and related changes in tests needed for the upgrade, etc.: http://cr.openjdk.java.net/~jlahoda/8203891/webrev.00/joptsimple.delta/ > Probably the biggest issue with this upgrade is that for two subsequent parameters: "--libs=", "/tmp" "/tmp" used to be interpreted as the parameter of "libs", but now the "libs" parameter is empty (as there's nothing behind the '=').
This is a reasonable fix and no whitespace is expected following `=`.
See the changes to test/jdk/tools/jmod/JmodTest.java for an example. Hopefully, this is a reasonable change.
114 "--libs=" + libDir.toString(),
Alternatively, you can take out `=` and run it as jmod --libs /tmp "--libs", libDir.toString(),
Yes, that would work as well, but there are already invocations like this in the test. So I opted for using "--libs=" + ... and similar, so that both variants are covered by the test. Thanks, Jan
Mandy
On 6/5/18 12:29 AM, Jan Lahoda wrote:
Yes, that would work as well, but there are already invocations like this in the test. So I opted for using "--libs=" + ... and similar, so that both variants are covered by the test.
That is fine to keep it as is. The change looks good. Mandy
On 31/05/18 10:11, Jan Lahoda wrote:
Hi,
I'd like to upgrade the JOpt Simple library we are using to version 5.0.4.
Bug: https://bugs.openjdk.java.net/browse/JDK-8203891 Complete webrev: http://cr.openjdk.java.net/~jlahoda/8203891/webrev.00/complete/
Delta webrev only showing (all) JDK changes in JOpt Simple and related changes in tests needed for the upgrade, etc.: http://cr.openjdk.java.net/~jlahoda/8203891/webrev.00/joptsimple.delta/
Probably the biggest issue with this upgrade is that for two subsequent parameters: "--libs=", "/tmp" "/tmp" used to be interpreted as the parameter of "libs", but now the "libs" parameter is empty (as there's nothing behind the '='). See the changes to test/jdk/tools/jmod/JmodTest.java for an example. Hopefully, this is a reasonable change.
How does this look?
Thanks Jan, I think this looks good. -Chris.
participants (4)
-
Alan Bateman
-
Chris Hegarty
-
Jan Lahoda
-
mandy chung