f6/cdcf96fce9576c00dad9d6d2458b1c511421a3

   1 Return-Path: <tomi.ollila@iki.fi>\r
   2 X-Original-To: notmuch@notmuchmail.org\r
   3 Delivered-To: notmuch@notmuchmail.org\r
   4 Received: from localhost (localhost [127.0.0.1])\r
   5         by olra.theworths.org (Postfix) with ESMTP id 1EDAE431FB6\r
   6         for <notmuch@notmuchmail.org>; Mon,  3 Sep 2012 07:58:34 -0700 (PDT)\r
   7 X-Virus-Scanned: Debian amavisd-new at olra.theworths.org\r
   8 X-Spam-Flag: NO\r
   9 X-Spam-Score: 0\r
  10 X-Spam-Level: \r
  11 X-Spam-Status: No, score=0 tagged_above=-999 required=5 tests=[none]\r
  12         autolearn=disabled\r
  13 Received: from olra.theworths.org ([127.0.0.1])\r
  14         by localhost (olra.theworths.org [127.0.0.1]) (amavisd-new, port 10024)\r
  15         with ESMTP id syRZT04Vq1dH for <notmuch@notmuchmail.org>;\r
  16         Mon,  3 Sep 2012 07:58:32 -0700 (PDT)\r
  17 Received: from guru.guru-group.fi (guru.guru-group.fi [46.183.73.34])\r
  18         by olra.theworths.org (Postfix) with ESMTP id 9E486431FAF\r
  19         for <notmuch@notmuchmail.org>; Mon,  3 Sep 2012 07:58:32 -0700 (PDT)\r
  20 Received: from guru.guru-group.fi (localhost [IPv6:::1])\r
  21         by guru.guru-group.fi (Postfix) with ESMTP id 0DCB31000E5;\r
  22         Mon,  3 Sep 2012 17:58:39 +0300 (EEST)\r
  23 From: Tomi Ollila <tomi.ollila@iki.fi>\r
  24 To: Michal Nazarewicz <mina86@mina86.com>, notmuch@notmuchmail.org\r
  25 Subject: Re: [PATCH V2 1/2] devel: add release-checks.sh\r
  26 In-Reply-To: <xa1t7gsbwbxe.fsf@mina86.com>\r
  27 References: <1346491928-2356-1-git-send-email-tomi.ollila@iki.fi>\r
  28         <xa1t7gsbwbxe.fsf@mina86.com>\r
  29 User-Agent: Notmuch/0.14+11~gd9bf007 (http://notmuchmail.org) Emacs/24.2.1\r
  30         (x86_64-unknown-linux-gnu)\r
  31 X-Face: HhBM'cA~<r"^Xv\KRN0P{vn'Y"Kd;zg_y3S[4)KSN~s?O\"QPoL\r
  32         $[Xv_BD:i/F$WiEWax}R(MPS`^UaptOGD`*/=@\1lKoVa9tnrg0TW?"r7aRtgk[F\r
  33         !)g;OY^,BjTbr)Np:%c_o'jj,Z\r
  34 Date: Mon, 03 Sep 2012 17:58:38 +0300\r
  35 Message-ID: <m2sjazdug1.fsf@guru.guru-group.fi>\r
  36 MIME-Version: 1.0\r
  37 Content-Type: text/plain; charset=utf-8\r
  38 Content-Transfer-Encoding: quoted-printable\r
  39 X-BeenThere: notmuch@notmuchmail.org\r
  40 X-Mailman-Version: 2.1.13\r
  41 Precedence: list\r
  42 List-Id: "Use and development of the notmuch mail system."\r
  43         <notmuch.notmuchmail.org>\r
  44 List-Unsubscribe: <http://notmuchmail.org/mailman/options/notmuch>,\r
  45         <mailto:notmuch-request@notmuchmail.org?subject=unsubscribe>\r
  46 List-Archive: <http://notmuchmail.org/pipermail/notmuch>\r
  47 List-Post: <mailto:notmuch@notmuchmail.org>\r
  48 List-Help: <mailto:notmuch-request@notmuchmail.org?subject=help>\r
  49 List-Subscribe: <http://notmuchmail.org/mailman/listinfo/notmuch>,\r
  50         <mailto:notmuch-request@notmuchmail.org?subject=subscribe>\r
  51 X-List-Received-Date: Mon, 03 Sep 2012 14:58:34 -0000\r
  52 \r
  53 On Mon, Sep 03 2012, Michal Nazarewicz <mina86@mina86.com> wrote:\r
  54 \r
  55 > Tomi Ollila <tomi.ollila@iki.fi> writes:\r
  56 >> diff --git a/devel/release-checks.sh b/devel/release-checks.sh\r
  57 >> new file mode 100755\r
  58 >> index 0000000..7dadefa\r
  59 >> --- /dev/null\r
  60 >> +++ b/devel/release-checks.sh\r
  61 >> @@ -0,0 +1,211 @@\r
  62 >> +#!/usr/bin/env bash\r
  63 >\r
  64 > On a side note, the whole script could be relatively easily rewritten\r
  65 > not to use bash at all and work with plain POSIX shell.\r
  66 \r
  67 The 'set -o pipefail' is hard to do using POSIX shell. I've been hit\r
  68 by this when 'find' part in find | sort' failed (due to a typo).\r
  69 \r
  70 When building release we can choose shell in more relaxed manner and\r
  71 expect release builder have a recent bash shell -- actually tests\r
  72 require bash 4...\r
  73 \r
  74 >> +\r
  75 >> +set -eu\r
  76 >> +#set -x # or enter bash -x ... on command line\r
  77 >> +\r
  78 >> +if [ x"${BASH_VERSION-}" =3D x ]\r
  79 >\r
  80 > -n perhaps?\r
  81 \r
  82 Before this test we cannot know what shell is run; the x"$var" =3D x=20\r
  83 is the most robust way to do this test. Well, if $BASH_VERSION had=20\r
  84 some value then we'd miss this error message and failed in\r
  85 set -o pipefail\r
  86 \r
  87 If we'd require bash 4 then the test [ x"${BASHPID-}" !=3D x$$ ]\r
  88 would be very hard to get wrong...\r
  89 \r
  90 >> +then        echo\r
  91 >> +    echo "Please execute this script using 'bash' interpreter"\r
  92 >> +    echo\r
  93 >> +    exit 1\r
  94 >> +fi\r
  95 >> +\r
  96 >> +set -o pipefail # bash feature\r
  97 >> +\r
  98 >> +# Avoid locale-specific differences in output of executed commands\r
  99 >> +LANG=3DC LC_ALL=3DC; export LANG LC_ALL\r
 100 >> +\r
 101 >> +readonly DEFAULT_IFS=3D"$IFS"\r
 102 >\r
 103 > Never used.\r
 104 \r
 105 True, it used to be. Can be removed in followup patch.\r
 106 \r
 107 >> +\r
 108 >> +readonly PV_FILE=3D'bindings/python/notmuch/version.py'\r
 109 >> +\r
 110 >> +# Using array here turned out to be unnecessarily complicated\r
 111 >> +emsgs=3D''\r
 112 >> +append_emsg ()\r
 113 >> +{\r
 114 >> +    emsgs=3D"${emsgs:+$emsgs\n}  $1"\r
 115 >> +}\r
 116 >> +\r
 117 >> +for f in ./version debian/changelog NEWS "$PV_FILE"\r
 118 >> +do\r
 119 >> +    test -f $f || { append_emsg "File '$f' is missing"; continue; }\r
 120 >> +    test -r $f || { append_emsg "File '$f' is unreadable"; continue; }\r
 121 >> +    test -s $f ||   append_emsg "File '$f' is empty"\r
 122 >\r
 123 > if ! [ -f "$f" ]; then\r
 124 >       append_emsg "File '$f' is missing"\r
 125 > elif ! [ -r "$f" ]; then\r
 126 >       append_emsg "File '$f' is unreadable"\r
 127 > elif ! [ -s "$f" ]; then\r
 128 >       append_emsg "File '$f' is empty"\r
 129 > fi\r
 130 \r
 131 IMHO this short-circuited or (||) version works well due to this\r
 132 negation handling. The $f's should have been quoted ("$f"), though\r
 133 (in case some of the items contain whitespace the construct=20\r
 134 would't fail). I am open to other opinions, though.\r
 135 \r
 136 >> +done\r
 137 >> +\r
 138 >> +if [ -n "$emsgs" ]\r
 139 >> +then\r
 140 >> +    echo 'Release files problems; fix these and try again:'\r
 141 >> +    echo -e "$emsgs"\r
 142 >> +    exit 1\r
 143 >> +fi\r
 144 >> +\r
 145 >> +if read VERSION\r
 146 >> +then\r
 147 >> +    if read rest\r
 148 >> +    then    echo "'version' file contains more than one line"\r
 149 >> +            exit 1\r
 150 >> +    fi\r
 151 >> +else\r
 152 >> +    echo "Reading './version' file failed (suprisingly!)"\r
 153 >> +    exit 1\r
 154 >> +fi < ./version\r
 155 >> +\r
 156 >> +readonly VERSION\r
 157 >> +\r
 158 >> +verfail ()\r
 159 >> +{\r
 160 >> +    echo No.\r
 161 >> +    echo "$@"\r
 162 >> +    echo "Please follow the instructions in RELEASING to choose a version"\r
 163 >> +    exit 1\r
 164 >> +}\r
 165 >> +\r
 166 >> +echo -n "Checking that '$VERSION' is good with digits and periods... "\r
 167 >> +if [ -z "${VERSION//[0123456789.]/}" ] # bash feature\r
 168 >> +then\r
 169 >> +    case $VERSION in\r
 170 >> +            .*)     verfail "'$VERSION' begins with a period" ;;\r
 171 >> +            *.)     verfail "'$VERSION' ends with a period" ;;\r
 172 >> +            *..*)   verfail "'$VERSION' contains two consecutive periods" ;;\r
 173 >> +            *.*)    echo Yes. ;;\r
 174 >> +            *)      verfail "'$VERSION' is a single number" ;;\r
 175 >> +    esac\r
 176 >> +else\r
 177 >> +    verfail "'$VERSION' contains other characters than digits and periods"\r
 178 >> +fi\r
 179 >\r
 180 > The outer condition can be put inside of case as so:\r
 181 >\r
 182 >       *[^0-9.]*)      verfail "'$VERSION' contains other characters than digits and=\r
 183  periods"\r
 184 >\r
 185 > This makes it more readable by putting all checks in case rather than\r
 186 > splitting one to an outer if.\r
 187 \r
 188 That's true! Also, one less bashish cannot be bad thing to have.\r
 189 \r
 190 >> +\r
 191 >> +\r
 192 >> +# In the rest of this file, tests collect list of errors to be fixed\r
 193 >> +\r
 194 >> +echo -n "Checking that this is Debian package for notmuch... "\r
 195 >> +read deb_notmuch deb_version rest < debian/changelog\r
 196 >> +if [ "$deb_notmuch" =3D 'notmuch' ]\r
 197 >\r
 198 > if [ x"$deb_notmuch" =3D xnotmuch ]\r
 199 >\r
 200 > And so in the rest of the conditions below.\r
 201 \r
 202 builtin bash '[' seems to be robust in these cases(*) ... but I'm now break=\r
 203 ing\r
 204 my own rule about not using most portable expressions; maybe it is just\r
 205 ugliness of the format in so many places. I'm not changing unless there is\r
 206 more desire for it :)\r
 207 \r
 208 (*) at least both of these work:\r
 209     bash -c 'set -eu; foo=3D"-a"; [ "$foo" =3D -a ]'\r
 210     bash -c 'set -eu; foo=3D"-z"; [ "$foo" =3D -z ]'\r
 211 \r
 212 >\r
 213 >> +then\r
 214 >> +    echo Yes.\r
 215 >> +else\r
 216 >> +    echo No.\r
 217 >> +    append_emsg "Package name '$deb_notmuch' is not 'notmuch' in debian/ch=\r
 218 angelog"\r
 219 >> +fi\r
 220 >> +\r
 221 >> +echo -n "Checking that Debian package version is $VERSION-1... "\r
 222 >> +\r
 223 >> +if [ "$deb_version" =3D "($VERSION-1)" ]\r
 224 >> +then\r
 225 >> +    echo Yes.\r
 226 >> +else\r
 227 >> +    echo No.\r
 228 >> +    append_emsg "Version '$deb_version' is not '($VERSION-1)' in debian/ch=\r
 229 angelog"\r
 230 >> +fi\r
 231 >> +\r
 232 >> +echo -n "Checking that python bindings version is $VERSION... "\r
 233 >> +py_version=3D`python -c "execfile('$PV_FILE'); print __VERSION__"`\r
 234 >> +if [ "$py_version" =3D "$VERSION" ]\r
 235 >> +then\r
 236 >> +    echo Yes.\r
 237 >> +else\r
 238 >> +    echo No.\r
 239 >> +    append_emsg "Version '$py_version' is not '$VERSION' in $PV_FILE"\r
 240 >> +fi\r
 241 >> +\r
 242 >> +echo -n "Checking that this is Notmuch NEWS... "\r
 243 >> +read news_notmuch news_version news_date < NEWS\r
 244 >> +if [ "$news_notmuch" =3D "Notmuch" ]\r
 245 >> +then\r
 246 >> +    echo Yes.\r
 247 >> +else\r
 248 >> +    echo No.\r
 249 >> +    append_emsg "First word '$news_notmuch' is not 'Notmuch' in NEWS file"\r
 250 >> +fi\r
 251 >> +\r
 252 >> +echo -n "Checking that NEWS version is $VERSION... "\r
 253 >> +if [ "$news_version" =3D "$VERSION" ]\r
 254 >> +then\r
 255 >> +    echo Yes.\r
 256 >> +else\r
 257 >> +    echo No.\r
 258 >> +    append_emsg "Version '$news_version' in NEWS file is not '$VERSION'"\r
 259 >> +fi\r
 260 >> +\r
 261 >> +#eval `date '+year=3D%Y mon=3D%m day=3D%d'`\r
 262 >> +today0utc=3D`date --date=3D0Z +%s` # gnu date feature\r
 263 >> +\r
 264 >> +echo -n "Checking that NEWS date is right... "\r
 265 >> +case $news_date in\r
 266 >> + '('[2-9][0-9][0-9][0-9]-[01][0-9]-[0123][0-9]')')\r
 267 >> +    newsdate0utc=3D`nd=3D${news_date#\\(}; date --date=3D"${nd%)} 0Z" +%s`\r
 268 >> +    ddiff=3D$((newsdate0utc - today0utc))\r
 269 >> +    if [ $ddiff -lt -86400 ] # since beginning of yesterday...\r
 270 >> +    then\r
 271 >> +            echo No.\r
 272 >> +            append_emsg "Date $news_date in NEWS file is too much in the past"\r
 273 >> +    elif [ $ddiff -gt 172800 ] # up to end of tomorrow...\r
 274 >> +    then\r
 275 >> +            echo No.\r
 276 >> +            append_emsg "Date $news_date in NEWS file is too much in the future"\r
 277 >> +    else\r
 278 >> +            echo Yes.\r
 279 >> +    fi ;;\r
 280 >> + *)\r
 281 >> +    echo No.\r
 282 >> +    append_emsg "Date '$news_date' in NEWS file is not in format (yyyy-mm-=\r
 283 dd)"\r
 284 >> +esac\r
 285 >> +\r
 286 >> +readonly DATE=3D${news_date//[()]/} # bash feature\r
 287 >> +manthdata ()\r
 288 >> +{\r
 289 >> +    set x $*\r
 290 >\r
 291 > Uh?  Did you mean =E2=80=9Cset -- $*=E2=80=9D?\r
 292 \r
 293 Nope, set in some shells don't know '--' (now I'm following the most\r
 294 portable code snippet principle) so with 'x' and just referencing\r
 295 positional parameters with number one larger than with '--' does the trick.\r
 296 \r
 297 >\r
 298 >> +    if [ $# !=3D 7 ]\r
 299 >> +    then\r
 300 >> +            append_emsg "'$mp' has too many '.TH' lines"\r
 301 >> +            man_mismatch=3D1\r
 302 >> +    fi\r
 303 >> +    man_date=3D${5-} man_version=3D${7-}\r
 304 >> +}\r
 305 >> +\r
 306 >> +echo -n "Checking that manual page dates and versions are $DATE and $VE=\r
 307 RSION... "\r
 308 >> +manfiles=3D`find man -type f | sort`\r
 309 >> +man_pages_ok=3DYes\r
 310 >> +for mp in $manfiles\r
 311 >> +do\r
 312 >> +    case $mp in *.[0-9]) ;; # fall below this 'case ... esac'\r
 313 >\r
 314 > Perhaps new line after =E2=80=9Cin=E2=80=9D?\r
 315 \r
 316 Can be added...\r
 317 \r
 318 >\r
 319 >> +            */Makefile.local | */Makefile ) continue ;;\r
 320 >> +            */.gitignore)   continue ;;\r
 321 >> +            *.bak)          continue ;;\r
 322 >> +\r
 323 >> +            *)      append_emsg "'$mp': extra file"\r
 324 >> +                    man_pages_ok=3DNo\r
 325 >> +                    continue\r
 326 >> +    esac\r
 327 >> +    manthdata `sed -n '/^[.]TH NOTMUCH/ { y/"/ /; p; }' "$mp"`\r
 328 >\r
 329 > Alternatively =E2=80=9C\.=E2=80=9D instead of =E2=80=9C[.]=E2=80=9D.\r
 330 \r
 331 I prefer [.] as a more robust alternative -- backslashes may get=20\r
 332 "expanded away" in some cases; using [.] just frees me from checking\r
 333 whether that may happen.\r
 334 \r
 335 >> +    if [ "$man_version" !=3D "$VERSION" ]\r
 336 >> +    then    append_emsg "Version '$man_version' is not '$VERSION' in $mp"\r
 337 >> +            mman_pages_ok=3DNo\r
 338 >> +    fi\r
 339 >> +    if [ "$man_date" !=3D "$DATE" ]\r
 340 >> +    then    append_emsg "DATE '$man_date' is not '$DATE' in $mp"\r
 341 >> +            man_pages_ok=3DNo\r
 342 >> +    fi\r
 343 >> +done\r
 344 >> +echo $man_pages_ok.\r
 345 >> +\r
 346 >> +if [ -n "$emsgs" ]\r
 347 >> +then\r
 348 >> +    echo\r
 349 >> +    echo 'Release check failed; check these issues:'\r
 350 >> +    echo -e "$emsgs"\r
 351 >> +    exit 1\r
 352 >> +fi\r
 353 >> +\r
 354 >> +echo 'All checks this script executed completed successfully.'\r
 355 >> +echo 'Make sure that everything else mentioned in RELEASING'\r
 356 >> +echo 'file is in order, too.'\r
 357 >> +\r
 358 >> +exit 0\r
 359 >\r
 360 > Unnecessary.\r
 361 \r
 362 True.=20\r
 363 \r
 364 Thanks for the review, I'll probably send new version tomorrow...\r
 365 \r
 366 >> +\r
 367 >> +# Local variables:\r
 368 >> +# mode: shell-script\r
 369 >> +# sh-basic-offset: 8\r
 370 >> +# tab-width: 8\r
 371 >> +# End:\r
 372 >> +# vi: set sw=3D8 ts=3D8\r
 373 >\r
 374 > --=20\r
 375 > Best regards,                                         _     _\r
 376 > .o. | Liege of Serenely Enlightened Majesty of      o' \,=3D./ `o\r
 377 > ..o | Computer Science,  Micha=C5=82 =E2=80=9Cmina86=E2=80=9D Nazarewicz =\r
 378    (o o)\r
 379 \r
 380 Tomi\r