On Nov 24, 2010, at 4:08 PM, David Holmes wrote:

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

It's just a name, but we were looking for a name that told you it would be ignored if MILESTONE=fcs. 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'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/

-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


Reply via email to