Andy Wingo wrote:
> On Fri 06 Jul 2012 16:32, Jim Meyering <j...@meyering.net> writes:
>
>> It'd be nice to say "why" this change is useful.
>> At worst, just refer to the URL for this mailing list thread.
>
> OK.
>
>> 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:
>
> OK.
>
>>> # 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?
Hi Andy,
That looks good, so far.
> Fixed to only match strictly when --match is passed. I couldn't figure
> out a way to do it without the gymnastics.
...
How about writing a function -- with description similar to the comments
you've added below -- and doing something like the following?
Then we should be able to avoid that duplication.
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_desc "$prefix" "$match"` \
...
> -if test -n "$v"
> -then
> - : # use $v
> -# Otherwise, if there is at least one git commit involving the working
> -# 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=`printf '%s\n' "$v" | sed "$tag_sed_script"` \
> - && case $v in
> - $prefix[0-9]*) ;;
> - *) (exit 1) ;;
> - esac
> -then
> +function massage_git_version() {
> # Is this a new git that lists number of commits since the last
> # tag or the previous older version that did not?
> # Newer: v6.10-77-g0f8faeb
> @@ -184,6 +177,38 @@ then
> # Remove the "g" in git describe's output string, to save a byte.
> v=`echo "$v" | sed 's/-/./;s/\(.*\)-g/\1-/'`;
> v_from_git=1
> +}
> +
> +if test -n "$v"
> +then
> + : # use $v
> +# Otherwise, if the user specified --match, look for a matching tag.
> +elif test -n "$match"
> +then
> + if test "`git log -1 --pretty=format:x . 2>&1`" = x \
> + && v=`git describe --abbrev=4 --match="$match" HEAD 2>/dev/null` \
> + && v=`printf '%s\n' "$v" | sed "$tag_sed_script"` \
> + && case $v in
> + $prefix[0-9]*) ;;
> + *) (exit 1) ;;
> + esac
> + then
> + massage_git_version
> + else
> + v=UNKNOWN
> + fi
> +# Otherwise, look for a tag starting with --prefix, or the first tag we
> +# find, sed it, and see if it looks like a version.
> +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=`printf '%s\n' "$v" | sed "$tag_sed_script"` \
> + && case $v in
> + $prefix[0-9]*) ;;
> + *) (exit 1) ;;
> + esac
> +then
> + massage_git_version
> else
> v=UNKNOWN
> fi
> --
> 1.7.10
