Thread (1 message) 1 message, 1 author, 2016-06-15

Re: [PATCH] Don't ignore write failure from git-diff, git-log, etc.

From: Junio C Hamano <hidden>
Date: 2016-06-15 22:43:13

Jim Meyering [off-list ref] writes:
Subject: [PATCH] Don't ignore write failure from git-diff, git-log, etc.
...
You can demonstrate this with git's own --version output, too:
(but git --help detects the failure without this patch)

    $ ./git --version > /dev/full
    fatal: write failure on standard output: No space left on device
    [Exit 128]

Note that the fcntl test (for whether the fileno may be closed) is
required in order to avoid EBADF upon closing an already-closed stdout,
as would happen for each git command that already closes stdout; I think
update-index was the one I noticed in the failure of t5400, before I
added that test.

Also, to be consistent, don't ignore EPIPE write failures.

Signed-off-by: Jim Meyering <redacted>
I do not think anybody has much objection about the change to
handle_internal_command() in git.c you made.  Earlier we relied
on exit(3) to close still open filehandles (while ignoring
errors), and you made the close explicit in order to detect
errors.

But "to be consistent" above is not a very good justification.

In the presense of shells that give "annoying" behaviour (we
have to remember that not everybody have enough privilege,
expertise, or motivation to update /bin/sh or install their own
copy in $HOME/bin/sh), "EPIPE is different from other errors" is
a more practical point of view, I'd have to say.  IOW, it is not
clear if it is a good thing "to be consistent" to begin with.

I would have appreciated if this were two separate patches.  I
think the EPIPE one is an independent issue.  We could even make
it a configuration option to ignore EPIPE ;-)
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help