Re: [PATCH] Fix race and deadlock when sending pack

Daniel Barkalow <barkalow@iabervon.org> writes:

In order to add reporting mechanism later, I think we need to be able to identify the protocol version in a backward compatible way, something like the "server capabilities hidden behind the NUL" trick we did for fetch-pack/upload-pack protocol. Once that is in place, it does not cause harm even if the current protocol program connects hooks' stdout to send-pack, at least in theory. If we take Paul's patch now, however, it would add more work for us later when we do that protocol change, because we will need to wrap the output from the hook in the pkt-line interface in the new protocol, in order to give that back to the stdout of send-pack. Considering that, I think we may want to drop Paul's patch and declare that hooks stdout does not come back to send-pack.

Honestly speaking, I do not really care where stdout of hooks go as long as that does not cause breakage/deadlocks, and I think your earlier patch on December 7th is serving us well enough; we needed to have told users to do an "exec 1>somewhere" in their hooks before that fix, which was not nice at all (and we even forgot to tell them that). If people want to send the output to a log file, they can do so; if they want e-mails, they can do so; if they want to show the output to the pusher, they can do 1>&2; all inside their hooks. I do "echo nitfol | at now" and love the way that I do not have to worry about how "at" command gives me back execution report via e-mail at all ;-).

I think we agree that /dev/null is one of the sane choices as you did in your earlier fix. Duping stderr would have been another sane choice, but I honestly do not think we care much either way.