Sender: Michal Nazarewicz <mpn@google.com>
From: Michal Nazarewicz <mina86@mina86.com>
To: Tomi Ollila <tomi.ollila@iki.fi>, notmuch@notmuchmail.org
Subject: Re: [PATCH V2 1/2] devel: add release-checks.sh
In-Reply-To: <1346491928-2356-1-git-send-email-tomi.ollila@iki.fi>
Organization: http://mina86.com/
References: <1346491928-2356-1-git-send-email-tomi.ollila@iki.fi>
User-Agent: Notmuch/0.14+2~g416b120 (http://notmuchmail.org) Emacs/24.2.50.1
	(x86_64-unknown-linux-gnu)
Face: iVBORw0KGgoAAAANSUhEUgAAADAAAAAwBAMAAAClLOS0AAAAJFBMVEWbfGlUPDDHgE57V0jUupKjgIObY0PLrom9mH4dFRK4gmjPs41MxjOgAAACQElEQVQ4jW3TMWvbQBQHcBk1xE6WyALX1069oZBMlq+ouUwpEQQ6uRjttkWP4CmBgGM0BQLBdPFZYPsyFUo6uEtKDQ7oy/U96XR2Ux8ehH/89Z6enqxBcS7Lg81jmSuujrfCZcLI/TYYvbGj+jbgFpHJ/bqQAUISj8iLyu4LuFHJTosxsucO4jSDNE0Hq3hwK/ceQ5sx97b8LcUDsILfk+ovHkOIsMbBfg43VuQ5Ln9YAGCkUdKJoXR9EclFBhixy3EGVz1K6eEkhxCAkeMMnqoAhAKwhoUJkDrCqvbecaYINlFKSRS1i12VKH1XpUd4qxL876EkMcDvHj3s5RBajHHMlA5iK32e0C7VgG0RlzFPvoYHZLRmAC0BmNcBruhkE0KsMsbEc62ZwUJDxWUdMsMhVqovoT96i/DnX/ASvz/6hbCabELLk/6FF/8PNpPCGqcZTGFcBhhAaZZDbQPaAB3+KrWWy2XgbYDNIinkdWAFcCpraDE/knwe5DBqGmgzESl1p2E4MWAz0VUPgYYzmfWb9yS4vCvgsxJriNTHoIBz5YteBvg+VGISQWUqhMiByPIPpygeDBE6elD973xWwKkEiHZAHKjhuPsFnBuArrzxtakRcISv+XMIPl4aGBUJm8Emk7qBYU8IlgNEIpiJhk/No24jHwkKTFHDWfPniR4iw5vJaw2nzSjfq2zffcE/GDjRC2dn0J0XwPAbDL84TvaFCJEU4Oml9pRyEUhR3Cl2t01AoEjRbs0sYugp14/4X5n4pU4EHHnMAAAAAElFTkSuQmCC
Date: Mon, 03 Sep 2012 14:03:41 +0200
Message-ID: <xa1t7gsbwbxe.fsf@mina86.com>
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="=-=-="
Cc: Tomi Ollila <tomi.ollila@iki.fi>
Precedence: list

--=-=-=
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: quoted-printable

Tomi Ollila <tomi.ollila@iki.fi> writes:
> diff --git a/devel/release-checks.sh b/devel/release-checks.sh
> new file mode 100755
> index 0000000..7dadefa
> --- /dev/null
> +++ b/devel/release-checks.sh
> @@ -0,0 +1,211 @@
> +#!/usr/bin/env bash

On a side note, the whole script could be relatively easily rewritten
not to use bash at all and work with plain POSIX shell.

> +
> +set -eu
> +#set -x # or enter bash -x ... on command line
> +
> +if [ x"${BASH_VERSION-}" =3D x ]

-n perhaps?

> +then	echo
> +	echo "Please execute this script using 'bash' interpreter"
> +	echo
> +	exit 1
> +fi
> +
> +set -o pipefail # bash feature
> +
> +# Avoid locale-specific differences in output of executed commands
> +LANG=3DC LC_ALL=3DC; export LANG LC_ALL
> +
> +readonly DEFAULT_IFS=3D"$IFS"

Never used.

> +
> +readonly PV_FILE=3D'bindings/python/notmuch/version.py'
> +
> +# Using array here turned out to be unnecessarily complicated
> +emsgs=3D''
> +append_emsg ()
> +{
> +	emsgs=3D"${emsgs:+$emsgs\n}  $1"
> +}
> +
> +for f in ./version debian/changelog NEWS "$PV_FILE"
> +do
> +	test -f $f || { append_emsg "File '$f' is missing"; continue; }
> +	test -r $f || { append_emsg "File '$f' is unreadable"; continue; }
> +	test -s $f ||   append_emsg "File '$f' is empty"

if ! [ -f "$f" ]; then
	append_emsg "File '$f' is missing"
elif ! [ -r "$f" ]; then
	append_emsg "File '$f' is unreadable"
elif ! [ -s "$f" ]; then
	append_emsg "File '$f' is empty"
fi

> +done
> +
> +if [ -n "$emsgs" ]
> +then
> +	echo 'Release files problems; fix these and try again:'
> +	echo -e "$emsgs"
> +	exit 1
> +fi
> +
> +if read VERSION
> +then
> +	if read rest
> +	then	echo "'version' file contains more than one line"
> +		exit 1
> +	fi
> +else
> +	echo "Reading './version' file failed (suprisingly!)"
> +	exit 1
> +fi < ./version
> +
> +readonly VERSION
> +
> +verfail ()
> +{
> +	echo No.
> +	echo "$@"
> +	echo "Please follow the instructions in RELEASING to choose a version"
> +	exit 1
> +}
> +
> +echo -n "Checking that '$VERSION' is good with digits and periods... "
> +if [ -z "${VERSION//[0123456789.]/}" ] # bash feature
> +then
> +	case $VERSION in
> +		.*)	verfail "'$VERSION' begins with a period" ;;
> +		*.)	verfail "'$VERSION' ends with a period" ;;
> +		*..*)	verfail "'$VERSION' contains two consecutive periods" ;;
> +		*.*)	echo Yes. ;;
> +		*)	verfail "'$VERSION' is a single number" ;;
> +	esac
> +else
> +	verfail "'$VERSION' contains other characters than digits and periods"
> +fi

