RFR: 8072379: Implement jdk.Version and jdk.OracleVersion
Hi. Please review the new classes jdk.Version and jdk.OracleVersion. These are simple Java APIs to parse, validate, and compare version numbers. Bug 8072379: Implement jdk.Version and jdk.OracleVersion https://bugs.openjdk.java.net/browse/JDK-8072379 Webrev http://cr.openjdk.java.net/~iris/verona/8072379/webrev.1/ JavaDoc http://cr.openjdk.java.net/~iris/verona/8072379/doc.1/jdk/Version.html http://cr.openjdk.java.net/~iris/verona/8072379/doc.1/jdk/OracleVersion.html The .java files are predominantly javadoc. The code is relatively straight-forward. jdk.Version is the representation of the JDK version string as described in JEP 223 ([0], 8061493). The javadoc is largely taken from the description section in the JEP. The API is described in the "API" section. jdk.OracleVersion extends jdk.Version and is the representation of the Oracle JDK version string. Its only purpose is to interpret the fourth element of the version number as a patch release. There are some minor discrepancies between the implementation and the JEP. The JEP will need to be updated. The most notable is the name of the package ("jdk" vs. the original "jdk.util"). The rename was recommended by Mark. Thanks, iris [0] http://openjdk.java.net/jeps/223
On 25/11/2015 01:54, Iris Clark wrote:
Hi.
Please review the new classes jdk.Version and jdk.OracleVersion. These are simple Java APIs to parse, validate, and compare version numbers.
Is jdk.OracleVersion really intended to be pushed to OpenJDK? Should it be final? There are few imports at the top that don't appear to be used. jdk.Version looks good. Has there been any thought/suggestions yet on which module this API will move too? I suspect the docs build will need to be looked at. The javadoc for jdk.Exported is currently generated in the javac tree javadoc for some reason (maybe because that annotation started out in the langtools repo). -Alan
Hi, Alan. Thanks for looking.
Is jdk.OracleVersion really intended to be pushed to OpenJDK?
That class should be available to users of the Oracle JDK. It is not applicable to OpenJDK.
Should it be final?
Yes.
There are few imports at the top that don't appear to be used.
Oops. Thought I cleaned those up. Here's the diff for both of these changes to OracleVersion: 28,33d27 < import java.util.regex.Matcher; < import java.util.regex.Pattern; < import java.util.ArrayList; < import java.util.List; < import java.util.Optional; < 86c80 < public class OracleVersion extends Version --- > public final class OracleVersion extends Version
jdk.Version looks good. Has there been any thought/suggestions yet on which module this API will move too?
No. My best guess is jdk.dev for Version but I'm willing to take suggestions. I'd prefer to keep Version and OracleVersion in the same module, but if they need to be split, I'll need to change the access modifier for the Version constructor.
I suspect the docs build will need to be looked at.
Definitely. I was surprised (but pleased) that javadoc ran on the new files, but the actual bundle surprised me. It is not correct for either jdk.Exported or these classes. I was unable to find a more suitable existing javadoc bundle. Is there one? Regards, iris -----Original Message----- From: Alan Bateman Sent: Wednesday, November 25, 2015 4:02 AM To: Iris Clark; core-libs-dev@openjdk.java.net Cc: verona-dev@openjdk.java.net Subject: Re: RFR: 8072379: Implement jdk.Version and jdk.OracleVersion On 25/11/2015 01:54, Iris Clark wrote:
Hi.
Please review the new classes jdk.Version and jdk.OracleVersion. These are simple Java APIs to parse, validate, and compare version numbers.
Is jdk.OracleVersion really intended to be pushed to OpenJDK? Should it be final? There are few imports at the top that don't appear to be used. jdk.Version looks good. Has there been any thought/suggestions yet on which module this API will move too? I suspect the docs build will need to be looked at. The javadoc for jdk.Exported is currently generated in the javac tree javadoc for some reason (maybe because that annotation started out in the langtools repo). -Alan
On Nov 24, 2015, at 5:54 PM, Iris Clark <iris.clark@oracle.com> wrote:
Hi.
Please review the new classes jdk.Version and jdk.OracleVersion. These are simple Java APIs to parse, validate, and compare version numbers.
Webrev
This looks good to me. Alan raises a good question whether jdk.OracleVersion is intended to be pushed to OpenJDK. We will need to find a home for this jdk.* API since java.base only exports Java SE API per the design principles in JEP 200 The Modular JDK. It’s fine to file an issue to follow up this. W.r.t. the docs build, jdk is listed in top/make/common/NON_CORE_PKGS.gmk but there is no target in make/Javadoc.gmk building it. You will also need to find a docs page to publish this new API as well (the current way is a page linked from the layer cake and this may be changed). Mandy
Hi, Mandy. Thanks for looking at this.
We will need to find a home for this jdk.* API since java.base only exports Java SE API per the design principles in JEP 200 The Modular JDK. It’s fine to file an issue to follow up this.
I've filed this bug and assigned it to myself: 8144062: Determine appropriate module for jdk.Version and jdk.OracleVersion https://bugs.openjdk.java.net/browse/JDK-8144062
You will also need to find a docs page to publish this new API as well (the current way is a page linked from the layer cake and this may be changed).
Yes, I'm aware of these discussions. Since I suspect that they won't be resolved within the next week or so, I've filed this bug: 8144069: Determine correct publication for jdk.Version and jdk.OracleVersion APIs https://bugs.openjdk.java.net/browse/JDK-8144069 Regards, iris -----Original Message----- From: Mandy Chung Sent: Wednesday, November 25, 2015 8:48 AM To: Iris Clark Cc: core-libs-dev@openjdk.java.net; verona-dev@openjdk.java.net Subject: Re: RFR: 8072379: Implement jdk.Version and jdk.OracleVersion
On Nov 24, 2015, at 5:54 PM, Iris Clark <iris.clark@oracle.com> wrote:
Hi.
Please review the new classes jdk.Version and jdk.OracleVersion. These are simple Java APIs to parse, validate, and compare version numbers.
Webrev
This looks good to me. Alan raises a good question whether jdk.OracleVersion is intended to be pushed to OpenJDK. We will need to find a home for this jdk.* API since java.base only exports Java SE API per the design principles in JEP 200 The Modular JDK. It’s fine to file an issue to follow up this. W.r.t. the docs build, jdk is listed in top/make/common/NON_CORE_PKGS.gmk but there is no target in make/Javadoc.gmk building it. You will also need to find a docs page to publish this new API as well (the current way is a page linked from the layer cake and this may be changed). Mandy
Hello, On 11/25/2015 8:48 AM, Mandy Chung wrote:
On Nov 24, 2015, at 5:54 PM, Iris Clark <iris.clark@oracle.com> wrote:
Hi.
Please review the new classes jdk.Version and jdk.OracleVersion. These are simple Java APIs to parse, validate, and compare version numbers.
Webrev
http://cr.openjdk.java.net/~irisa/verona/8072379/webrev.1/ This looks good to me. Alan raises a good question whether jdk.OracleVersion is intended to be pushed to OpenJDK.
I share the concerns that have been raised about the naming and placement of a class named "OracleVersion". Is the intention that downstream JDK distributions, such as IcedTea, whether based on OpenJDK or otherwise, would provide their own specialization of the jdk.Version class? If so, should there be some kind of provider interface to find system's the version interpreter? For example, what class, if any would be expected to provide detailed analysis of the version string emitted by, say, a OpenJDK build used as the reference implementation of Java SE 9? The equals / hashCode behavior of Version and OracleVersion is bit surprising and I think somewhat underspecified given the possibility of defining subclasses. The (overridable) equals method in Version says "Two Versions are equal if and only if they represent the same version string." If that is interpreted to mean the Versions are equal if their toString output is equal, the toString spec says "Returns a string representation of this version." and the (overridable) implementation in Version looks at versions, pre, build, and optional. Since OracleVersion does not override equals, a plain Version object and an OracleVersion object can compare as equal. Additionally, a VersionWithDifferentToStringOutput("1.2.3") object which overrides toString would not be semantically equivalent to a Version("1.2.3") object. Worse, there could be a non-communicability behavior in the equals method since (new Version("1.2.3")).equals(new VersionWithDifferentToStringOutput("1.2.3")) could return "true" while (new VersionWithDifferentToStringOutput("1.2.3")).equals(new Version("1.2.3")) could easily and legitimately return "false" by the specification. How is this API supposed to behave if the component version strings have a numerical value greater than Integer.MAX_VALUE? Was using Longs to record numerical versions rather than Integers considered? Thanks, -Joe
PS If the concepts the two classes Version and OracleVersion are trying to capture is a "Vendor-Version" then perhaps that can be surfaced more directly in the API. That is, if the basic notion is to interpret a version string in a way appropriate to and specialized for a given vendor of the JDK (a la the java.vendor system property), then perhaps a type like // API sketch public final class VendorVersion { public VendorVersion(String vendor, Version version, Comparator<Version> versionComp>) { ...} @Override public boolean equals(VendorVersion vv) { // Usual instance of checks return Objects.equals(vendor, vv.vendor()) && versionComp.version(), vv.version()); } int compareVersion(VendorVersion vv) { if (!vendor.equals(vv.vendor())) // throw an exception return versionComp(version, vv.version); } // ... } might serve the purposes at hand. HTH, -Joe On 11/25/2015 1:31 PM, joe darcy wrote:
Hello,
On 11/25/2015 8:48 AM, Mandy Chung wrote:
On Nov 24, 2015, at 5:54 PM, Iris Clark <iris.clark@oracle.com> wrote:
Hi.
Please review the new classes jdk.Version and jdk.OracleVersion. These are simple Java APIs to parse, validate, and compare version numbers.
Webrev
http://cr.openjdk.java.net/~irisa/verona/8072379/webrev.1/ This looks good to me. Alan raises a good question whether jdk.OracleVersion is intended to be pushed to OpenJDK.
I share the concerns that have been raised about the naming and placement of a class named "OracleVersion".
Is the intention that downstream JDK distributions, such as IcedTea, whether based on OpenJDK or otherwise, would provide their own specialization of the jdk.Version class?
If so, should there be some kind of provider interface to find system's the version interpreter?
For example, what class, if any would be expected to provide detailed analysis of the version string emitted by, say, a OpenJDK build used as the reference implementation of Java SE 9?
The equals / hashCode behavior of Version and OracleVersion is bit surprising and I think somewhat underspecified given the possibility of defining subclasses.
The (overridable) equals method in Version says "Two Versions are equal if and only if they represent the same version string." If that is interpreted to mean the Versions are equal if their toString output is equal, the toString spec says "Returns a string representation of this version." and the (overridable) implementation in Version looks at versions, pre, build, and optional.
Since OracleVersion does not override equals, a plain Version object and an OracleVersion object can compare as equal. Additionally, a VersionWithDifferentToStringOutput("1.2.3") object which overrides toString would not be semantically equivalent to a Version("1.2.3") object. Worse, there could be a non-communicability behavior in the equals method since
(new Version("1.2.3")).equals(new VersionWithDifferentToStringOutput("1.2.3"))
could return "true" while
(new VersionWithDifferentToStringOutput("1.2.3")).equals(new Version("1.2.3"))
could easily and legitimately return "false" by the specification.
How is this API supposed to behave if the component version strings have a numerical value greater than Integer.MAX_VALUE?
Was using Longs to record numerical versions rather than Integers considered?
Thanks,
-Joe
PPS Perhaps this is already planned as future work, but it might be a kindness to those analyzing JDK version strings if there was a class that did a "best effort" at understanding Sun and Oracle JDK version strings pre-Verona to those post-Verona. In other words, a version class where new JdkVersion("1.8.0_25") would have major() == 8 and minor() == 25, but have the specified meaning for strings for Verona strings where the first group was numerically 9 or larger. Cheers, -Joe On 11/25/2015 3:57 PM, joe darcy wrote:
PS If the concepts the two classes Version and OracleVersion are trying to capture is a "Vendor-Version" then perhaps that can be surfaced more directly in the API. That is, if the basic notion is to interpret a version string in a way appropriate to and specialized for a given vendor of the JDK (a la the java.vendor system property), then perhaps a type like
// API sketch public final class VendorVersion { public VendorVersion(String vendor, Version version, Comparator<Version> versionComp>) { ...}
@Override public boolean equals(VendorVersion vv) { // Usual instance of checks return Objects.equals(vendor, vv.vendor()) && versionComp.version(), vv.version()); }
int compareVersion(VendorVersion vv) { if (!vendor.equals(vv.vendor())) // throw an exception return versionComp(version, vv.version); }
// ... }
might serve the purposes at hand.
HTH,
-Joe
On 11/25/2015 1:31 PM, joe darcy wrote:
Hello,
On 11/25/2015 8:48 AM, Mandy Chung wrote:
On Nov 24, 2015, at 5:54 PM, Iris Clark <iris.clark@oracle.com> wrote:
Hi.
Please review the new classes jdk.Version and jdk.OracleVersion. These are simple Java APIs to parse, validate, and compare version numbers.
Webrev
http://cr.openjdk.java.net/~irisa/verona/8072379/webrev.1/ This looks good to me. Alan raises a good question whether jdk.OracleVersion is intended to be pushed to OpenJDK.
I share the concerns that have been raised about the naming and placement of a class named "OracleVersion".
Is the intention that downstream JDK distributions, such as IcedTea, whether based on OpenJDK or otherwise, would provide their own specialization of the jdk.Version class?
If so, should there be some kind of provider interface to find system's the version interpreter?
For example, what class, if any would be expected to provide detailed analysis of the version string emitted by, say, a OpenJDK build used as the reference implementation of Java SE 9?
The equals / hashCode behavior of Version and OracleVersion is bit surprising and I think somewhat underspecified given the possibility of defining subclasses.
The (overridable) equals method in Version says "Two Versions are equal if and only if they represent the same version string." If that is interpreted to mean the Versions are equal if their toString output is equal, the toString spec says "Returns a string representation of this version." and the (overridable) implementation in Version looks at versions, pre, build, and optional.
Since OracleVersion does not override equals, a plain Version object and an OracleVersion object can compare as equal. Additionally, a VersionWithDifferentToStringOutput("1.2.3") object which overrides toString would not be semantically equivalent to a Version("1.2.3") object. Worse, there could be a non-communicability behavior in the equals method since
(new Version("1.2.3")).equals(new VersionWithDifferentToStringOutput("1.2.3"))
could return "true" while
(new VersionWithDifferentToStringOutput("1.2.3")).equals(new Version("1.2.3"))
could easily and legitimately return "false" by the specification.
How is this API supposed to behave if the component version strings have a numerical value greater than Integer.MAX_VALUE?
Was using Longs to record numerical versions rather than Integers considered?
Thanks,
-Joe
Hi, Joe. I've filed the following JDK 9 RFE to provide this functionality: 8145794: Provide Verona interpretation of historical JDK version strings https://bugs.openjdk.java.net/browse/JDK-8145794 Thanks, iris -----Original Message----- From: joe darcy Sent: Sunday, November 29, 2015 4:18 PM To: Mandy Chung; Iris Clark Cc: verona-dev@openjdk.java.net; core-libs-dev@openjdk.java.net Subject: Re: RFR: 8072379: Implement jdk.Version and jdk.OracleVersion PPS Perhaps this is already planned as future work, but it might be a kindness to those analyzing JDK version strings if there was a class that did a "best effort" at understanding Sun and Oracle JDK version strings pre-Verona to those post-Verona. In other words, a version class where new JdkVersion("1.8.0_25") would have major() == 8 and minor() == 25, but have the specified meaning for strings for Verona strings where the first group was numerically 9 or larger. Cheers, -Joe
Hi, Joe. It is not the intention that all JDK distributions must provide their own specializations of jdk.Version. However, downstream users may wish to provide additional semantics to the version string via encapsulation rather than inheritance. In order to facilitate this, the following bug has been filed to consider that case: 8145793: Provide vendor-specific interpretation of a JDK version string https://bugs.openjdk.java.net/browse/JDK-8145793 Thanks, iris -----Original Message----- From: joe darcy Sent: Wednesday, November 25, 2015 3:58 PM To: Mandy Chung; Iris Clark Cc: core-libs-dev@openjdk.java.net; verona-dev@openjdk.java.net Subject: Re: RFR: 8072379: Implement jdk.Version and jdk.OracleVersion PS If the concepts the two classes Version and OracleVersion are trying to capture is a "Vendor-Version" then perhaps that can be surfaced more directly in the API. That is, if the basic notion is to interpret a version string in a way appropriate to and specialized for a given vendor of the JDK (a la the java.vendor system property), then perhaps a type like // API sketch public final class VendorVersion { public VendorVersion(String vendor, Version version, Comparator<Version> versionComp>) { ...} @Override public boolean equals(VendorVersion vv) { // Usual instance of checks return Objects.equals(vendor, vv.vendor()) && versionComp.version(), vv.version()); } int compareVersion(VendorVersion vv) { if (!vendor.equals(vv.vendor())) // throw an exception return versionComp(version, vv.version); } // ... } might serve the purposes at hand. HTH, -Joe
Hi, Joe. Thanks for the review comments.
Is the intention that downstream JDK distributions, such as IcedTea, whether based on OpenJDK or otherwise, would provide their own specialization of the jdk.Version class?
No. Downstream users may provide their own specialization, but there is no requirement that they must do so. jdk.OracleVersion has been removed. The functionality it provided, access to the fourth element of the version number, is trivially accessed by existing methods in jdk.Version. I've filed the following bug to address the more general use case: 8145793: Provide vendor-specific interpretation of a JDK version string https://bugs.openjdk.java.net/browse/JDK-8145793
The equals / hashCode behavior of Version and OracleVersion is bit surprising and I think somewhat underspecified given the possibility of defining subclasses.
Given the above regarding downstream use, I've make jdk.Version final and the constructor is now private. Oracle or any other JDK distribution wishing to impose additional semantics to the version string interpretation will need to do so via encapsulation rather than inheritance.
How is this API supposed to behave if the component version strings have a numerical value greater than Integer.MAX_VALUE?
From the specification of the only instance method, Version.parse():
* @throws NumberFormatException * If an element of the version number or the build number cannot * be represented as an {@link Integer}
Was using Longs to record numerical versions rather than Integers considered?
Yes. In an informal poll conducted during implementation, it was thought that 2^32 bits would be more than sufficient to represent the components of typical version numbers. An updated webrev is forthcoming. Thanks, iris PS: I believe that we are both out the week of 21 December, so I don't expect to hear back from you until the New Year.
Hi, Joe, Roger, Alan, Magnus, and Mandy. At the end of December (shortly before the Christmas/Winter break and my vacation), I provided responses to your messages and an updated webrev: http://cr.openjdk.java.net/~iris/verona/8072379/webrev.2/ I didn't hear from anybody, so I'd like to optimistically assume that you were satisfied. Is that correct? For you convenience, here's a reference to the December and November threads: http://mail.openjdk.java.net/pipermail/core-libs-dev/2015-December/037062.ht... http://mail.openjdk.java.net/pipermail/core-libs-dev/2015-November/036904.ht... I'd like to wrap up this work for the initial implementation of jdk.Version soon. Regards, iris -----Original Message----- From: Iris Clark Sent: Friday, December 18, 2015 1:55 PM To: Joe Darcy; Mandy Chung Cc: core-libs-dev@openjdk.java.net; verona-dev@openjdk.java.net Subject: RE: RFR: 8072379: Implement jdk.Version and jdk.OracleVersion Hi, Joe. Thanks for the review comments.
Is the intention that downstream JDK distributions, such as IcedTea, whether based on OpenJDK or otherwise, would provide their own specialization of the jdk.Version class?
No. Downstream users may provide their own specialization, but there is no requirement that they must do so. jdk.OracleVersion has been removed. The functionality it provided, access to the fourth element of the version number, is trivially accessed by existing methods in jdk.Version. I've filed the following bug to address the more general use case: 8145793: Provide vendor-specific interpretation of a JDK version string https://bugs.openjdk.java.net/browse/JDK-8145793
The equals / hashCode behavior of Version and OracleVersion is bit surprising and I think somewhat underspecified given the possibility of defining subclasses.
Given the above regarding downstream use, I've make jdk.Version final and the constructor is now private. Oracle or any other JDK distribution wishing to impose additional semantics to the version string interpretation will need to do so via encapsulation rather than inheritance.
How is this API supposed to behave if the component version strings have a numerical value greater than Integer.MAX_VALUE?
From the specification of the only instance method, Version.parse():
* @throws NumberFormatException * If an element of the version number or the build number cannot * be represented as an {@link Integer}
Was using Longs to record numerical versions rather than Integers considered?
Yes. In an informal poll conducted during implementation, it was thought that 2^32 bits would be more than sufficient to represent the components of typical version numbers. An updated webrev is forthcoming. Thanks, iris PS: I believe that we are both out the week of 21 December, so I don't expect to hear back from you until the New Year.
On 11/01/2016 21:44, Iris Clark wrote:
Hi, Joe, Roger, Alan, Magnus, and Mandy.
At the end of December (shortly before the Christmas/Winter break and my vacation), I provided responses to your messages and an updated webrev:
http://cr.openjdk.java.net/~iris/verona/8072379/webrev.2/
I didn't hear from anybody, so I'd like to optimistically assume that you were satisfied. Is that correct?
For you convenience, here's a reference to the December and November threads:
http://mail.openjdk.java.net/pipermail/core-libs-dev/2015-December/037062.ht... http://mail.openjdk.java.net/pipermail/core-libs-dev/2015-November/036904.ht...
I'd like to wrap up this work for the initial implementation of jdk.Version soon.
I think this looks good but we'll to decide which module to put this in. It can't be java.base (see design principles in JEP 200). If it's going into java.base temporarily then the top-level modules.xml will need to be updated to export the "jdk" package. -Alan
Hi, Alan. Thanks for looking at this (hopefully) one last time.
It can't be java.base (see design principles in JEP 200). If it's going into java.base temporarily then the top-level modules.xml will need to be updated to export the "jdk" package.
This diff has been applied to modules.xml: diff -r 6fefc5bce180 modules.xml --- a/modules.xml Wed Jan 13 13:56:19 2016 +0000 +++ b/modules.xml Wed Jan 13 13:46:56 2016 -0800 @@ -205,6 +205,9 @@ <name>javax.security.cert</name> </export> <export> + <name>jdk</name> + </export> + <export> <name>jdk.net</name> </export> <export> It essentially reverts your 8049422 change [0] to that file. I will not re-add jdk to the javadoc bundle for javac trees API since that is not an appropriate location. I filed the following bug to track publication of jdk.Version: 8144069: Determine correct publication for jdk.Version API https://bugs.openjdk.java.net/browse/JDK-8144069 When this came up earlier, I filed this bug to track finding a more appropriate module for jdk.Version: 8144062: Determine appropriate module for jdk.Version https://bugs.openjdk.java.net/browse/JDK-8144062 Thanks, iris [0] http://hg.openjdk.java.net/jdk9/dev/rev/1bee5efa73e3 -----Original Message----- From: Alan Bateman Sent: Tuesday, January 12, 2016 7:41 AM To: Iris Clark; core-libs-dev@openjdk.java.net; verona-dev@openjdk.java.net Subject: Re: RFR: 8072379: Implement jdk.Version and jdk.OracleVersion On 11/01/2016 21:44, Iris Clark wrote:
Hi, Joe, Roger, Alan, Magnus, and Mandy.
At the end of December (shortly before the Christmas/Winter break and my vacation), I provided responses to your messages and an updated webrev:
http://cr.openjdk.java.net/~iris/verona/8072379/webrev.2/
I didn't hear from anybody, so I'd like to optimistically assume that you were satisfied. Is that correct?
For you convenience, here's a reference to the December and November threads:
http://mail.openjdk.java.net/pipermail/core-libs-dev/2015-December/037062.ht...
http://mail.openjdk.java.net/pipermail/core-libs-dev/2015-November/036 904.html
I'd like to wrap up this work for the initial implementation of jdk.Version soon.
I think this looks good but we'll to decide which module to put this in. It can't be java.base (see design principles in JEP 200). If it's going into java.base temporarily then the top-level modules.xml will need to be updated to export the "jdk" package. -Alan
Hi, Some comment on the regex (and also the JEP): ([1-9][0-9]*(((\.0)*\.[1-9][0-9]*)*)*)(\-([a-zA-Z0-9]+))?((\+)(0|[1-9][0-9]*)?)?(-([\-a-zA-Z0-9\.]+))? - The outer most quantifier in (((\.0)*\.[1-9][0-9]*)*)* is redundant and a source of catastrophic backtracking. You only need ((\.0)*\.[1-9][0-9]*)*. - The outside - in PRE part (\-([a-zA-Z0-9]+))? doesn't need escaping. Simplify it to (-([a-zA-Z0-9]+))? - Do you want to allow only a plus without the number in the BUILD part? Why do you capture the +? ((\+)(0|[1-9][0-9]*)?)? - Dot loses meaning in the character class, and hyphen loses meaning at the beginning or at the end of the character class. You can simplify the OPT part to (-([-a-zA-Z0-9.]+))? - You might want to consider using named-capturing groups instead of numbered capturing groups. Also, consider using non-capturing groups for groups you don't need to extract. Best regards, Hong Dai Thanh. -----Original Message----- From: core-libs-dev [mailto:core-libs-dev-bounces@openjdk.java.net] On Behalf Of Iris Clark Sent: Thursday, 14 January, 2016 4:55 AM To: Alan Bateman <alan.bateman@oracle.com>; core-libs-dev@openjdk.java.net; verona-dev@openjdk.java.net Subject: RE: RFR: 8072379: Implement jdk.Version and jdk.OracleVersion Hi, Alan. Thanks for looking at this (hopefully) one last time.
It can't be java.base (see design principles in JEP 200). If it's going into java.base temporarily then the top-level modules.xml will need to be updated to export the "jdk" package.
This diff has been applied to modules.xml: diff -r 6fefc5bce180 modules.xml --- a/modules.xml Wed Jan 13 13:56:19 2016 +0000 +++ b/modules.xml Wed Jan 13 13:46:56 2016 -0800 @@ -205,6 +205,9 @@ <name>javax.security.cert</name> </export> <export> + <name>jdk</name> + </export> + <export> <name>jdk.net</name> </export> <export> It essentially reverts your 8049422 change [0] to that file. I will not re-add jdk to the javadoc bundle for javac trees API since that is not an appropriate location. I filed the following bug to track publication of jdk.Version: 8144069: Determine correct publication for jdk.Version API https://bugs.openjdk.java.net/browse/JDK-8144069 When this came up earlier, I filed this bug to track finding a more appropriate module for jdk.Version: 8144062: Determine appropriate module for jdk.Version https://bugs.openjdk.java.net/browse/JDK-8144062 Thanks, iris [0] http://hg.openjdk.java.net/jdk9/dev/rev/1bee5efa73e3 -----Original Message----- From: Alan Bateman Sent: Tuesday, January 12, 2016 7:41 AM To: Iris Clark; core-libs-dev@openjdk.java.net; verona-dev@openjdk.java.net Subject: Re: RFR: 8072379: Implement jdk.Version and jdk.OracleVersion On 11/01/2016 21:44, Iris Clark wrote:
Hi, Joe, Roger, Alan, Magnus, and Mandy.
At the end of December (shortly before the Christmas/Winter break and my vacation), I provided responses to your messages and an updated webrev:
http://cr.openjdk.java.net/~iris/verona/8072379/webrev.2/
I didn't hear from anybody, so I'd like to optimistically assume that you were satisfied. Is that correct?
For you convenience, here's a reference to the December and November threads:
http://mail.openjdk.java.net/pipermail/core-libs-dev/2015-December/037 062.html
http://mail.openjdk.java.net/pipermail/core-libs-dev/2015-November/036 904.html
I'd like to wrap up this work for the initial implementation of jdk.Version soon.
I think this looks good but we'll to decide which module to put this in. It can't be java.base (see design principles in JEP 200). If it's going into java.base temporarily then the top-level modules.xml will need to be updated to export the "jdk" package. -Alan
Hi, Hong. Thanks again for looking closely at the regular expression and providing comments.
- The outer most quantifier in (((\.0)*\.[1-9][0-9]*)*)* is redundant and a source of catastrophic backtracking. You only need ((\.0)*\.[1-9][0-9]*)*. - The outside - in PRE part (\-([a-zA-Z0-9]+))? doesn't need escaping. Simplify it to (-([a-zA-Z0-9]+))?
Both done.
- Do you want to allow only a plus without the number in the BUILD ? part? Why do you capture the +? ((\+)(0|[1-9][0-9]*)?)?
I need to capture the plus to distinguish between cases where an empty build is allowed (e.g. "9+-foo") and when it is not ("9+"). See code in Version.java, line 226-230 and in Basic.java, line 98, 107-109. (Note that we use the empty "+" to distinguish "9-foo" from "9+-foo".)
- Dot loses meaning in the character class, and hyphen loses meaning at the beginning or at the end of the character class. You can simplify the OPT part to (-([-a-zA-Z0-9.]+))?
Done.
- You might want to consider using named-capturing groups instead of numbered capturing groups. Also, consider using non-capturing groups for groups you don't need to extract.
Done. This is the (hopefully) final version of the webrev and javadoc for the initial implementation of this API: http://cr.openjdk.java.net/~iris/verona/8072379/webrev.3/index.html http://cr.openjdk.java.net/~iris/verona/8072379/doc.3/jdk/Version.html I've filed the following bug to update the spec: 8148822: (spec) Regex in jdk.Version and JEP 223 should match https://bugs.openjdk.java.net/browse/JDK-8148822 These are the corresponding changes to the JEP which I'll apply in the next day or so: $ diff jep-open-mr11.md jep-open-mr12.md 84c84 < `^[1-9][0-9]*(((\.0)*\.[1-9][0-9]*)*)*$`. The sequence may be of arbitrary ---
`^[1-9][0-9]*((\.0)*\.[1-9][0-9]*)*$`. The sequence may be of arbitrary 166c166 < - <a name="descSTR"/> $OPT, matching `([-a-zA-Z0-9\.]+)` --- Additional
- <a name="descSTR"/> $OPT, matching `([-a-zA-Z0-9.]+)` --- Additional
Thanks again for the recommendations. Regards, iris -----Original Message----- From: Thanh Hong Dai [mailto:hdthanh@tma.com.vn] Sent: Wednesday, January 13, 2016 7:53 PM To: Iris Clark; Alan Bateman; core-libs-dev@openjdk.java.net; verona-dev@openjdk.java.net Subject: RE: RFR: 8072379: Implement jdk.Version and jdk.OracleVersion Hi, Some comment on the regex (and also the JEP): ([1-9][0-9]*(((\.0)*\.[1-9][0-9]*)*)*)(\-([a-zA-Z0-9]+))?((\+)(0|[1-9][0-9]*)?)?(-([\-a-zA-Z0-9\.]+))? - The outer most quantifier in (((\.0)*\.[1-9][0-9]*)*)* is redundant and a source of catastrophic backtracking. You only need ((\.0)*\.[1-9][0-9]*)*. - The outside - in PRE part (\-([a-zA-Z0-9]+))? doesn't need escaping. Simplify it to (-([a-zA-Z0-9]+))? - Do you want to allow only a plus without the number in the BUILD part? Why do you capture the +? ((\+)(0|[1-9][0-9]*)?)? - Dot loses meaning in the character class, and hyphen loses meaning at the beginning or at the end of the character class. You can simplify the OPT part to (-([-a-zA-Z0-9.]+))? - You might want to consider using named-capturing groups instead of numbered capturing groups. Also, consider using non-capturing groups for groups you don't need to extract. Best regards, Hong Dai Thanh. -----Original Message----- From: core-libs-dev [mailto:core-libs-dev-bounces@openjdk.java.net] On Behalf Of Iris Clark Sent: Thursday, 14 January, 2016 4:55 AM To: Alan Bateman <alan.bateman@oracle.com>; core-libs-dev@openjdk.java.net; verona-dev@openjdk.java.net Subject: RE: RFR: 8072379: Implement jdk.Version and jdk.OracleVersion Hi, Alan. Thanks for looking at this (hopefully) one last time.
It can't be java.base (see design principles in JEP 200). If it's going into java.base temporarily then the top-level modules.xml will need to be updated to export the "jdk" package.
This diff has been applied to modules.xml: diff -r 6fefc5bce180 modules.xml --- a/modules.xml Wed Jan 13 13:56:19 2016 +0000 +++ b/modules.xml Wed Jan 13 13:46:56 2016 -0800 @@ -205,6 +205,9 @@ <name>javax.security.cert</name> </export> <export> + <name>jdk</name> + </export> + <export> <name>jdk.net</name> </export> <export> It essentially reverts your 8049422 change [0] to that file. I will not re-add jdk to the javadoc bundle for javac trees API since that is not an appropriate location. I filed the following bug to track publication of jdk.Version: 8144069: Determine correct publication for jdk.Version API https://bugs.openjdk.java.net/browse/JDK-8144069 When this came up earlier, I filed this bug to track finding a more appropriate module for jdk.Version: 8144062: Determine appropriate module for jdk.Version https://bugs.openjdk.java.net/browse/JDK-8144062 Thanks, iris [0] http://hg.openjdk.java.net/jdk9/dev/rev/1bee5efa73e3 -----Original Message----- From: Alan Bateman Sent: Tuesday, January 12, 2016 7:41 AM To: Iris Clark; core-libs-dev@openjdk.java.net; verona-dev@openjdk.java.net Subject: Re: RFR: 8072379: Implement jdk.Version and jdk.OracleVersion On 11/01/2016 21:44, Iris Clark wrote:
Hi, Joe, Roger, Alan, Magnus, and Mandy.
At the end of December (shortly before the Christmas/Winter break and my vacation), I provided responses to your messages and an updated webrev:
http://cr.openjdk.java.net/~iris/verona/8072379/webrev.2/
I didn't hear from anybody, so I'd like to optimistically assume that you were satisfied. Is that correct?
For you convenience, here's a reference to the December and November threads:
http://mail.openjdk.java.net/pipermail/core-libs-dev/2015-December/037 062.html
http://mail.openjdk.java.net/pipermail/core-libs-dev/2015-November/036 904.html
I'd like to wrap up this work for the initial implementation of jdk.Version soon.
I think this looks good but we'll to decide which module to put this in. It can't be java.base (see design principles in JEP 200). If it's going into java.base temporarily then the top-level modules.xml will need to be updated to export the "jdk" package. -Alan
Dear Iris, On closer look, there seems to be some conflicting definition of version string. In JEP: http://openjdk.java.net/jeps/223 $VNUM(-$PRE)?(\+$BUILD)?(-$OPT)? In Javadoc: http://cr.openjdk.java.net/~iris/verona/8072379/doc.3/jdk/Version.html $VNUM(-$PRE)?(\+($BUILD)?(-$OPT)?)? In implementation: The regex follows JEP's definition. The JEP & implementation allows -$OPT to be specified without +, but the Javadoc one does not allow that. For example, "9-pre-opt" is allowed by JEP, but disallowed by Javadoc.
I need to capture the plus to distinguish between cases where an empty build is allowed (e.g. "9+-foo") and when it is not ("9+"). See code in Version.java, line 226-230 and in Basic.java, line 98, 107-109. (Note that we use the empty "+" to distinguish "9-foo" from "9+-foo".)
Understood, but I didn't see any part of the JEP or the Javadoc explaining that + is needed to make the parser recognize the text followed as options instead of pre-release identifier. It would be great if that is added. Best regards, Hong Dai Thanh. -----Original Message----- From: Iris Clark [mailto:iris.clark@oracle.com] Sent: Tuesday, 2 February, 2016 12:51 PM To: Thanh Hong Dai <hdthanh@tma.com.vn>; Alan Bateman <alan.bateman@oracle.com>; core-libs-dev@openjdk.java.net; verona-dev@openjdk.java.net Subject: RE: RFR: 8072379: Implement jdk.Version and jdk.OracleVersion Hi, Hong. Thanks again for looking closely at the regular expression and providing comments.
- The outer most quantifier in (((\.0)*\.[1-9][0-9]*)*)* is redundant and a source of catastrophic backtracking. You only need ((\.0)*\.[1-9][0-9]*)*. - The outside - in PRE part (\-([a-zA-Z0-9]+))? doesn't need escaping. Simplify it to (-([a-zA-Z0-9]+))?
Both done.
- Do you want to allow only a plus without the number in the BUILD ? part? Why do you capture the +? ((\+)(0|[1-9][0-9]*)?)?
I need to capture the plus to distinguish between cases where an empty build is allowed (e.g. "9+-foo") and when it is not ("9+"). See code in Version.java, line 226-230 and in Basic.java, line 98, 107-109. (Note that we use the empty "+" to distinguish "9-foo" from "9+-foo".)
- Dot loses meaning in the character class, and hyphen loses meaning at the beginning or at the end of the character class. You can simplify the OPT part to (-([-a-zA-Z0-9.]+))?
Done.
- You might want to consider using named-capturing groups instead of numbered capturing groups. Also, consider using non-capturing groups for groups you don't need to extract.
Done. This is the (hopefully) final version of the webrev and javadoc for the initial implementation of this API: http://cr.openjdk.java.net/~iris/verona/8072379/webrev.3/index.html http://cr.openjdk.java.net/~iris/verona/8072379/doc.3/jdk/Version.html I've filed the following bug to update the spec: 8148822: (spec) Regex in jdk.Version and JEP 223 should match https://bugs.openjdk.java.net/browse/JDK-8148822 These are the corresponding changes to the JEP which I'll apply in the next day or so: $ diff jep-open-mr11.md jep-open-mr12.md 84c84 < `^[1-9][0-9]*(((\.0)*\.[1-9][0-9]*)*)*$`. The sequence may be of arbitrary ---
`^[1-9][0-9]*((\.0)*\.[1-9][0-9]*)*$`. The sequence may be of arbitrary 166c166 < - <a name="descSTR"/> $OPT, matching `([-a-zA-Z0-9\.]+)` --- Additional
- <a name="descSTR"/> $OPT, matching `([-a-zA-Z0-9.]+)` --- Additional
Thanks again for the recommendations. Regards, iris -----Original Message----- From: Thanh Hong Dai [mailto:hdthanh@tma.com.vn] Sent: Wednesday, January 13, 2016 7:53 PM To: Iris Clark; Alan Bateman; core-libs-dev@openjdk.java.net; verona-dev@openjdk.java.net Subject: RE: RFR: 8072379: Implement jdk.Version and jdk.OracleVersion Hi, Some comment on the regex (and also the JEP): ([1-9][0-9]*(((\.0)*\.[1-9][0-9]*)*)*)(\-([a-zA-Z0-9]+))?((\+)(0|[1-9][0-9]*)?)?(-([\-a-zA-Z0-9\.]+))? - The outer most quantifier in (((\.0)*\.[1-9][0-9]*)*)* is redundant and a source of catastrophic backtracking. You only need ((\.0)*\.[1-9][0-9]*)*. - The outside - in PRE part (\-([a-zA-Z0-9]+))? doesn't need escaping. Simplify it to (-([a-zA-Z0-9]+))? - Do you want to allow only a plus without the number in the BUILD part? Why do you capture the +? ((\+)(0|[1-9][0-9]*)?)? - Dot loses meaning in the character class, and hyphen loses meaning at the beginning or at the end of the character class. You can simplify the OPT part to (-([-a-zA-Z0-9.]+))? - You might want to consider using named-capturing groups instead of numbered capturing groups. Also, consider using non-capturing groups for groups you don't need to extract. Best regards, Hong Dai Thanh. -----Original Message----- From: core-libs-dev [mailto:core-libs-dev-bounces@openjdk.java.net] On Behalf Of Iris Clark Sent: Thursday, 14 January, 2016 4:55 AM To: Alan Bateman <alan.bateman@oracle.com>; core-libs-dev@openjdk.java.net; verona-dev@openjdk.java.net Subject: RE: RFR: 8072379: Implement jdk.Version and jdk.OracleVersion Hi, Alan. Thanks for looking at this (hopefully) one last time.
It can't be java.base (see design principles in JEP 200). If it's going into java.base temporarily then the top-level modules.xml will need to be updated to export the "jdk" package.
This diff has been applied to modules.xml: diff -r 6fefc5bce180 modules.xml --- a/modules.xml Wed Jan 13 13:56:19 2016 +0000 +++ b/modules.xml Wed Jan 13 13:46:56 2016 -0800 @@ -205,6 +205,9 @@ <name>javax.security.cert</name> </export> <export> + <name>jdk</name> + </export> + <export> <name>jdk.net</name> </export> <export> It essentially reverts your 8049422 change [0] to that file. I will not re-add jdk to the javadoc bundle for javac trees API since that is not an appropriate location. I filed the following bug to track publication of jdk.Version: 8144069: Determine correct publication for jdk.Version API https://bugs.openjdk.java.net/browse/JDK-8144069 When this came up earlier, I filed this bug to track finding a more appropriate module for jdk.Version: 8144062: Determine appropriate module for jdk.Version https://bugs.openjdk.java.net/browse/JDK-8144062 Thanks, iris [0] http://hg.openjdk.java.net/jdk9/dev/rev/1bee5efa73e3 -----Original Message----- From: Alan Bateman Sent: Tuesday, January 12, 2016 7:41 AM To: Iris Clark; core-libs-dev@openjdk.java.net; verona-dev@openjdk.java.net Subject: Re: RFR: 8072379: Implement jdk.Version and jdk.OracleVersion On 11/01/2016 21:44, Iris Clark wrote:
Hi, Joe, Roger, Alan, Magnus, and Mandy.
At the end of December (shortly before the Christmas/Winter break and my vacation), I provided responses to your messages and an updated webrev:
http://cr.openjdk.java.net/~iris/verona/8072379/webrev.2/
I didn't hear from anybody, so I'd like to optimistically assume that you were satisfied. Is that correct?
For you convenience, here's a reference to the December and November threads:
http://mail.openjdk.java.net/pipermail/core-libs-dev/2015-December/037 062.html
http://mail.openjdk.java.net/pipermail/core-libs-dev/2015-November/036 904.html
I'd like to wrap up this work for the initial implementation of jdk.Version soon.
I think this looks good but we'll to decide which module to put this in. It can't be java.base (see design principles in JEP 200). If it's going into java.base temporarily then the top-level modules.xml will need to be updated to export the "jdk" package. -Alan
Hi. Thanks again for looking at this change.
The JEP & implementation allows -$OPT to be specified without +, but the Javadoc one does not allow that. For example, "9-pre-opt" is allowed by JEP, but disallowed by Javadoc.
The JavaDoc and JEP are both slightly wrong in different ways. The implementation is correct. Note that it enforces additional constraints beyond just the regular expression. I've added a few more regression tests to verify that the behavior is as expected. This is the updated webrev and javadoc: http://cr.openjdk.java.net/~iris/verona/8072379/webrev.4/ http://cr.openjdk.java.net/~iris/verona/8072379/doc.4/jdk/Version.html (unchanged from doc.3)
Understood, but I didn't see any part of the JEP or the Javadoc explaining that + is needed to make the parser recognize the text followed as options instead of pre-release identifier. It would be great if that is added.
I've created a new bug to cover updating both the javadoc and JEP for this case. https://bugs.openjdk.java.net/browse/JDK-8148877 JDK-8148877: (spec) Specify when an empty '+' is required in a version string This is the key sentence in the description: '+' is required for empty build when OPT is present and PRE is not. "9+-opt" and "9-pre-opt" are legal. "9+" is not. Regards, iris -----Original Message----- From: Thanh Hong Dai [mailto:hdthanh@tma.com.vn] Sent: Monday, February 01, 2016 11:48 PM To: Iris Clark; Alan Bateman; core-libs-dev@openjdk.java.net Subject: RE: RFR: 8072379: Implement jdk.Version and jdk.OracleVersion Dear Iris, On closer look, there seems to be some conflicting definition of version string. In JEP: http://openjdk.java.net/jeps/223 $VNUM(-$PRE)?(\+$BUILD)?(-$OPT)? In Javadoc: http://cr.openjdk.java.net/~iris/verona/8072379/doc.3/jdk/Version.html $VNUM(-$PRE)?(\+($BUILD)?(-$OPT)?)? In implementation: The regex follows JEP's definition. The JEP & implementation allows -$OPT to be specified without +, but the Javadoc one does not allow that. For example, "9-pre-opt" is allowed by JEP, but disallowed by Javadoc.
I need to capture the plus to distinguish between cases where an empty build is allowed (e.g. "9+-foo") and when it is not ("9+"). See code in Version.java, line 226-230 and in Basic.java, line 98, 107-109. (Note that we use the empty "+" to distinguish "9-foo" from "9+-foo".)
Understood, but I didn't see any part of the JEP or the Javadoc explaining that + is needed to make the parser recognize the text followed as options instead of pre-release identifier. It would be great if that is added. Best regards, Hong Dai Thanh.
Dear Iris,
The JavaDoc and JEP are both slightly wrong in different ways. The implementation is correct. Note that it enforces additional constraints beyond just the regular expression.
It would be great if those additional constraints are also documented outside the code (JavaDoc or JEP). Also, please update the JavaDoc at line 48, 113, optionally line 130 of Version.java, plus the comment at line 160 of Version.java to match the JEP, since you confirm "9-pre-opt" is valid. Best regards, Hong Dai Thanh. -----Original Message----- From: Iris Clark [mailto:iris.clark@oracle.com] Sent: Wednesday, 3 February, 2016 6:16 AM To: Thanh Hong Dai <hdthanh@tma.com.vn>; Alan Bateman <alan.bateman@oracle.com>; core-libs-dev@openjdk.java.net Subject: RE: RFR: 8072379: Implement jdk.Version and jdk.OracleVersion Hi. Thanks again for looking at this change.
The JEP & implementation allows -$OPT to be specified without +, but the Javadoc one does not allow that. For example, "9-pre-opt" is allowed by JEP, but disallowed by Javadoc.
The JavaDoc and JEP are both slightly wrong in different ways. The implementation is correct. Note that it enforces additional constraints beyond just the regular expression. I've added a few more regression tests to verify that the behavior is as expected. This is the updated webrev and javadoc: http://cr.openjdk.java.net/~iris/verona/8072379/webrev.4/ http://cr.openjdk.java.net/~iris/verona/8072379/doc.4/jdk/Version.html (unchanged from doc.3)
Understood, but I didn't see any part of the JEP or the Javadoc explaining that + is needed to make the parser recognize the text followed as options instead of pre-release identifier. It would be great if that is added.
I've created a new bug to cover updating both the javadoc and JEP for this case. https://bugs.openjdk.java.net/browse/JDK-8148877 JDK-8148877: (spec) Specify when an empty '+' is required in a version string This is the key sentence in the description: '+' is required for empty build when OPT is present and PRE is not. "9+-opt" and "9-pre-opt" are legal. "9+" is not. Regards, iris -----Original Message----- From: Thanh Hong Dai [mailto:hdthanh@tma.com.vn] Sent: Monday, February 01, 2016 11:48 PM To: Iris Clark; Alan Bateman; core-libs-dev@openjdk.java.net Subject: RE: RFR: 8072379: Implement jdk.Version and jdk.OracleVersion Dear Iris, On closer look, there seems to be some conflicting definition of version string. In JEP: http://openjdk.java.net/jeps/223 $VNUM(-$PRE)?(\+$BUILD)?(-$OPT)? In Javadoc: http://cr.openjdk.java.net/~iris/verona/8072379/doc.3/jdk/Version.html $VNUM(-$PRE)?(\+($BUILD)?(-$OPT)?)? In implementation: The regex follows JEP's definition. The JEP & implementation allows -$OPT to be specified without +, but the Javadoc one does not allow that. For example, "9-pre-opt" is allowed by JEP, but disallowed by Javadoc.
I need to capture the plus to distinguish between cases where an empty build is allowed (e.g. "9+-foo") and when it is not ("9+"). See code in Version.java, line 226-230 and in Basic.java, line 98, 107-109. (Note that we use the empty "+" to distinguish "9-foo" from "9+-foo".)
Understood, but I didn't see any part of the JEP or the Javadoc explaining that + is needed to make the parser recognize the text followed as options instead of pre-release identifier. It would be great if that is added. Best regards, Hong Dai Thanh.
Hi.
Also, please update the JavaDoc at line 48, 113, optionally line 130 of Version.java, plus the comment at line 160 of Version.java to match the JEP, since you confirm "9-pre-opt" is valid.
I've added the specific Version.java line numbers to 8148877. My sponsor will push the changes for 8072379 soon. Regards, Iris -----Original Message----- From: Thanh Hong Dai [mailto:hdthanh@tma.com.vn] Sent: Tuesday, February 02, 2016 7:08 PM To: Iris Clark; Alan Bateman; core-libs-dev@openjdk.java.net Subject: RE: RFR: 8072379: Implement jdk.Version and jdk.OracleVersion Dear Iris,
The JavaDoc and JEP are both slightly wrong in different ways. The implementation is correct. Note that it enforces additional constraints beyond just the regular expression.
It would be great if those additional constraints are also documented outside the code (JavaDoc or JEP). Also, please update the JavaDoc at line 48, 113, optionally line 130 of Version.java, plus the comment at line 160 of Version.java to match the JEP, since you confirm "9-pre-opt" is valid. Best regards, Hong Dai Thanh. -----Original Message----- From: Iris Clark [mailto:iris.clark@oracle.com] Sent: Wednesday, 3 February, 2016 6:16 AM To: Thanh Hong Dai <hdthanh@tma.com.vn>; Alan Bateman <alan.bateman@oracle.com>; core-libs-dev@openjdk.java.net Subject: RE: RFR: 8072379: Implement jdk.Version and jdk.OracleVersion Hi. Thanks again for looking at this change.
The JEP & implementation allows -$OPT to be specified without +, but the Javadoc one does not allow that. For example, "9-pre-opt" is allowed by JEP, but disallowed by Javadoc.
The JavaDoc and JEP are both slightly wrong in different ways. The implementation is correct. Note that it enforces additional constraints beyond just the regular expression. I've added a few more regression tests to verify that the behavior is as expected. This is the updated webrev and javadoc: http://cr.openjdk.java.net/~iris/verona/8072379/webrev.4/ http://cr.openjdk.java.net/~iris/verona/8072379/doc.4/jdk/Version.html (unchanged from doc.3)
Understood, but I didn't see any part of the JEP or the Javadoc explaining that + is needed to make the parser recognize the text followed as options instead of pre-release identifier. It would be great if that is added.
I've created a new bug to cover updating both the javadoc and JEP for this case. https://bugs.openjdk.java.net/browse/JDK-8148877 JDK-8148877: (spec) Specify when an empty '+' is required in a version string This is the key sentence in the description: '+' is required for empty build when OPT is present and PRE is not. "9+-opt" and "9-pre-opt" are legal. "9+" is not. Regards, iris -----Original Message----- From: Thanh Hong Dai [mailto:hdthanh@tma.com.vn] Sent: Monday, February 01, 2016 11:48 PM To: Iris Clark; Alan Bateman; core-libs-dev@openjdk.java.net Subject: RE: RFR: 8072379: Implement jdk.Version and jdk.OracleVersion Dear Iris, On closer look, there seems to be some conflicting definition of version string. In JEP: http://openjdk.java.net/jeps/223 $VNUM(-$PRE)?(\+$BUILD)?(-$OPT)? In Javadoc: http://cr.openjdk.java.net/~iris/verona/8072379/doc.3/jdk/Version.html $VNUM(-$PRE)?(\+($BUILD)?(-$OPT)?)? In implementation: The regex follows JEP's definition. The JEP & implementation allows -$OPT to be specified without +, but the Javadoc one does not allow that. For example, "9-pre-opt" is allowed by JEP, but disallowed by Javadoc.
I need to capture the plus to distinguish between cases where an empty build is allowed (e.g. "9+-foo") and when it is not ("9+"). See code in Version.java, line 226-230 and in Basic.java, line 98, 107-109. (Note that we use the empty "+" to distinguish "9-foo" from "9+-foo".)
Understood, but I didn't see any part of the JEP or the Javadoc explaining that + is needed to make the parser recognize the text followed as options instead of pre-release identifier. It would be great if that is added. Best regards, Hong Dai Thanh.
On 13/01/2016 21:54, Iris Clark wrote:
: This diff has been applied to modules.xml: This looks okay.
:
When this came up earlier, I filed this bug to track finding a more appropriate module for jdk.Version:
8144062: Determine appropriate module for jdk.Version https://bugs.openjdk.java.net/browse/JDK-8144062
Thanks, I've changed this to be a P2 as we will need to decide on this soon. -Alan.
On Jan 11, 2016, at 1:44 PM, Iris Clark <iris.clark@oracle.com> wrote:
Hi, Joe, Roger, Alan, Magnus, and Mandy.
At the end of December (shortly before the Christmas/Winter break and my vacation), I provided responses to your messages and an updated webrev:
It’s good to see that jdk.OracleVersion has been removed and use jdk.Version to obtain the fourth element of the version number. This patch looks good in general. Minor comment: 273 current = parse(System.getProperty("java.version")); System.getProperty will do a permission check and it needs to be called within a doPrivileged block. 154 * @see <a href="http://openjdk.java.net/jeps/223">JEP 223: New Version-String Scheme</a> Does the javadoc have the essential information from this JEP? Wonder if this @see is necessary. Alan already comment on the “jdk” package that needs to find the proper module to export it (that’s a future RFE) and modules.xml should be updated. Mandy
Hi, Mandy. Thanks for looking at this webrev again.
273 current = parse(System.getProperty("java.version"));
System.getProperty will do a permission check and it needs to be called within a doPrivileged block.
Nice catch! I've added the permission check and the associated @throws.
154 * @see <a href="http://openjdk.java.net/jeps/223">JEP 223: New Version-String Scheme</a>
Does the javadoc have the essential information from this JEP? Wonder if this @see is necessary.
I thought it might be nice to include the JEP reference as it contains additional data about versioning beyond the JavaDoc for this method; however, it's not critical to the understanding or use of this class. For some reason I'd thought that there was precedence in including JEP numbers as @see references, but I find no evidence of that. I've removed the @see. These are the diffs to address both of your comments: $ diff Version.java_save Version.java 28a29,30
import java.security.AccessController; import java.security.PrivilegedAction; 154,155d155 < * @see <a href="http://openjdk.java.net/jeps/223">JEP 223: New Version-String Scheme</a> < * 268a269,274 * @throws SecurityException * If a security manager exists and its {@link * SecurityManager#checkPropertyAccess(String) * checkPropertyAccess} method does not allow access to the * system property "java.version" * 272,273c278,285 < if (current == null) < current = parse(System.getProperty("java.version"));
if (current == null) { current = parse(AccessController.doPrivileged( new PrivilegedAction<>() { public String run() { return System.getProperty("java.version"); } })); }
Regards, iris -----Original Message----- From: Mandy Chung Sent: Tuesday, January 12, 2016 2:26 PM To: Iris Clark Cc: Mandy Chung; Joe Darcy; Magnus Ihse Bursie; Roger Riggs; Alan Bateman; core-libs-dev; verona-dev@openjdk.java.net Subject: Re: RFR: 8072379: Implement jdk.Version and jdk.OracleVersion
On Jan 11, 2016, at 1:44 PM, Iris Clark <iris.clark@oracle.com> wrote:
Hi, Joe, Roger, Alan, Magnus, and Mandy.
At the end of December (shortly before the Christmas/Winter break and my vacation), I provided responses to your messages and an updated webrev:
It’s good to see that jdk.OracleVersion has been removed and use jdk.Version to obtain the fourth element of the version number. This patch looks good in general. Minor comment: 273 current = parse(System.getProperty("java.version")); System.getProperty will do a permission check and it needs to be called within a doPrivileged block. 154 * @see <a href="http://openjdk.java.net/jeps/223">JEP 223: New Version-String Scheme</a> Does the javadoc have the essential information from this JEP? Wonder if this @see is necessary. Alan already comment on the “jdk” package that needs to find the proper module to export it (that’s a future RFE) and modules.xml should be updated. Mandy
On Jan 13, 2016, at 9:27 PM, Iris Clark <iris.clark@oracle.com> wrote:
These are the diffs to address both of your comments:
$ diff Version.java_save Version.java 28a29,30
import java.security.AccessController; import java.security.PrivilegedAction; 154,155d155 < * @see <a href="http://openjdk.java.net/jeps/223">JEP 223: New Version-String Scheme</a> < * 268a269,274 * @throws SecurityException * If a security manager exists and its {@link * SecurityManager#checkPropertyAccess(String) * checkPropertyAccess} method does not allow access to the * system property "java.version" * 272,273c278,285 < if (current == null) < current = parse(System.getProperty("java.version"));
if (current == null) { current = parse(AccessController.doPrivileged( new PrivilegedAction<>() { public String run() { return System.getProperty("java.version"); } })); }
Looks fine. Thanks Mandy
Hi Iris, Catching up on old reviews, the regular expression for versions is given as ^[1-9][0-9]*(((\.0)*\.[1-9][0-9]*)*)*$ Is this equivalent to ^[1-9][0-9]*(\.[0-9])* Trying to put this in words, "The version number starts with a sequence of digits where the leading digit is not "0". Afterward, there are zero or more period separated groups of digits with no restrictions on the digits. Note that this implies The version number ends with a digit group." I suggest changing the class-level javadoc discussing comparisons to defer to the various compare methods. It would be helpful to not which compare methods are consistent with equals and which are not. Thanks, -Joe On 1/11/2016 1:44 PM, Iris Clark wrote:
Hi, Joe, Roger, Alan, Magnus, and Mandy.
At the end of December (shortly before the Christmas/Winter break and my vacation), I provided responses to your messages and an updated webrev:
http://cr.openjdk.java.net/~iris/verona/8072379/webrev.2/
I didn't hear from anybody, so I'd like to optimistically assume that you were satisfied. Is that correct?
For you convenience, here's a reference to the December and November threads:
http://mail.openjdk.java.net/pipermail/core-libs-dev/2015-December/037062.ht... http://mail.openjdk.java.net/pipermail/core-libs-dev/2015-November/036904.ht...
I'd like to wrap up this work for the initial implementation of jdk.Version soon.
Regards, iris
-----Original Message----- From: Iris Clark Sent: Friday, December 18, 2015 1:55 PM To: Joe Darcy; Mandy Chung Cc: core-libs-dev@openjdk.java.net; verona-dev@openjdk.java.net Subject: RE: RFR: 8072379: Implement jdk.Version and jdk.OracleVersion
Hi, Joe.
Thanks for the review comments.
Is the intention that downstream JDK distributions, such as IcedTea, whether based on OpenJDK or otherwise, would provide their own specialization of the jdk.Version class? No. Downstream users may provide their own specialization, but there is no requirement that they must do so.
jdk.OracleVersion has been removed. The functionality it provided, access to the fourth element of the version number, is trivially accessed by existing methods in jdk.Version.
I've filed the following bug to address the more general use case:
8145793: Provide vendor-specific interpretation of a JDK version string https://bugs.openjdk.java.net/browse/JDK-8145793
The equals / hashCode behavior of Version and OracleVersion is bit surprising and I think somewhat underspecified given the possibility of defining subclasses. Given the above regarding downstream use, I've make jdk.Version final and the constructor is now private. Oracle or any other JDK distribution wishing to impose additional semantics to the version string interpretation will need to do so via encapsulation rather than inheritance.
How is this API supposed to behave if the component version strings have a numerical value greater than Integer.MAX_VALUE? From the specification of the only instance method, Version.parse():
* @throws NumberFormatException * If an element of the version number or the build number cannot * be represented as an {@link Integer}
Was using Longs to record numerical versions rather than Integers considered? Yes. In an informal poll conducted during implementation, it was thought that 2^32 bits would be more than sufficient to represent the components of typical version numbers.
An updated webrev is forthcoming.
Thanks, iris
PS: I believe that we are both out the week of 21 December, so I don't expect to hear back from you until the New Year.
Hi Joe, On 1/13/16 2:06 AM, Joseph D. Darcy wrote:
Hi Iris,
Catching up on old reviews, the regular expression for versions is given as
^[1-9][0-9]*(((\.0)*\.[1-9][0-9]*)*)*$
Is this equivalent to
^[1-9][0-9]*(\.[0-9])*
Iris's regexp will disallow trailing .0 9.0.1 will match, but 9.0 and 9.0.0 will not - the version should be plain 9 in those latter cases as documented in the javadoc. I believe this is important because it ensures that the version string is always in canonical form, which makes it possible to simply compare the 'versions' list (List<Integer>) within the various variations of equals... So the regexp matching will eliminate non canonical forms without having to perform further checks down the road. Took me a while to figure it, I had to look at the test to pinpoint it. Now that I understand, I find it quite elegant :-) best regards, -- daniel
Hi Daniel, On 1/13/2016 1:01 AM, Daniel Fuchs wrote:
Hi Joe,
On 1/13/16 2:06 AM, Joseph D. Darcy wrote:
Hi Iris,
Catching up on old reviews, the regular expression for versions is given as
^[1-9][0-9]*(((\.0)*\.[1-9][0-9]*)*)*$
Is this equivalent to
^[1-9][0-9]*(\.[0-9])*
Iris's regexp will disallow trailing .0
9.0.1 will match, but 9.0 and 9.0.0 will not - the version should be plain 9 in those latter cases as documented in the javadoc.
I believe this is important because it ensures that the version string is always in canonical form, which makes it possible to simply compare the 'versions' list (List<Integer>) within the various variations of equals...
So the regexp matching will eliminate non canonical forms without having to perform further checks down the road. Took me a while to figure it, I had to look at the test to pinpoint it. Now that I understand, I find it quite elegant :-)
Ah okay, I missed those points when reading over the regular expression. I suggest some discussion be added to explain and document that those aspects of the design are intentional. Thanks, -Joe
Hi, Joe.
I suggest changing the class-level javadoc discussing comparisons to defer to the various compare methods. It would be helpful to not which compare methods are consistent with equals and which are not.
I've reworked the class and method javadoc for the comparison methods so that comparison details exist in only one place. I believe that this resolves the remaining spec issue to get an initial implementation of jdk.Version into jdk9/dev. After I've enhanced the regular expression as suggested elsewhere in this thread I'll post the updated webrev. Thanks, iris
Hi Iris, Do you consider an option to let community reuse JDK versioning style for their own purposes. Probably defining an interface with basic default methods which can be extended by various libraries to provide unified way to gather version information from MANIFEST.MF, ClassLoader's jars and simply to allow developers inventing one wheel lesser. /Victor --- Original message --- From: "Iris Clark" <iris.clark@oracle.com> Date: 11 January 2016, 23:45:56
Hi, Joe, Roger, Alan, Magnus, and Mandy.
At the end of December (shortly before the Christmas/Winter break and my vacation), I provided responses to your messages and an updated webrev:
http://cr.openjdk.java.net/~iris/verona/8072379/webrev.2/
I didn't hear from anybody, so I'd like to optimistically assume that you were satisfied. Is that correct?
For you convenience, here's a reference to the December and November threads:
http://mail.openjdk.java.net/pipermail/core-libs-dev/2015-December/037062.ht... http://mail.openjdk.java.net/pipermail/core-libs-dev/2015-November/036904.ht...
I'd like to wrap up this work for the initial implementation of jdk.Version soon.
Regards, iris
On 2016-01-11 22:44, Iris Clark wrote:
Hi, Joe, Roger, Alan, Magnus, and Mandy.
At the end of December (shortly before the Christmas/Winter break and my vacation), I provided responses to your messages and an updated webrev:
http://cr.openjdk.java.net/~iris/verona/8072379/webrev.2/
I didn't hear from anybody, so I'd like to optimistically assume that you were satisfied. Is that correct?
I'm basically happy. As other reviewers, I think the regexes are a bit tough to parse (regexes always are), and some additional comments on the effect would be appreciated. Thanh Hong Dai had some good suggestions as well on how to improve readability. I noted that your java source file had a lot of initial tabs instead of spaces. I'm not sure if we have any code guidelines about this, but I have not encountered that in the JDK source base before, so I'd recommend you turn them into spaces. /Magnus
For you convenience, here's a reference to the December and November threads:
http://mail.openjdk.java.net/pipermail/core-libs-dev/2015-December/037062.ht... http://mail.openjdk.java.net/pipermail/core-libs-dev/2015-November/036904.ht...
I'd like to wrap up this work for the initial implementation of jdk.Version soon.
Regards, iris
-----Original Message----- From: Iris Clark Sent: Friday, December 18, 2015 1:55 PM To: Joe Darcy; Mandy Chung Cc: core-libs-dev@openjdk.java.net; verona-dev@openjdk.java.net Subject: RE: RFR: 8072379: Implement jdk.Version and jdk.OracleVersion
Hi, Joe.
Thanks for the review comments.
Is the intention that downstream JDK distributions, such as IcedTea, whether based on OpenJDK or otherwise, would provide their own specialization of the jdk.Version class? No. Downstream users may provide their own specialization, but there is no requirement that they must do so.
jdk.OracleVersion has been removed. The functionality it provided, access to the fourth element of the version number, is trivially accessed by existing methods in jdk.Version.
I've filed the following bug to address the more general use case:
8145793: Provide vendor-specific interpretation of a JDK version string https://bugs.openjdk.java.net/browse/JDK-8145793
The equals / hashCode behavior of Version and OracleVersion is bit surprising and I think somewhat underspecified given the possibility of defining subclasses. Given the above regarding downstream use, I've make jdk.Version final and the constructor is now private. Oracle or any other JDK distribution wishing to impose additional semantics to the version string interpretation will need to do so via encapsulation rather than inheritance.
How is this API supposed to behave if the component version strings have a numerical value greater than Integer.MAX_VALUE? From the specification of the only instance method, Version.parse():
* @throws NumberFormatException * If an element of the version number or the build number cannot * be represented as an {@link Integer}
Was using Longs to record numerical versions rather than Integers considered? Yes. In an informal poll conducted during implementation, it was thought that 2^32 bits would be more than sufficient to represent the components of typical version numbers.
An updated webrev is forthcoming.
Thanks, iris
PS: I believe that we are both out the week of 21 December, so I don't expect to hear back from you until the New Year.
Hi, Magnus. Thanks for taking the time to look at this again.
I noted that your java source file had a lot of initial tabs instead of spaces. I'm not sure if we have any code guidelines about this,
jcheck prevents tabs in comments and source. I remove them, one of my editors adds them when I make changes. It's annoying. They won't be in the checked in source. Regards, iris -----Original Message----- From: Magnus Ihse Bursie Sent: Thursday, January 14, 2016 3:38 AM To: Iris Clark; Joe Darcy; Mandy Chung; Roger Riggs; Alan Bateman Cc: core-libs-dev@openjdk.java.net; verona-dev@openjdk.java.net Subject: Re: RFR: 8072379: Implement jdk.Version and jdk.OracleVersion On 2016-01-11 22:44, Iris Clark wrote:
Hi, Joe, Roger, Alan, Magnus, and Mandy.
At the end of December (shortly before the Christmas/Winter break and my vacation), I provided responses to your messages and an updated webrev:
http://cr.openjdk.java.net/~iris/verona/8072379/webrev.2/
I didn't hear from anybody, so I'd like to optimistically assume that you were satisfied. Is that correct?
I'm basically happy. As other reviewers, I think the regexes are a bit tough to parse (regexes always are), and some additional comments on the effect would be appreciated. Thanh Hong Dai had some good suggestions as well on how to improve readability. I noted that your java source file had a lot of initial tabs instead of spaces. I'm not sure if we have any code guidelines about this, but I have not encountered that in the JDK source base before, so I'd recommend you turn them into spaces. /Magnus
For you convenience, here's a reference to the December and November threads:
http://mail.openjdk.java.net/pipermail/core-libs-dev/2015-December/037062.ht...
http://mail.openjdk.java.net/pipermail/core-libs-dev/2015-November/036 904.html
I'd like to wrap up this work for the initial implementation of jdk.Version soon.
Regards, iris
-----Original Message----- From: Iris Clark Sent: Friday, December 18, 2015 1:55 PM To: Joe Darcy; Mandy Chung Cc: core-libs-dev@openjdk.java.net; verona-dev@openjdk.java.net Subject: RE: RFR: 8072379: Implement jdk.Version and jdk.OracleVersion
Hi, Joe.
Thanks for the review comments.
Is the intention that downstream JDK distributions, such as IcedTea, whether based on OpenJDK or otherwise, would provide their own specialization of the jdk.Version class? No. Downstream users may provide their own specialization, but there is no requirement that they must do so.
jdk.OracleVersion has been removed. The functionality it provided, access to the fourth element of the version number, is trivially accessed by existing methods in jdk.Version.
I've filed the following bug to address the more general use case:
8145793: Provide vendor-specific interpretation of a JDK version string https://bugs.openjdk.java.net/browse/JDK-8145793
The equals / hashCode behavior of Version and OracleVersion is bit surprising and I think somewhat underspecified given the possibility of defining subclasses. Given the above regarding downstream use, I've make jdk.Version final and the constructor is now private. Oracle or any other JDK distribution wishing to impose additional semantics to the version string interpretation will need to do so via encapsulation rather than inheritance.
How is this API supposed to behave if the component version strings have a numerical value greater than Integer.MAX_VALUE? From the specification of the only instance method, Version.parse():
* @throws NumberFormatException * If an element of the version number or the build number cannot * be represented as an {@link Integer}
Was using Longs to record numerical versions rather than Integers considered? Yes. In an informal poll conducted during implementation, it was thought that 2^32 bits would be more than sufficient to represent the components of typical version numbers.
An updated webrev is forthcoming.
Thanks, iris
PS: I believe that we are both out the week of 21 December, so I don't expect to hear back from you until the New Year.
Hi, Mandy. Thanks so much for pushing the changeset for the initial implementation of jdk.Version! Regards, iris -----Original Message----- From: Iris Clark Sent: Monday, January 11, 2016 1:45 PM To: Iris Clark; Joe Darcy; Mandy Chung; Magnus Ihse Bursie; Roger Riggs; Alan Bateman Cc: core-libs-dev@openjdk.java.net; verona-dev@openjdk.java.net Subject: RE: RFR: 8072379: Implement jdk.Version and jdk.OracleVersion Hi, Joe, Roger, Alan, Magnus, and Mandy. At the end of December (shortly before the Christmas/Winter break and my vacation), I provided responses to your messages and an updated webrev: http://cr.openjdk.java.net/~iris/verona/8072379/webrev.2/ I didn't hear from anybody, so I'd like to optimistically assume that you were satisfied. Is that correct? For you convenience, here's a reference to the December and November threads: http://mail.openjdk.java.net/pipermail/core-libs-dev/2015-December/037062.ht... http://mail.openjdk.java.net/pipermail/core-libs-dev/2015-November/036904.ht... I'd like to wrap up this work for the initial implementation of jdk.Version soon. Regards, iris -----Original Message----- From: Iris Clark Sent: Friday, December 18, 2015 1:55 PM To: Joe Darcy; Mandy Chung Cc: core-libs-dev@openjdk.java.net; verona-dev@openjdk.java.net Subject: RE: RFR: 8072379: Implement jdk.Version and jdk.OracleVersion Hi, Joe. Thanks for the review comments.
Is the intention that downstream JDK distributions, such as IcedTea, whether based on OpenJDK or otherwise, would provide their own specialization of the jdk.Version class?
No. Downstream users may provide their own specialization, but there is no requirement that they must do so. jdk.OracleVersion has been removed. The functionality it provided, access to the fourth element of the version number, is trivially accessed by existing methods in jdk.Version. I've filed the following bug to address the more general use case: 8145793: Provide vendor-specific interpretation of a JDK version string https://bugs.openjdk.java.net/browse/JDK-8145793
The equals / hashCode behavior of Version and OracleVersion is bit surprising and I think somewhat underspecified given the possibility of defining subclasses.
Given the above regarding downstream use, I've make jdk.Version final and the constructor is now private. Oracle or any other JDK distribution wishing to impose additional semantics to the version string interpretation will need to do so via encapsulation rather than inheritance.
How is this API supposed to behave if the component version strings have a numerical value greater than Integer.MAX_VALUE?
From the specification of the only instance method, Version.parse():
* @throws NumberFormatException * If an element of the version number or the build number cannot * be represented as an {@link Integer}
Was using Longs to record numerical versions rather than Integers considered?
Yes. In an informal poll conducted during implementation, it was thought that 2^32 bits would be more than sufficient to represent the components of typical version numbers. An updated webrev is forthcoming. Thanks, iris PS: I believe that we are both out the week of 21 December, so I don't expect to hear back from you until the New Year.
Sure thing. Glad to see this going in. Mandy
On Feb 2, 2016, at 7:17 PM, Iris Clark <iris.clark@oracle.com> wrote:
Hi, Mandy.
Thanks so much for pushing the changeset for the initial implementation of jdk.Version!
Regards, iris
-----Original Message----- From: Iris Clark Sent: Monday, January 11, 2016 1:45 PM To: Iris Clark; Joe Darcy; Mandy Chung; Magnus Ihse Bursie; Roger Riggs; Alan Bateman Cc: core-libs-dev@openjdk.java.net; verona-dev@openjdk.java.net Subject: RE: RFR: 8072379: Implement jdk.Version and jdk.OracleVersion
Hi, Joe, Roger, Alan, Magnus, and Mandy.
At the end of December (shortly before the Christmas/Winter break and my vacation), I provided responses to your messages and an updated webrev:
http://cr.openjdk.java.net/~iris/verona/8072379/webrev.2/
I didn't hear from anybody, so I'd like to optimistically assume that you were satisfied. Is that correct?
For you convenience, here's a reference to the December and November threads:
http://mail.openjdk.java.net/pipermail/core-libs-dev/2015-December/037062.ht... http://mail.openjdk.java.net/pipermail/core-libs-dev/2015-November/036904.ht...
I'd like to wrap up this work for the initial implementation of jdk.Version soon.
Regards, iris
-----Original Message----- From: Iris Clark Sent: Friday, December 18, 2015 1:55 PM To: Joe Darcy; Mandy Chung Cc: core-libs-dev@openjdk.java.net; verona-dev@openjdk.java.net Subject: RE: RFR: 8072379: Implement jdk.Version and jdk.OracleVersion
Hi, Joe.
Thanks for the review comments.
Is the intention that downstream JDK distributions, such as IcedTea, whether based on OpenJDK or otherwise, would provide their own specialization of the jdk.Version class?
No. Downstream users may provide their own specialization, but there is no requirement that they must do so.
jdk.OracleVersion has been removed. The functionality it provided, access to the fourth element of the version number, is trivially accessed by existing methods in jdk.Version.
I've filed the following bug to address the more general use case:
8145793: Provide vendor-specific interpretation of a JDK version string https://bugs.openjdk.java.net/browse/JDK-8145793
The equals / hashCode behavior of Version and OracleVersion is bit surprising and I think somewhat underspecified given the possibility of defining subclasses.
Given the above regarding downstream use, I've make jdk.Version final and the constructor is now private. Oracle or any other JDK distribution wishing to impose additional semantics to the version string interpretation will need to do so via encapsulation rather than inheritance.
How is this API supposed to behave if the component version strings have a numerical value greater than Integer.MAX_VALUE?
From the specification of the only instance method, Version.parse():
* @throws NumberFormatException * If an element of the version number or the build number cannot * be represented as an {@link Integer}
Was using Longs to record numerical versions rather than Integers considered?
Yes. In an informal poll conducted during implementation, it was thought that 2^32 bits would be more than sufficient to represent the components of typical version numbers.
An updated webrev is forthcoming.
Thanks, iris
PS: I believe that we are both out the week of 21 December, so I don't expect to hear back from you until the New Year.
On 03/02/2016 03:17, Iris Clark wrote:
Hi, Mandy.
Thanks so much for pushing the changeset for the initial implementation of jdk.Version!
Good to have this in but now we need to decide on where it should live. It's JDK-specific so we'll need it exported by a JDK module rather than java.base. -Alan
Hi, Alan.
Good to have this in but now we need to decide on where it should live. It's JDK-specific so we'll need it exported by a JDK module rather than java.base.
8144062 was next on my list. Do you have any suggestions for the module? Of the existing modules in the jdk repository, jdk.dev seems to have the most appropriate name. It appears to only contain jimage so I'm not sure about that module's charter. Thanks, iris -----Original Message----- From: Alan Bateman Sent: Wednesday, February 03, 2016 12:53 AM To: Iris Clark; Mandy Chung Cc: verona-dev@openjdk.java.net; core-libs-dev@openjdk.java.net Subject: Re: RFR: 8072379: Implement jdk.Version and jdk.OracleVersion On 03/02/2016 03:17, Iris Clark wrote:
Hi, Mandy.
Thanks so much for pushing the changeset for the initial implementation of jdk.Version!
Good to have this in but now we need to decide on where it should live. It's JDK-specific so we'll need it exported by a JDK module rather than java.base. -Alan
Hi, Alan, Mandy, and Mark. After exploring a few module options (jdk.Version, jdk.dev [0]), it looks like the best choice is to move jdk.Version to java.lang.Runtime.Version (a nested class of Runtime). It supports the values returned by the java.{vm.}?version and java.{vm.}?specification.version system properties. By making Version an SE API, it may be exported by the java.base module. Please review the following changes to move jdk.Version to jdk.lang.Runtime.Version. Note that beyond the package name and class declaration (to static), the only other changes in Version are related to indentation. Bug 8144062: Determine appropriate module for jdk.Version https://bugs.openjdk.java.net/browse/JDK-8144062 webrev http://cr.openjdk.java.net/~iris/verona/8144062/webrev.0/ Additionally, the following changes to JEP 223 [1] (modulo indentation) will be made: 252,253c252,253 < A simple JDK-specific Java API to parse, validate, and compare version < strings will be defined: ---
A simple Java API to parse, validate, and compare version strings will be defined as a nested class of Runtime: 255c255 < package jdk;
package java.lang;
258a259
public class Runtime {
283a285
}
309a312,315
The existing specification for the above properties in `System.getProperties()` will be modified to indicate that the return values for these properties may be interpreted as a `java.lang.Runtime.Version`.
Thanks, Iris [0] jdk.dev renamed to jdk.jlink in jigsaw/jake. [1] http://openjdk.java.net/jeps/223
On 25/02/2016 05:59, Iris Clark wrote:
Hi, Alan, Mandy, and Mark.
After exploring a few module options (jdk.Version, jdk.dev [0]), it looks like the best choice is to move jdk.Version to java.lang.Runtime.Version (a nested class of Runtime). It supports the values returned by the java.{vm.}?version and java.{vm.}?specification.version system properties. By making Version an SE API, it may be exported by the java.base module.
Please review the following changes to move jdk.Version to jdk.lang.Runtime.Version. Note that beyond the package name and class declaration (to static), the only other changes in Version are related to indentation.
This looks good. We should probably change the synopsis on the JIRA issue to make it clearer that it is promoting Version to a standard API. Once this is in then the multi-release JAR file patch can be updated to make use of this. -Alan.
On 25 Feb 2016, at 09:06, Alan Bateman <Alan.Bateman@oracle.com> wrote:
On 25/02/2016 05:59, Iris Clark wrote:
Hi, Alan, Mandy, and Mark.
After exploring a few module options (jdk.Version, jdk.dev [0]), it looks like the best choice is to move jdk.Version to java.lang.Runtime.Version (a nested class of Runtime). It supports the values returned by the java.{vm.}?version and java.{vm.}?specification.version system properties. By making Version an SE API, it may be exported by the java.base module.
Please review the following changes to move jdk.Version to jdk.lang.Runtime.Version. Note that beyond the package name and class declaration (to static), the only other changes in Version are related to indentation.
This looks good. We should probably change the synopsis on the JIRA issue to make it clearer that it is promoting Version to a standard API.
Dunno if the following was discussed for the review of internal impl. I think it would be better to internally maintain an int[] for the version numbers rather than List<Integer>, that is more efficient in terms of space and comparing/equality (you can even use the new array comparison methods, thus the compareVersion becomes a one-liner). It’s ok to take the hit for returning List<Integer> a simple AbstractList wrapper (also implementing RandomAccess) will suffice [*].
Once this is in then the multi-release JAR file patch can be updated to make use of this.
And since this will now be part of SE it might be possible to revisit (as a follow on issue) the definition and use of the JarFile.Release enum by MR-JARs. Paul. [*] public List<Integer> version() { class VersionList extends AbstractList<Integer> implements RandomAccess { @Override public Integer get(int index) { return version[index]; } @Override public int size() { return version.length; } } return new VersionList(); }
On Feb 24, 2016, at 9:59 PM, Iris Clark <iris.clark@oracle.com> wrote:
Hi, Alan, Mandy, and Mark.
After exploring a few module options (jdk.Version, jdk.dev [0]), it looks like the best choice is to move jdk.Version to java.lang.Runtime.Version (a nested class of Runtime). It supports the values returned by the java.{vm.}?version and java.{vm.}?specification.version system properties. By making Version an SE API, it may be exported by the java.base module.
Please review the following changes to move jdk.Version to jdk.lang.Runtime.Version. Note that beyond the package name and class declaration (to static), the only other changes in Version are related to indentation.
Bug
8144062: Determine appropriate module for jdk.Version https://bugs.openjdk.java.net/browse/JDK-8144062
webrev
942 * A representation of the JDK version-string version string for the runtime instead of JDK One suggestion is to move the details about fourth and later elements to @implSpec or @implNote. Have you considered moving the static Version::current method to Runtime::version? Mandy
Hi Iris, I see the JEP says JDK specific, but does that rule out putting the version API in a Java.* package? It would support the values found in the java.version, etc properties. Perhaps as an nested class of System or Runtime? Version.java: Line 213: Seems a bit wasteful to reparse the string after the matcher has done its work; but does not use the groups for the version components. 229: The IAE("Terminal...") exception may not be that easily understood; can it say concisely that the build and optional elements are missing 264: current() should cache the value from the parse; it is likely to be called more than once and parsing an version string is a relatively expensive. 334: the naming of one of the version elements as 'optional' may be confusing because optional is an adjective. Especially when the element is optional and the method named optional returns an Optional. Can the name be a cogent noun. how about 'info' for informational string 396: I'm not sure why BigInteger is needed; other than perhaps because it has a constructor that takes a string. 433: Optional has a .ifPresent(xxx) method that could be used to streamline the code. pre.ifPresent( v -> sb.append("-").append(v)); OracleVersion.java: Can this be renamed more functionally to reflect that the 4'th component is a patch number. It might be useful to folks other than Oracle. 107: the constructor should be declared private since it is not needed outside the class. I would have preferred the Tests to be written using TestNG. Thanks, Roger On 11/24/2015 8:54 PM, Iris Clark wrote:
Hi.
Please review the new classes jdk.Version and jdk.OracleVersion. These are simple Java APIs to parse, validate, and compare version numbers.
Bug
8072379: Implement jdk.Version and jdk.OracleVersion https://bugs.openjdk.java.net/browse/JDK-8072379
Webrev
http://cr.openjdk.java.net/~iris/verona/8072379/webrev.1/
JavaDoc
http://cr.openjdk.java.net/~iris/verona/8072379/doc.1/jdk/Version.html http://cr.openjdk.java.net/~iris/verona/8072379/doc.1/jdk/OracleVersion.html
The .java files are predominantly javadoc. The code is relatively straight-forward.
jdk.Version is the representation of the JDK version string as described in JEP 223 ([0], 8061493). The javadoc is largely taken from the description section in the JEP. The API is described in the "API" section.
jdk.OracleVersion extends jdk.Version and is the representation of the Oracle JDK version string. Its only purpose is to interpret the fourth element of the version number as a patch release.
There are some minor discrepancies between the implementation and the JEP. The JEP will need to be updated. The most notable is the name of the package ("jdk" vs. the original "jdk.util"). The rename was recommended by Mark.
Thanks, iris
Hi, Roger. Thanks for the feedback.
I see the JEP says JDK specific, but does that rule out putting the version API in a Java.* package?
For now, we should avoid including this API in java.*. If for no other reason but that we may need to evolve the API in an update release.
Version.java:
229: The IAE("Terminal...") exception may not be that easily understood; can it say concisely that the build and optional elements are missing
I've changed it to this which is technically more accurate (but verbose): IAE("'+' found with neither build or optional components: ':" + s + "'"). If you have a more succinct suggestion, let me know.
264: current() should cache the value from the parse; it is likely to be called more than once and parsing an version string is a relatively expensive.
Done.
334: the naming of one of the version elements as 'optional' may be confusing because optional is an adjective. Especially when the element is optional and the method named optional returns an Optional. Can the name be a cogent noun. how about 'info' for informational string
The term "optional" to refer to this portion of the string is used in quite a few places, both inside and outside of OpenJDK code. I don't think that it would be helpful to refer to this by two different names. If you feel strongly that the name should be changed, I can file an RFE to investigate the full impact to do so.
396: I'm not sure why BigInteger is needed; other than perhaps because it has a constructor that takes a string.
The components of $VNUM are constrained to be ints, hence if a string of numbers greater than Integer.MAX_VALUE is passed, construction of the Version will fail. $PRE is an arbitrary length String which may consist entirely of digits. As long as the string of digits may be represented as a String, there is no constraint on how many digits there are. The only time when the number of digits is a potential problem is during Version comparison. In the case when both of the Version objects contain $PRE consisting entirely of digits, we need to consider the case where the number of digits would result in a integer larger than Integer.MAX_VALUE.
433: Optional has a .ifPresent(xxx) method that could be used to streamline the code. pre.ifPresent( v -> sb.append("-").append(v));
Done.
OracleVersion.java: Can this be renamed more functionally to reflect that the 4'th component is a patch number. It might be useful to folks other than Oracle.
107: the constructor should be declared private since it is not needed outside the class.
OracleVersion.java is gone. The functionality it provided, access to the fourth element of the version number, is trivially accessed by existing methods in jdk.Version. jdk.Version is now final, with a private constructor. An updated webrev is forthcoming. Regards, iris -----Original Message----- From: Roger Riggs Sent: Wednesday, November 25, 2015 1:23 PM To: core-libs-dev@openjdk.java.net Subject: Re: RFR: 8072379: Implement jdk.Version and jdk.OracleVersion Hi Iris, I see the JEP says JDK specific, but does that rule out putting the version API in a Java.* package? It would support the values found in the java.version, etc properties. Perhaps as an nested class of System or Runtime? Version.java: Line 213: Seems a bit wasteful to reparse the string after the matcher has done its work; but does not use the groups for the version components. 229: The IAE("Terminal...") exception may not be that easily understood; can it say concisely that the build and optional elements are missing 264: current() should cache the value from the parse; it is likely to be called more than once and parsing an version string is a relatively expensive. 334: the naming of one of the version elements as 'optional' may be confusing because optional is an adjective. Especially when the element is optional and the method named optional returns an Optional. Can the name be a cogent noun. how about 'info' for informational string 396: I'm not sure why BigInteger is needed; other than perhaps because it has a constructor that takes a string. 433: Optional has a .ifPresent(xxx) method that could be used to streamline the code. pre.ifPresent( v -> sb.append("-").append(v)); OracleVersion.java: Can this be renamed more functionally to reflect that the 4'th component is a patch number. It might be useful to folks other than Oracle. 107: the constructor should be declared private since it is not needed outside the class. I would have preferred the Tests to be written using TestNG. Thanks, Roger
On 2015-11-25 02:54, Iris Clark wrote:
Hi.
Please review the new classes jdk.Version and jdk.OracleVersion. These are simple Java APIs to parse, validate, and compare version numbers.
Bug
8072379: Implement jdk.Version and jdk.OracleVersion https://bugs.openjdk.java.net/browse/JDK-8072379
Webrev
Hi Iris, I thought the end agreement was that the + should always be present even if build was empty, if opt was present but not pre. That is, "9-foo" should unambigiously parse as vnum=9 and pre="foo", while "9-+foo" should umambigously parse as vnum=9 and opt="foo". The javadoc does not seem to reflect this. I've also had to read and re-read the regexp and parsing logic in the constructor to convince myself that this (most likely) will be correctly handled. Perhaps a few comments around this would be helpful? The comparison of two version strings which differs only in "pre" does not adhere to the principle of astonishment. The documentation states: "Pre-release identifiers are compared numerically when they consist only of digits, and lexiographically otherwise. Numeric identifiers are considered to be less than non-numeric identifiers." But consider the following version strings: 9-01 9-01a 9-02 9-003b That sequence would be ordered like this, which I find highly surprising. 9-01 9-02 9-003b 9-01a I'm also surprised that equals() does not produce the same result as compareTo(). If opt is to be ignored, surely it should be so in equals() as well? /Magnus
JavaDoc
http://cr.openjdk.java.net/~iris/verona/8072379/doc.1/jdk/Version.html http://cr.openjdk.java.net/~iris/verona/8072379/doc.1/jdk/OracleVersion.html
The .java files are predominantly javadoc. The code is relatively straight-forward.
jdk.Version is the representation of the JDK version string as described in JEP 223 ([0], 8061493). The javadoc is largely taken from the description section in the JEP. The API is described in the "API" section.
jdk.OracleVersion extends jdk.Version and is the representation of the Oracle JDK version string. Its only purpose is to interpret the fourth element of the version number as a patch release.
There are some minor discrepancies between the implementation and the JEP. The JEP will need to be updated. The most notable is the name of the package ("jdk" vs. the original "jdk.util"). The rename was recommended by Mark.
Thanks, iris
Another comment below... On 11/27/2015 6:36 AM, Magnus Ihse Bursie wrote:
On 2015-11-25 02:54, Iris Clark wrote:
Hi.
Please review the new classes jdk.Version and jdk.OracleVersion. These are simple Java APIs to parse, validate, and compare version numbers.
Bug
8072379: Implement jdk.Version and jdk.OracleVersion https://bugs.openjdk.java.net/browse/JDK-8072379
Webrev
Hi Iris,
I thought the end agreement was that the + should always be present even if build was empty, if opt was present but not pre. That is, "9-foo" should unambigiously parse as vnum=9 and pre="foo", while "9-+foo" should umambigously parse as vnum=9 and opt="foo". The javadoc does not seem to reflect this.
I've also had to read and re-read the regexp and parsing logic in the constructor to convince myself that this (most likely) will be correctly handled. Perhaps a few comments around this would be helpful?
The comparison of two version strings which differs only in "pre" does not adhere to the principle of astonishment.
The documentation states: "Pre-release identifiers are compared numerically when they consist only of digits, and lexiographically otherwise. Numeric identifiers are considered to be less than non-numeric identifiers." But consider the following version strings:
9-01 9-01a 9-02 9-003b
That sequence would be ordered like this, which I find highly surprising. 9-01 9-02 9-003b 9-01a
I'm also surprised that equals() does not produce the same result as compareTo(). If opt is to be ignored, surely it should be so in equals() as well?
I'm one of the maintainers of BigDecimal, one of the few Comparable classes in the JDK where compareTo is "inconsistent with equals" (see "Effective Java, 2nd edition", Item 12 - Considering implementing Comparable). It is consistently surprising to users if compareTo is inconsistent with equals ;-) Therefore, if at all possible it is preferable to have compareTo be *consistent* rather than *inconsistent* with equals; although there are cases where the inconsistency is necessary or at least defensible. I suggest providing multiple compareFoo methods for version that did or did not include certain components, some of these could be consistent with equals and others not. HTH, -Joe
Hi, Joe. You make a good point regarding the inconsistency between compareTo (which ignores $OPT) and equals (which includes $OPT). To resolve this difference, I will introduce two new methods so that users may choose to uniformly consider $OPT in comparisons. We now have the following: public int compareTo(Version ob); public int compareToIgnoreOpt(Version ob); public boolean equals(Version ob); public boolean equalsIgnoreOpt(Version ob); The names parallel String.{compareTo|equals}{IgnoreCase}*. I've re-arranged the code a bit around the implementation and tests for equals*() and compareTo*() to take advantage of code re-use. Thanks, iris -----Original Message----- From: joe darcy Sent: Monday, November 30, 2015 5:33 PM To: Magnus Ihse Bursie; Iris Clark; core-libs-dev@openjdk.java.net Cc: verona-dev@openjdk.java.net Subject: Re: RFR: 8072379: Implement jdk.Version and jdk.OracleVersion Another comment below... On 11/27/2015 6:36 AM, Magnus Ihse Bursie wrote:
On 2015-11-25 02:54, Iris Clark wrote:
Hi.
Please review the new classes jdk.Version and jdk.OracleVersion. These are simple Java APIs to parse, validate, and compare version numbers.
Bug
8072379: Implement jdk.Version and jdk.OracleVersion https://bugs.openjdk.java.net/browse/JDK-8072379
Webrev
Hi Iris,
I thought the end agreement was that the + should always be present even if build was empty, if opt was present but not pre. That is, "9-foo" should unambigiously parse as vnum=9 and pre="foo", while "9-+foo" should umambigously parse as vnum=9 and opt="foo". The javadoc does not seem to reflect this.
I've also had to read and re-read the regexp and parsing logic in the constructor to convince myself that this (most likely) will be correctly handled. Perhaps a few comments around this would be helpful?
The comparison of two version strings which differs only in "pre" does not adhere to the principle of astonishment.
The documentation states: "Pre-release identifiers are compared numerically when they consist only of digits, and lexiographically otherwise. Numeric identifiers are considered to be less than non-numeric identifiers." But consider the following version strings:
9-01 9-01a 9-02 9-003b
That sequence would be ordered like this, which I find highly surprising. 9-01 9-02 9-003b 9-01a
I'm also surprised that equals() does not produce the same result as compareTo(). If opt is to be ignored, surely it should be so in equals() as well?
I'm one of the maintainers of BigDecimal, one of the few Comparable classes in the JDK where compareTo is "inconsistent with equals" (see "Effective Java, 2nd edition", Item 12 - Considering implementing Comparable). It is consistently surprising to users if compareTo is inconsistent with equals ;-) Therefore, if at all possible it is preferable to have compareTo be *consistent* rather than *inconsistent* with equals; although there are cases where the inconsistency is necessary or at least defensible. I suggest providing multiple compareFoo methods for version that did or did not include certain components, some of these could be consistent with equals and others not. HTH, -Joe
Hi, Magnus. Thanks for the review comments.
I thought the end agreement was that the + should always be present even if build was empty, if opt was present but not pre. That is, "9-foo" should unambigiously parse as vnum=9 and pre="foo", while "9-+foo" should umambigously parse as vnum=9 and opt="foo". The javadoc does not seem to reflect this.
You have correctly summarized the expectation and the implementation. I've made the following update the class javadoc: From: * <blockquote><pre> * $VNUM(-$PRE)?(\+$BUILD)?(-$OPT)? * </pre></blockquote> To: * <blockquote><pre> * $VNUM(-$PRE)?(\+($BUILD)?(-$OPT)?)? * </pre></blockquote>
The comparison of two version strings which differs only in "pre" does not adhere to the principle of astonishment.
The documentation states: "Pre-release identifiers are compared numerically when they consist only of digits, and lexiographically otherwise. Numeric identifiers are considered to be less than non-numeric identifiers." But consider the following version strings:
9-01 9-01a 9-02 9-003b
That sequence would be ordered like this, which I find highly surprising. 9-01 9-02 9-003b 9-01a
The current choice of comparison is motivated by Semantic Versioning. A goal of jdk.Version is to subset this scheme. The sorting may be counter-intuitive and it is possible that in the future we may decide that perfect alignment with semver.org is not critical. For now, we're settling with documenting the behavior.
I'm also surprised that equals() does not produce the same result as compareTo(). If opt is to be ignored, surely it should be so in equals() as well?
Based on your comment and Joe's, I've added comparison methods compareTo()/equals() and compareToIgnoreOpt()/equalsIngnoreOpt() for consistency. I'll be sending an updated webrev shortly. Thanks, iris
Hi. Updated webrev and JavaDoc: Webrev http://cr.openjdk.java.net/~iris/verona/8072379/webrev.2/ JavaDoc http://cr.openjdk.java.net/~iris/verona/8072379/doc.2/jdk/Version.html Thanks, iris PS: Please note that I'll be out the week of 21 December, back on 4 January. Access to e-mail will be intermittent at best.
Iris, Did you consider to split version string into array of groups first (using String.split()), then validate each group separately? It may make the code better readable and more resilient to possible future changes. -Dmitry On 2015-11-25 04:54, Iris Clark wrote:
Hi.
Please review the new classes jdk.Version and jdk.OracleVersion. These are simple Java APIs to parse, validate, and compare version numbers.
Bug
8072379: Implement jdk.Version and jdk.OracleVersion https://bugs.openjdk.java.net/browse/JDK-8072379
Webrev
http://cr.openjdk.java.net/~iris/verona/8072379/webrev.1/
JavaDoc
http://cr.openjdk.java.net/~iris/verona/8072379/doc.1/jdk/Version.html http://cr.openjdk.java.net/~iris/verona/8072379/doc.1/jdk/OracleVersion.html
The .java files are predominantly javadoc. The code is relatively straight-forward.
jdk.Version is the representation of the JDK version string as described in JEP 223 ([0], 8061493). The javadoc is largely taken from the description section in the JEP. The API is described in the "API" section.
jdk.OracleVersion extends jdk.Version and is the representation of the Oracle JDK version string. Its only purpose is to interpret the fourth element of the version number as a patch release.
There are some minor discrepancies between the implementation and the JEP. The JEP will need to be updated. The most notable is the name of the package ("jdk" vs. the original "jdk.util"). The rename was recommended by Mark.
Thanks, iris
-- Dmitry Samersoff Oracle Java development team, Saint Petersburg, Russia * I would love to change the world, but they won't give me the sources.
participants (12)
-
Alan Bateman
-
Daniel Fuchs
-
Dmitry Samersoff
-
Iris Clark
-
joe darcy
-
Joseph D. Darcy
-
Magnus Ihse Bursie
-
Mandy Chung
-
Paul Sandoz
-
Roger Riggs
-
Thanh Hong Dai
-
Victor Polischuk