From 96335bcf4d64c29add3692fb41671190123cf44e Mon Sep 17 00:00:00 2001 From: Jeff King Date: Fri, 17 Jun 2016 19:38:47 -0400 Subject: run-command: add pipe_command helper We already have capture_command(), which captures the stdout of a command in a way that avoids deadlocks. But sometimes we need to do more I/O, like capturing stderr as well, or sending data to stdin. It's easy to write code that deadlocks racily in these situations depending on how fast the command reads its input, or in which order it writes its output. Let's give callers an easy interface for doing this the right way, similar to what capture_command() did for the simple case. The whole thing is backed by a generic poll() loop that can feed an arbitrary number of buffers to descriptors, and fill an arbitrary number of strbufs from other descriptors. This seems like overkill, but the resulting code is actually a bit cleaner than just handling the three descriptors (because the output code for stdout/stderr is effectively duplicated, so being able to loop is a benefit). Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- run-command.c | 152 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 147 insertions(+), 5 deletions(-) (limited to 'run-command.c') diff --git a/run-command.c b/run-command.c index af0c8a10df..33bc63a1de 100644 --- a/run-command.c +++ b/run-command.c @@ -864,19 +864,161 @@ int run_hook_le(const char *const *env, const char *name, ...) return ret; } -int capture_command(struct child_process *cmd, struct strbuf *buf, size_t hint) +struct io_pump { + /* initialized by caller */ + int fd; + int type; /* POLLOUT or POLLIN */ + union { + struct { + const char *buf; + size_t len; + } out; + struct { + struct strbuf *buf; + size_t hint; + } in; + } u; + + /* returned by pump_io */ + int error; /* 0 for success, otherwise errno */ + + /* internal use */ + struct pollfd *pfd; +}; + +static int pump_io_round(struct io_pump *slots, int nr, struct pollfd *pfd) +{ + int pollsize = 0; + int i; + + for (i = 0; i < nr; i++) { + struct io_pump *io = &slots[i]; + if (io->fd < 0) + continue; + pfd[pollsize].fd = io->fd; + pfd[pollsize].events = io->type; + io->pfd = &pfd[pollsize++]; + } + + if (!pollsize) + return 0; + + if (poll(pfd, pollsize, -1) < 0) { + if (errno == EINTR) + return 1; + die_errno("poll failed"); + } + + for (i = 0; i < nr; i++) { + struct io_pump *io = &slots[i]; + + if (io->fd < 0) + continue; + + if (!(io->pfd->revents & (POLLOUT|POLLIN|POLLHUP|POLLERR|POLLNVAL))) + continue; + + if (io->type == POLLOUT) { + ssize_t len = xwrite(io->fd, + io->u.out.buf, io->u.out.len); + if (len < 0) { + io->error = errno; + close(io->fd); + io->fd = -1; + } else { + io->u.out.buf += len; + io->u.out.len -= len; + if (!io->u.out.len) { + close(io->fd); + io->fd = -1; + } + } + } + + if (io->type == POLLIN) { + ssize_t len = strbuf_read_once(io->u.in.buf, + io->fd, io->u.in.hint); + if (len < 0) + io->error = errno; + if (len <= 0) { + close(io->fd); + io->fd = -1; + } + } + } + + return 1; +} + +static int pump_io(struct io_pump *slots, int nr) +{ + struct pollfd *pfd; + int i; + + for (i = 0; i < nr; i++) + slots[i].error = 0; + + ALLOC_ARRAY(pfd, nr); + while (pump_io_round(slots, nr, pfd)) + ; /* nothing */ + free(pfd); + + /* There may be multiple errno values, so just pick the first. */ + for (i = 0; i < nr; i++) { + if (slots[i].error) { + errno = slots[i].error; + return -1; + } + } + return 0; +} + + +int pipe_command(struct child_process *cmd, + const char *in, size_t in_len, + struct strbuf *out, size_t out_hint, + struct strbuf *err, size_t err_hint) { - cmd->out = -1; + struct io_pump io[3]; + int nr = 0; + + if (in) + cmd->in = -1; + if (out) + cmd->out = -1; + if (err) + cmd->err = -1; + if (start_command(cmd) < 0) return -1; - if (strbuf_read(buf, cmd->out, hint) < 0) { - close(cmd->out); + if (in) { + io[nr].fd = cmd->in; + io[nr].type = POLLOUT; + io[nr].u.out.buf = in; + io[nr].u.out.len = in_len; + nr++; + } + if (out) { + io[nr].fd = cmd->out; + io[nr].type = POLLIN; + io[nr].u.in.buf = out; + io[nr].u.in.hint = out_hint; + nr++; + } + if (err) { + io[nr].fd = cmd->err; + io[nr].type = POLLIN; + io[nr].u.in.buf = err; + io[nr].u.in.hint = err_hint; + nr++; + } + + if (pump_io(io, nr) < 0) { finish_command(cmd); /* throw away exit code */ return -1; } - close(cmd->out); return finish_command(cmd); } -- cgit v1.3 From 9445b4921e3e996b2d38d58c594f95d63a72dcea Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Tue, 16 Aug 2016 15:14:27 +0200 Subject: rev-parse: respect core.hooksPath in --git-path The idea of the --git-path option is not only to avoid having to prefix paths with the output of --git-dir all the time, but also to respect overrides for specific common paths inside the .git directory (e.g. `git rev-parse --git-path objects` will report the value of the environment variable GIT_OBJECT_DIRECTORY, if set). When introducing the core.hooksPath setting, we forgot to adjust git_path() accordingly. This patch fixes that. While at it, revert the special-casing of core.hooksPath in run-command.c, as it is now no longer needed. Signed-off-by: Johannes Schindelin Signed-off-by: Junio C Hamano --- path.c | 2 ++ run-command.c | 5 +---- t/t1350-config-hooks-path.sh | 6 ++++++ 3 files changed, 9 insertions(+), 4 deletions(-) (limited to 'run-command.c') diff --git a/path.c b/path.c index 969b494d72..ad81856d53 100644 --- a/path.c +++ b/path.c @@ -379,6 +379,8 @@ static void adjust_git_path(struct strbuf *buf, int git_dir_len) get_index_file(), strlen(get_index_file())); else if (git_db_env && dir_prefix(base, "objects")) replace_dir(buf, git_dir_len + 7, get_object_directory()); + else if (git_hooks_path && dir_prefix(base, "hooks")) + replace_dir(buf, git_dir_len + 5, git_hooks_path); else if (git_common_dir_env) update_common_dir(buf, git_dir_len, NULL); } diff --git a/run-command.c b/run-command.c index aa85cd5c3c..c72601056c 100644 --- a/run-command.c +++ b/run-command.c @@ -815,10 +815,7 @@ const char *find_hook(const char *name) static struct strbuf path = STRBUF_INIT; strbuf_reset(&path); - if (git_hooks_path) - strbuf_addf(&path, "%s/%s", git_hooks_path, name); - else - strbuf_git_path(&path, "hooks/%s", name); + strbuf_git_path(&path, "hooks/%s", name); if (access(path.buf, X_OK) < 0) return NULL; return path.buf; diff --git a/t/t1350-config-hooks-path.sh b/t/t1350-config-hooks-path.sh index 5e3fb3a6af..f1f9aee9f5 100755 --- a/t/t1350-config-hooks-path.sh +++ b/t/t1350-config-hooks-path.sh @@ -34,4 +34,10 @@ test_expect_success 'Check that various forms of specifying core.hooksPath work' test_cmp expect actual ' +test_expect_success 'git rev-parse --git-path hooks' ' + git config core.hooksPath .git/custom-hooks && + git rev-parse --git-path hooks/abc >actual && + test .git/custom-hooks/abc = "$(cat actual)" +' + test_done -- cgit v1.3