Re: [PATCH 3/3] Add sideband status report to git-archive protocol
- From
- Franck Bui-Huu <vagabon.xyz@gmail.com>
- Date
- Sep 10, 2006, 19:15 UTC
- Message-ID
- <cda58cb80609101215h36b0f7d7m990ed3011cfda9d2@mail.gmail.com>
- In-Reply-To
- <7v1wqkt2v4.fsf_-_@assigned-by-dhcp.cox.net>
2006/9/10, Junio C Hamano <junkio@cox.net>:
Show 14 quoted lines
> Using the refactored sideband code from existing upload-pack protocol, > this lets the error condition and status output sent from the remote > process to be shown locally. > > Signed-off-by: Junio C Hamano <junkio@cox.net> > --- > > * This does it in a "stupid" way. Ideally, write(2) to fd 1 > and 2 in each archiver backend could be wrapped with > something like upload-pack.c::send_client_data() that does > straight write(2) to the original destination when it is not > driven by upload-pack, or use send_sideband() when it is, and > that way we can lose two pipes and the multiplexer process. >
Well I started that part the same way you do because I liked the fact that archiver backend implementation needn't to know how to send out data. It's totaly transparent for it. It just need to write its payload on stdout.
Show 5 quoted lines
> > builtin-archive.c | 24 ++++++++++-- > builtin-upload-archive.c | 92 +++++++++++++++++++++++++++++++++++++++++++++- > 2 files changed, 108 insertions(+), 8 deletions(-) >
[snip]
Show 85 quoted lines
> +int cmd_upload_archive(int argc, const char **argv, const char *prefix)
> +{
> + pid_t writer;
> + int fd1[2], fd2[2];
> + /*
> + * Set up sideband subprocess.
> + *
> + * We (parent) monitor and read from child, sending its fd#1 and fd#2
> + * multiplexed out to our fd#1. If the child dies, we tell the other
> + * end over channel #3.
> + */
> + if (pipe(fd1) < 0 || pipe(fd2) < 0) {
> + int err = errno;
> + packet_write(1, "NACK pipe failed on the remote side\n");
> + die("upload-archive: %s", strerror(err));
> + }
> + writer = fork();
> + if (writer < 0) {
> + int err = errno;
> + packet_write(1, "NACK fork failed on the remote side\n");
> + die("upload-archive: %s", strerror(err));
> + }
> + if (!writer) {
> + /* child - connect fd#1 and fd#2 to the pipe */
> + dup2(fd1[1], 1);
> + dup2(fd2[1], 2);
> + close(fd1[1]); close(fd2[1]);
> + close(fd1[0]); close(fd2[0]); /* we do not read from pipe */
> +
> + exit(run_upload_archive(argc, argv, prefix));
> + }
> +
> + /* parent - read from child, multiplex and send out to fd#1 */
> + close(fd1[1]); close(fd2[1]); /* we do not write to pipe */
> packet_write(1, "ACK\n");
> packet_flush(1);
>
> - return ar.write_archive(&ar.args);
> -}
> + while (1) {
> + struct pollfd pfd[2];
> + char buf[16384];
> + ssize_t sz;
> + pid_t pid;
> + int status;
> +
> + pfd[0].fd = fd1[0];
> + pfd[0].events = POLLIN;
> + pfd[1].fd = fd2[0];
> + pfd[1].events = POLLIN;
> + if (poll(pfd, 2, -1) < 0) {
> + if (errno != EINTR) {
> + error("poll failed resuming: %s",
> + strerror(errno));
> + sleep(1);
> + }
> + continue;
> + }
> + if (pfd[0].revents & (POLLIN|POLLHUP)) {
> + /* Data stream ready */
> + sz = read(pfd[0].fd, buf, sizeof(buf));
> + send_sideband(1, 1, buf, sz, DEFAULT_PACKET_MAX);
> + }
> + if (pfd[1].revents & (POLLIN|POLLHUP)) {
> + /* Status stream ready */
> + sz = read(pfd[1].fd, buf, sizeof(buf));
> + send_sideband(1, 2, buf, sz, DEFAULT_PACKET_MAX);
> + }
>
> + if (((pfd[0].revents | pfd[1].revents) & POLLHUP) == 0)
> + continue;
> + /* did it die? */
> + pid = waitpid(writer, &status, WNOHANG);
> + if (!pid) {
> + fprintf(stderr, "Hmph, HUP?\n");
> + continue;
> + }
> + if (!WIFEXITED(status) || WEXITSTATUS(status) > 0)
> + send_sideband(1, 3, deadchild, strlen(deadchild),
> + DEFAULT_PACKET_MAX);
> + packet_flush(1);
> + break;
> + }
> + return 0;
> +}Why all of this is part of upload-archive ? Shouldn't we put that code in daemon.c ? We could use a new service flag to ask daemon.c to start the service with the sideband multiplexer process already setup and plugged with the sercive is going to start ? Hence others future services could use it.
--
Franck