Re: Problem with r255775 include/mk-osreldate.sh

On Wed, Sep 25, 2013 at 12:16:47PM -0600, Ian Lepore wrote:
| On Wed, 2013-09-25 at 10:52 -0700, Doug Ambrisko wrote:
| > I don't know if others have run into this but I hit a problem with
| > include/mk-osreldate.sh.  It does a set -e to exit on commands failing
| > and sources in sys/conf/newvers.sh to get various things set.
| > In newvers.sh it does a bunch of
| > 	<commmand>
| > 	if [ $? -eq 0 ]; then
| > to decide what to do when it passes or fails.  Unfortunately, when
| > it fails due to the "set -e" it just exits and doesn't do the
| > else clause.  For me I check out a svn tree then build in a chroot.
| > In the chroot svn was failing then not creating a osreldate.h
| > resulting in the build dying.  This happened on two different machines
| > of which I use this method.
| > 
| > Removing the set -e in mk-osreldate.sh "fixed" my problem.  It should
| > probably be reworked to not depend on set -e and print errors when things
| > fail.  I guess newvers.sh could be reworked to do
| > 	if <command> ; then
| > which should pass set -e.
| > 
| > What do folks think?  It would be good to get this fixed before MFC
| > and before 10 is released.
| 
| For such a "simple" little change, this sure has been problematic.
| There are as many ways for it to fail as there are ways to arrange
| checkout-and-build workflows, apparently.
| 
| I've been mostly inclined to stay away from any big changes in
| newvers.sh for fear of breaking it when it's used in some way I'm not
| familiar with (such as building a release).  Sticking with that theory,
| I'd be inclined to leave it alone again, and not push the 'set -e'
| problem into its world, and instead do something like the attached.  

Yes, I'd be nervous to touch newvers.sh as well.
 
| My thinking is that newvers.sh does a variety of things, only some of
| which are germane to the needs of mk-osreldate.h, so have mk-osreldate
| check for just what it needs, and let newvers.sh take care of its
| internal errors however it likes.

Index: include/mk-osreldate.sh
===================================================================
--- include/mk-osreldate.sh	(revision 255775)
+++ include/mk-osreldate.sh	(working copy)
_at__at_ -25,8 +25,6 _at__at_
 #
 # $FreeBSD$
 
-set -e
-
 CURDIR=$(pwd)
 ECHO=${ECHO:=echo}
 
_at__at_ -37,6 +35,12 _at__at_ ${ECHO} creating osreldate.h from newvers.sh
 
 export PARAMFILE="${PARAM_H:=$CURDIR/../sys/sys/param.h}"
 . "${NEWVERS_SH:=$CURDIR/../sys/conf/newvers.sh}"
+
+if [ -z "${COPYRIGHT}" -o -z "${RELDATE}" ] ; then
+    ${ECHO} "newvers.sh did not generate required information"
+    exit 1
+fi
+
 cat > $tmpfile <<EOF
 $COPYRIGHT
 #ifdef _KERNEL


Your patch worked for me.

Thanks,

Doug A.