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

Mon Nov 29 11:10:10 PST 2010

Kelly,

So what JPRT version string will be?

Kelly O'Hair wrote:
> John,
> 
> Can you or someone from the hotspot team review the hotspot part of this:
> 
> 6987107: Add NONFCS_BUILD_INFO variable to add to but not 
> modify MILESTONE in version string
> http://cr.openjdk.java.net/~ohair/openjdk7/nonfcs-version2/webrev/
> 
> -kto
> 
> Begin forwarded message:
> 
>> *From: *"Kelly O'Hair" <kelly.ohair at oracle.com>
>> *Date: *November 24, 2010 18:14:48 PM PST
>> *To: *David Holmes <David.Holmes at oracle.com>
>> *Cc: *John Coomes <John.Coomes at oracle.com>, David Katleman 
>> <david.katleman at oracle.com>, build-dev <build-dev at openjdk.java.net>, 
>> "christine.lu at oracle.com Lu" <christine.lu at oracle.com>
>> *Subject: **Re: Need reviewer: NONFCS_BUILD_INFO to add to the non-fcs 
>> version string*
>>
>>
>> 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
>>>>>>>>>>
>>
> 