The outer condition can be put inside of case as so:

	*[^0-9.]*)	verfail "'$VERSION' contains other characters than digits and p=
eriods"

This makes it more readable by putting all checks in case rather than
splitting one to an outer if.

> +
> +
> +# In the rest of this file, tests collect list of errors to be fixed
> +
> +echo -n "Checking that this is Debian package for notmuch... "
> +read deb_notmuch deb_version rest < debian/changelog
> +if [ "$deb_notmuch" =3D 'notmuch' ]

if [ x"$deb_notmuch" =3D xnotmuch ]

And so in the rest of the conditions below.

> +then
> +	echo Yes.
> +else
> +	echo No.
> +	append_emsg "Package name '$deb_notmuch' is not 'notmuch' in debian/cha=
ngelog"
> +fi
> +
> +echo -n "Checking that Debian package version is $VERSION-1... "
> +
> +if [ "$deb_version" =3D "($VERSION-1)" ]
> +then
> +	echo Yes.
> +else
> +	echo No.
> +	append_emsg "Version '$deb_version' is not '($VERSION-1)' in debian/cha=
ngelog"
> +fi
> +
> +echo -n "Checking that python bindings version is $VERSION... "
> +py_version=3D`python -c "execfile('$PV_FILE'); print __VERSION__"`
> +if [ "$py_version" =3D "$VERSION" ]
> +then
> +	echo Yes.
> +else
> +	echo No.
> +	append_emsg "Version '$py_version' is not '$VERSION' in $PV_FILE"
> +fi
> +
> +echo -n "Checking that this is Notmuch NEWS... "
> +read news_notmuch news_version news_date < NEWS
> +if [ "$news_notmuch" =3D "Notmuch" ]
> +then
> +	echo Yes.
> +else
> +	echo No.
> +	append_emsg "First word '$news_notmuch' is not 'Notmuch' in NEWS file"
> +fi
> +
> +echo -n "Checking that NEWS version is $VERSION... "
> +if [ "$news_version" =3D "$VERSION" ]
> +then
> +	echo Yes.
> +else
> +	echo No.
> +	append_emsg "Version '$news_version' in NEWS file is not '$VERSION'"
> +fi
> +
> +#eval `date '+year=3D%Y mon=3D%m day=3D%d'`
> +today0utc=3D`date --date=3D0Z +%s` # gnu date feature
> +
> +echo -n "Checking that NEWS date is right... "
> +case $news_date in
> + '('[2-9][0-9][0-9][0-9]-[01][0-9]-[0123][0-9]')')
> +	newsdate0utc=3D`nd=3D${news_date#\\(}; date --date=3D"${nd%)} 0Z" +%s`
> +	ddiff=3D$((newsdate0utc - today0utc))
> +	if [ $ddiff -lt -86400 ] # since beginning of yesterday...
> +	then
> +		echo No.
> +		append_emsg "Date $news_date in NEWS file is too much in the past"
> +	elif [ $ddiff -gt 172800 ] # up to end of tomorrow...
> +	then
> +		echo No.
> +		append_emsg "Date $news_date in NEWS file is too much in the future"
> +	else
> +		echo Yes.
> +	fi ;;
> + *)
> +	echo No.
> +	append_emsg "Date '$news_date' in NEWS file is not in format (yyyy-mm-d=
d)"
> +esac
> +
> +readonly DATE=3D${news_date//[()]/} # bash feature
> +manthdata ()
> +{
> +	set x $*

Uh?  Did you mean =E2=80=9Cset -- $*=E2=80=9D?

> +	if [ $# !=3D 7 ]
> +	then
> +		append_emsg "'$mp' has too many '.TH' lines"
> +		man_mismatch=3D1
> +	fi
> +	man_date=3D${5-} man_version=3D${7-}
> +}
> +
> +echo -n "Checking that manual page dates and versions are $DATE and $VER=
SION... "
> +manfiles=3D`find man -type f | sort`
> +man_pages_ok=3DYes
> +for mp in $manfiles
> +do
> +	case $mp in *.[0-9]) ;; # fall below this 'case ... esac'

Perhaps new line after =E2=80=9Cin=E2=80=9D?

> +		*/Makefile.local | */Makefile ) continue ;;
> +		*/.gitignore)	continue ;;
> +		*.bak)		continue ;;
> +
> +		*)	append_emsg "'$mp': extra file"
> +			man_pages_ok=3DNo
> +			continue
> +	esac
> +	manthdata `sed -n '/^[.]TH NOTMUCH/ { y/"/ /; p; }' "$mp"`

