RFR: 8072379: Implement jdk.Version and jdk.OracleVersion

Tue Feb 2 07:48:29 UTC 2016

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



