From 4fa5b63713741736b526edce8e8b88ea1e535b0e Mon Sep 17 00:00:00 2001 From: Tomi Ollila Date: Mon, 3 Sep 2012 17:58:38 +0300 Subject: [PATCH] Re: [PATCH V2 1/2] devel: add release-checks.sh --- f6/cdcf96fce9576c00dad9d6d2458b1c511421a3 | 380 ++++++++++++++++++++++ 1 file changed, 380 insertions(+) create mode 100644 f6/cdcf96fce9576c00dad9d6d2458b1c511421a3 diff --git a/f6/cdcf96fce9576c00dad9d6d2458b1c511421a3 b/f6/cdcf96fce9576c00dad9d6d2458b1c511421a3 new file mode 100644 index 000000000..684db71ba --- /dev/null +++ b/f6/cdcf96fce9576c00dad9d6d2458b1c511421a3 @@ -0,0 +1,380 @@ +Return-Path: +X-Original-To: notmuch@notmuchmail.org +Delivered-To: notmuch@notmuchmail.org +Received: from localhost (localhost [127.0.0.1]) + by olra.theworths.org (Postfix) with ESMTP id 1EDAE431FB6 + for ; Mon, 3 Sep 2012 07:58:34 -0700 (PDT) +X-Virus-Scanned: Debian amavisd-new at olra.theworths.org +X-Spam-Flag: NO +X-Spam-Score: 0 +X-Spam-Level: +X-Spam-Status: No, score=0 tagged_above=-999 required=5 tests=[none] + autolearn=disabled +Received: from olra.theworths.org ([127.0.0.1]) + by localhost (olra.theworths.org [127.0.0.1]) (amavisd-new, port 10024) + with ESMTP id syRZT04Vq1dH for ; + Mon, 3 Sep 2012 07:58:32 -0700 (PDT) +Received: from guru.guru-group.fi (guru.guru-group.fi [46.183.73.34]) + by olra.theworths.org (Postfix) with ESMTP id 9E486431FAF + for ; Mon, 3 Sep 2012 07:58:32 -0700 (PDT) +Received: from guru.guru-group.fi (localhost [IPv6:::1]) + by guru.guru-group.fi (Postfix) with ESMTP id 0DCB31000E5; + Mon, 3 Sep 2012 17:58:39 +0300 (EEST) +From: Tomi Ollila +To: Michal Nazarewicz , notmuch@notmuchmail.org +Subject: Re: [PATCH V2 1/2] devel: add release-checks.sh +In-Reply-To: +References: <1346491928-2356-1-git-send-email-tomi.ollila@iki.fi> + +User-Agent: Notmuch/0.14+11~gd9bf007 (http://notmuchmail.org) Emacs/24.2.1 + (x86_64-unknown-linux-gnu) +X-Face: HhBM'cA~ +MIME-Version: 1.0 +Content-Type: text/plain; charset=utf-8 +Content-Transfer-Encoding: quoted-printable +X-BeenThere: notmuch@notmuchmail.org +X-Mailman-Version: 2.1.13 +Precedence: list +List-Id: "Use and development of the notmuch mail system." + +List-Unsubscribe: , + +List-Archive: +List-Post: +List-Help: +List-Subscribe: , + +X-List-Received-Date: Mon, 03 Sep 2012 14:58:34 -0000 + +On Mon, Sep 03 2012, Michal Nazarewicz wrote: + +> Tomi Ollila 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. + +The 'set -o pipefail' is hard to do using POSIX shell. I've been hit +by this when 'find' part in find | sort' failed (due to a typo). + +When building release we can choose shell in more relaxed manner and +expect release builder have a recent bash shell -- actually tests +require bash 4... + +>> + +>> +set -eu +>> +#set -x # or enter bash -x ... on command line +>> + +>> +if [ x"${BASH_VERSION-}" =3D x ] +> +> -n perhaps? + +Before this test we cannot know what shell is run; the x"$var" =3D x=20 +is the most robust way to do this test. Well, if $BASH_VERSION had=20 +some value then we'd miss this error message and failed in +set -o pipefail + +If we'd require bash 4 then the test [ x"${BASHPID-}" !=3D x$$ ] +would be very hard to get wrong... + +>> +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. + +True, it used to be. Can be removed in followup patch. + +>> + +>> +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 + +IMHO this short-circuited or (||) version works well due to this +negation handling. The $f's should have been quoted ("$f"), though +(in case some of the items contain whitespace the construct=20 +would't fail). I am open to other opinions, though. + +>> +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= + periods" +> +> This makes it more readable by putting all checks in case rather than +> splitting one to an outer if. + +That's true! Also, one less bashish cannot be bad thing to have. + +>> + +>> + +>> +# 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. + +builtin bash '[' seems to be robust in these cases(*) ... but I'm now break= +ing +my own rule about not using most portable expressions; maybe it is just +ugliness of the format in so many places. I'm not changing unless there is +more desire for it :) + +(*) at least both of these work: + bash -c 'set -eu; foo=3D"-a"; [ "$foo" =3D -a ]' + bash -c 'set -eu; foo=3D"-z"; [ "$foo" =3D -z ]' + +> +>> +then +>> + echo Yes. +>> +else +>> + echo No. +>> + append_emsg "Package name '$deb_notmuch' is not 'notmuch' in debian/ch= +angelog" +>> +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/ch= +angelog" +>> +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-= +dd)" +>> +esac +>> + +>> +readonly DATE=3D${news_date//[()]/} # bash feature +>> +manthdata () +>> +{ +>> + set x $* +> +> Uh? Did you mean =E2=80=9Cset -- $*=E2=80=9D? + +Nope, set in some shells don't know '--' (now I'm following the most +portable code snippet principle) so with 'x' and just referencing +positional parameters with number one larger than with '--' does the trick. + +> +>> + 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 $VE= +RSION... " +>> +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? + +Can be added... + +> +>> + */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. + +I prefer [.] as a more robust alternative -- backslashes may get=20 +"expanded away" in some cases; using [.] just frees me from checking +whether that may happen. + +>> + 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. + +True.=20 + +Thanks for the review, I'll probably send new version tomorrow... + +>> + +>> +# 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) + +Tomi -- 2.26.2