Alternatively =E2=80=9C\.=E2=80=9D instead of =E2=80=9C[.]=E2=80=9D.

> +	if [ "$man_version" !=3D "$VERSION" ]
> +	then	append_emsg "Version '$man_version' is not '$VERSION' in $mp"
> +		mman_pages_ok=3DNo
> +	fi
> +	if [ "$man_date" !=3D "$DATE" ]
> +	then	append_emsg "DATE '$man_date' is not '$DATE' in $mp"
> +		man_pages_ok=3DNo
> +	fi
> +done
> +echo $man_pages_ok.
> +
> +if [ -n "$emsgs" ]
> +then
> +	echo
> +	echo 'Release check failed; check these issues:'
> +	echo -e "$emsgs"
> +	exit 1
> +fi
> +
> +echo 'All checks this script executed completed successfully.'
> +echo 'Make sure that everything else mentioned in RELEASING'
> +echo 'file is in order, too.'
> +
> +exit 0

Unnecessary.

> +
> +# Local variables:
> +# mode: shell-script
> +# sh-basic-offset: 8
> +# tab-width: 8
> +# End:
> +# vi: set sw=3D8 ts=3D8

--=20
Best regards,                                         _     _
.o. | Liege of Serenely Enlightened Majesty of      o' \,=3D./ `o
..o | Computer Science,  Micha=C5=82 =E2=80=9Cmina86=E2=80=9D Nazarewicz   =
 (o o)
ooo +----<email/xmpp: mpn@google.com>--------------ooO--(_)--Ooo--
--=-=-=
Content-Type: multipart/signed; boundary="==-=-=";
	micalg=pgp-sha1; protocol="application/pgp-signature"

--==-=-=
Content-Type: text/plain


--==-=-=
Content-Type: application/pgp-signature

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.10 (GNU/Linux)

iQIcBAEBAgAGBQJQRJydAAoJECBgQBJQdR/0nucP/R6zc1d5OupCMuJSpCoO7Ivh
4dK7QsW5D52tm9gBP7ZCVZUVI1r8hUKpolH8sQ4HW+D3bedmMdb49ys5BwoKLALh
jh7T7Okuz4kOkoRTgN/9XOo+uYUO7b0NkmXTlhUS0B2qPAPmcMR7W9qPx7raBP50
cZTQi2pDiYNHXdWUH7DV8DbqaphZkElQU2DJdVcjGginCey9z4weJN3wY9MWDhtk
RYHr3xzkxHIdpMdpzj6+m4UGU4e70TmeqZMDOQn+5fC8TqwZpKSGqaEX3i3Mq9e1
LcypWwQyTqTK4wOzR4I5qYcLTW6joNVpTH7eqTeZ9kyNrBuZYTszQgRBAt8No325
s189aUlGsBKy0tyxAL/UEpvCO8lTnk0+7iJHavdRaKzMGFT6y5i6WQDuruUDGmeh
TL/zNYrdmNTQDCjvi2FbaPsX6BhPKANnHElb0iyCU4GSfNwJ/LQm0e6Z4OfOSUg8
hW6kL1QsSoqIsKThjkBqeA++2uf+GBKjAh1uzvJlWxmn+kz8pD/1WoHnH1w/RccJ
z3QJ2Wuky5NxmSAL4+op6rXry9IcorGvELRs/LUhaMHT3FwpyH6G0JlT82L+FsqB
P7/h+Vmc59OWfCXCzlepeLjqDQLQwRGzmo9XCBWEm0q6K3QbVVuxN/LyPgZ1Elzz
6UtZ6WBOKNqeDcRMtXhF
=iuqg
-----END PGP SIGNATURE-----
--==-=-=--

--=-=-=--