Re: add --match argument to git-version-gen

[Jim Meyering]

Andy Wingo wrote:

> On Fri 06 Jul 2012 15:56, Stefano Lattarini <address@hidden> writes:
>
>> On 07/06/2012 03:53 PM, Andy Wingo wrote:
>>>
>>> @@ -121,6 +126,9 @@ if test -z "$tarball_version_file"; then
>>>      exit 1
>>>  fi
>>>
>>> +echo $match
>>>
>> Huh?  Forgotten debugging code perhaps?
>>
>>> +match="${match:-$prefix\*}"
>>> +echo $match
>>>
>> Likewise.
>
> Sigh, yes!  Sorry about that.  Was just about to reply to myself, but
> you were faster :)  Here's the right version, with a bugfix to the match
> expansion so it works correctly if you don't pass --match.
>
>
>>From ded42750d65cf976b8ff2874fcca5de91b2526cb Mon Sep 17 00:00:00 2001
> From: Andy Wingo <address@hidden>
> Date: Fri, 6 Jul 2012 15:51:05 +0200
> Subject: [PATCH] git-version-gen: add --match argument
>
> * build-aux/git-version-gen (Options): Add --match argument.
>   (v_from_git): Remove path without --match.

It'd be nice to say "why" this change is useful.
At worst, just refer to the URL for this mailing list thread.

>  build-aux/git-version-gen |   19 ++++++++++++-------
>  1 file changed, 12 insertions(+), 7 deletions(-)
>
> diff --git a/build-aux/git-version-gen b/build-aux/git-version-gen
> index 0fa9063..0b51154 100755
> --- a/build-aux/git-version-gen
> +++ b/build-aux/git-version-gen
> @@ -1,6 +1,6 @@
>  #!/bin/sh
>  # Print a version string.
> -scriptversion=2012-03-18.17; # UTC
> +scriptversion=2012-07-06.14; # UTC
>
>  # Copyright (C) 2007-2012 Free Software Foundation, Inc.
>  #
> @@ -85,20 +85,25 @@ Print a version string.
>
>  Options:
>
> -   --prefix           prefix of git tags (default 'v')
> +   --prefix     prefix of git tags to strip from version (default 'v')
> +   --match      pattern for git tags to match (default: '\$prefix*')
>
> -   --help             display this help and exit
> -   --version          output version information and exit
> +   --help       display this help and exit
> +   --version    output version information and exit
>
> -Running without arguments will suffice in most cases."
> +Running without arguments will suffice in most cases.  If no --match
> +argument is given, only match tags that begin with the --prefix."
>
>  prefix=v
> +unset match
> +unset tag_sed_script

Protecting against envvars by those names is a good idea.
That's a fix that merits mention in the ChangeLog, if not
a separate commit.
However, please don't use "unset" and rather just set them to
the empty string as is done just below -- oh, and also initialize
tarball_version_file:

    match=
    tag_sed_script=
    tarball_version_file=


>  while test $# -gt 0; do
>    case $1 in
>      --help) echo "$usage"; exit 0;;
>      --version) echo "$version"; exit 0;;
>      --prefix) shift; prefix="$1";;
> +    --match) shift; match="$1";;
>      -*)
>        echo "$0: Unknown option '$1'." >&2
>        echo "$0: Try '--help' for more information." >&2
> @@ -121,6 +126,7 @@ if test -z "$tarball_version_file"; then
>      exit 1
>  fi
>
> +match="${match:-$prefix*}"
>  tag_sed_script="${tag_sed_script:-s/x/x/}"
>
>  nl='
> @@ -151,8 +157,7 @@ then
>  # directory, and "git describe" output looks sensible, use that to
>  # derive a version string.
>  elif test "`git log -1 --pretty=format:x . 2>&1`" = x \
> -    && v=`git describe --abbrev=4 --match="$prefix*" HEAD 2>/dev/null \
> -          || git describe --abbrev=4 HEAD 2>/dev/null` \
> +    && v=`git describe --abbrev=4 --match="$match" HEAD 2>/dev/null` \

Why remove the second git invocation here?
I can see why you'd do that when --match has been specified,
but what about all of us who use the default (no --match option).
Do you really want to add the "feature" that git-version-gen now
fails when there is no "v*" tag, whereas before that worked fine?