Re: [PATCH 3/3] Add sideband status report to git-archive protocol

2006/9/10, Junio C Hamano <junkio@cox.net>:
> 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.

>
>  builtin-archive.c        |   24 ++++++++++--
>  builtin-upload-archive.c |   92 +++++++++++++++++++++++++++++++++++++++++++++-
>  2 files changed, 108 insertions(+), 8 deletions(-)
>

[snip]

> +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
-
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html