Re: [PATCH] push: Use sideband channel for hook messages

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

 



Shawn O. Pearce schrieb:
> Rather than sending hook messages over stderr, and losing them
> entirely on git:// and smart HTTP transports,

I don't think "losing them entirely" is true for the git:// protocol:
git-daemon writes receive-pack's stderr to the syslog.

The question is whether hook errors are intended for the remote side or
for the repository owner. Generally, I'd say for the latter. But since
your patch is about pushing, a repository owner must already trust the
remote side, and then it can be argued that in this case errors can be
sent to the remote.

> diff --git a/run-command.c b/run-command.c
> index cf2d8f7..7d1fd88 100644
> --- a/run-command.c
> +++ b/run-command.c
> @@ -94,6 +94,9 @@ fail_pipe:
>  		else if (need_err) {
>  			dup2(fderr[1], 2);
>  			close_pair(fderr);
> +		} else if (cmd->err > 1) {
> +			dup2(cmd->err, 2);
> +			close(cmd->err);
>  		}
>  
>  		if (cmd->no_stdout)
> @@ -228,6 +231,8 @@ fail_pipe:
>  
>  	if (need_err)
>  		close(fderr[1]);
> +	else if (cmd->err)
> +		close(cmd->err);
>  
>  	return 0;
>  }

This requires similar adjustments in the Windows part.

Documentation/technical/api-runcommand.txt should be an update, too.

> @@ -326,10 +331,19 @@ static unsigned __stdcall run_thread(void *data)
>  int start_async(struct async *async)
>  {
>  	int pipe_out[2];
> +	int proc_fd, call_fd;
>  
>  	if (pipe(pipe_out) < 0)
>  		return error("cannot create pipe: %s", strerror(errno));
> -	async->out = pipe_out[0];
> +
> +	if (async->is_reader) {
> +		proc_fd = pipe_out[0];
> +		call_fd = pipe_out[1];
> +	} else {
> +		call_fd = pipe_out[0];
> +		proc_fd = pipe_out[1];
> +	}
> +	async->out = call_fd;

I don't particularly like this approach because it restricts the async
procedures to a one-way communication.

What would you think about passing both channels to the async callback,
and the communicating parties must agree on which channel they communicate
by closing the unused one? It would require slight changes to all current
async users, though. (It also requires in the threaded case that we pass
dup()s of the pipe channels.)

-- Hannes

--
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]