Need reviewer: NONFCS_BUILD_INFO to add to the non-fcs version string

[David Holmes]

Kelly O'Hair said the following on 11/25/10 09:55:
> 
> On Nov 24, 2010, at 3:47 PM, David Holmes wrote:
> 
>> John Coomes said the following on 11/25/10 09:16:
>>> Kelly O'Hair (kelly.ohair at oracle.com) wrote:
>>>> Dang... just shoot me now. :^(
>>>>
>>>> Try this:
>>>>   http://cr.openjdk.java.net/~ohair/openjdk7/nonfcs-version/webrev/
>>>>
>>>> Sorry about that.
>>> Aren't we using the term GA (general availability) instead of FCS
>>> these days?
>>
>> Going further why is this even an issue? In all interesting cases 
>> HOTSPOT_BUILD_VERSION should be set on the command-line. The Makefile 
>> only needs to give a default if it is not set. So why set 
>> NONFCS_BUILD_INFO when you can set HOTSPOT_BUILD_VERSION in the first 
>> place?
> 
> I'm trying to avoid having to specify a specific variable for every 
> component of the jdk when we want to add specific build information to 
> the version string.

Ok. So why not just USER_BUILD_INFO or CUSTOM_BUILD_INFO ? There's 
really no tie to "FCS" or not. In fact why not use USER_RELEASE_SUFFIX - 
see below ..

> When full builds of jdk7 are done, we wanted a single variable that 
> would add some extra identification string to all version strings.
> Hudson systems and JPRT can use this to uniquely identify all full 
> builds so that testing teams can report an exact build on bugs.

I see. I'm not sure why you re-export the variable here 
(jdk/.../Defs.gmk) as it doesn't appear to get used anywhere else:

+   ifndef NONFCS_BUILD_INFO
       BUILD_DATE := $(shell $(DATE) '+%Y_%m_%d_%H_%M')
       CLEAN_USERNAME := $(shell $(ECHO) "$(USER)" | $(TR) -d -c 
'[:alnum:]')
       USER_RELEASE_SUFFIX := $(shell $(ECHO) 
"$(CLEAN_USERNAME)_$(BUILD_DATE)" | $(TR) '[:upper:]' '[:lower:]' )
!     NONFCS_BUILD_INFO = $(USER_RELEASE_SUFFIX)
!   endif
!   export NONFCS_BUILD_INFO
!   FULL_VERSION = $(RELEASE)-$(NONFCS_BUILD_INFO)-$(BUILD_NUMBER)

It would seem a little simpler/cleaner to me to instead just do:

    ifndef NONFCS_BUILD_INFO
       BUILD_DATE := $(shell $(DATE) '+%Y_%m_%d_%H_%M')
       CLEAN_USERNAME := $(shell $(ECHO) "$(USER)" | $(TR) -d -c 
'[:alnum:]')
       USER_RELEASE_SUFFIX := $(shell $(ECHO) 
"$(CLEAN_USERNAME)_$(BUILD_DATE)" | $(TR) '[:upper:]' '[:lower:]' )
    else
      USER_RELEASE_SUFFIX := $(NONFCS_BUILD_INFO)
    endif
    FULL_VERSION = $(RELEASE)-$(USER_RELEASE_SUFFIX)-$(BUILD_NUMBER)

And this would be even simpler if USER_RELEASE_SUFFIX were the variable 
that was set externally in the first place.

    ifndef USER_RELEASE_SUFFIX
       BUILD_DATE := $(shell $(DATE) '+%Y_%m_%d_%H_%M')
       CLEAN_USERNAME := $(shell $(ECHO) "$(USER)" | $(TR) -d -c 
'[:alnum:]')
       USER_RELEASE_SUFFIX := $(shell $(ECHO) 
"$(CLEAN_USERNAME)_$(BUILD_DATE)" | $(TR) '[:upper:]' '[:lower:]' )
    endif
    FULL_VERSION = $(RELEASE)-$(USER_RELEASE_SUFFIX)-$(BUILD_NUMBER)


Cheers,
David


>>
>> Any why use the same NONFCS_BUILD_INFO for Hotspot and the JDK when 
>> they typically report different version strings anyway ???
> 
> It's extra build information, not really a change in the component 
> version number.
> 
> -kto
> 
>>
>> David
>>
>>> Aside from that, looks fine.
>>> -John
>>>> On Nov 24, 2010, at 11:28 AM, Mark Wielaard wrote:
>>>>
>>>>> On Wed, 2010-11-24 at 11:19 -0800, Kelly O'Hair wrote:
>>>>>> I need a reviewer for this change:
>>>>>>
>>>>>> 6987107: Add NONFCS_BUILD_INFO variable to add to but not modify
>>>>>> MILESTONE in version string
>>>>>> http://javaweb.sfbay.sun.com/~ohair/webrevs/jdk7/nonfcs-version/webrev/ 
>>>>>>
>>>>> Bit hard to review if the host isn't reachable :)
>>>>> Could you just attach the patch to your email,
>>>>> or post it on some publicly reachable machine?
>>>>>
>>>>> Thanks,
>>>>>
>>>>> Mark
>>>>>
>