Re: fatal: ambiguous message

[Eric Blake <eblake@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 2793 bytes --]

[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@gmail.com> 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@redhat.com    +1-801-349-2682
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 619 bytes --]