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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Jan 5, 2008 1:29 AM, Junio C Hamano <gitster@xxxxxxxxx> wrote:
> "Marco Costalba" <mcostalba@xxxxxxxxx> 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.

> 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
-
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux