hg: verona/stage/jdk: 8087202: Add support for PATCH field and remove unused fields of new version string

[Magnus Ihse Bursie]

On 2015-08-25 23:16, Alejandro E Murillo wrote:
>
>
> On 8/19/2015 12:06 PM, Magnus Ihse Bursie wrote:
>>
>>
>> On 2015-08-19 14:47, Alejandro E Murillo wrote:
>>>
>>>
>>> On 8/18/2015 6:19 AM, Magnus Ihse Bursie wrote:
>>>> On 2015-06-22 22:08, alejandro.murillo at oracle.com wrote:
>>>>> Changeset: 60387e5d6822
>>>>> Author:    amurillo
>>>>> Date:      2015-06-19 10:58 -0700
>>>>> URL: http://hg.openjdk.java.net/verona/stage/jdk/rev/60387e5d6822
>>>>>
>>>>> 8087202: Add support for PATCH field and remove unused fields of 
>>>>> new version string
>>>>> Reviewed-by: dholmes, alanb
>>>>>
>>>>> ! make/mapfiles/libjava/mapfile-vers
>>>>> ! src/java.base/share/classes/sun/misc/Version.java.template
>>>>> ! src/java.base/share/native/include/jvm.h
>>>>> ! src/java.base/share/native/libjava/Version.c
>>>>> ! src/java.base/share/native/libjava/jdk_util.c
>>>>> ! test/sun/misc/Version/Version.java
>>>>>
>>>> Hi Alejandro,
>>>>
>>>> A question about test/sun/misc/Version/Version.java. The toString() 
>>>> function seems to not follow the JEP rules about no trailing zeros? 
>>>> Or am I missing something?
>>>>
>>>> /Magnus
>>> Hi Magnus,
>>> welcome back
>>
>> Thanks :)
>>> haven't seen that code in a while now,
>>> but I suspect it might need some tweaking to avoid outputting
>>> extra zeros when some version fields are not defined.
>>> I was off yesterday and off today and Friday
>>> might not have time to look at it Thursday,
>>> so I won't probably get to that until next week,
>>> so feel free to work on that if you have a chance,
>>
>> I'll gladly leave it to you. :) I just noticed that it looked weird 
>> when I skimmed through the accumulated mails and pushes.
> I went ahead and attempted to change the class, so that during the 
> parsing
> of the version string, the values for non existing fields are set to 
> -1 (instead of zero),
> in this way "toString()" can skip the printing of those non existing 
> fields,
> but by doing so, the default values for non existing fields in the class
> differ from the default values for non existing fields by the build 
> system,
> which currently sets them to 0.
>
> for example, if  the "java.runtime.version" system property is set to
> "9-internal+0-2015-08-24-214734.amurillo.8134365-verona-stage"
> then after parsing that, the toString() now outputs: "9+0"
> and internally, since minor, security and patch are no defined,
> the test class set them to -1, but by default, at build time we are 
> setting
> the non existing  values to zero, so the test will fail when comparing
> the parsed values with the actual JDK values for those fields.
This is incorrect. According to the spec, unspecified values for minor 
and security should be considered equal to 0.

>
> So if I'm not missing something here, if we don't want these trailing 
> zeros,
> we might need to define a standard way/value (other than zero) to 
> identify
> non defined (present) fields.
>
> I believe this came up in another thread,  but with regard to build 
> number 0,
> which currently indicates no build number provided,
> have we reached an agreement on that?

I do not think we have reached an agreement. I believe the current 
revision of the spec stipulates that a build with number 0 is allowed, 
and that this should be something different from having no build number.

I think this is a bad decision. This makes build number behave 
differently from the other numeric formats on the build, and is 
generally hard to implement, since it means that a special value (e.g. 
-1) will need to be used in non-string contexts, e.g. when storing the 
value as an integer, and all users of that field will need to understand 
how to handle that -1 value.

>
> Will that be possible to set them to -1 (instead of 0) if not provided 
> to configure?
> that might require some changes in the build system, similar to what I 
> did for
> toString() in the test class, as to make sure VERSION_SHORT
> and VERSION_STRING, etc  do not include fields with "-1"
> will that be feasible?

Setting the build version to -1 in the makefiles if left unspecified is 
trivial. However, for the reasons stated above, I'm reluctant to do this.

/Magnus