fatal: ambiguous message
A message that is not in the archive (sent off-list or not yet received): [email protected]
A message that is not in the archive (sent off-list or not yet received): [email protected]
Quoted 6 lines · Eric Blake
Quoted 3 lines · Bruce Korb
Hi,
Is this fatal? If so, how come it continued?
It's fatal to git-version-gen, which did not continue.
git-version-gen has but two fatal conditions: invalid arguments yielding a usage message and an unreadable "tarball version file". That is not this message, but might be clarified with:
v=`cat $tarball_version_file 2>&1` || {
echo "$0 error: unreadable tarball version file $1: $v" >&2
exit 1
}In any event, the invocation is:
./git-version-gen .tarball-version
and the file ".tarball-version" does not exist, hence git-version-gen should not fail at all. So, this message says, "fatal: ..." and comes from git and all three "git" invocations redirect stderr to /dev/null. The fact that we see it is a git bug. Error messages should be directed to stderr and thus written to /dev/null.
So, git-version-gen is correct to continue, but git should fail with a message that names the program that fails ("git") and should direct the message to stderr.
Note to GIT list: the message in question:
fatal: ambiguous argument 'v0.1..HEAD': unknown revision \
or path not in the working tree.
Thanks! Regards, Bruce
Hi,
Context: http://git.savannah.gnu.org/gitweb/?p=autoconf.git;a=blob;f=build-aux/git-version-gen
Quoted 4 lines · Bruce Korb
So, this message says, "fatal: ..."
and comes from git and all three "git" invocations redirect stderr to/dev/null. The fact that we see it is a git bug. Error messages should be directed to stderr and thus written to/dev/null.
Were you been able to reproduce that outside the script?
Quoted 3 lines
So, git-version-gen is correct to continue, but git should fail with a message that names the program that fails ("git") and should direct the message to stderr.
No thoughts on this part. git has at least four kinds of message it sends to stderr (fatal:, warning:, error:, and usage:) but I am not sure it is useful to distinguish them ---
git add: pathspec 'nonsense' did not match any files
might be nicer.
build-aux/git-version-gen +1 -1
@@ -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
Hi Jonathan,
Quoted 1 line · Jonathan Nieder
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....
build-aux/git-version-gen +68 -48
@@ -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> ' + git status >/dev/null 2>&1 || { + printf UNKNOWN + exit 0 + } -# First see if there is a tarball-only version file. -# then try "git describe", then default. -if test -f $tarball_version_file -then - v=`cat $tarball_version_file` || exit 1 - case $v in - *$nl*) v= ;; # reject multi-line output - [0-9]*) ;; - *) v= ;; + test "`git log -1 --pretty=format:x . 2>&1`" = x || { + printf UNKNOWN + exit 0 + } + + X=`git describe --abbrev=4 --match='v*' HEAD || \ + git describe --abbrev=4 HEAD` || { + printf UNKNOWN + exit 0 + } + + case "$X" in + v[0-9]* ) : ;; + * ) + printf UNKNOWN + exit 0 + ;; esac - test -z "$v" \ - && echo "$0: WARNING: $tarball_version_file seems to be damaged" 1>&2 -fi -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='v*' HEAD 2>/dev/null \ - || git describe --abbrev=4 HEAD 2>/dev/null` \ - && v=`printf '%s\n' "$v" | sed "$tag_sed_script"` \ - && case $v in - v[0-9]*) ;; - *) (exit 1) ;; - esac -then # 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 # Older: v6.10-g0f8faeb - case $v in + case $X in *-*-*) : git describe is okay three part flavor ;; *-*) : git describe is older two part flavor # Recreate the number of commits and rewrite such that the # result is the same as if we were using the newer version # of git describe. - vtag=`echo "$v" | sed 's/-.*//'` + vtag=`echo "$X" | sed 's/-.*//'` numcommits=`git rev-list "$vtag"..HEAD | wc -l` - v=`echo "$v" | sed "s/\(.*\)-\(.*\)/\1-$numcommits-\2/"`; + X=`echo "$X" | sed "s/\(.*\)-\(.*\)/\1-$numcommits-\2/"`; ;; esac + # Don't declare a version "dirty" merely because a time stamp has changed. + silent_git update-index --refresh >/dev/null 2>&1 + + dirty=`git diff-index --name-only HEAD` || dirty= + case "$dirty" in + '') ;; + *) # Append the suffix only if there isn't one already. + case $X in + *-dirty) ;; + *) X="$X-dirty" ;; + esac + ;; + esac + # Change the first '-' to a '.', so version-comparing tools work properly. # Remove the "g" in git describe's output string, to save a byte. - v=`echo "$v" | sed 's/-/./;s/\(.*\)-g/\1-/'`; + echo "$X" | sed 's/^v//;s/-/./;s/\(.*\)-g/\1-/' +} + +# First see if there is a tarball-only version file. +# then try "git describe", then default. +if test -f $tarball_version_file +then + v=`cat $tarball_version_file` || exit 1 + case $v in + *$nl*) v= ;; # reject multi-line output + [0-9]*) ;; + *) v= ;; + esac + test -z "$v" \ + && echo "$0: WARNING: $tarball_version_file seems to be damaged" 1>&2 else - v=UNKNOWN + v= fi -v=`echo "$v" |sed 's/^v//'` - -# Don't declare a version "dirty" merely because a time stamp has changed. -git update-index --refresh > /dev/null 2>&1 +if test -n "$v" +then + : # use $v -dirty=`sh -c 'git diff-index --name-only HEAD' 2>/dev/null` || dirty= -case "$dirty" in - '') ;; - *) # Append the suffix only if there isn't one already. - case $v in - *-dirty) ;; - *) v="$v-dirty" ;; - esac ;; -esac +else + # 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. + # + v=`get_ver` 2>/dev/null +fi # Omit the trailing newline, so that m4_esyscmd can use the result directly. echo "$v" | tr -d "$nl"
_______________________________________________
Autoconf mailing list
[email protected]
http://lists.gnu.org/mailman/listinfo/autoconf
[redirecting to bug-gnulib as the owner of the git-version-gen script in question; replies can drop other lists]
Quoted 5 lines · Bruce Korb
Hi Jonathan,
Quoted 1 line · Jonathan Nieder
Were you been able to reproduce that outside the script?
Quoted 8 lines
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:
build-aux/git-version-gen then · line 125
Quoted 13 lines
build-aux/git-version-gen +1
-1@@ -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:
Quoted 13 lines
build-aux/git-version-gen +1
-1@@ -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. #
build-aux/git-version-gen tag_sed_script="${2:-s/x/x/}" · line 83
Quoted 8 lines
build-aux/git-version-gen +3
-2@@ -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.
Signature · 2 lines
Eric Blake [email protected] +1-801-349-2682 Libvirt virtualization library http://libvirt.org