RFR 8030011: Update Hotspot version string output

[Alejandro E Murillo]

On 4/22/2014 6:12 PM, Vladimir Kozlov wrote:
> On 4/22/14 4:42 PM, Alejandro E Murillo wrote:
>>
>> On 4/21/2014 1:18 PM, Vladimir Kozlov wrote:
>>> Hi Alejandro,
>>>
>>> I don't think we need to rename make/hotspot_version file. It is still
>>> used to set JVM's version string and not JDK's version.
>>
>>>
>>> Next assert message is not consistent with previous messages which use
>>> "vm", I think it should be "vm" here too:
>>>
>>>   DEBUG_ONLY(assert_digits(vm_build_num, offset, "wrong JDK build
>>> number"));
>> we do not have hotspot build number anymore, so I think we should not
>> mention hotspot build numberls
>
> Can we simple say "wrong build number"?
sounds good!
>
> So you don't want update build number in make/*_version files? ;)
> Yes, I see in your example of JPRT build VM does not have build number 
> anymore.
it's gone!
>
>>>
>>> Abstract_VM_Version::jvm_version() should include micro version. See
>>> JVM_GetVersionInfo() in jvm.cpp and jvm_version_info in
>>> jdk/src/share/javavm/export/jvm.h.
>> I added that here, is that what you are referring?
>> http://cr.openjdk.java.net/~amurillo/9/8030011/src/share/vm/runtime/vmStructs.cpp.udiff.html 
>>
>>
>> http://cr.openjdk.java.net/~amurillo/9/8030011/src/share/vm/runtime/vm_version.hpp.udiff.html 
>>
>
> No, I mean next code should encode micro version too:
>
>  unsigned int Abstract_VM_Version::jvm_version() {
>    return ((Abstract_VM_Version::vm_major_version() & 0xFF) << 24) |
>           ((Abstract_VM_Version::vm_minor_version() & 0xFF) << 16) |
> +         ((Abstract_VM_Version::vm_micro_version() & 0xFF) <<  8) |
>           (Abstract_VM_Version::vm_build_number() & 0xFF);
>  }
>
you are right. I recall having added that earlier :(
it's back int the webrev
>>
>>>
>>> Use corresponding test in jdk for testing of these changes:
>>>
>>> jdk/test/sun/misc/Version/Version.java
>> yeah, I run that test as part of jprt full builds,
>> That test handles both JDK and Hotspot express versions
>
> Good.
>
>>>
>>> jvm.h: Next comment is not accurate:
>>>
>>> +    /* VM version string: JDK version string     */
>>>
>>> If we build VM separately (for example, in JPRT) VM version will not
>>> be JDK version in which VM is installed. It will take numbers either
>>> from passed make parameters or from make/hotspot_version. I think it
>>> should say:
>>>
>>> +    /* VM version string follows the JDK release version naming
>>> convention
>>> +     * <major>.<minor>.<micro>-bxx[-<identifier>][-<debug_flavor>]
>>>
>>> Based on your examples [-<identifier>][-<debug_flavor>] is still used
>>> so it should be reflected in the comment.
>> Let me make that explicit.
>>>
>>> Don't remove next comments from vm_version.cpp but fix it ("follow the
>>> JDK release"):
>>>
>>> -// HOTSPOT_RELEASE_VERSION must follow the release version naming
>>> convention
>>> -// <major_ver>.<minor_ver>-b<nn>[-<identifier>][-<debug_target>]
>> ok
>>>
>>> You did not show default VM version example when VM is built locally
>>> by engineer.
>> It will be similar to the hotspot only jprt build. There will be a
>> mismatch,
>> I tested by dropping the resulting libjvm  into a promoted build, so it
>> looks like this:
>>
>> java version "1.9.0-ea"
>> Java(TM) SE Runtime Environment (build 1.9.0-ea-b01)
>> Java HotSpot(TM) 64-Bit Server VM (build 1.9.0-internal-debug, mixed 
>> mode)
>
> okay
>
>>
>>>
>>> Please test that correct version string is constructed when you build
>>> VM using make/build.sh, for example 'sh make/build.sh debug LP64=1'
>> Haven't tested that, thanks for pointing that out.
>
> thank you
>
>>>
>>> Next comment change in buildtree.make is not correct because
>>> HOTSPOT_RELEASE_VERSION make parameter does not include
>>> HOTSPOT_BUILD_VERSION:
>>>
>>> -# HOTSPOT_RELEASE_VERSION - <major>.<minor>-b<nn> (11.0-b07)
>>> +# HOTSPOT_RELEASE_VERSION - JRE_RELEASE_VERSION plus
>>> HOTSPOT_BUILD_VERSION
>>>
>>> see JPRT logs where HOTSPOT_BUILD_VERSION is set separately.
>> let me check that again
>>>
>>> I think next change in make/defs.make is not safe (removing make
>>> parameter) due to complexity of our builds:
>>>
>>> -MAKE_ARGS += HOTSPOT_RELEASE_VERSION=$(HOTSPOT_RELEASE_VERSION)
>> I checked the code, and HOTSPOT_RELEASE_VERSION is only used in
>> vm_version.cpp,
>> so I think is fine to remove it. Note that if we keep it there, since
>> the JDK version string
>> sometimes might have time stamps, it may affect ccache,  that's why at
>> some point they
>> separated the options for vm_version.cpp from the options for other
>> components. See this
>> comment on vm,make:
>>
>> # This is VERY important! The version define must only be supplied to
>> vm_version.o
>> # If not, ccache will not re-use the cache at all, since the version
>> string might contain
>> # a time and date.
>>
>
> I was concern that it will not be passed into nested make so that, for 
> example, buildtree.make will not get it.
I see. I don't think it's needed

I think I incorporated all the changes David, John and you suggested and 
started some sanity testing;
Here's is the latest webrev:

http://cr.openjdk.java.net/~amurillo/9/8030011/

Please review (don't forget to refresh each file on your browser) and 
let me know if I missed anything.
thanks guys!

-- 
Alejandro