[PATCH] Make GIT-VERSION-GEN tolerate missing git describe command

[PATCH] Make GIT-VERSION-GEN tolerate missing git describe command

[John Ellson]

I think it is probably a bug that "git non_existent_command"
returns its error message to stdout without an error, where
"git-non_existent_command" behaves differently and does return an
error.

Older versions of git did not implement "git describe"  and
GIT-VERSION-GEN produces an empty version string if run on
a system with such a git installed.  The consequence
is that "make rpm" fails.

This patch fixes GIT-VERSION-GEN so that it works in the
absence of a working "git describe"

----------------------------------------

diff --git a/GIT-VERSION-GEN b/GIT-VERSION-GEN
index 196402c..845b9dc 100755
--- a/GIT-VERSION-GEN
+++ b/GIT-VERSION-GEN
@@ -2,7 +2,7 @@

  GVF=GIT-VERSION-FILE

-VN=$(git describe --abbrev=4 HEAD 2>/dev/null) || VN=v1.0.GIT
+VN=$(git-describe --abbrev=4 HEAD 2>/dev/null) || VN=v1.0.GIT
  VN=$(expr "$VN" : v'\(.*\)')
  if test -r $GVF
  then

----------------------------------------


Signed-off-by: John Ellson <ellson@research.att.com>

Re: [PATCH] Make GIT-VERSION-GEN tolerate missing git describe command

[Linus Torvalds]

On Fri, 30 Dec 2005, John Ellson wrote:
>
> I think it is probably a bug that "git non_existent_command"
> returns its error message to stdout without an error, where
> "git-non_existent_command" behaves differently and does return an
> error.
> 
> Older versions of git did not implement "git describe"  and
> GIT-VERSION-GEN produces an empty version string if run on
> a system with such a git installed.  The consequence
> is that "make rpm" fails.
> 
> This patch fixes GIT-VERSION-GEN so that it works in the
> absence of a working "git describe"

Shouldn't you make "git.c" return an error too, so that "git-describe" and 
"git describe" both fail properly?

I realize that you'd want to do your patch _too_ (in case somebody has an 
old version of "git" installed), but I just think it would be sensible to 
fix the problem that causes this in the first place..

Continuing to output to stdout rather than stderr is probably a good idea 
(so that it's easy to do "git help | less" or something), but yeah, I 
think an unrecognized command should at least return an error.

		Linus

Re: [PATCH] Make GIT-VERSION-GEN tolerate missing git describe command

[John Ellson]

Linus Torvalds wrote:
> On Fri, 30 Dec 2005, John Ellson wrote:
>   
>> I think it is probably a bug that "git non_existent_command"
>> returns its error message to stdout without an error, where
>> "git-non_existent_command" behaves differently and does return an
>> error.
>>
>> Older versions of git did not implement "git describe"  and
>> GIT-VERSION-GEN produces an empty version string if run on
>> a system with such a git installed.  The consequence
>> is that "make rpm" fails.
>>
>> This patch fixes GIT-VERSION-GEN so that it works in the
>> absence of a working "git describe"
>>     
>
> Shouldn't you make "git.c" return an error too, so that "git-describe" and 
> "git describe" both fail properly?
>
> I realize that you'd want to do your patch _too_ (in case somebody has an 
> old version of "git" installed), but I just think it would be sensible to 
> fix the problem that causes this in the first place..
>
> Continuing to output to stdout rather than stderr is probably a good idea 
> (so that it's easy to do "git help | less" or something), but yeah, I 
> think an unrecognized command should at least return an error.
>
> 		Linus
>   

I checked and that bug has been fixed since the older version of git that
was causing me problems. "git non_existent_command" now returns 1.
However, the error message was also changed to
goto stderr, which it sounds like you disagree with?   Personally
I don't have a problem with it, although a real "git-help" command
might be a good idea too.

John

Re: [PATCH] Make GIT-VERSION-GEN tolerate missing git describe command

[Linus Torvalds]

On Fri, 30 Dec 2005, John Ellson wrote:
> 
> I checked and that bug has been fixed since the older version of git that
> was causing me problems. "git non_existent_command" now returns 1.

Ahh, good.

> However, the error message was also changed to goto stderr, which it 
> sounds like you disagree with?

Not that I can see:

	[torvalds@g5 ~]$ git --version
	git version 1.0.6-g58e3
	[torvalds@g5 ~]$ git hjsdhjas > /dev/null 
	[torvalds@g5 ~]$ echo $?
	1

so yes, it returns a proper error, and it outputs the help message to 
stdout, not stderr. Which is fine, because it means that you can indeed do

	git help | less

and it will do the right thing (apart from printing the line

	git: 'help' is not a git-command

which is a bit sad).

		Linus

Re: [PATCH] Make GIT-VERSION-GEN tolerate missing git describe command

[John Ellson]

John Ellson wrote:
> I think it is probably a bug that "git non_existent_command"
> returns its error message to stdout without an error, where
> "git-non_existent_command" behaves differently and does return an
> error.

BTW.  Its the old shell-script version of "git" that fails to return an error on 
non_existent_commands.    The newer C version of "git" correctly returns an 
error code.

The reason that this is a sufficiently serious problem to require a fix is that 
the broken version of "git" is in the git-core-0.99.9a-2.fc5.i386.rpm that is 
currently in Fedora Core Extras development.

John

Re: [PATCH] Make GIT-VERSION-GEN tolerate missing git describe command

[H. Peter Anvin]

John Ellson wrote:
> John Ellson wrote:
> 
>> I think it is probably a bug that "git non_existent_command"
>> returns its error message to stdout without an error, where
>> "git-non_existent_command" behaves differently and does return an
>> error.
> 
> 
> BTW.  Its the old shell-script version of "git" that fails to return an 
> error on non_existent_commands.    The newer C version of "git" 
> correctly returns an error code.
> 
> The reason that this is a sufficiently serious problem to require a fix 
> is that the broken version of "git" is in the 
> git-core-0.99.9a-2.fc5.i386.rpm that is currently in Fedora Core Extras 
> development.
> 

It has been fixed.  What's broken is that Fedora Extras still contains 
0.99.9a which is totally ancient.

	-hpa