{"thread":{"id":"45232","subject":"[PATCH v2] convert: add \"status=delayed\" to filter process protocol","startedAt":"2017-02-26T18:48:28Z","lastAt":"2017-04-09T18:48:23Z","messageCount":7,"participants":["Lars Schneider","Jeff King","Jakub Narębski"],"isPatch":true,"patchVersion":2,"patchTotal":null},"messages":[{"id":"312693","messageId":"20170226184816.30010-1-larsxschneider@gmail.com","threadId":"45232","inReplyTo":null,"subject":"[PATCH v2] convert: add \"status=delayed\" to filter process protocol","fromName":"Lars Schneider","fromEmail":"larsxschneider@gmail.com","sentAt":"2017-02-26T18:48:16Z","receivedAt":"2017-02-26T18:48:28Z","isPatch":true,"sender":{"key":"larsxschneider@gmail.com","avatar":"https://avatars.githubusercontent.com/u/477434?v=4"},"body":"Some `clean` / `smudge` filters might require a significant amount of\ntime to process a single blob. During this process the Git checkout\noperation is blocked and Git needs to wait until the filter is done to\ncontinue with the checkout.\n\nTeach the filter process protocol (introduced in edcc858) to accept the\nstatus \"delayed\" as response to a filter request. Upon this response Git\ncontinues with the checkout operation and asks the filter to process the\nblob again after all other blobs have been processed.\n\nGit has a multiple code paths that checkout a blob. Support delayed\ncheckouts only in `clone` (in unpack-trees.c) and `checkout` operations.\n\nSigned-off-by: Lars Schneider <larsxschneider@gmail.com>\n---\n\nHi,\n\nin v1 Junio criticized the \"convert.h\" interface of this patch [1].\nAfter talking to Peff I think I understand Junio's point and I would\nlike to get your feedback on the new approach here. Please ignore all\nchanges behind async_convert_to_working_tree() and async_filter_finish()\nfor now as I plan to change the implementation as soon as the interface\nis in an acceptable state.\n\nThe new interface also addresses Torsten's feedback and leaves\nconvert_to_working_tree() as is [2].\n\nI also use '>' for numeric comparisons in Perl as suggested by Eric [3].\n\nPlease note, I rebased the patch to v2.12 as v1 did not apply clean on\nmaster anymore.\n\nThanks,\nLars\n\n[1] http://public-inbox.org/git/xmqqa8b115ll.fsf@gitster.mtv.corp.google.com/\n[2] http://public-inbox.org/git/20170108201415.GA3569@tb-raspi/\n[3] http://public-inbox.org/git/20170108204517.GA13779@starla/\n\n\nRFC: http://public-inbox.org/git/D10F7C47-14E8-465B-8B7A-A09A1B28A39F@gmail.com/\n v1: http://public-inbox.org/git/20170108191736.47359-1-larsxschneider@gmail.com/\n\n\nNotes:\n    Base Ref: v2.12.0\n    Web-Diff: https://github.com/larsxschneider/git/commit/13d5b37021\n    Checkout: git fetch https://github.com/larsxschneider/git filter-process/delay-v2 && git checkout 13d5b37021\n\n Documentation/gitattributes.txt |  9 ++++++\n builtin/checkout.c              |  1 +\n cache.h                         |  1 +\n convert.c                       | 68 +++++++++++++++++++++++++++++++++--------\n convert.h                       | 13 ++++++++\n entry.c                         | 29 +++++++++++++++---\n t/t0021-conversion.sh           | 53 ++++++++++++++++++++++++++++++++\n t/t0021/rot13-filter.pl         | 19 ++++++++++++\n unpack-trees.c                  |  1 +\n 9 files changed, 176 insertions(+), 18 deletions(-)\n\ndiff --git a/Documentation/gitattributes.txt b/Documentation/gitattributes.txt\nindex e0b66c1220..f6bad8db40 100644\n--- a/Documentation/gitattributes.txt\n+++ b/Documentation/gitattributes.txt\n@@ -473,6 +473,15 @@ packet:          git< 0000  # empty content!\n packet:          git< 0000  # empty list, keep \"status=success\" unchanged!\n ------------------------\n\n+If the request cannot be fulfilled within a reasonable amount of time\n+then the filter can respond with a \"delayed\" status and a flush packet.\n+Git will perform the same request at a later point in time, again. The\n+filter can delay a response multiple times for a single request.\n+------------------------\n+packet:          git< status=delayed\n+packet:          git< 0000\n+------------------------\n+\n In case the filter cannot or does not want to process the content,\n it is expected to respond with an \"error\" status.\n ------------------------\ndiff --git a/builtin/checkout.c b/builtin/checkout.c\nindex f174f50303..742e8742cd 100644\n--- a/builtin/checkout.c\n+++ b/builtin/checkout.c\n@@ -369,6 +369,7 @@ static int checkout_paths(const struct checkout_opts *opts,\n \t\t\tpos = skip_same_name(ce, pos) - 1;\n \t\t}\n \t}\n+\terrs |= checkout_delayed_entries(&state);\n\n \tif (write_locked_index(&the_index, lock_file, COMMIT_LOCK))\n \t\tdie(_(\"unable to write new index file\"));\ndiff --git a/cache.h b/cache.h\nindex 61fc86e6d7..66dde99a79 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -1434,6 +1434,7 @@ struct checkout {\n\n #define TEMPORARY_FILENAME_LENGTH 25\n extern int checkout_entry(struct cache_entry *ce, const struct checkout *state, char *topath);\n+extern int checkout_delayed_entries(const struct checkout *state);\n\n struct cache_def {\n \tstruct strbuf path;\ndiff --git a/convert.c b/convert.c\nindex 4e17e45ed2..24d29f5c53 100644\n--- a/convert.c\n+++ b/convert.c\n@@ -4,6 +4,7 @@\n #include \"quote.h\"\n #include \"sigchain.h\"\n #include \"pkt-line.h\"\n+#include \"list.h\"\n\n /*\n  * convert.c - convert a file when checking it out and checking it in.\n@@ -38,6 +39,13 @@ struct text_stat {\n \tunsigned printable, nonprintable;\n };\n\n+static LIST_HEAD(delayed_item_queue_head);\n+\n+struct delayed_item {\n+\tvoid* item;\n+\tstruct list_head node;\n+};\n+\n static void gather_stats(const char *buf, unsigned long size, struct text_stat *stats)\n {\n \tunsigned long i;\n@@ -672,7 +680,7 @@ static struct cmd2process *start_multi_file_filter(struct hashmap *hashmap, cons\n }\n\n static int apply_multi_file_filter(const char *path, const char *src, size_t len,\n-\t\t\t\t   int fd, struct strbuf *dst, const char *cmd,\n+\t\t\t\t   int fd, struct strbuf *dst, int *delayed, const char *cmd,\n \t\t\t\t   const unsigned int wanted_capability)\n {\n \tint err;\n@@ -738,9 +746,14 @@ static int apply_multi_file_filter(const char *path, const char *src, size_t len\n \t\tgoto done;\n\n \tread_multi_file_filter_status(process->out, &filter_status);\n-\terr = strcmp(filter_status.buf, \"success\");\n-\tif (err)\n+\tif (delayed && !strcmp(filter_status.buf, \"delayed\")) {\n+\t\t*delayed = 1;\n \t\tgoto done;\n+\t} else {\n+\t\terr = strcmp(filter_status.buf, \"success\");\n+\t\tif (err)\n+\t\t\tgoto done;\n+\t}\n\n \terr = read_packetized_to_strbuf(process->out, &nbuf) < 0;\n \tif (err)\n@@ -787,8 +800,8 @@ static struct convert_driver {\n } *user_convert, **user_convert_tail;\n\n static int apply_filter(const char *path, const char *src, size_t len,\n-\t\t\tint fd, struct strbuf *dst, struct convert_driver *drv,\n-\t\t\tconst unsigned int wanted_capability)\n+\t\t\tint fd, struct strbuf *dst, int *delayed,\n+\t\t\tstruct convert_driver *drv, const unsigned int wanted_capability)\n {\n \tconst char *cmd = NULL;\n\n@@ -806,7 +819,7 @@ static int apply_filter(const char *path, const char *src, size_t len,\n \tif (cmd && *cmd)\n \t\treturn apply_single_file_filter(path, src, len, fd, dst, cmd);\n \telse if (drv->process && *drv->process)\n-\t\treturn apply_multi_file_filter(path, src, len, fd, dst, drv->process, wanted_capability);\n+\t\treturn apply_multi_file_filter(path, src, len, fd, dst, delayed, drv->process, wanted_capability);\n\n \treturn 0;\n }\n@@ -1152,7 +1165,7 @@ int would_convert_to_git_filter_fd(const char *path)\n \tif (!ca.drv->required)\n \t\treturn 0;\n\n-\treturn apply_filter(path, NULL, 0, -1, NULL, ca.drv, CAP_CLEAN);\n+\treturn apply_filter(path, NULL, 0, -1, NULL, NULL, ca.drv, CAP_CLEAN);\n }\n\n const char *get_convert_attr_ascii(const char *path)\n@@ -1189,7 +1202,7 @@ int convert_to_git(const char *path, const char *src, size_t len,\n\n \tconvert_attrs(&ca, path);\n\n-\tret |= apply_filter(path, src, len, -1, dst, ca.drv, CAP_CLEAN);\n+\tret |= apply_filter(path, src, len, -1, dst, NULL, ca.drv, CAP_CLEAN);\n \tif (!ret && ca.drv && ca.drv->required)\n \t\tdie(\"%s: clean filter '%s' failed\", path, ca.drv->name);\n\n@@ -1214,7 +1227,7 @@ void convert_to_git_filter_fd(const char *path, int fd, struct strbuf *dst,\n \tassert(ca.drv);\n \tassert(ca.drv->clean || ca.drv->process);\n\n-\tif (!apply_filter(path, NULL, 0, fd, dst, ca.drv, CAP_CLEAN))\n+\tif (!apply_filter(path, NULL, 0, fd, dst, NULL, ca.drv, CAP_CLEAN))\n \t\tdie(\"%s: clean filter '%s' failed\", path, ca.drv->name);\n\n \tcrlf_to_git(path, dst->buf, dst->len, dst, ca.crlf_action, checksafe);\n@@ -1222,7 +1235,7 @@ void convert_to_git_filter_fd(const char *path, int fd, struct strbuf *dst,\n }\n\n static int convert_to_working_tree_internal(const char *path, const char *src,\n-\t\t\t\t\t    size_t len, struct strbuf *dst,\n+\t\t\t\t\t    size_t len, struct strbuf *dst, int *delayed,\n \t\t\t\t\t    int normalizing)\n {\n \tint ret = 0, ret_filter = 0;\n@@ -1248,21 +1261,50 @@ static int convert_to_working_tree_internal(const char *path, const char *src,\n \t\t}\n \t}\n\n-\tret_filter = apply_filter(path, src, len, -1, dst, ca.drv, CAP_SMUDGE);\n+\tret_filter = apply_filter(path, src, len, -1, dst, delayed, ca.drv, CAP_SMUDGE);\n \tif (!ret_filter && ca.drv && ca.drv->required)\n \t\tdie(\"%s: smudge filter %s failed\", path, ca.drv->name);\n\n \treturn ret | ret_filter;\n }\n\n+int async_convert_to_working_tree(const char *path, const char *src,\n+\t\t\t\t\t\t\t\t  size_t len, struct strbuf *dst, void *item)\n+{\n+\tint delayed = 0;\n+\tstruct delayed_item *delayed_item;\n+\tif (convert_to_working_tree_internal(path, src, len, dst, &delayed, 0)) {\n+\t\tif (delayed) {\n+\t\t\tdelayed_item = xmalloc(sizeof(*delayed_item));\n+\t\t\tdelayed_item->item = item;\n+\t\t\tlist_add_tail(&delayed_item->node, &delayed_item_queue_head);\n+\t\t\treturn ASYNC_FILTER_DELAYED;\n+\t\t}\n+\t\treturn ASYNC_FILTER_SUCCESS;\n+\t}\n+\treturn ASYNC_FILTER_FAIL;\n+}\n+\n+void* async_filter_finish(void)\n+{\n+\tstruct delayed_item *head;\n+\tif (!list_empty(&delayed_item_queue_head)) {\n+\t\thead = list_first_entry(&delayed_item_queue_head,\n+\t\t\tstruct delayed_item, node);\n+\t\tlist_del(&head->node);\n+\t\treturn head->item;\n+\t}\n+\treturn NULL;\n+}\n+\n int convert_to_working_tree(const char *path, const char *src, size_t len, struct strbuf *dst)\n {\n-\treturn convert_to_working_tree_internal(path, src, len, dst, 0);\n+\treturn convert_to_working_tree_internal(path, src, len, dst, NULL, 0);\n }\n\n int renormalize_buffer(const char *path, const char *src, size_t len, struct strbuf *dst)\n {\n-\tint ret = convert_to_working_tree_internal(path, src, len, dst, 1);\n+\tint ret = convert_to_working_tree_internal(path, src, len, dst, NULL, 1);\n \tif (ret) {\n \t\tsrc = dst->buf;\n \t\tlen = dst->len;\ndiff --git a/convert.h b/convert.h\nindex 82871a11d5..acc016de9f 100644\n--- a/convert.h\n+++ b/convert.h\n@@ -4,6 +4,15 @@\n #ifndef CONVERT_H\n #define CONVERT_H\n\n+enum async_filter {\n+\tASYNC_FILTER_SUCCESS = 0,\n+\tASYNC_FILTER_FAIL = 1,\n+\tASYNC_FILTER_DELAYED = 2\n+};\n+\n+extern enum async_filter async_filter;\n+\n+\n enum safe_crlf {\n \tSAFE_CRLF_FALSE = 0,\n \tSAFE_CRLF_FAIL = 1,\n@@ -42,6 +51,10 @@ extern int convert_to_git(const char *path, const char *src, size_t len,\n \t\t\t  struct strbuf *dst, enum safe_crlf checksafe);\n extern int convert_to_working_tree(const char *path, const char *src,\n \t\t\t\t   size_t len, struct strbuf *dst);\n+extern int async_convert_to_working_tree(const char *path, const char *src,\n+\t\t\t\t\t size_t len, struct strbuf *dst,\n+\t\t\t\t\t void *item);\n+extern void* async_filter_finish(void);\n extern int renormalize_buffer(const char *path, const char *src, size_t len,\n \t\t\t      struct strbuf *dst);\n static inline int would_convert_to_git(const char *path)\ndiff --git a/entry.c b/entry.c\nindex c6eea240b6..d15e69a55e 100644\n--- a/entry.c\n+++ b/entry.c\n@@ -177,11 +177,17 @@ static int write_entry(struct cache_entry *ce,\n \t\t/*\n \t\t * Convert from git internal format to working tree format\n \t\t */\n-\t\tif (ce_mode_s_ifmt == S_IFREG &&\n-\t\t    convert_to_working_tree(ce->name, new, size, &buf)) {\n-\t\t\tfree(new);\n-\t\t\tnew = strbuf_detach(&buf, &newsize);\n-\t\t\tsize = newsize;\n+\t\tif (ce_mode_s_ifmt == S_IFREG) {\n+\t\t\tret = async_convert_to_working_tree(ce->name, new, size, &buf, ce);\n+\t\t\tif (ret == ASYNC_FILTER_SUCCESS) {\n+\t\t\t\tfree(new);\n+\t\t\t\tnew = strbuf_detach(&buf, &newsize);\n+\t\t\t\tsize = newsize;\n+\t\t\t}\n+\t\t\telse if (ret == ASYNC_FILTER_DELAYED) {\n+\t\t\t\tfree(new);\n+\t\t\t\tgoto finish;\n+\t\t\t}\n \t\t}\n\n \t\tfd = open_output_fd(path, ce, to_tempfile);\n@@ -291,3 +297,16 @@ int checkout_entry(struct cache_entry *ce,\n \tcreate_directories(path.buf, path.len, state);\n \treturn write_entry(ce, path.buf, state, 0);\n }\n+\n+int checkout_delayed_entries(const struct checkout *state)\n+{\n+\tstruct cache_entry *ce;\n+\tint errs = 0;\n+\n+\twhile ((ce = async_filter_finish())) {\n+\t\tce->ce_flags &= ~CE_UPDATE;\n+\t\terrs |= checkout_entry(ce, state, NULL);\n+\t}\n+\n+\treturn errs;\n+}\ndiff --git a/t/t0021-conversion.sh b/t/t0021-conversion.sh\nindex 161f560446..8ae5b1a521 100755\n--- a/t/t0021-conversion.sh\n+++ b/t/t0021-conversion.sh\n@@ -701,4 +701,57 @@ test_expect_success PERL 'invalid process filter must fail (and not hang!)' '\n \t)\n '\n\n+test_expect_success PERL 'delayed checkout in process filter' '\n+\ttest_config_global filter.protocol.process \"rot13-filter.pl clean smudge\" &&\n+\ttest_config_global filter.protocol.required true &&\n+\trm -rf repo &&\n+\tmkdir repo &&\n+\t(\n+\t\tcd repo &&\n+\t\tgit init &&\n+\t\techo \"*.r filter=protocol\" >.gitattributes &&\n+\t\tcp \"$TEST_ROOT/test.o\" test.r &&\n+\t\tcp \"$TEST_ROOT/test.o\" test-delay1.r &&\n+\t\tcp \"$TEST_ROOT/test.o\" test-delay3.r &&\n+\t\tgit add . &&\n+\t\tgit commit -m \"test commit 1\"\n+\t) &&\n+\n+\tS=$(file_size repo/test.r) &&\n+\trm -rf repo-cloned &&\n+\tfilter_git clone repo repo-cloned &&\n+\tcat >expected.log <<-EOF &&\n+\t\tSTART\n+\t\tinit handshake complete\n+\t\tIN: smudge test.r $S [OK] -- OUT: $S . [OK]\n+\t\tIN: smudge test-delay1.r $S [OK] -- OUT: $S [DELAYED]\n+\t\tIN: smudge test-delay1.r $S [OK] -- OUT: $S . [OK]\n+\t\tIN: smudge test-delay3.r $S [OK] -- OUT: $S [DELAYED]\n+\t\tIN: smudge test-delay3.r $S [OK] -- OUT: $S [DELAYED]\n+\t\tIN: smudge test-delay3.r $S [OK] -- OUT: $S [DELAYED]\n+\t\tIN: smudge test-delay3.r $S [OK] -- OUT: $S . [OK]\n+\t\tSTOP\n+\tEOF\n+\ttest_cmp_count expected.log repo-cloned/rot13-filter.log &&\n+\n+\t(\n+\t\tcd repo-cloned &&\n+\t\trm *.r rot13-filter.log &&\n+\t\tfilter_git checkout . &&\n+\t\tcat >expected.log <<-EOF &&\n+\t\t\tSTART\n+\t\t\tinit handshake complete\n+\t\t\tIN: smudge test.r $S [OK] -- OUT: $S . [OK]\n+\t\t\tIN: smudge test-delay1.r $S [OK] -- OUT: $S [DELAYED]\n+\t\t\tIN: smudge test-delay1.r $S [OK] -- OUT: $S . [OK]\n+\t\t\tIN: smudge test-delay3.r $S [OK] -- OUT: $S [DELAYED]\n+\t\t\tIN: smudge test-delay3.r $S [OK] -- OUT: $S [DELAYED]\n+\t\t\tIN: smudge test-delay3.r $S [OK] -- OUT: $S [DELAYED]\n+\t\t\tIN: smudge test-delay3.r $S [OK] -- OUT: $S . [OK]\n+\t\t\tSTOP\n+\t\tEOF\n+\t\ttest_cmp_count expected.log rot13-filter.log\n+\t)\n+'\n+\n test_done\ndiff --git a/t/t0021/rot13-filter.pl b/t/t0021/rot13-filter.pl\nindex 617f581e56..ece0d314b4 100644\n--- a/t/t0021/rot13-filter.pl\n+++ b/t/t0021/rot13-filter.pl\n@@ -17,6 +17,10 @@\n #     operation then the filter signals that it cannot or does not want\n #     to process the file and any file after that is processed with the\n #     same command.\n+# (5) If data with a pathname that is a key in the DELAY hash is\n+#     processed (e.g. 'test-delay1.r') then the filter signals n times\n+#     to Git that the processing is delayed (n being the value of the\n+#     DELAY hash key).\n #\n\n use strict;\n@@ -25,6 +29,12 @@ use IO::File;\n\n my $MAX_PACKET_CONTENT_SIZE = 65516;\n my @capabilities            = @ARGV;\n+my $DELAY3 = 3;\n+my $DELAY1 = 1;\n+\n+my %DELAY;\n+$DELAY{'test-delay1.r'} = 1;\n+$DELAY{'test-delay3.r'} = 3;\n\n open my $debug, \">>\", \"rot13-filter.log\" or die \"cannot open log file: $!\";\n\n@@ -166,6 +176,15 @@ while (1) {\n \t\tpacket_txt_write(\"status=abort\");\n \t\tpacket_flush();\n \t}\n+\telsif ( $command eq \"smudge\" and\n+\t\t    exists $DELAY{$pathname} and\n+\t\t    $DELAY{$pathname} > 0 ) {\n+\t\t$DELAY{$pathname} = $DELAY{$pathname} - 1;\n+\t\tprint $debug \"[DELAYED]\\n\";\n+\t\t$debug->flush();\n+\t\tpacket_txt_write(\"status=delayed\");\n+\t\tpacket_flush();\n+\t}\n \telse {\n \t\tpacket_txt_write(\"status=success\");\n \t\tpacket_flush();\ndiff --git a/unpack-trees.c b/unpack-trees.c\nindex 3a8ee19fe8..6b3246db03 100644\n--- a/unpack-trees.c\n+++ b/unpack-trees.c\n@@ -315,6 +315,7 @@ static int check_updates(struct unpack_trees_options *o)\n \t\t\t}\n \t\t}\n \t}\n+\terrs |= checkout_delayed_entries(&state);\n \tstop_progress(&progress);\n \tif (o->update)\n \t\tgit_attr_set_direction(GIT_ATTR_CHECKIN, NULL);\n\nbase-commit: e7e07d5a4fcc2a203d9873968ad3e6bd4d7419d7\n--\n2.11.1\n\n"},{"id":"312713","messageId":"20170227095825.jhdspwy6oa6mvvvm@sigill.intra.peff.net","threadId":"45232","inReplyTo":"20170226184816.30010-1-larsxschneider@gmail.com","subject":"Re: [PATCH v2] convert: add \"status=delayed\" to filter process protocol","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-02-27T09:58:25Z","receivedAt":"2017-02-27T10:01:56Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sun, Feb 26, 2017 at 07:48:16PM +0100, Lars Schneider wrote:\n\n> +If the request cannot be fulfilled within a reasonable amount of time\n> +then the filter can respond with a \"delayed\" status and a flush packet.\n> +Git will perform the same request at a later point in time, again. The\n> +filter can delay a response multiple times for a single request.\n> +------------------------\n> +packet:          git< status=delayed\n> +packet:          git< 0000\n> +------------------------\n> +\n\nSo Git just asks for the same content again? I see two issues with that:\n\n  1. Does git have to feed the blob content again? That can be expensive\n     to access or to keep around in memory.\n\n  2. What happens when the item isn't ready on the second request? I can\n     think of a few options:\n\n       a. The filter immediately says \"nope, still delayed\". But then\n          Git ends up busy-looping with \"is this one ready yet?\"\n\n       b. The filter blocks until the item is ready. But then if other\n\t  items _are_ ready, Git cannot work on processing them. We lose\n\t  parallelism.\n\n       c. You could do a hybrid: block until _some_ item is ready, and\n          then issue \"delayed\" responses for everything that isn't\n\t  ready. Then if you assume that Git is looping over and over\n\t  through the set of objects, it will either block or pick up\n\t  _something_ on each loop.\n\n\t  But it makes a quadratic number of requests in the worst case.\n\t  E.g., imagine you have N items and the last one is available\n\t  first, then the second-to-last, and so on. You'll ask N times,\n\t  then N-1, then N-2, and so on.\n\nI think it would be much more efficient to do something like:\n\n  [Git issues a request and gives it an opaque index id]\n  git> command=smudge\n  git> pathname=foo\n  git> index=0\n  git> 0000\n  git> CONTENT\n  git> 0000\n\n  [The data isn't ready yet, so the filter tells us so...]\n  git< status=delayed\n  git< 0000\n\n  [Git may make other requests, that are either served or delayed]\n  git> command=smudge\n  git> pathname=foo\n  git> index=1\n  git> 0000\n  git< status=success\n  git< 0000\n  git< CONTENT\n  git< 0000\n\n  [Now Git has processed all of the items, and each one either has its\n   final status, or has been marked as delayed. So we ask for a delayed\n   item]\n  git> command=wait\n  git> 0000\n\n  [Some time may pass if nothing is ready. But eventually we get...]\n  git< status=success\n  git< index=0\n  git< 0000\n  git< CONTENT\n  git< 0000\n\nFrom Git's side, the loop is something like:\n\n  while (delayed_items > 0) {\n\t/* issue a wait, and get back the status/index pair */\n\tstatus = send_wait(&index);\n\tdelayed_items--;\n\n\t/*\n\t * use \"index\" to find the right item in our list of files;\n\t * the format can be opaque to the filter, so we could index\n\t * it however we like. But probably numeric indices in an array\n\t * are the simplest.\n\t */\n\tassert(index > 0 && index < nr_items);\n\titem[index].status = status;\n\tif (status == SUCCESS)\n\t\tread_content(&item[index]);\n  }\n\nand the filter side just attaches the \"index\" string to whatever its\ninternal queue structure is, and feeds it back verbatim when processing\nthat item finishes.\n\n-Peff\n"},{"id":"312714","messageId":"E4A6D866-D046-4CF3-8050-666FD6C98F7B@gmail.com","threadId":"45232","inReplyTo":"20170227095825.jhdspwy6oa6mvvvm@sigill.intra.peff.net","subject":"Re: [PATCH v2] convert: add \"status=delayed\" to filter process protocol","fromName":"Lars Schneider","fromEmail":"larsxschneider@gmail.com","sentAt":"2017-02-27T10:32:47Z","receivedAt":"2017-02-27T10:32:56Z","isPatch":true,"sender":{"key":"larsxschneider@gmail.com","avatar":"https://avatars.githubusercontent.com/u/477434?v=4"},"body":"\n> On 27 Feb 2017, at 10:58, Jeff King <peff@peff.net> wrote:\n> \n> On Sun, Feb 26, 2017 at 07:48:16PM +0100, Lars Schneider wrote:\n> \n>> +If the request cannot be fulfilled within a reasonable amount of time\n>> +then the filter can respond with a \"delayed\" status and a flush packet.\n>> +Git will perform the same request at a later point in time, again. The\n>> +filter can delay a response multiple times for a single request.\n>> +------------------------\n>> +packet:          git< status=delayed\n>> +packet:          git< 0000\n>> +------------------------\n>> +\n> \n> So Git just asks for the same content again? I see two issues with that:\n> \n>  1. Does git have to feed the blob content again? That can be expensive\n>     to access or to keep around in memory.\n> \n>  2. What happens when the item isn't ready on the second request? I can\n>     think of a few options:\n> \n>       a. The filter immediately says \"nope, still delayed\". But then\n>          Git ends up busy-looping with \"is this one ready yet?\"\n> \n>       b. The filter blocks until the item is ready. But then if other\n> \t  items _are_ ready, Git cannot work on processing them. We lose\n> \t  parallelism.\n> \n>       c. You could do a hybrid: block until _some_ item is ready, and\n>          then issue \"delayed\" responses for everything that isn't\n> \t  ready. Then if you assume that Git is looping over and over\n> \t  through the set of objects, it will either block or pick up\n> \t  _something_ on each loop.\n> \n> \t  But it makes a quadratic number of requests in the worst case.\n> \t  E.g., imagine you have N items and the last one is available\n> \t  first, then the second-to-last, and so on. You'll ask N times,\n> \t  then N-1, then N-2, and so on.\n\nI completely agree - I need to change that. However, the goal of the v2\niteration was to get the \"convert\" interface in an acceptable state.\nThat's what I intended to say in the patch comment section:\n\n    \"Please ignore all changes behind async_convert_to_working_tree() and \n     async_filter_finish() for now as I plan to change the implementation \n     as soon as the interface is in an acceptable state.\"\n\n> \n> I think it would be much more efficient to do something like:\n> \n>  [Git issues a request and gives it an opaque index id]\n>  git> command=smudge\n>  git> pathname=foo\n>  git> index=0\n>  git> 0000\n>  git> CONTENT\n>  git> 0000\n> \n>  [The data isn't ready yet, so the filter tells us so...]\n>  git< status=delayed\n>  git< 0000\n> \n>  [Git may make other requests, that are either served or delayed]\n>  git> command=smudge\n>  git> pathname=foo\n>  git> index=1\n>  git> 0000\n>  git< status=success\n>  git< 0000\n>  git< CONTENT\n>  git< 0000\n> \n>  [Now Git has processed all of the items, and each one either has its\n>   final status, or has been marked as delayed. So we ask for a delayed\n>   item]\n>  git> command=wait\n>  git> 0000\n> \n>  [Some time may pass if nothing is ready. But eventually we get...]\n>  git< status=success\n>  git< index=0\n>  git< 0000\n>  git< CONTENT\n>  git< 0000\n> \n> From Git's side, the loop is something like:\n> \n>  while (delayed_items > 0) {\n> \t/* issue a wait, and get back the status/index pair */\n> \tstatus = send_wait(&index);\n> \tdelayed_items--;\n> \n> \t/*\n> \t * use \"index\" to find the right item in our list of files;\n> \t * the format can be opaque to the filter, so we could index\n> \t * it however we like. But probably numeric indices in an array\n> \t * are the simplest.\n> \t */\n> \tassert(index > 0 && index < nr_items);\n> \titem[index].status = status;\n> \tif (status == SUCCESS)\n> \t\tread_content(&item[index]);\n>  }\n> \n> and the filter side just attaches the \"index\" string to whatever its\n> internal queue structure is, and feeds it back verbatim when processing\n> that item finishes.\n\nThat could work! I had something like that in mind:\n\nI teach Git a new command \"list_completed\" or similar. The filter\nblocks this call until at least one item is ready for Git. \nThen the filter responds with a list of paths that identify the\n\"ready items\". Then Git asks for these ready items just with the\npath and not with any content. Could that work? Wouldn't the path\nbe \"unique\" to identify a blob per filter run?\n\nThanks,\nLars\n"},{"id":"312716","messageId":"20170227105330.gv5qk6dszrv56kmg@sigill.intra.peff.net","threadId":"45232","inReplyTo":"E4A6D866-D046-4CF3-8050-666FD6C98F7B@gmail.com","subject":"Re: [PATCH v2] convert: add \"status=delayed\" to filter process protocol","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-02-27T10:53:30Z","receivedAt":"2017-02-27T10:54:02Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Feb 27, 2017 at 11:32:47AM +0100, Lars Schneider wrote:\n\n> I completely agree - I need to change that. However, the goal of the v2\n> iteration was to get the \"convert\" interface in an acceptable state.\n> That's what I intended to say in the patch comment section:\n> \n>     \"Please ignore all changes behind async_convert_to_working_tree() and \n>      async_filter_finish() for now as I plan to change the implementation \n>      as soon as the interface is in an acceptable state.\"\n\nAh, sorry, I missed that. I would think the underlying approach would\ninfluence the interface to some degree. But as long as the interface\nis sufficiently abstract, I think it gives you enough flexibility.\n\n> > From Git's side, the loop is something like:\n> > \n> >  while (delayed_items > 0) {\n> > \t/* issue a wait, and get back the status/index pair */\n> > \tstatus = send_wait(&index);\n> > \tdelayed_items--;\n> > \n> > \t/*\n> > \t * use \"index\" to find the right item in our list of files;\n> > \t * the format can be opaque to the filter, so we could index\n> > \t * it however we like. But probably numeric indices in an array\n> > \t * are the simplest.\n> > \t */\n> > \tassert(index > 0 && index < nr_items);\n> > \titem[index].status = status;\n> > \tif (status == SUCCESS)\n> > \t\tread_content(&item[index]);\n> >  }\n> > \n> > and the filter side just attaches the \"index\" string to whatever its\n> > internal queue structure is, and feeds it back verbatim when processing\n> > that item finishes.\n> \n> That could work! I had something like that in mind:\n> \n> I teach Git a new command \"list_completed\" or similar. The filter\n> blocks this call until at least one item is ready for Git. \n> Then the filter responds with a list of paths that identify the\n> \"ready items\". Then Git asks for these ready items just with the\n> path and not with any content. Could that work? Wouldn't the path\n> be \"unique\" to identify a blob per filter run?\n\nI think that could work, though I think there are few minor downsides\ncompared to what I wrote above:\n\n  - if you respond with \"these items are ready\", and then make Git ask\n    for each again, it's an extra round-trip for each set of ready\n    items. You could just say \"an item is ready; here it is\" in a single\n    response. For a local pipe the latency is probably negligible,\n    though.\n\n  - using paths as the index would probably work, but it means Git has\n    to use the path to find the \"struct checkout_entry\" again. Which\n    might mean a hashmap (though if you have them all in a sorted list,\n    I guess you could also do a binary search).\n\n  - Using an explicit index communicates to the filter not only what the\n    index is, but also that Git is prepared to accept a delayed response\n    for the item. For backwards compatibility, the filter would probably\n    advertise \"I have the 'delayed' capability\", and then Git could\n    choose to use it or not on a per-item basis. Realistically it would\n    not change from item to item, but rather operation to operation. So\n    that means we can easily convert the call-sites in Git to the async\n    approach incrementally. As each one is converted, it turns on the\n    flag that causes the filter code to send the \"index\" tag.\n\n-Peff\n"},{"id":"312789","messageId":"7fbf63df-c61f-3d4b-99a8-eea52049709d@gmail.com","threadId":"45232","inReplyTo":"E4A6D866-D046-4CF3-8050-666FD6C98F7B@gmail.com","subject":"Re: [PATCH v2] convert: add \"status=delayed\" to filter process protocol","fromName":"Jakub Narębski","fromEmail":"jnareb@gmail.com","sentAt":"2017-02-27T22:11:13Z","receivedAt":"2017-02-27T22:21:16Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"W dniu 27.02.2017 o 11:32, Lars Schneider pisze:\n> \n>> On 27 Feb 2017, at 10:58, Jeff King <peff@peff.net> wrote:\n>>\n>> On Sun, Feb 26, 2017 at 07:48:16PM +0100, Lars Schneider wrote:\n>>\n>>> +If the request cannot be fulfilled within a reasonable amount of time\n>>> +then the filter can respond with a \"delayed\" status and a flush packet.\n>>> +Git will perform the same request at a later point in time, again. The\n>>> +filter can delay a response multiple times for a single request.\n>>> +------------------------\n>>> +packet:          git< status=delayed\n>>> +packet:          git< 0000\n>>> +------------------------\n\nIs it something that happens instead of filter process sending the contents\nof file, or is it something that happens after sending some part of the\ncontents (maybe empty) instead of empty list to keep \"status=success\"\nunchanged or instead of \"status=error\" if there was a problem processing\nfile?\n\n>>> +\n>>\n>> So Git just asks for the same content again? I see two issues with that:\n>>\n>>  1. Does git have to feed the blob content again? That can be expensive\n>>     to access or to keep around in memory.\n>>\n>>  2. What happens when the item isn't ready on the second request? I can\n>>     think of a few options:\n>>\n>>       a. The filter immediately says \"nope, still delayed\". But then\n>>          Git ends up busy-looping with \"is this one ready yet?\"\n>>\n>>       b. The filter blocks until the item is ready. But then if other\n>> \t  items _are_ ready, Git cannot work on processing them. We lose\n>> \t  parallelism.\n>>\n>>       c. You could do a hybrid: block until _some_ item is ready, and\n>>          then issue \"delayed\" responses for everything that isn't\n>> \t  ready. Then if you assume that Git is looping over and over\n>> \t  through the set of objects, it will either block or pick up\n>> \t  _something_ on each loop.\n>>\n>> \t  But it makes a quadratic number of requests in the worst case.\n>> \t  E.g., imagine you have N items and the last one is available\n>> \t  first, then the second-to-last, and so on. You'll ask N times,\n>> \t  then N-1, then N-2, and so on.\n\nThe current solution is a 'busy loop' one that I wrote about[1][2],\nsee below.\n\n> \n> I completely agree - I need to change that. However, the goal of the v2\n> iteration was to get the \"convert\" interface in an acceptable state.\n> That's what I intended to say in the patch comment section:\n> \n>     \"Please ignore all changes behind async_convert_to_working_tree() and \n>      async_filter_finish() for now as I plan to change the implementation \n>      as soon as the interface is in an acceptable state.\"\n\nI think that it is more important to start with a good abstraction,\nand the proposal for protocol, rather than getting bogged down in\nimplementation details that may change as the idea for protocol\nextension changes.\n\n>>\n>> I think it would be much more efficient to do something like:\n>>\n>>  [Git issues a request and gives it an opaque index id]\n>>  git> command=smudge\n>>  git> pathname=foo\n>>  git> index=0\n>>  git> 0000\n>>  git> CONTENT\n>>  git> 0000\n>>\n>>  [The data isn't ready yet, so the filter tells us so...]\n>>  git< status=delayed\n>>  git< 0000\n\nSo is it only as replacement for \"status=success\" + contents or\n\"status=abort\", that is upfront before sending any part of the file?\n\nOr, as one can assume from the point of the paragraph with the\n\"status=delayed\", it is about replacing null list for success or\n\"status=error\" after sending some part (maybe empty) of a file,\nthat is:\n\n    [filter driver says that it can process contents]\n    git< status=success\n    git< 0000\n    git< PARTIAL_SMUDGED_CONTENT (maybe empty)\n    [there was some delay, for example one of shards is slow]\n    git< 0000\n    git< status=delayed\n    git< 0000\n\n>>\n>>  [Git may make other requests, that are either served or delayed]\n>>  git> command=smudge\n>>  git> pathname=foo\n>>  git> index=1\n>>  git> 0000\n>>  git< status=success\n>>  git< 0000\n>>  git< CONTENT\n>>  git< 0000\n>>\n>>  [Now Git has processed all of the items, and each one either has its\n>>   final status, or has been marked as delayed. So we ask for a delayed\n>>   item]\n>>  git> command=wait\n>>  git> 0000\n\nIn my proposal[2] I have called this \"command=continue\"... but at this\npoint it is bikeshedding.  I think \"command=wait\" (or \"await\" ;-))\nmight be better.\n\n>>\n>>  [Some time may pass if nothing is ready. But eventually we get...]\n>>  git< status=success\n\nOr\n\n    git< status=resumed\n\nIf it would not be undue burden on the filter driver process, we might\nrequire for it to say where to continue at (in bytes), e.g.\n\n    git< from=16426\n\nThat should, of course, go below index/pathname line.\n\n>>  git< index=0\n\nOr a filter driver could have used pathname as an index, that is\n\n    git< pathname=path/testfile.dat\n\n>>  git< 0000\n>>  git< CONTENT\n>>  git< 0000\n>>\n>> From Git's side, the loop is something like:\n>>\n>>  while (delayed_items > 0) {\n>> \t/* issue a wait, and get back the status/index pair */\n>> \tstatus = send_wait(&index);\n>> \tdelayed_items--;\n\nThis looks like my 'event loop' proposal[1][2], see below.\n\n>>\n>> \t/*\n>> \t * use \"index\" to find the right item in our list of files;\n>> \t * the format can be opaque to the filter, so we could index\n>> \t * it however we like. But probably numeric indices in an array\n>> \t * are the simplest.\n>> \t */\n>> \tassert(index > 0 && index < nr_items);\n>> \titem[index].status = status;\n>> \tif (status == SUCCESS)\n>> \t\tread_content(&item[index]);\n>>  }\n>>\n>> and the filter side just attaches the \"index\" string to whatever its\n>> internal queue structure is, and feeds it back verbatim when processing\n>> that item finishes.\n\nI have wrote about this for v1 version of this patch series:\n\n[1]: https://public-inbox.org/git/ec8078ef-8ff2-d26f-ef73-5ef612737eee@gmail.com/\n[2]: https://public-inbox.org/git/17fa31a5-8689-2766-952b-704f433a5b3a@gmail.com/\n\n> \n> That could work! I had something like that in mind:\n> \n> I teach Git a new command \"list_completed\" or similar. The filter\n> blocks this call until at least one item is ready for Git. \n> Then the filter responds with a list of paths that identify the\n> \"ready items\". Then Git asks for these ready items just with the\n> path and not with any content. Could that work? Wouldn't the path\n> be \"unique\" to identify a blob per filter run?\n\nWhy in the \"drain\" phase it is still Git that needs to ask filter for\ncontents, one file after another?  Wouldn't it be easier and simpler\nfor filter to finish sending contents, and send signal that it has\nfinished continue'ing?\n\nTo summarize my earlier emails, current proposal looks for me as if\nit were a \"busy loop\" solution, that is[2]:\n\n          while there are delayed paths:\n                  for each delayed path:\n                          request path from filter [a]\n                          fetch the thing (supporting \"delayed\") [b]\n                          if path done\n                                  do the caller's thing to use buf\n                                  remove it from delayed paths list\n\n\nFootnotes:\n----------\na) We don't send the Git-side contents of blob again, isn't it?\n   So we need some protocol extension / new understanding anyway.\n   for example that we don't send contents if we request path again.\nb) If path is not ready at all, filter protocol would send status=delayed\n   with empty contents.  This means that we would immediately go to the\n   next path, if there is one.\n\nThere are some cases where busy loop is preferable, but I don't think\nit is the case here.\n\n\nThe \"event loop\" solution, which is I think what Peff proposed, would\nbe something like this (from the protocol point of view, rather than\nfrom the Git code point of view)[1]:\n\n        while there are delayed paths:\n                get path that is ready from filter\n                fetch the thing to buf (supporting \"delayed\")\n                if path done\n                        do the caller's thing to use buf \n                        (e.g. finish checkout path, eof convert, etc.)\n\nWe can either trust filter process to tell us when it finished sending\ndelayed paths, or keep list of paths that are being delayed in Git.\n\n\nAlso, one thing that we need to be solved, assuming that the proposed\nextension allows to send partial data from filter to be delayed and\ncontinued later, is that Git needs to keep this partial response in buf;\nthis is because of precedence of gitattributes applying:\n\n  Interaction between checkin/checkout attributes\n  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^\n  In the check-in codepath, the worktree file is first converted with\n  `filter` driver (if specified and corresponding driver defined), then\n  the result is processed with `ident` (if specified), and then finally\n  with `text` (again, if specified and applicable).\n\n  In the check-out codepath, the blob content is first converted with\n  `text`, and then `ident` and fed to `filter`.\n\nOr maybe I misunderstood something; I am a bit confused by check-in\nvs check-out there...\n\n\nNote that with support of \"command=wait\" / \"command=continue\" Git\ncould interrupt sending contents, and drain unfinished files, if\nit needs it... then continue sending rest of files.  Well, if the\nprotocol extension is interpreted to allow for this.\n\nBest regards,\n-- \nJakub Narębski\n\n"},{"id":"316438","messageId":"011147E3-3D2F-4260-9296-A60557A8FE3A@gmail.com","threadId":"45232","inReplyTo":"20170227105330.gv5qk6dszrv56kmg@sigill.intra.peff.net","subject":"Re: [PATCH v2] convert: add \"status=delayed\" to filter process protocol","fromName":"Lars Schneider","fromEmail":"larsxschneider@gmail.com","sentAt":"2017-04-09T18:17:54Z","receivedAt":"2017-04-09T18:18:03Z","isPatch":true,"sender":{"key":"larsxschneider@gmail.com","avatar":"https://avatars.githubusercontent.com/u/477434?v=4"},"body":"\n> On 27 Feb 2017, at 11:53, Jeff King <peff@peff.net> wrote:\n> \n> On Mon, Feb 27, 2017 at 11:32:47AM +0100, Lars Schneider wrote:\n> \n>> ...\n> \n>>> From Git's side, the loop is something like:\n>>> \n>>> while (delayed_items > 0) {\n>>> \t/* issue a wait, and get back the status/index pair */\n>>> \tstatus = send_wait(&index);\n>>> \tdelayed_items--;\n>>> \n>>> \t/*\n>>> \t * use \"index\" to find the right item in our list of files;\n>>> \t * the format can be opaque to the filter, so we could index\n>>> \t * it however we like. But probably numeric indices in an array\n>>> \t * are the simplest.\n>>> \t */\n>>> \tassert(index > 0 && index < nr_items);\n>>> \titem[index].status = status;\n>>> \tif (status == SUCCESS)\n>>> \t\tread_content(&item[index]);\n>>> }\n>>> \n>>> and the filter side just attaches the \"index\" string to whatever its\n>>> internal queue structure is, and feeds it back verbatim when processing\n>>> that item finishes.\n>> \n>> That could work! I had something like that in mind:\n>> \n>> I teach Git a new command \"list_completed\" or similar. The filter\n>> blocks this call until at least one item is ready for Git. \n>> Then the filter responds with a list of paths that identify the\n>> \"ready items\". Then Git asks for these ready items just with the\n>> path and not with any content. Could that work? Wouldn't the path\n>> be \"unique\" to identify a blob per filter run?\n> \n> I think that could work, though I think there are few minor downsides\n> compared to what I wrote above:\n> \n>  - if you respond with \"these items are ready\", and then make Git ask\n>    for each again, it's an extra round-trip for each set of ready\n>    items. You could just say \"an item is ready; here it is\" in a single\n>    response. For a local pipe the latency is probably negligible,\n>    though.\n\nIt is true that the extra round-trip is not strictly necessary but I think\nit simplifies the protocol/the code as I can reuse the convert machinery \nas is.\n\n\n>  - using paths as the index would probably work, but it means Git has\n>    to use the path to find the \"struct checkout_entry\" again. Which\n>    might mean a hashmap (though if you have them all in a sorted list,\n>    I guess you could also do a binary search).\n\nAgreed. I changed my implementation to use an index following your\nsuggestion.\n\n>  - Using an explicit index communicates to the filter not only what the\n>    index is, but also that Git is prepared to accept a delayed response\n>    for the item. For backwards compatibility, the filter would probably\n>    advertise \"I have the 'delayed' capability\", and then Git could\n>    choose to use it or not on a per-item basis. Realistically it would\n>    not change from item to item, but rather operation to operation. So\n>    that means we can easily convert the call-sites in Git to the async\n>    approach incrementally. As each one is converted, it turns on the\n>    flag that causes the filter code to send the \"index\" tag.\n\nAgreed. I change the implementation accordingly and I will send out the\npatches shortly.\n\nThanks,\nLars\n"},{"id":"316439","messageId":"6410F3B1-3A85-4B2C-9425-17912F57622B@gmail.com","threadId":"45232","inReplyTo":"7fbf63df-c61f-3d4b-99a8-eea52049709d@gmail.com","subject":"Re: [PATCH v2] convert: add \"status=delayed\" to filter process protocol","fromName":"Lars Schneider","fromEmail":"larsxschneider@gmail.com","sentAt":"2017-04-09T18:41:02Z","receivedAt":"2017-04-09T18:48:23Z","isPatch":true,"sender":{"key":"larsxschneider@gmail.com","avatar":"https://avatars.githubusercontent.com/u/477434?v=4"},"body":"\n> On 27 Feb 2017, at 23:11, Jakub Narębski <jnareb@gmail.com> wrote:\n> \n> W dniu 27.02.2017 o 11:32, Lars Schneider pisze:\n>> \n>>> On 27 Feb 2017, at 10:58, Jeff King <peff@peff.net> wrote:\n>>> \n>>> On Sun, Feb 26, 2017 at 07:48:16PM +0100, Lars Schneider wrote:\n>>> \n>>>> +If the request cannot be fulfilled within a reasonable amount of time\n>>>> +then the filter can respond with a \"delayed\" status and a flush packet.\n>>>> +Git will perform the same request at a later point in time, again. The\n>>>> +filter can delay a response multiple times for a single request.\n>>>> +------------------------\n>>>> +packet:          git< status=delayed\n>>>> +packet:          git< 0000\n>>>> +------------------------\n> \n> Is it something that happens instead of filter process sending the contents\n\nCorrect! I'll clarify this in v3!\n\n\n>> \n>> I completely agree - I need to change that. However, the goal of the v2\n>> iteration was to get the \"convert\" interface in an acceptable state.\n>> That's what I intended to say in the patch comment section:\n>> \n>>    \"Please ignore all changes behind async_convert_to_working_tree() and \n>>     async_filter_finish() for now as I plan to change the implementation \n>>     as soon as the interface is in an acceptable state.\"\n> \n> I think that it is more important to start with a good abstraction,\n> and the proposal for protocol, rather than getting bogged down in\n> implementation details that may change as the idea for protocol\n> extension changes.\n\nI'll send out v3 shortly as proposal for a complete solution.\n\n\n>>> I think it would be much more efficient to do something like:\n>>> \n>>> [Git issues a request and gives it an opaque index id]\n>>> git> command=smudge\n>>> git> pathname=foo\n>>> git> index=0\n>>> git> 0000\n>>> git> CONTENT\n>>> git> 0000\n>>> \n>>> [The data isn't ready yet, so the filter tells us so...]\n>>> git< status=delayed\n>>> git< 0000\n> \n> So is it only as replacement for \"status=success\" + contents or\n> \"status=abort\", that is upfront before sending any part of the file?\n\nYes.\n\n\n> Or, as one can assume from the point of the paragraph with the\n> \"status=delayed\", it is about replacing null list for success or\n> \"status=error\" after sending some part (maybe empty) of a file,\n> that is:\n\nNo. As this would complicate things I don't want to support it. \n(and I clarified that in the docs in v3).\n\n\n> If it would not be undue burden on the filter driver process, we might\n> require for it to say where to continue at (in bytes), e.g.\n> \n>    git< from=16426\n> \n> That should, of course, go below index/pathname line.\n\nThis would make the protocol even more complicated. That's why I don't\nwant to support splitting the response.\n\n\n>>> git< index=0\n> \n> Or a filter driver could have used pathname as an index, that is\n> \n>    git< pathname=path/testfile.dat\n\nIn v3 I've used an index to help Git finding the right cache entry\nquickly.\n\n\n> \n>>> git< 0000\n>>> git< CONTENT\n>>> git< 0000\n>>> \n>>> From Git's side, the loop is something like:\n>>> \n>>> while (delayed_items > 0) {\n>>> \t/* issue a wait, and get back the status/index pair */\n>>> \tstatus = send_wait(&index);\n>>> \tdelayed_items--;\n> \n> This looks like my 'event loop' proposal[1][2], see below.\n\nI implemented something similar in v3.\n\n\n>> That could work! I had something like that in mind:\n>> \n>> I teach Git a new command \"list_completed\" or similar. The filter\n>> blocks this call until at least one item is ready for Git. \n>> Then the filter responds with a list of paths that identify the\n>> \"ready items\". Then Git asks for these ready items just with the\n>> path and not with any content. Could that work? Wouldn't the path\n>> be \"unique\" to identify a blob per filter run?\n> \n> Why in the \"drain\" phase it is still Git that needs to ask filter for\n> contents, one file after another?  Wouldn't it be easier and simpler\n> for filter to finish sending contents, and send signal that it has\n> finished continue'ing?\n> \n> To summarize my earlier emails, current proposal looks for me as if\n> it were a \"busy loop\" solution, that is[2]:\n\nIn v3 the implementation still uses kind of a busy loop (I expect the\nfilter to block if there nothing ready, yet). An event loop would\ncomplicate the protocol as the filter would need to initiate an action.\nRight now only Git initiates actions.\n\n\n> Footnotes:\n> ----------\n> a) We don't send the Git-side contents of blob again, isn't it?\n>   So we need some protocol extension / new understanding anyway.\n>   for example that we don't send contents if we request path again.\nCorrect - v3 doesn't send the content again.\n\n\n> Also, one thing that we need to be solved, assuming that the proposed\n> extension allows to send partial data from filter to be delayed and\n> continued later, is that Git needs to keep this partial response in buf;\n> this is because of precedence of gitattributes applying:\n\nAs mentioned above I don't want to support partial data as this\ncomplicates things and is of no use for my Git LFS problem case.\n\nThanks,\nLars\n\n"}]}