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

[Kelly O'Hair]

On Nov 24, 2010, at 6:06 PM, David Holmes wrote:

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

The whole way the versions are defined is a bit confusing, tricky  
stuff. Probably need a serious revamping someday.
But in general, yes, if MILESTONE=fcs most values come from the  
command line or RE scripts.

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

The jdk makefiles are heavily nested, make running make, and these  
Defs files are repeatedly parsed over and
over, maybe 100's of times for a top level Makefile?
The export pushes the variable into the environment so that the lower  
nested makefiles see the definition
as an environment variable and if you use ifndef then you avoid the  
recalculations or shell execs.
Solaris/Linux aren't bothered by the excessive execs that much, but  
Windows suffers from it.
I've been trying to lower the total exec count when the Defs files are  
parsed to make windows builds faster.

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

OK.

-kto

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