[redirecting to bug-gnulib as the owner of the git-version-gen script in
question; replies can drop other lists]
On 01/02/2011 05:16 PM, Bruce Korb wrote:
> Hi Jonathan,
>
> On Sun, Jan 2, 2011 at 10:34 AM, Jonathan Nieder <jrnieder@xxxxxxxxx> wrote:
>> Were you been able to reproduce that outside the script?
>
> No, I was blind to the invocation. You found it. I was looking
> without seeing. Thank you.
>
> Given that shells without functions can be considered sufficiently
> obsolete to not be a consideration, perhaps a better solution is
> to put the I-don't-care-about-error-messages code into a separate
> function with stderr redirected. Doing that turned out messier
> than I had hoped....
Jonathan's patch:
> diff --git a/build-aux/git-version-gen b/build-aux/git-version-gen
> index 5617eb8..119d7aa 100755
> --- a/build-aux/git-version-gen
> +++ b/build-aux/git-version-gen
> @@ -119,7 +119,7 @@ then
> # result is the same as if we were using the newer version
> # of git describe.
> vtag=`echo "$v" | sed 's/-.*//'`
> - numcommits=`git rev-list "$vtag"..HEAD | wc -l`
> + numcommits=`git rev-list "$vtag"..HEAD 2>/dev/null | wc -l`
> v=`echo "$v" | sed "s/\(.*\)-\(.*\)/\1-$numcommits-\2/"`;
> ;;
> esac
makes sense to suppress the error message from leaking (whether or not
git can be improved to have the error message claim which program is
issuing the message); but there's still the nagging issue that because
git output is fed to a pipe, there's no way to check $? to see if git
failed, in order to properly react to that situation.
Bruce's patch mixes refactoring with bug fixing, making it a bit harder
to read, and introduced a bug in its own right:
> diff --git a/build-aux/git-version-gen b/build-aux/git-version-gen
> index c278f6a..8a238b0 100755
> --- a/build-aux/git-version-gen
> +++ b/build-aux/git-version-gen
> @@ -1,6 +1,6 @@
> #!/bin/sh
> # Print a version string.
> -scriptversion=2010-10-13.20; # UTC
> +scriptversion=2011-01-03.00; # UTC
>
> # Copyright (C) 2007-2011 Free Software Foundation, Inc.
> #
> @@ -78,76 +78,96 @@ tag_sed_script="${2:-s/x/x/}"
> nl='
> '
>
> -# Avoid meddling by environment variable of the same name.
> -v=
> +get_ver()
> +{
> + local PS4='>gv> '
Portable scripts CANNOT use local (since POSIX does not require it), and
setting PS4 is not commonly done in portable scripting.
I'll probably end up writing yet a third approach, which collects git
rev-list output into a temporary variable in order to correctly detect
failures, without refactoring into a helper function.
--
Eric Blake eblake@xxxxxxxxxx +1-801-349-2682
Libvirt virtualization library http://libvirt.org
Attachment:
signature.asc
Description: OpenPGP digital signature