From: Ben Peart <peartben@gmail.com>
To: git@vger.kernel.org
Cc: benpeart@microsoft.com, christian.couder@gmail.com,
larsxschneider@gmail.com
Subject: [PATCH v1 3/3] convert: use new sub-process module for filter processes
Date: Wed, 22 Mar 2017 12:52:20 -0400 [thread overview]
Message-ID: <20170322165220.5660-4-benpeart@microsoft.com> (raw)
In-Reply-To: <20170322165220.5660-1-benpeart@microsoft.com>
The filter.<driver>.process code was refactored into a separate
sub-process module to facilitate reuse in future patch series. This
centralizes the logic in a single place to simplify maintenance. As a
side benefit, it also improves the readability of convert.c.
Signed-off-by: Ben Peart <benpeart@microsoft.com>
---
convert.c | 154 ++++++++++----------------------------------------------------
1 file changed, 24 insertions(+), 130 deletions(-)
diff --git a/convert.c b/convert.c
index 8d652bf27c..ca8e0d26c0 100644
--- a/convert.c
+++ b/convert.c
@@ -4,6 +4,7 @@
#include "quote.h"
#include "sigchain.h"
#include "pkt-line.h"
+#include "sub-process.h"
/*
* convert.c - convert a file when checking it out and checking it in.
@@ -497,132 +498,30 @@ static int apply_single_file_filter(const char *path, const char *src, size_t le
#define CAP_SMUDGE (1u<<1)
struct cmd2process {
- struct hashmap_entry ent; /* must be the first member! */
+ struct subprocess_entry subprocess;
unsigned int supported_capabilities;
- const char *cmd;
- struct child_process process;
};
-static int cmd_process_map_initialized;
-static struct hashmap cmd_process_map;
-
-static int cmd2process_cmp(const struct cmd2process *e1,
- const struct cmd2process *e2,
- const void *unused)
-{
- return strcmp(e1->cmd, e2->cmd);
-}
-
-static struct cmd2process *find_multi_file_filter_entry(struct hashmap *hashmap, const char *cmd)
-{
- struct cmd2process key;
- hashmap_entry_init(&key, strhash(cmd));
- key.cmd = cmd;
- return hashmap_get(hashmap, &key, NULL);
-}
-
-static int packet_write_list(int fd, const char *line, ...)
+static int start_multi_file_filter_fn(struct subprocess_entry *subprocess)
{
- va_list args;
int err;
- va_start(args, line);
- for (;;) {
- if (!line)
- break;
- if (strlen(line) > LARGE_PACKET_DATA_MAX)
- return -1;
- err = packet_write_fmt_gently(fd, "%s\n", line);
- if (err)
- return err;
- line = va_arg(args, const char*);
- }
- va_end(args);
- return packet_flush_gently(fd);
-}
-
-static void read_multi_file_filter_status(int fd, struct strbuf *status)
-{
- struct strbuf **pair;
- char *line;
- for (;;) {
- line = packet_read_line(fd, NULL);
- if (!line)
- break;
- pair = strbuf_split_str(line, '=', 2);
- if (pair[0] && pair[0]->len && pair[1]) {
- /* the last "status=<foo>" line wins */
- if (!strcmp(pair[0]->buf, "status=")) {
- strbuf_reset(status);
- strbuf_addbuf(status, pair[1]);
- }
- }
- strbuf_list_free(pair);
- }
-}
-
-static void kill_multi_file_filter(struct hashmap *hashmap, struct cmd2process *entry)
-{
- if (!entry)
- return;
-
- entry->process.clean_on_exit = 0;
- kill(entry->process.pid, SIGTERM);
- finish_command(&entry->process);
-
- hashmap_remove(hashmap, entry, NULL);
- free(entry);
-}
-
-static void stop_multi_file_filter(struct child_process *process)
-{
- sigchain_push(SIGPIPE, SIG_IGN);
- /* Closing the pipe signals the filter to initiate a shutdown. */
- close(process->in);
- close(process->out);
- sigchain_pop(SIGPIPE);
- /* Finish command will wait until the shutdown is complete. */
- finish_command(process);
-}
-
-static struct cmd2process *start_multi_file_filter(struct hashmap *hashmap, const char *cmd)
-{
- int err;
- struct cmd2process *entry;
+ struct cmd2process *entry = (struct cmd2process *)subprocess;
struct child_process *process;
- const char *argv[] = { cmd, NULL };
struct string_list cap_list = STRING_LIST_INIT_NODUP;
char *cap_buf;
const char *cap_name;
- entry = xmalloc(sizeof(*entry));
- entry->cmd = cmd;
- entry->supported_capabilities = 0;
- process = &entry->process;
-
- child_process_init(process);
- process->argv = argv;
- process->use_shell = 1;
- process->in = -1;
- process->out = -1;
- process->clean_on_exit = 1;
- process->clean_on_exit_handler = stop_multi_file_filter;
-
- if (start_command(process)) {
- error("cannot fork to run external filter '%s'", cmd);
- return NULL;
- }
-
- hashmap_entry_init(entry, strhash(cmd));
+ process = subprocess_get_child_process(&entry->subprocess);
sigchain_push(SIGPIPE, SIG_IGN);
- err = packet_write_list(process->in, "git-filter-client", "version=2", NULL);
+ err = packet_write_list_gently(process->in, "git-filter-client", "version=2", NULL);
if (err)
goto done;
err = strcmp(packet_read_line(process->out, NULL), "git-filter-server");
if (err) {
- error("external filter '%s' does not support filter protocol version 2", cmd);
+ error("external filter '%s' does not support filter protocol version 2", subprocess->cmd);
goto done;
}
err = strcmp(packet_read_line(process->out, NULL), "version=2");
@@ -632,7 +531,7 @@ static struct cmd2process *start_multi_file_filter(struct hashmap *hashmap, cons
if (err)
goto done;
- err = packet_write_list(process->in, "capability=clean", "capability=smudge", NULL);
+ err = packet_write_list_gently(process->in, "capability=clean", "capability=smudge", NULL);
for (;;) {
cap_buf = packet_read_line(process->out, NULL);
@@ -651,7 +550,7 @@ static struct cmd2process *start_multi_file_filter(struct hashmap *hashmap, cons
} else {
warning(
"external filter '%s' requested unsupported filter capability '%s'",
- cmd, cap_name
+ subprocess->cmd, cap_name
);
}
@@ -661,14 +560,10 @@ static struct cmd2process *start_multi_file_filter(struct hashmap *hashmap, cons
done:
sigchain_pop(SIGPIPE);
- if (err || errno == EPIPE) {
- error("initialization for external filter '%s' failed", cmd);
- kill_multi_file_filter(hashmap, entry);
- return NULL;
- }
+ if (err || errno == EPIPE)
+ err = err ? err : errno;
- hashmap_add(hashmap, entry);
- return entry;
+ return err;
}
static int apply_multi_file_filter(const char *path, const char *src, size_t len,
@@ -682,22 +577,20 @@ static int apply_multi_file_filter(const char *path, const char *src, size_t len
struct strbuf filter_status = STRBUF_INIT;
const char *filter_type;
- if (!cmd_process_map_initialized) {
- cmd_process_map_initialized = 1;
- hashmap_init(&cmd_process_map, (hashmap_cmp_fn) cmd2process_cmp, 0);
- entry = NULL;
- } else {
- entry = find_multi_file_filter_entry(&cmd_process_map, cmd);
- }
+ entry = (struct cmd2process *)subprocess_find_entry(cmd);
fflush(NULL);
if (!entry) {
- entry = start_multi_file_filter(&cmd_process_map, cmd);
- if (!entry)
+ entry = xmalloc(sizeof(*entry));
+ entry->supported_capabilities = 0;
+
+ if (subprocess_start(&entry->subprocess, cmd, start_multi_file_filter_fn)) {
+ free(entry);
return 0;
+ }
}
- process = &entry->process;
+ process = subprocess_get_child_process(&entry->subprocess);
if (!(wanted_capability & entry->supported_capabilities))
return 0;
@@ -737,7 +630,7 @@ static int apply_multi_file_filter(const char *path, const char *src, size_t len
if (err)
goto done;
- read_multi_file_filter_status(process->out, &filter_status);
+ subprocess_read_status(process->out, &filter_status);
err = strcmp(filter_status.buf, "success");
if (err)
goto done;
@@ -746,7 +639,7 @@ static int apply_multi_file_filter(const char *path, const char *src, size_t len
if (err)
goto done;
- read_multi_file_filter_status(process->out, &filter_status);
+ subprocess_read_status(process->out, &filter_status);
err = strcmp(filter_status.buf, "success");
done:
@@ -768,7 +661,8 @@ static int apply_multi_file_filter(const char *path, const char *src, size_t len
* Force shutdown and restart if another blob requires filtering.
*/
error("external filter '%s' failed", cmd);
- kill_multi_file_filter(&cmd_process_map, entry);
+ subprocess_stop((struct subprocess_entry *)entry);
+ free(entry);
}
} else {
strbuf_swap(dst, &nbuf);
--
2.12.0.gvfs.1.42.g0b7328eac2
next prev parent reply other threads:[~2017-03-22 17:02 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-03-22 16:52 [PATCH v1 0/3] Add support for downloading blobs on demand Ben Peart
2017-03-22 16:52 ` [PATCH v1 1/3] pkt-line: add packet_write_list_gently() Ben Peart
2017-03-22 20:21 ` Junio C Hamano
2017-03-24 12:34 ` Ben Peart
2017-03-22 16:52 ` [PATCH v1 2/3] sub-process: refactor the filter process code into a reusable module Ben Peart
2017-03-23 6:16 ` Junio C Hamano
2017-03-24 12:39 ` Ben Peart
2017-03-24 16:10 ` Junio C Hamano
2017-03-24 17:15 ` Junio C Hamano
2017-03-27 22:04 ` Ben Peart
2017-03-22 16:52 ` Ben Peart [this message]
2017-03-25 11:59 ` [PATCH v1 0/3] Add support for downloading blobs on demand Duy Nguyen
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: http://vger.kernel.org/majordomo-info.html
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20170322165220.5660-4-benpeart@microsoft.com \
--to=peartben@gmail.com \
--cc=benpeart@microsoft.com \
--cc=christian.couder@gmail.com \
--cc=git@vger.kernel.org \
--cc=larsxschneider@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).