Date: Wed, 25 Sep 2013 11:55:47 -0700 From: Doug Ambrisko <ambrisko@ambrisko.com> To: Ian Lepore <ian@FreeBSD.org> Cc: freebsd-current@FreeBSD.org Subject: Re: Problem with r255775 include/mk-osreldate.sh Message-ID: <20130925185546.GA17895@ambrisko.com> In-Reply-To: <1380133007.1197.209.camel@revolution.hippie.lan> References: <20130925175210.GA56575@ambrisko.com> <1380133007.1197.209.camel@revolution.hippie.lan>
next in thread | previous in thread | raw e-mail | index | archive | help
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)
@@ -25,8 +25,6 @@
#
# $FreeBSD$
-set -e
-
CURDIR=$(pwd)
ECHO=${ECHO:=echo}
@@ -37,6 +35,12 @@ ${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.
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20130925185546.GA17895>