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 11:38:
> On Nov 24, 2010, at 4:08 PM, David Holmes wrote:
>> 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 ..
> 
> It's just a name, but we were looking for a name that told you it would 
> be ignored if MILESTONE=fcs.

I see - that's not obvious from the webrev. I guess somewhere in there 
is some "magic" MILESTONE treatment. That said for a real fcs build I 
expect the complete version strings to specified by the "release" 
process on the build invocation not defaulting to whatever is in the 
Makefiles.

> USER_RELEASE_SUFFIX would work, and your suggestion below works too, 
> although I still want to
> export USER_RELEASE_SUFFIX so that shell logic isn't repeated over and 
> over, hard on windows systems.

I'm still unclear to where you are exporting this value? One of the doc 
makefiles utilizes USER_RELEASE_SUFFIX but doesn't need it to be 
exported presently.

> I'll need to change all NONFCS_BUILD_INFO to USER_RELEASE_SUFFIX in the 
> hotspot files.
> 
> That also cures John's complaint about the "fcs" name too.
> 
> Check this one out:
>   http://cr.openjdk.java.net/~ohair/openjdk7/nonfcs-version2/webrev/

I can give Thumbs Up as is.

Thanks,
David

> -kto
> 
>>
>>> 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
>>>>>>>
>