From: "Marco Costalba" <mcostalba@gmail.com>
To: "Junio C Hamano" <gitster@pobox.com>
Cc: "Git Mailing List" <git@vger.kernel.org>
Subject: Re: [PATCH] git stash: one bug and one feature request
Date: Sat, 5 Jan 2008 09:25:59 +0100 [thread overview]
Message-ID: <e5bfff550801050025g6758bfb6p751e69e93d4299be@mail.gmail.com> (raw)
In-Reply-To: <7vy7b5glmr.fsf@gitster.siamese.dyndns.org>
On Jan 5, 2008 1:29 AM, Junio C Hamano <gitster@pobox.com> wrote:
> "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.
> 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
next prev parent reply other threads:[~2008-01-05 8:26 UTC|newest]
Thread overview: 19+ messages / expand[flat|nested] mbox.gz Atom feed top
2008-01-04 16:14 [PATCH] git stash: one bug and one feature request Marco Costalba
2008-01-04 16:36 ` Brandon Casey
2008-01-04 17:30 ` Pascal Obry
2008-01-04 17:51 ` Jakub Narebski
2008-01-04 19:36 ` Brian Swetland
2008-01-04 21:04 ` Jeff King
[not found] ` <e5bfff550801040944p7f8e722asfa726b34a4a712fa@mail.gmail.com>
2008-01-04 18:00 ` Brandon Casey
2008-01-04 18:05 ` Marco Costalba
2008-01-04 18:34 ` Brandon Casey
2008-01-04 18:46 ` Marco Costalba
2008-01-05 0:31 ` Junio C Hamano
2008-01-05 0:29 ` Junio C Hamano
2008-01-05 8:25 ` Marco Costalba [this message]
2008-01-05 8:36 ` Junio C Hamano
2008-01-05 8:57 ` Marco Costalba
2008-01-05 9:06 ` Junio C Hamano
2008-01-05 6:41 ` Wayne Davison
2008-01-05 6:52 ` Junio C Hamano
2008-01-05 8:31 ` Marco Costalba
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=e5bfff550801050025g6758bfb6p751e69e93d4299be@mail.gmail.com \
--to=mcostalba@gmail.com \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).