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.cl...@oracle.com] Sent: Tuesday, 2 February, 2016 12:51 PM To: Thanh Hong Dai <hdth...@tma.com.vn>; Alan Bateman <alan.bate...@oracle.com>; core-libs-dev@openjdk.java.net; verona-...@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:hdth...@tma.com.vn] Sent: Wednesday, January 13, 2016 7:53 PM To: Iris Clark; Alan Bateman; core-libs-dev@openjdk.java.net; verona-...@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-boun...@openjdk.java.net] On Behalf Of Iris Clark Sent: Thursday, 14 January, 2016 4:55 AM To: Alan Bateman <alan.bate...@oracle.com>; core-libs-dev@openjdk.java.net; verona-...@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-...@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
