Re: [PATCH] git stash: one bug and one feature request

From: Marco Costalba <mcostalba@gmail.com>
Date: Jan 5, 2008, 08:25 UTC
Message-ID: <e5bfff550801050025g6758bfb6p751e69e93d4299be@mail.gmail.com>
In-Reply-To: <7vy7b5glmr.fsf@gitster.siamese.dyndns.org>

On Jan 5, 2008 1:29 AM, Junio C Hamano <gitster@pobox.com> wrote:

Show 11 quoted lines

> "Marco Costalba" <mcostalba@gmail.com> writes:
>
> > Otherwise git-stash is unusable by scripts that check
> > stderr to detect fail/success of launched command.
>
> Sorry, but I happen to disagree with your notion of "having
> something on stderr is an error" to begin with.  I think scripts
> written that way are either simply bogus, or are working around
> a defect in the underlying command it calls (perhaps it does not
> signal error with exit status properly).
>

I understand your point. The problem is that in git there isn't an unique way to test success/fail for any command, as example, regarding checking the exit status:

$ git status; echo $? # On branch master nothing to commit (working directory clean) 1

You get a value different from zero also in case of no error. The checking for stderr I have found is more reliable for the git command/scripts I use.

Show 6 quoted lines

> A command that produces machine parsable output should write
> that out to stdout, and if it needs to emit other informational
> messages meant for human consumption (this includes progress
> bars), that should be sent to stderr so that scripts can get the
> meat of the output without having to filter cruft out.
>

I agree with this, but I fail to see the machine parsable output and human consumption sideband info in case of git-stash that I would say does not foreseen machine parsable output at all, so in this case choice of writing to stderr is less clear to me.

> If the command does not signal an error by exiting with non-zero
> status, that would be a bug indeed and you can fix that instead,
> I think.
>

If we don't want to have general rule for exit status and stderr at least we could add a -q option to git stash, altough I would prefer git stash writing on stdout if is not an error.

Please let me explain again why I need a reliable way to detect success/fail of a command. When a function wants to execute a git command it passes a string with the command + arguments to a low level routine, say run(), that is command agnostic. This run() function adapts and formats the command line according to the OS environment then runs the command, saves the results and check for an error, the result buffer is then passed as is to the caller that has the semantic knowledge of what the command have produced.

This low level run() should know nothing about the semantic of the command or the outputted data, but should detect command failing, because failing reporting framework is unified and is the same for each type of command.

A good and reliable way is to check for stderr, because it happens to be more reliable then exit codes.

Please note that also gitk uses the same approach, indeed from http://ftp.tcl.tk/man/tcl8.5/tutorial/Tcl26.html you can read:

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

The 'exec' treats any output to standard error to be an indication that the external program failed. This is simply a conservative assumption: many programs behave that way and they are sloppy in setting return codes.

Some programs however write to standard error without intending this as an indication of an error. You can guard against this from upsetting your script by using the catch command:

if { [catch { exec ls *.tcl } msg] } {
   puts "Something seems to have gone wrong but we will ignore it"
}

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

Indeed in gitk you find something like

    # Unfortunately git-cherry-pick writes stuff to stderr even when
    # no error occurs, and exec takes that as an indication of error...
    if {[catch {exec sh -c "git cherry-pick -r $rowmenuid 2>&1"} err]} {
	notbusy cherrypick
	error_popup $err
	return
    }

I can also black list not commonly behaving programs, but in case of git-stash a fail to see why to choose a not standard behaviour when not needed.

Thanks Marco