{"thread":{"id":"26175","subject":"Re: fatal: ambiguous message","startedAt":"2011-01-02T18:03:55Z","lastAt":"2011-01-03T15:04:20Z","messageCount":4,"participants":["Bruce Korb","Jonathan Nieder","Eric Blake"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"158821","messageId":"4D20BE0B.6040104@gmail.com","threadId":"26175","inReplyTo":"4D1DFF96.4010004@redhat.com","subject":"Re: fatal: ambiguous message","fromName":"Bruce Korb","fromEmail":"bruce.korb@gmail.com","sentAt":"2011-01-02T18:03:55Z","receivedAt":"2011-01-02T18:03:55Z","isPatch":false,"sender":{"key":"bruce.korb@gmail.com","avatar":"https://gravatar.com/avatar/86d91467dc7cc8466a9d133a7b93a5d21233052017c144c9a6f6f7e5110344d0?d=mp&s=160"},"body":"On 12/31/10 08:06, Eric Blake wrote:\n> On 12/30/2010 06:37 PM, Bruce Korb wrote:\n>> Hi,\n>>\n>> Is this fatal?  If so, how come it continued?\n> \n> It's fatal to git-version-gen, which did not continue.\n\ngit-version-gen has but two fatal conditions: invalid arguments\nyielding a usage message and an unreadable \"tarball version file\".\nThat is not this message, but might be clarified with:\n\n    v=`cat $tarball_version_file 2>&1` || {\n        echo \"$0 error: unreadable tarball version file $1:  $v\" >&2\n        exit 1\n    }\n\nIn any event, the invocation is:\n   ./git-version-gen .tarball-version\nand the file \".tarball-version\" does not exist, hence git-version-gen\nshould not fail at all.  So, this message says, \"fatal: ...\"\nand comes from git and all three \"git\" invocations redirect stderr to\n/dev/null.  The fact that we see it is a git bug.  Error messages\nshould be directed to stderr and thus written to /dev/null.\n\nSo, git-version-gen is correct to continue, but git should fail\nwith a message that names the program that fails (\"git\") and\nshould direct the message to stderr.\n\nNote to GIT list: the message in question:\n\n    fatal: ambiguous argument 'v0.1..HEAD': unknown revision \\\n       or path not in the working tree.\n\nThanks!  Regards, Bruce\n"},{"id":"158822","messageId":"20110102183453.GA13463@burratino","threadId":"26175","inReplyTo":"4D20BE0B.6040104@gmail.com","subject":"Re: fatal: ambiguous message","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2011-01-02T18:34:53Z","receivedAt":"2011-01-02T18:34:53Z","isPatch":false,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Hi,\n\nContext: http://git.savannah.gnu.org/gitweb/?p=autoconf.git;a=blob;f=build-aux/git-version-gen\n\nBruce Korb wrote:\n>                          So, this message says, \"fatal: ...\"\n> and comes from git and all three \"git\" invocations redirect stderr to\n> /dev/null.  The fact that we see it is a git bug.  Error messages\n> should be directed to stderr and thus written to /dev/null.\n\nWere you been able to reproduce that outside the script?\n\n> So, git-version-gen is correct to continue, but git should fail\n> with a message that names the program that fails (\"git\") and\n> should direct the message to stderr.\n\nNo thoughts on this part.  git has at least four kinds of message it\nsends to stderr (fatal:, warning:, error:, and usage:) but I am\nnot sure it is useful to distinguish them ---\n\n\tgit add: pathspec 'nonsense' did not match any files\n\nmight be nicer.\n\ndiff --git a/build-aux/git-version-gen b/build-aux/git-version-gen\nindex 5617eb8..119d7aa 100755\n--- a/build-aux/git-version-gen\n+++ b/build-aux/git-version-gen\n@@ -119,7 +119,7 @@ then\n \t    # result is the same as if we were using the newer version\n \t    # of git describe.\n \t    vtag=`echo \"$v\" | sed 's/-.*//'`\n-\t    numcommits=`git rev-list \"$vtag\"..HEAD | wc -l`\n+\t    numcommits=`git rev-list \"$vtag\"..HEAD 2>/dev/null | wc -l`\n \t    v=`echo \"$v\" | sed \"s/\\(.*\\)-\\(.*\\)/\\1-$numcommits-\\2/\"`;\n \t    ;;\n     esac\n"},{"id":"158824","messageId":"4D211555.1040502@gmail.com","threadId":"26175","inReplyTo":"20110102183453.GA13463@burratino","subject":"Re: fatal: ambiguous message","fromName":"Bruce Korb","fromEmail":"bruce.korb@gmail.com","sentAt":"2011-01-03T00:16:21Z","receivedAt":"2011-01-03T00:16:21Z","isPatch":false,"sender":{"key":"bruce.korb@gmail.com","avatar":"https://gravatar.com/avatar/86d91467dc7cc8466a9d133a7b93a5d21233052017c144c9a6f6f7e5110344d0?d=mp&s=160"},"body":"Hi Jonathan,\n\nOn Sun, Jan 2, 2011 at 10:34 AM, Jonathan Nieder <jrnieder@gmail.com> wrote:\n> Were you been able to reproduce that outside the script?\n\nNo, I was blind to the invocation.  You found it.  I was looking\nwithout seeing.  Thank you.\n\nGiven that shells without functions can be considered sufficiently\nobsolete to not be a consideration, perhaps a better solution is\nto put the I-don't-care-about-error-messages code into a separate\nfunction with stderr redirected.  Doing that turned out messier\nthan I had hoped....\n\n\n\ndiff --git a/build-aux/git-version-gen b/build-aux/git-version-gen\nindex c278f6a..8a238b0 100755\n--- a/build-aux/git-version-gen\n+++ b/build-aux/git-version-gen\n@@ -1,6 +1,6 @@\n #!/bin/sh\n # Print a version string.\n-scriptversion=2010-10-13.20; # UTC\n+scriptversion=2011-01-03.00; # UTC\n \n # Copyright (C) 2007-2011 Free Software Foundation, Inc.\n #\n@@ -78,76 +78,96 @@ tag_sed_script=\"${2:-s/x/x/}\"\n nl='\n '\n \n-# Avoid meddling by environment variable of the same name.\n-v=\n+get_ver()\n+{\n+    local PS4='>gv> '\n+    git status >/dev/null 2>&1 || {\n+        printf UNKNOWN\n+        exit 0\n+    }\n \n-# First see if there is a tarball-only version file.\n-# then try \"git describe\", then default.\n-if test -f $tarball_version_file\n-then\n-    v=`cat $tarball_version_file` || exit 1\n-    case $v in\n-\t*$nl*) v= ;; # reject multi-line output\n-\t[0-9]*) ;;\n-\t*) v= ;;\n+    test \"`git log -1 --pretty=format:x . 2>&1`\" = x || {\n+        printf UNKNOWN\n+        exit 0\n+    }\n+\n+    X=`git describe --abbrev=4 --match='v*' HEAD || \\\n+        git describe --abbrev=4 HEAD` || {\n+        printf UNKNOWN\n+        exit 0\n+    }\n+\n+    case \"$X\" in\n+    v[0-9]* ) : ;;\n+    * )\n+        printf UNKNOWN\n+        exit 0\n+        ;;\n     esac\n-    test -z \"$v\" \\\n-\t&& echo \"$0: WARNING: $tarball_version_file seems to be damaged\" 1>&2\n-fi\n \n-if test -n \"$v\"\n-then\n-    : # use $v\n-# Otherwise, if there is at least one git commit involving the working\n-# directory, and \"git describe\" output looks sensible, use that to\n-# derive a version string.\n-elif test \"`git log -1 --pretty=format:x . 2>&1`\" = x \\\n-    && v=`git describe --abbrev=4 --match='v*' HEAD 2>/dev/null \\\n-\t  || git describe --abbrev=4 HEAD 2>/dev/null` \\\n-    && v=`printf '%s\\n' \"$v\" | sed \"$tag_sed_script\"` \\\n-    && case $v in\n-\t v[0-9]*) ;;\n-\t *) (exit 1) ;;\n-       esac\n-then\n     # Is this a new git that lists number of commits since the last\n     # tag or the previous older version that did not?\n     #   Newer: v6.10-77-g0f8faeb\n     #   Older: v6.10-g0f8faeb\n-    case $v in\n+    case $X in\n \t*-*-*) : git describe is okay three part flavor ;;\n \t*-*)\n \t    : git describe is older two part flavor\n \t    # Recreate the number of commits and rewrite such that the\n \t    # result is the same as if we were using the newer version\n \t    # of git describe.\n-\t    vtag=`echo \"$v\" | sed 's/-.*//'`\n+\t    vtag=`echo \"$X\" | sed 's/-.*//'`\n \t    numcommits=`git rev-list \"$vtag\"..HEAD | wc -l`\n-\t    v=`echo \"$v\" | sed \"s/\\(.*\\)-\\(.*\\)/\\1-$numcommits-\\2/\"`;\n+\t    X=`echo \"$X\" | sed \"s/\\(.*\\)-\\(.*\\)/\\1-$numcommits-\\2/\"`;\n \t    ;;\n     esac\n \n+    # Don't declare a version \"dirty\" merely because a time stamp has changed.\n+    silent_git update-index --refresh >/dev/null 2>&1\n+\n+    dirty=`git diff-index --name-only HEAD` || dirty=\n+    case \"$dirty\" in\n+    '') ;;\n+    *) # Append the suffix only if there isn't one already.\n+\tcase $X in\n+\t  *-dirty) ;;\n+\t  *) X=\"$X-dirty\" ;;\n+\tesac\n+        ;;\n+    esac\n+\n     # Change the first '-' to a '.', so version-comparing tools work properly.\n     # Remove the \"g\" in git describe's output string, to save a byte.\n-    v=`echo \"$v\" | sed 's/-/./;s/\\(.*\\)-g/\\1-/'`;\n+    echo \"$X\" | sed 's/^v//;s/-/./;s/\\(.*\\)-g/\\1-/'\n+}\n+\n+# First see if there is a tarball-only version file.\n+# then try \"git describe\", then default.\n+if test -f $tarball_version_file\n+then\n+    v=`cat $tarball_version_file` || exit 1\n+    case $v in\n+\t*$nl*) v= ;; # reject multi-line output\n+\t[0-9]*) ;;\n+\t*) v= ;;\n+    esac\n+    test -z \"$v\" \\\n+\t&& echo \"$0: WARNING: $tarball_version_file seems to be damaged\" 1>&2\n else\n-    v=UNKNOWN\n+    v=\n fi\n \n-v=`echo \"$v\" |sed 's/^v//'`\n-\n-# Don't declare a version \"dirty\" merely because a time stamp has changed.\n-git update-index --refresh > /dev/null 2>&1\n+if test -n \"$v\"\n+then\n+    : # use $v\n \n-dirty=`sh -c 'git diff-index --name-only HEAD' 2>/dev/null` || dirty=\n-case \"$dirty\" in\n-    '') ;;\n-    *) # Append the suffix only if there isn't one already.\n-\tcase $v in\n-\t  *-dirty) ;;\n-\t  *) v=\"$v-dirty\" ;;\n-\tesac ;;\n-esac\n+else\n+    # Otherwise, if there is at least one git commit involving the\n+    # working directory, and \"git describe\" output looks sensible, use\n+    # that to derive a version string.\n+    #\n+    v=`get_ver` 2>/dev/null\n+fi\n \n # Omit the trailing newline, so that m4_esyscmd can use the result directly.\n echo \"$v\" | tr -d \"$nl\"\n\n\n_______________________________________________\nAutoconf mailing list\nAutoconf@gnu.org\nhttp://lists.gnu.org/mailman/listinfo/autoconf\n"},{"id":"158847","messageId":"4D21E574.50404@redhat.com","threadId":"26175","inReplyTo":"4D211555.1040502@gmail.com","subject":"Re: fatal: ambiguous message","fromName":"Eric Blake","fromEmail":"eblake@redhat.com","sentAt":"2011-01-03T15:04:20Z","receivedAt":"2011-01-03T15:04:20Z","isPatch":false,"sender":{"key":"eblake@redhat.com","avatar":"https://avatars.githubusercontent.com/u/32933908?v=4"},"body":"[redirecting to bug-gnulib as the owner of the git-version-gen script in\nquestion; replies can drop other lists]\n\nOn 01/02/2011 05:16 PM, Bruce Korb wrote:\n> Hi Jonathan,\n> \n> On Sun, Jan 2, 2011 at 10:34 AM, Jonathan Nieder <jrnieder@gmail.com> wrote:\n>> Were you been able to reproduce that outside the script?\n> \n> No, I was blind to the invocation.  You found it.  I was looking\n> without seeing.  Thank you.\n> \n> Given that shells without functions can be considered sufficiently\n> obsolete to not be a consideration, perhaps a better solution is\n> to put the I-don't-care-about-error-messages code into a separate\n> function with stderr redirected.  Doing that turned out messier\n> than I had hoped....\n\nJonathan's patch:\n\n> diff --git a/build-aux/git-version-gen b/build-aux/git-version-gen\n> index 5617eb8..119d7aa 100755\n> --- a/build-aux/git-version-gen\n> +++ b/build-aux/git-version-gen\n> @@ -119,7 +119,7 @@ then\n>  \t    # result is the same as if we were using the newer version\n>  \t    # of git describe.\n>  \t    vtag=`echo \"$v\" | sed 's/-.*//'`\n> -\t    numcommits=`git rev-list \"$vtag\"..HEAD | wc -l`\n> +\t    numcommits=`git rev-list \"$vtag\"..HEAD 2>/dev/null | wc -l`\n>  \t    v=`echo \"$v\" | sed \"s/\\(.*\\)-\\(.*\\)/\\1-$numcommits-\\2/\"`;\n>  \t    ;;\n>      esac\n\nmakes sense to suppress the error message from leaking (whether or not\ngit can be improved to have the error message claim which program is\nissuing the message); but there's still the nagging issue that because\ngit output is fed to a pipe, there's no way to check $? to see if git\nfailed, in order to properly react to that situation.\n\nBruce's patch mixes refactoring with bug fixing, making it a bit harder\nto read, and introduced a bug in its own right:\n\n> diff --git a/build-aux/git-version-gen b/build-aux/git-version-gen\n> index c278f6a..8a238b0 100755\n> --- a/build-aux/git-version-gen\n> +++ b/build-aux/git-version-gen\n> @@ -1,6 +1,6 @@\n>  #!/bin/sh\n>  # Print a version string.\n> -scriptversion=2010-10-13.20; # UTC\n> +scriptversion=2011-01-03.00; # UTC\n>  \n>  # Copyright (C) 2007-2011 Free Software Foundation, Inc.\n>  #\n> @@ -78,76 +78,96 @@ tag_sed_script=\"${2:-s/x/x/}\"\n>  nl='\n>  '\n>  \n> -# Avoid meddling by environment variable of the same name.\n> -v=\n> +get_ver()\n> +{\n> +    local PS4='>gv> '\n\nPortable scripts CANNOT use local (since POSIX does not require it), and\nsetting PS4 is not commonly done in portable scripting.\n\nI'll probably end up writing yet a third approach, which collects git\nrev-list output into a temporary variable in order to correctly detect\nfailures, without refactoring into a helper function.\n\n-- \nEric Blake   eblake@redhat.com    +1-801-349-2682\nLibvirt virtualization library http://libvirt.org\n\n"}]}