Le 5 juil. 2012 à 15:24, Akim Demaille a écrit :
> Hi friends,
>
> I just installed this.
>
> # simple check: no question marks on line 3 of NEWS
> -test "$(sed -n 3p NEWS)" = "$noteworthy_stub" \
> +test "$(sed -n 3p NEWS)" != "$noteworthy_stub" \
> || die 'line 3 of NEWS looks fishy!'
This was wrong, the = was right, it is the comments that
are misleading. I'm trying to make a beta of Bison, and
because an initial run changed the NEWS file (which I did
not notice) the NEWS line was changed, and this check
was triggered. I thought that this check was ensuring
that the template was properly filled, but actually,
it's exactly the opposite: this is run _before_.
I'm sorry for noise. I propose the following patch
which is first performing the checks, to avoid useless
changes of NEWS, and comment/message fixes.
Also, meanwhile Jim answered:
>> # simple check: no question marks on line 3 of NEWS
>> -test "$(sed -n 3p NEWS)" = "$noteworthy_stub" \
>> +test "$(sed -n 3p NEWS)" != "$noteworthy_stub" \
>> || die 'line 3 of NEWS looks fishy!'
>
> First, that's sort of like a double negative.
> This is a more readable equivalent of what you've committed:
>
> test "$(sed -n 3p NEWS)" = "$noteworthy_stub" \
> && die 'line 3 of NEWS looks fishy!'
Well, while I agree in general, I don't have the same
feeling in this case, for two reasons. The first one
being that under set -e, && propagate the failure,
and the script will die. || is safer than && when
not in a condition (inside an if for instance). And second,
this pattern (check || die) implements assert is a quite
visual way. Using &&, on the contrary IMHO, would make
things harder to read (because not consistent).
Anyway, this was wrong in this place.
Here is my proposal:
From 6df4446131184e583ebc2c472a626fed07aab012 Mon Sep 17 00:00:00 2001
From: Akim Demaille <a...@lrde.epita.fr>
Date: Thu, 5 Jul 2012 15:41:20 +0200
Subject: [PATCH] do-release-commit-and-tag: fix the previous commit
* build-aux/do-release-commit-and-tag: Actually the test was right,
but the error message was wrong.
Fix comment, and improve error message.
Perform check first, so that NEWS is not modified uselessly.
---
ChangeLog | 6 ++++++
build-aux/do-release-commit-and-tag | 41 ++++++++++++++++++++++++-------------
2 files changed, 33 insertions(+), 14 deletions(-)
diff --git a/ChangeLog b/ChangeLog
index c257cb2..57478ed 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,11 @@
2012-07-05 Akim Demaille <a...@lrde.epita.fr>
+ do-release-commit-and-tag: fix the previous commit
+ * build-aux/do-release-commit-and-tag: Actually the test was right,
+ but the error message was wrong.
+ Fix comment, and improve error message.
+ Perform check first, so that NEWS is not modified uselessly.
+
do-release-commit-and-tag: fix typo
* build-aux/do-release-commit-and-tag: Be sure that NEWS does
_not_ start with a stub.
diff --git a/build-aux/do-release-commit-and-tag
b/build-aux/do-release-commit-and-tag
index 329d0eb..78223be 100755
--- a/build-aux/do-release-commit-and-tag
+++ b/build-aux/do-release-commit-and-tag
@@ -3,7 +3,7 @@
# controlled .prev-version file, automate the procedure by which we record
# the date, release-type and version string in the NEWS file. That commit
# will serve to identify the release, so apply a signed tag to it as well.
-VERSION=2012-07-05.13 # UTC
+VERSION=2012-07-05.14 # UTC
# Note: this is a bash script (could be zsh or dash)
@@ -77,6 +77,10 @@ EOF
exit
}
+## ------ ##
+## Main. ##
+## ------ ##
+
branch=master
builddir=.
@@ -107,6 +111,11 @@ test $# = 2 \
ver=$1
type=$2
+
+## ---------------------- ##
+## First, sanity checks. ##
+## ---------------------- ##
+
# Verify that $ver looks like a version number, and...
echo "$ver"|grep -E '^[0-9][0-9.]*[0-9]$' > /dev/null \
|| die "invalid version: $ver"
@@ -124,32 +133,36 @@ case $type in
*) die "invalid release type: $type";;
esac
+# No dirt allowed.
+case $(git diff-index --name-only HEAD) in
+ '') ;;
+ *) die 'this tree is dirty; commit your changes first';;
+esac
+
+# Ensure the current branch name is correct:
+curr_br=$(git rev-parse --symbolic-full-name HEAD)
+test "$curr_br" = refs/heads/$branch || die not on branch $branch
+
# Extract package name from Makefile.
Makefile=$builddir/Makefile
pkg=$(sed -n 's/^PACKAGE = \(.*\)/\1/p' "$Makefile") \
|| die "failed to determine package name from $Makefile"
-# simple check: no question marks on line 3 of NEWS
-test "$(sed -n 3p NEWS)" != "$noteworthy_stub" \
- || die 'line 3 of NEWS looks fishy!'
+# Check that line 3 of NEWS is the stub line about to be filled.
+test "$(sed -n 3p NEWS)" = "$noteworthy_stub" \
+ || die "line 3 of NEWS must be exactly '$noteworthy_stub'"
-# No dirt allowed.
-case $(git diff-index --name-only HEAD) in
- '') ;;
- *) die 'this tree is dirty; commit your changes first';;
-esac
+## --------------- ##
+## Then, changes. ##
+## --------------- ##
-# update NEWS to have today's date, plus desired version number and $type
+# Update NEWS to have today's date, plus desired version number and $type.
perl -MPOSIX -ni -e 'my $today = strftime "%F", localtime time;' \
-e 'my ($type, $ver) = qw('"$type $ver"');' \
-e 'my $pfx = "'"$noteworthy"'";' \
-e 'print $.==3 ? "$pfx $ver ($today) [$type]\n" : $_' \
NEWS || die 'failed to update NEWS'
-# Ensure the current branch name is correct:
-curr_br=$(git rev-parse --symbolic-full-name HEAD)
-test "$curr_br" = refs/heads/$branch || die not on branch $branch
-
printf "version $ver\n\n* NEWS: Record release date.\n" \
| git commit -F - -a || die 'git commit failed'
git tag -s -m "$pkg $ver" v$ver HEAD || die 'git tag failed'
--
1.7.11.1
