cvs commit: src/sys/conf newvers.sh

[Doug Barton]

Colin Percival wrote:
> cperciva    2005-08-19 03:56:45 UTC
> 
>   FreeBSD src repository
> 
>   Modified files:
>     sys/conf             newvers.sh 
>   Log:
>   Forced commit to note that the preceeding commit also makes it possible
>   to override the BRANCH specified in newvers.sh via a BRANCH_OVERRIDE
>   environment variable.  This is useful for my FreeBSD Update builds, and
>   might possibly be useful for someone else at some point.
>   
>   Revision  Changes    Path
>   1.72      +0 -0      src/sys/conf/newvers.sh
> 
> http://www.FreeBSD.org/cgi/cvsweb.cgi/src/sys/conf/newvers.sh.diff?&r1=1.71&r2=1.72&f=h

I have 3 comments about this commit, none of which are intended to be critical.

1. A "better" way (IMO) to write:

if [ "X${BRANCH_OVERRIDE}" != "X" ]; then

is

case "${BRANCH_OVERRIDE}" in
'')	;;
*)	BRANCH=${BRANCH_OVERRIDE} ;;
esac

The original reason for writing it this way was to avoid the call to 
test(1), because case is a shell builtin. This is a style issue, and not a 
"must have," but I thought I'd mention it.

2. I've had the following in my newvers.sh file for years, it helps me 
identify exactly when a given kernel was built (which may or may not 
coincide with the age of the sources):

RELEASE="${REVISION}-${BRANCH}-`/bin/date +%m%d`"

I like your override idea here, but I'd rather not have to keep track of 
what the current value of $BRANCH is so that I can include it in my own 
override variable. What would be more useful to me (and arguably more useful 
in general, although once again I will not press the point) would be some 
way to add a string to the BRANCH or RELEASE variables. This would also 
allow you to simplify the code. I'd have done the following:

Index: newvers.sh
===================================================================
RCS file: /usr/local/ncvs/src/sys/conf/newvers.sh,v
retrieving revision 1.70
diff -u -r1.70 newvers.sh
--- newvers.sh  11 Jul 2005 08:34:49 -0000      1.70
+++ newvers.sh  19 Aug 2005 08:25:12 -0000
@@ -32,7 +32,7 @@

  TYPE="FreeBSD"
  REVISION="7.0"
-BRANCH="CURRENT"
+BRANCH="CURRENT${BRANCH_TAG}"
  RELEASE="${REVISION}-${BRANCH}"
  VERSION="${TYPE} ${RELEASE}"

This will be a noop when not defined, but then I could do something like 
this in my build environment:

BRANCH_TAG="-`/bin/date +%m%d`"

and have it work.

You are of course free to ignore all this blathering, it's just a suggestion. :)

3. Even in the hectic environment leading up to a release, I dislike 
insta-MFCs. I admire your desire to get as much done in as short a time as 
possible, but everyone makes mistakes, and even a 24 hour period between a 
commit to HEAD and an MFC can make life easier for everyone in the case of 
an error, and generally does not "cost" us anything. This is simply an 
opinion, and a request to think about the topic. It is not a specific 
objection to this commit.

Hope this helps,

Doug

-- 

     This .signature sanitized for your protection