From 42b0a86c0ed90d48c2df510e3b26b9bd007ec0a4 Mon Sep 17 00:00:00 2001 From: Lars Schneider Date: Wed, 28 Jun 2017 23:29:50 +0200 Subject: convert: put the flags field before the flag itself for consistent style Suggested-by: Jeff King Signed-off-by: Lars Schneider Signed-off-by: Junio C Hamano --- convert.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) (limited to 'convert.c') diff --git a/convert.c b/convert.c index f1e168bc30..9907e3b9ba 100644 --- a/convert.c +++ b/convert.c @@ -597,12 +597,12 @@ static int apply_multi_file_filter(const char *path, const char *src, size_t len } process = &entry->subprocess.process; - if (!(wanted_capability & entry->supported_capabilities)) + if (!(entry->supported_capabilities & wanted_capability)) return 0; - if (CAP_CLEAN & wanted_capability) + if (wanted_capability & CAP_CLEAN) filter_type = "clean"; - else if (CAP_SMUDGE & wanted_capability) + else if (wanted_capability & CAP_SMUDGE) filter_type = "smudge"; else die("unexpected filter type"); @@ -703,9 +703,9 @@ static int apply_filter(const char *path, const char *src, size_t len, if (!dst) return 1; - if ((CAP_CLEAN & wanted_capability) && !drv->process && drv->clean) + if ((wanted_capability & CAP_CLEAN) && !drv->process && drv->clean) cmd = drv->clean; - else if ((CAP_SMUDGE & wanted_capability) && !drv->process && drv->smudge) + else if ((wanted_capability & CAP_SMUDGE) && !drv->process && drv->smudge) cmd = drv->smudge; if (cmd && *cmd) -- cgit v1.2.3 From 9364fc298a32b19fbfe545974cbf9e46d5c7e67f Mon Sep 17 00:00:00 2001 From: Lars Schneider Date: Wed, 28 Jun 2017 23:29:51 +0200 Subject: convert: move multiple file filter error handling to separate function Refactoring the filter error handling is useful for the subsequent patch 'convert: add "status=delayed" to filter process protocol'. In addition, replace the parentheses around the empty "if" block with a single semicolon to adhere to the Git style guide. Signed-off-by: Lars Schneider Signed-off-by: Junio C Hamano --- convert.c | 47 ++++++++++++++++++++++++++--------------------- 1 file changed, 26 insertions(+), 21 deletions(-) (limited to 'convert.c') diff --git a/convert.c b/convert.c index 9907e3b9ba..e55c034d86 100644 --- a/convert.c +++ b/convert.c @@ -565,6 +565,29 @@ done: return err; } +static void handle_filter_error(const struct strbuf *filter_status, + struct cmd2process *entry, + const unsigned int wanted_capability) { + if (!strcmp(filter_status->buf, "error")) + ; /* The filter signaled a problem with the file. */ + else if (!strcmp(filter_status->buf, "abort") && wanted_capability) { + /* + * The filter signaled a permanent problem. Don't try to filter + * files with the same command for the lifetime of the current + * Git process. + */ + entry->supported_capabilities &= ~wanted_capability; + } else { + /* + * Something went wrong with the protocol filter. + * Force shutdown and restart if another blob requires filtering. + */ + error("external filter '%s' failed", entry->subprocess.cmd); + subprocess_stop(&subprocess_map, &entry->subprocess); + free(entry); + } +} + static int apply_multi_file_filter(const char *path, const char *src, size_t len, int fd, struct strbuf *dst, const char *cmd, const unsigned int wanted_capability) @@ -656,28 +679,10 @@ static int apply_multi_file_filter(const char *path, const char *src, size_t len done: sigchain_pop(SIGPIPE); - if (err) { - if (!strcmp(filter_status.buf, "error")) { - /* The filter signaled a problem with the file. */ - } else if (!strcmp(filter_status.buf, "abort")) { - /* - * The filter signaled a permanent problem. Don't try to filter - * files with the same command for the lifetime of the current - * Git process. - */ - entry->supported_capabilities &= ~wanted_capability; - } else { - /* - * Something went wrong with the protocol filter. - * Force shutdown and restart if another blob requires filtering. - */ - error("external filter '%s' failed", cmd); - subprocess_stop(&subprocess_map, &entry->subprocess); - free(entry); - } - } else { + if (err) + handle_filter_error(&filter_status, entry, wanted_capability); + else strbuf_swap(dst, &nbuf); - } strbuf_release(&nbuf); return !err; } -- cgit v1.2.3 From 1514c8edd62d96006cd1de31e906ed5798dd4681 Mon Sep 17 00:00:00 2001 From: Lars Schneider Date: Fri, 30 Jun 2017 22:41:27 +0200 Subject: convert: refactor capabilities negotiation The code to negotiate long running filter capabilities was very repetitive for new capabilities. Replace the repetitive conditional statements with a table-driven approach. This is useful for the subsequent patch 'convert: add "status=delayed" to filter process protocol'. Suggested-by: Junio C Hamano Signed-off-by: Lars Schneider Signed-off-by: Junio C Hamano --- convert.c | 39 +++++++++++++++++++++++++++------------ 1 file changed, 27 insertions(+), 12 deletions(-) (limited to 'convert.c') diff --git a/convert.c b/convert.c index e55c034d86..d13e505dfb 100644 --- a/convert.c +++ b/convert.c @@ -507,7 +507,7 @@ static struct hashmap subprocess_map; static int start_multi_file_filter_fn(struct subprocess_entry *subprocess) { - int err; + int err, i; struct cmd2process *entry = (struct cmd2process *)subprocess; struct string_list cap_list = STRING_LIST_INIT_NODUP; char *cap_buf; @@ -515,6 +515,14 @@ static int start_multi_file_filter_fn(struct subprocess_entry *subprocess) struct child_process *process = &subprocess->process; const char *cmd = subprocess->cmd; + static const struct { + const char *name; + unsigned int cap; + } known_caps[] = { + { "clean", CAP_CLEAN }, + { "smudge", CAP_SMUDGE }, + }; + sigchain_push(SIGPIPE, SIG_IGN); err = packet_writel(process->in, "git-filter-client", "version=2", NULL); @@ -533,7 +541,15 @@ static int start_multi_file_filter_fn(struct subprocess_entry *subprocess) if (err) goto done; - err = packet_writel(process->in, "capability=clean", "capability=smudge", NULL); + for (i = 0; i < ARRAY_SIZE(known_caps); ++i) { + err = packet_write_fmt_gently( + process->in, "capability=%s\n", known_caps[i].name); + if (err) + goto done; + } + err = packet_flush_gently(process->in); + if (err) + goto done; for (;;) { cap_buf = packet_read_line(process->out, NULL); @@ -545,16 +561,15 @@ static int start_multi_file_filter_fn(struct subprocess_entry *subprocess) continue; cap_name = cap_list.items[1].string; - if (!strcmp(cap_name, "clean")) { - entry->supported_capabilities |= CAP_CLEAN; - } else if (!strcmp(cap_name, "smudge")) { - entry->supported_capabilities |= CAP_SMUDGE; - } else { - warning( - "external filter '%s' requested unsupported filter capability '%s'", - cmd, cap_name - ); - } + i = ARRAY_SIZE(known_caps) - 1; + while (i >= 0 && strcmp(cap_name, known_caps[i].name)) + i--; + + if (i >= 0) + entry->supported_capabilities |= known_caps[i].cap; + else + warning("external filter '%s' requested unsupported filter capability '%s'", + cmd, cap_name); string_list_clear(&cap_list, 0); } -- cgit v1.2.3 From 2841e8f81cb2820024804b9341577be1d0ce1240 Mon Sep 17 00:00:00 2001 From: Lars Schneider Date: Fri, 30 Jun 2017 22:41:28 +0200 Subject: convert: add "status=delayed" to filter process protocol Some `clean` / `smudge` filters may require a significant amount of time to process a single blob (e.g. the Git LFS smudge filter might perform network requests). During this process the Git checkout operation is blocked and Git needs to wait until the filter is done to continue with the checkout. Teach the filter process protocol, introduced in edcc8581 ("convert: add filter..process option", 2016-10-16), to accept the status "delayed" as response to a filter request. Upon this response Git continues with the checkout operation. After the checkout operation Git calls "finish_delayed_checkout" which queries the filter for remaining blobs. If the filter is still working on the completion, then the filter is expected to block. If the filter has completed all remaining blobs then an empty response is expected. Git has a multiple code paths that checkout a blob. Support delayed checkouts only in `clone` (in unpack-trees.c) and `checkout` operations for now. The optimization is most effective in these code paths as all files of the tree are processed. Signed-off-by: Lars Schneider Signed-off-by: Junio C Hamano --- convert.c | 110 +++++++++++++++++++++++++++++++++++++++++++++++++++++--------- 1 file changed, 94 insertions(+), 16 deletions(-) (limited to 'convert.c') diff --git a/convert.c b/convert.c index d13e505dfb..12a0b3eafb 100644 --- a/convert.c +++ b/convert.c @@ -496,6 +496,7 @@ static int apply_single_file_filter(const char *path, const char *src, size_t le #define CAP_CLEAN (1u<<0) #define CAP_SMUDGE (1u<<1) +#define CAP_DELAY (1u<<2) struct cmd2process { struct subprocess_entry subprocess; /* must be the first member! */ @@ -521,6 +522,7 @@ static int start_multi_file_filter_fn(struct subprocess_entry *subprocess) } known_caps[] = { { "clean", CAP_CLEAN }, { "smudge", CAP_SMUDGE }, + { "delay", CAP_DELAY }, }; sigchain_push(SIGPIPE, SIG_IGN); @@ -605,9 +607,11 @@ static void handle_filter_error(const struct strbuf *filter_status, static int apply_multi_file_filter(const char *path, const char *src, size_t len, int fd, struct strbuf *dst, const char *cmd, - const unsigned int wanted_capability) + const unsigned int wanted_capability, + struct delayed_checkout *dco) { int err; + int can_delay = 0; struct cmd2process *entry; struct child_process *process; struct strbuf nbuf = STRBUF_INIT; @@ -662,6 +666,14 @@ static int apply_multi_file_filter(const char *path, const char *src, size_t len if (err) goto done; + if ((entry->supported_capabilities & CAP_DELAY) && + dco && dco->state == CE_CAN_DELAY) { + can_delay = 1; + err = packet_write_fmt_gently(process->in, "can-delay=1\n"); + if (err) + goto done; + } + err = packet_flush_gently(process->in); if (err) goto done; @@ -677,14 +689,73 @@ static int apply_multi_file_filter(const char *path, const char *src, size_t len if (err) goto done; - err = strcmp(filter_status.buf, "success"); + if (can_delay && !strcmp(filter_status.buf, "delayed")) { + string_list_insert(&dco->filters, cmd); + string_list_insert(&dco->paths, path); + } else { + /* The filter got the blob and wants to send us a response. */ + err = strcmp(filter_status.buf, "success"); + if (err) + goto done; + + err = read_packetized_to_strbuf(process->out, &nbuf) < 0; + if (err) + goto done; + + err = subprocess_read_status(process->out, &filter_status); + if (err) + goto done; + + err = strcmp(filter_status.buf, "success"); + } + +done: + sigchain_pop(SIGPIPE); + + if (err) + handle_filter_error(&filter_status, entry, wanted_capability); + else + strbuf_swap(dst, &nbuf); + strbuf_release(&nbuf); + return !err; +} + + +int async_query_available_blobs(const char *cmd, struct string_list *available_paths) +{ + int err; + char *line; + struct cmd2process *entry; + struct child_process *process; + struct strbuf filter_status = STRBUF_INIT; + + assert(subprocess_map_initialized); + entry = (struct cmd2process *)subprocess_find_entry(&subprocess_map, cmd); + if (!entry) { + error("external filter '%s' is not available anymore although " + "not all paths have been filtered", cmd); + return 0; + } + process = &entry->subprocess.process; + sigchain_push(SIGPIPE, SIG_IGN); + + err = packet_write_fmt_gently( + process->in, "command=list_available_blobs\n"); if (err) goto done; - err = read_packetized_to_strbuf(process->out, &nbuf) < 0; + err = packet_flush_gently(process->in); if (err) goto done; + while ((line = packet_read_line(process->out, NULL))) { + const char *path; + if (skip_prefix(line, "pathname=", &path)) + string_list_insert(available_paths, xstrdup(path)); + else + ; /* ignore unknown keys */ + } + err = subprocess_read_status(process->out, &filter_status); if (err) goto done; @@ -695,10 +766,7 @@ done: sigchain_pop(SIGPIPE); if (err) - handle_filter_error(&filter_status, entry, wanted_capability); - else - strbuf_swap(dst, &nbuf); - strbuf_release(&nbuf); + handle_filter_error(&filter_status, entry, 0); return !err; } @@ -713,7 +781,8 @@ static struct convert_driver { static int apply_filter(const char *path, const char *src, size_t len, int fd, struct strbuf *dst, struct convert_driver *drv, - const unsigned int wanted_capability) + const unsigned int wanted_capability, + struct delayed_checkout *dco) { const char *cmd = NULL; @@ -731,7 +800,8 @@ static int apply_filter(const char *path, const char *src, size_t len, if (cmd && *cmd) return apply_single_file_filter(path, src, len, fd, dst, cmd); else if (drv->process && *drv->process) - return apply_multi_file_filter(path, src, len, fd, dst, drv->process, wanted_capability); + return apply_multi_file_filter(path, src, len, fd, dst, + drv->process, wanted_capability, dco); return 0; } @@ -1072,7 +1142,7 @@ int would_convert_to_git_filter_fd(const char *path) if (!ca.drv->required) return 0; - return apply_filter(path, NULL, 0, -1, NULL, ca.drv, CAP_CLEAN); + return apply_filter(path, NULL, 0, -1, NULL, ca.drv, CAP_CLEAN, NULL); } const char *get_convert_attr_ascii(const char *path) @@ -1109,7 +1179,7 @@ int convert_to_git(const char *path, const char *src, size_t len, convert_attrs(&ca, path); - ret |= apply_filter(path, src, len, -1, dst, ca.drv, CAP_CLEAN); + ret |= apply_filter(path, src, len, -1, dst, ca.drv, CAP_CLEAN, NULL); if (!ret && ca.drv && ca.drv->required) die("%s: clean filter '%s' failed", path, ca.drv->name); @@ -1134,7 +1204,7 @@ void convert_to_git_filter_fd(const char *path, int fd, struct strbuf *dst, assert(ca.drv); assert(ca.drv->clean || ca.drv->process); - if (!apply_filter(path, NULL, 0, fd, dst, ca.drv, CAP_CLEAN)) + if (!apply_filter(path, NULL, 0, fd, dst, ca.drv, CAP_CLEAN, NULL)) die("%s: clean filter '%s' failed", path, ca.drv->name); crlf_to_git(path, dst->buf, dst->len, dst, ca.crlf_action, checksafe); @@ -1143,7 +1213,7 @@ void convert_to_git_filter_fd(const char *path, int fd, struct strbuf *dst, static int convert_to_working_tree_internal(const char *path, const char *src, size_t len, struct strbuf *dst, - int normalizing) + int normalizing, struct delayed_checkout *dco) { int ret = 0, ret_filter = 0; struct conv_attrs ca; @@ -1168,21 +1238,29 @@ static int convert_to_working_tree_internal(const char *path, const char *src, } } - ret_filter = apply_filter(path, src, len, -1, dst, ca.drv, CAP_SMUDGE); + ret_filter = apply_filter( + path, src, len, -1, dst, ca.drv, CAP_SMUDGE, dco); if (!ret_filter && ca.drv && ca.drv->required) die("%s: smudge filter %s failed", path, ca.drv->name); return ret | ret_filter; } +int async_convert_to_working_tree(const char *path, const char *src, + size_t len, struct strbuf *dst, + void *dco) +{ + return convert_to_working_tree_internal(path, src, len, dst, 0, dco); +} + int convert_to_working_tree(const char *path, const char *src, size_t len, struct strbuf *dst) { - return convert_to_working_tree_internal(path, src, len, dst, 0); + return convert_to_working_tree_internal(path, src, len, dst, 0, NULL); } int renormalize_buffer(const char *path, const char *src, size_t len, struct strbuf *dst) { - int ret = convert_to_working_tree_internal(path, src, len, dst, 1); + int ret = convert_to_working_tree_internal(path, src, len, dst, 1, NULL); if (ret) { src = dst->buf; len = dst->len; -- cgit v1.2.3 From fa64a2fdbeedd98c5f24d1662bcc470a8449abcf Mon Sep 17 00:00:00 2001 From: Jonathan Tan Date: Wed, 26 Jul 2017 11:17:29 -0700 Subject: sub-process: refactor handshake to common function Refactor, into a common function, the version and capability negotiation done when invoking a long-running process as a clean or smudge filter. This will be useful for other Git code that needs to interact similarly with a long-running process. As you can see in the change to t0021, this commit changes the error message reported when the long-running process does not introduce itself with the expected "server"-terminated line. Originally, the error message reports that the filter "does not support filter protocol version 2", differentiating between the old single-file filter protocol and the new multi-file filter protocol - I have updated it to something more generic and useful. Signed-off-by: Jonathan Tan Signed-off-by: Junio C Hamano --- convert.c | 75 ++++++--------------------------------------------------------- 1 file changed, 7 insertions(+), 68 deletions(-) (limited to 'convert.c') diff --git a/convert.c b/convert.c index 972bf8aec2..4936fcc26b 100644 --- a/convert.c +++ b/convert.c @@ -513,78 +513,17 @@ static struct hashmap subprocess_map; static int start_multi_file_filter_fn(struct subprocess_entry *subprocess) { - int err, i; - struct cmd2process *entry = (struct cmd2process *)subprocess; - struct string_list cap_list = STRING_LIST_INIT_NODUP; - char *cap_buf; - const char *cap_name; - struct child_process *process = &subprocess->process; - const char *cmd = subprocess->cmd; - - static const struct { - const char *name; - unsigned int cap; - } known_caps[] = { + static int versions[] = {2, 0}; + static struct subprocess_capability capabilities[] = { { "clean", CAP_CLEAN }, { "smudge", CAP_SMUDGE }, { "delay", CAP_DELAY }, + { NULL, 0 } }; - - sigchain_push(SIGPIPE, SIG_IGN); - - err = packet_writel(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); - goto done; - } - err = strcmp(packet_read_line(process->out, NULL), "version=2"); - if (err) - goto done; - err = packet_read_line(process->out, NULL) != NULL; - if (err) - goto done; - - for (i = 0; i < ARRAY_SIZE(known_caps); ++i) { - err = packet_write_fmt_gently( - process->in, "capability=%s\n", known_caps[i].name); - if (err) - goto done; - } - err = packet_flush_gently(process->in); - if (err) - goto done; - - for (;;) { - cap_buf = packet_read_line(process->out, NULL); - if (!cap_buf) - break; - string_list_split_in_place(&cap_list, cap_buf, '=', 1); - - if (cap_list.nr != 2 || strcmp(cap_list.items[0].string, "capability")) - continue; - - cap_name = cap_list.items[1].string; - i = ARRAY_SIZE(known_caps) - 1; - while (i >= 0 && strcmp(cap_name, known_caps[i].name)) - i--; - - if (i >= 0) - entry->supported_capabilities |= known_caps[i].cap; - else - warning("external filter '%s' requested unsupported filter capability '%s'", - cmd, cap_name); - - string_list_clear(&cap_list, 0); - } - -done: - sigchain_pop(SIGPIPE); - - return err; + struct cmd2process *entry = (struct cmd2process *)subprocess; + return subprocess_handshake(subprocess, "git-filter", versions, NULL, + capabilities, + &entry->supported_capabilities); } static void handle_filter_error(const struct strbuf *filter_status, -- cgit v1.2.3