{"thread":{"id":"44310","subject":"[PATCH v11 00/14] Git filter protocol","startedAt":"2016-10-16T23:20:53Z","lastAt":"2016-11-07T02:35:45Z","messageCount":25,"participants":["larsxschneider@gmail.com","Junio C Hamano","Johannes Sixt","Lars Schneider"],"isPatch":true,"patchVersion":11,"patchTotal":14},"messages":[{"id":"304235","messageId":"20161016232038.84951-1-larsxschneider@gmail.com","threadId":"44310","inReplyTo":null,"subject":"[PATCH v11 00/14] Git filter protocol","fromName":"","fromEmail":"larsxschneider@gmail.com","sentAt":"2016-10-16T23:20:24Z","receivedAt":"2016-10-16T23:20:53Z","isPatch":true,"sender":{"key":"larsxschneider@gmail.com","avatar":"https://avatars.githubusercontent.com/u/477434?v=4"},"body":"From: Lars Schneider \n\nThe goal of this series is to avoid launching a new clean/smudge filter\nprocess for each file that is filtered.\n\nA short summary about v1 to v5 can be found here:\nhttps://git.github.io/rev_news/2016/08/17/edition-18/\n\nThis series is also published on web:\nhttps://github.com/larsxschneider/git/pull/15\n\nPatches 1 and 2 are cleanups and not strictly necessary for the series.\nPatches 3 to 12 are required preparation. Patch 13 is the main patch.\nPatch 14 adds an example how to use the Git filter protocol in contrib.\n\nThanks a lot to\n Jakub, Junio, Ramsay, Dscho, Torsten and Peff\nfor very helpful reviews,\nLars\n\n## Changes since v10\n * clarify documentation (Jakub)\n * change \"=true\" to \"capability=\" in protocol (Jakub)\n * make stop_multi_file_filter() static (Ramsay)\n * add flush packet after version negotiation for consistency (Jakub)\n * fix pid_t translation error on Windows (Dscho)\n * fix Unquoted references in tests (Junio)\n * remove smudge invocation count in tests (Junio)\n\n\nLars Schneider (14):\n convert: quote filter names in error messages\n convert: modernize tests\n run-command: move check_pipe() from write_or_die to run_command\n run-command: add clean_on_exit_handler\n pkt-line: rename packet_write() to packet_write_fmt()\n pkt-line: extract set_packet_header()\n pkt-line: add packet_write_fmt_gently()\n pkt-line: add packet_flush_gently()\n pkt-line: add packet_write_gently()\n pkt-line: add functions to read/write flush terminated packet streams\n convert: make apply_filter() adhere to standard Git error handling\n convert: prepare filter..process option\n convert: add filter..process option\n contrib/long-running-filter: add long running filter example\n\n Documentation/gitattributes.txt | 157 ++++++++++-\n builtin/archive.c | 4 +-\n builtin/receive-pack.c | 4 +-\n builtin/remote-ext.c | 4 +-\n builtin/upload-archive.c | 4 +-\n connect.c | 2 +-\n contrib/long-running-filter/example.pl | 128 +++++++++\n convert.c | 375 +++++++++++++++++++++---\n daemon.c | 2 +-\n http-backend.c | 2 +-\n pkt-line.c | 152 +++++++++-\n pkt-line.h | 12 +-\n run-command.c | 39 ++-\n run-command.h | 4 +-\n shallow.c | 2 +-\n t/t0021-conversion.sh | 502 ++++++++++++++++++++++++++++++---\n t/t0021/rot13-filter.pl | 192 +++++++++++++\n upload-pack.c | 30 +-\n write_or_die.c | 13 -\n 19 files changed, 1495 insertions(+), 133 deletions(-)\n create mode 100755 contrib/long-running-filter/example.pl\n create mode 100755 t/t0021/rot13-filter.pl\n\n\n\n## Interdiff (v10..v11)\n\ndiff --git a/Documentation/gitattributes.txt b/Documentation/gitattributes.txt\nindex a182ef2..976243a 100644\n--- a/Documentation/gitattributes.txt\n+++ b/Documentation/gitattributes.txt\n@@ -297,9 +297,11 @@ upon checkin. By default these commands process only a single\n blob and terminate. If a long running `process` filter is used\n in place of `clean` and/or `smudge` filters, then Git can process\n all blobs with a single filter command invocation for the entire\n-life of a single Git command, for example `git add --all`. See\n-section below for the description of the protocol used to\n-communicate with a `process` filter.\n+life of a single Git command, for example `git add --all`. If a\n+long running `process` filter is configured then it always takes\n+precedence over a configured single blob filter. See section\n+below for the description of the protocol used to communicate with\n+a `process` filter.\n\n One use of the content filtering is to massage the content into a shape\n that is more convenient for the platform, filesystem, and the user to use.\n@@ -393,10 +395,10 @@ text and therefore are terminated by a LF.\n\n Git starts the filter when it encounters the first file\n that needs to be cleaned or smudged. After the filter started\n-Git sends a welcome message (\"git-filter-client\"), a list of\n-supported protocol version numbers, and a flush packet. Git expects\n-to read a welcome response message (\"git-filter-server\") and exactly\n-one protocol version number from the previously sent list. All further\n+Git sends a welcome message (\"git-filter-client\"), a list of supported\n+protocol version numbers, and a flush packet. Git expects to read a welcome\n+response message (\"git-filter-server\"), exactly one protocol version number\n+from the previously sent list, and a flush packet. All further\n communication will be based on the selected version. The remaining\n protocol description below documents \"version=2\". Please note that\n \"version=42\" in the example below does not exist and is only there\n@@ -414,12 +416,13 @@ packet: git> version=42\n packet: git> 0000\n packet: git< git-filter-server\n packet: git< version=2\n-packet: git> clean=true\n-packet: git> smudge=true\n-packet: git> not-yet-invented=true\n+packet: git< 0000\n+packet: git> capability=clean\n+packet: git> capability=smudge\n+packet: git> capability=not-yet-invented\n packet: git> 0000\n-packet: git< clean=true\n-packet: git< smudge=true\n+packet: git< capability=clean\n+packet: git< capability=smudge\n packet: git< 0000\n ------------------------\n Supported filter capabilities in version 2 are \"clean\" and\n@@ -428,7 +431,7 @@ Supported filter capabilities in version 2 are \"clean\" and\n Afterwards Git sends a list of \"key=value\" pairs terminated with\n a flush packet. The list will contain at least the filter command\n (based on the supported capabilities) and the pathname of the file\n-to filter relative to the repository root. Right after these packets\n+to filter relative to the repository root. Right after the flush packet\n Git sends the content split in zero or more pkt-line packets and a\n flush packet to terminate content. Please note, that the filter\n must not send any response before it received the content and the\n@@ -447,7 +450,10 @@ problems then the list must contain a \"success\" status. Right after\n these packets the filter is expected to send the content in zero\n or more pkt-line packets and a flush packet at the end. Finally, a\n second list of \"key=value\" pairs terminated with a flush packet\n-is expected. The filter can change the status in the second list.\n+is expected. The filter can change the status in the second list\n+or keep the status as is with an empty list. Please note that the\n+empty list must be terminated with a flush packet regardless.\n+\n ------------------------\n packet: git< status=success\n packet: git< 0000\n@@ -457,7 +463,7 @@ packet: git< 0000 # empty list, keep \"status=success\" unchanged!\n ------------------------\n\n If the result content is empty then the filter is expected to respond\n-with a \"success\" status and an empty list.\n+with a \"success\" status and a flush packet to signal the empty content.\n ------------------------\n packet: git< status=success\n packet: git< 0000\n@@ -466,9 +472,7 @@ packet: git< 0000 # empty list, keep \"status=success\" unchanged!\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. Depending on the\n-`filter..required` flag Git will interpret that as error\n-but it will not stop or restart the filter process.\n+it is expected to respond with an \"error\" status.\n ------------------------\n packet: git< status=error\n packet: git< 0000\n@@ -476,9 +480,7 @@ packet: git< 0000\n\n If the filter experiences an error during processing, then it can\n send the status \"error\" after the content was (partially or\n-completely) sent. Depending on the `filter..required` flag\n-Git will interpret that as error but it will not stop or restart the\n-filter process.\n+completely) sent.\n ------------------------\n packet: git< status=success\n packet: git< 0000\n@@ -488,31 +490,31 @@ packet: git< status=error\n packet: git< 0000\n ------------------------\n\n-If the filter dies during the communication or does not adhere to\n-the protocol then Git will stop the filter process and restart it\n-with the next file that needs to be processed. Depending on the\n-`filter..required` flag Git will interpret that as error.\n-\n-The error handling for all cases above mimic the behavior of\n-the `filter..clean` / `filter..smudge` error\n-handling.\n-\n In case the filter cannot or does not want to process the content\n as well as any future content for the lifetime of the Git process,\n-it is expected to respond with an \"abort\" status at any point in\n-the protocol. Depending on the `filter..required` flag Git\n-will interpret that as error for the content as well as any future\n-content for the lifetime of the Git process but it will not stop or\n-restart the filter process.\n+then it is expected to respond with an \"abort\" status at any point\n+in the protocol.\n ------------------------\n packet: git< status=abort\n packet: git< 0000\n ------------------------\n\n+Git neither stops nor restarts the filter process in case the\n+\"error\"/\"abort\" status is set. However, Git sets its exit code\n+according to the `filter..required` flag, mimicking the\n+behavior of the `filter..clean` / `filter..smudge`\n+mechanism.\n+\n+If the filter dies during the communication or does not adhere to\n+the protocol then Git will stop the filter process and restart it\n+with the next file that needs to be processed. Depending on the\n+`filter..required` flag Git will interpret that as error.\n+\n After the filter has processed a blob it is expected to wait for\n the next \"key=value\" list containing a command. Git will close\n the command pipe on exit. The filter is expected to detect EOF\n-and exit gracefully on its own.\n+and exit gracefully on its own. Git will wait until the filter\n+process has stopped.\n\n A long running filter demo implementation can be found in\n `contrib/long-running-filter/example.pl` located in the Git\n@@ -520,10 +522,6 @@ core repository. If you develop your own long running filter\n process then the `GIT_TRACE_PACKET` environment variables can be\n very helpful for debugging (see linkgit:git[1]).\n\n-If a `filter..process` command is configured then it\n-always takes precedence over a configured `filter..clean`\n-or `filter..smudge` command.\n-\n Please note that you cannot use an existing `filter..clean`\n or `filter..smudge` command with `filter..process`\n because the former two use a different inter process communication\ndiff --git a/contrib/long-running-filter/example.pl b/contrib/long-running-filter/example.pl\nindex f4102d2..3945705 100755\n--- a/contrib/long-running-filter/example.pl\n+++ b/contrib/long-running-filter/example.pl\n@@ -70,13 +70,14 @@ sub packet_flush {\n\n packet_txt_write(\"git-filter-server\");\n packet_txt_write(\"version=2\");\n+packet_flush();\n\n-( packet_txt_read() eq ( 0, \"clean=true\" ) ) || die \"bad capability\";\n-( packet_txt_read() eq ( 0, \"smudge=true\" ) ) || die \"bad capability\";\n+( packet_txt_read() eq ( 0, \"capability=clean\" ) ) || die \"bad capability\";\n+( packet_txt_read() eq ( 0, \"capability=smudge\" ) ) || die \"bad capability\";\n ( packet_bin_read() eq ( 1, \"\" ) ) || die \"bad capability end\";\n\n-packet_txt_write(\"clean=true\");\n-packet_txt_write(\"smudge=true\");\n+packet_txt_write(\"capability=clean\");\n+packet_txt_write(\"capability=smudge\");\n packet_flush();\n\n while (1) {\ndiff --git a/convert.c b/convert.c\nindex 1d89632..9d2aa68 100644\n--- a/convert.c\n+++ b/convert.c\n@@ -567,7 +567,7 @@ static void kill_multi_file_filter(struct hashmap *hashmap, struct cmd2process *\n \tfree(entry);\n }\n\n-void stop_multi_file_filter(struct child_process *process)\n+static void stop_multi_file_filter(struct child_process *process)\n {\n \tsigchain_push(SIGPIPE, SIG_IGN);\n \t/* Closing the pipe signals the filter to initiate a shutdown. */\n@@ -622,8 +622,11 @@ static struct cmd2process *start_multi_file_filter(struct hashmap *hashmap, cons\n \terr = strcmp(packet_read_line(process->out, NULL), \"version=2\");\n \tif (err)\n \t\tgoto done;\n+\terr = packet_read_line(process->out, NULL) != NULL;\n+\tif (err)\n+\t\tgoto done;\n\n-\terr = packet_write_list(process->in, \"clean=true\", \"smudge=true\", NULL);\n+\terr = packet_write_list(process->in, \"capability=clean\", \"capability=smudge\", NULL);\n\n \tfor (;;) {\n \t\tcap_buf = packet_read_line(process->out, NULL);\n@@ -631,10 +634,10 @@ static struct cmd2process *start_multi_file_filter(struct hashmap *hashmap, cons\n \t\t\tbreak;\n \t\tstring_list_split_in_place(&cap_list, cap_buf, '=', 1);\n\n-\t\tif (cap_list.nr != 2 || strcmp(cap_list.items[1].string, \"true\"))\n+\t\tif (cap_list.nr != 2 || strcmp(cap_list.items[0].string, \"capability\"))\n \t\t\tcontinue;\n\n-\t\tcap_name = cap_list.items[0].string;\n+\t\tcap_name = cap_list.items[1].string;\n \t\tif (!strcmp(cap_name, \"clean\")) {\n \t\t\tentry->supported_capabilities |= CAP_CLEAN;\n \t\t} else if (!strcmp(cap_name, \"smudge\")) {\ndiff --git a/run-command.c b/run-command.c\nindex e5fd6ff..ca905a9 100644\n--- a/run-command.c\n+++ b/run-command.c\n@@ -36,7 +36,10 @@ static void cleanup_children(int sig, int in_signal)\n \t\tif (p->process && !in_signal) {\n \t\t\tstruct child_process *process = p->process;\n \t\t\tif (process->clean_on_exit_handler) {\n-\t\t\t\ttrace_printf(\"trace: run_command: running exit handler for pid %d\", p->pid);\n+\t\t\t\ttrace_printf(\n+\t\t\t\t\t\"trace: run_command: running exit handler for pid %\"\n+\t\t\t\t\tPRIuMAX, (uintmax_t)p->pid\n+\t\t\t\t);\n \t\t\t\tprocess->clean_on_exit_handler(process);\n \t\t\t}\n \t\t}\ndiff --git a/t/t0021-conversion.sh b/t/t0021-conversion.sh\nindex 9f892c0..a20b9f5 100755\n--- a/t/t0021-conversion.sh\n+++ b/t/t0021-conversion.sh\n@@ -31,38 +31,35 @@ filter_git () {\n \trm -f git-stderr.log\n }\n\n-# Count unique lines in two files and compare them.\n-test_cmp_count () {\n-\tfor FILE in $@\n-\tdo\n-\t\tsort $FILE | uniq -c | sed \"s/^[ ]*//\" >$FILE.tmp\n-\t\tcat $FILE.tmp >$FILE\n-\tdone &&\n-\ttest_cmp $@\n-}\n-\n-# Count unique lines except clean invocations in two files and compare\n-# them. Clean invocations are not counted because their number can vary.\n+# Compare two files and ensure that `clean` and `smudge` respectively are\n+# called at least once if specified in the `expect` file. The actual\n+# invocation count is not relevant because their number can vary.\n # c.f. http://public-inbox.org/git/xmqqshv18i8i.fsf@gitster.mtv.corp.google.com/\n-test_cmp_count_except_clean () {\n-\tfor FILE in $@\n+test_cmp_count () {\n+\texpect=$1\n+\tactual=$2\n+\tfor FILE in \"$expect\" \"$actual\"\n \tdo\n-\t\tsort $FILE | uniq -c | sed \"s/^[ ]*//\" |\n-\t\t\tsed \"s/^\\([0-9]\\) IN: clean/x IN: clean/\" >$FILE.tmp\n-\t\tcat $FILE.tmp >$FILE\n+\t\tsort \"$FILE\" | uniq -c | sed \"s/^[ ]*//\" |\n+\t\t\tsed \"s/^\\([0-9]\\) IN: clean/x IN: clean/\" |\n+\t\t\tsed \"s/^\\([0-9]\\) IN: smudge/x IN: smudge/\" >\"$FILE.tmp\" &&\n+\t\tmv \"$FILE.tmp\" \"$FILE\"\n \tdone &&\n-\ttest_cmp $@\n+\ttest_cmp \"$expect\" \"$actual\"\n }\n\n-# Compare two files but exclude clean invocations because they can vary.\n+# Compare two files but exclude all `clean` invocations because Git can\n+# call `clean` zero or more times.\n # c.f. http://public-inbox.org/git/xmqqshv18i8i.fsf@gitster.mtv.corp.google.com/\n test_cmp_exclude_clean () {\n-\tfor FILE in $@\n+\texpect=$1\n+\tactual=$2\n+\tfor FILE in \"$expect\" \"$actual\"\n \tdo\n-\t\tgrep -v \"IN: clean\" $FILE >$FILE.tmp\n-\t\tcat $FILE.tmp >$FILE\n+\t\tgrep -v \"IN: clean\" \"$FILE\" >\"$FILE.tmp\" &&\n+\t\tmv \"$FILE.tmp\" \"$FILE\"\n \tdone &&\n-\ttest_cmp $@\n+\ttest_cmp \"$expect\" \"$actual\"\n }\n\n # Check that the contents of two files are equal and that their rot13 version\n@@ -395,7 +392,7 @@ test_expect_success PERL 'required process filter should filter data' '\n \t\t\tIN: clean testsubdir/test3 '\\''sq'\\'',\\$x.r $S3 [OK] -- OUT: $S3 . [OK]\n \t\t\tSTOP\n \t\tEOF\n-\t\ttest_cmp_count_except_clean expected.log rot13-filter.log &&\n+\t\ttest_cmp_count expected.log rot13-filter.log &&\n\n \t\trm -f test2.r \"testsubdir/test3 '\\''sq'\\'',\\$x.r\" &&\n\ndiff --git a/t/t0021/rot13-filter.pl b/t/t0021/rot13-filter.pl\nindex 1a6959c..ae4c50f 100755\n--- a/t/t0021/rot13-filter.pl\n+++ b/t/t0021/rot13-filter.pl\n@@ -95,13 +95,14 @@ $debug->flush();\n\n packet_txt_write(\"git-filter-server\");\n packet_txt_write(\"version=2\");\n+packet_flush();\n\n-( packet_txt_read() eq ( 0, \"clean=true\" ) ) || die \"bad capability\";\n-( packet_txt_read() eq ( 0, \"smudge=true\" ) ) || die \"bad capability\";\n+( packet_txt_read() eq ( 0, \"capability=clean\" ) ) || die \"bad capability\";\n+( packet_txt_read() eq ( 0, \"capability=smudge\" ) ) || die \"bad capability\";\n ( packet_bin_read() eq ( 1, \"\" ) ) || die \"bad capability end\";\n\n foreach (@capabilities) {\n-\tpacket_txt_write( $_ . \"=true\" );\n+\tpacket_txt_write( \"capability=\" . $_ );\n }\n packet_flush();\n print $debug \"init handshake complete\\n\";\n\n--\n2.10.0\n\n"},{"id":"304236","messageId":"20161016232038.84951-2-larsxschneider@gmail.com","threadId":"44310","inReplyTo":"20161016232038.84951-1-larsxschneider@gmail.com","subject":"[PATCH v11 01/14] convert: quote filter names in error messages","fromName":"","fromEmail":"larsxschneider@gmail.com","sentAt":"2016-10-16T23:20:25Z","receivedAt":"2016-10-16T23:20:56Z","isPatch":true,"sender":{"key":"larsxschneider@gmail.com","avatar":"https://avatars.githubusercontent.com/u/477434?v=4"},"body":"From: Lars Schneider \n\nGit filter driver commands with spaces (e.g. `filter.sh foo`) are hard\nto read in error messages. Quote them to improve the readability.\n\nSigned-off-by: Lars Schneider \nSigned-off-by: Junio C Hamano \n---\n convert.c | 12 ++++++------\n 1 file changed, 6 insertions(+), 6 deletions(-)\n\ndiff --git a/convert.c b/convert.c\nindex 077f5e6..986c239 100644\n--- a/convert.c\n+++ b/convert.c\n@@ -412,7 +412,7 @@ static int filter_buffer_or_fd(int in, int out, void *data)\n \tchild_process.out = out;\n \n \tif (start_command(&child_process))\n-\t\treturn error(\"cannot fork to run external filter %s\", params->cmd);\n+\t\treturn error(\"cannot fork to run external filter '%s'\", params->cmd);\n \n \tsigchain_push(SIGPIPE, SIG_IGN);\n \n@@ -430,13 +430,13 @@ static int filter_buffer_or_fd(int in, int out, void *data)\n \tif (close(child_process.in))\n \t\twrite_err = 1;\n \tif (write_err)\n-\t\terror(\"cannot feed the input to external filter %s\", params->cmd);\n+\t\terror(\"cannot feed the input to external filter '%s'\", params->cmd);\n \n \tsigchain_pop(SIGPIPE);\n \n \tstatus = finish_command(&child_process);\n \tif (status)\n-\t\terror(\"external filter %s failed %d\", params->cmd, status);\n+\t\terror(\"external filter '%s' failed %d\", params->cmd, status);\n \n \tstrbuf_release(&cmd);\n \treturn (write_err || status);\n@@ -477,15 +477,15 @@ static int apply_filter(const char *path, const char *src, size_t len, int fd,\n \t\treturn 0;\t/* error was already reported */\n \n \tif (strbuf_read(&nbuf, async.out, len) < 0) {\n-\t\terror(\"read from external filter %s failed\", cmd);\n+\t\terror(\"read from external filter '%s' failed\", cmd);\n \t\tret = 0;\n \t}\n \tif (close(async.out)) {\n-\t\terror(\"read from external filter %s failed\", cmd);\n+\t\terror(\"read from external filter '%s' failed\", cmd);\n \t\tret = 0;\n \t}\n \tif (finish_async(&async)) {\n-\t\terror(\"external filter %s failed\", cmd);\n+\t\terror(\"external filter '%s' failed\", cmd);\n \t\tret = 0;\n \t}\n \n-- \n2.10.0\n\n"},{"id":"304237","messageId":"20161016232038.84951-3-larsxschneider@gmail.com","threadId":"44310","inReplyTo":"20161016232038.84951-1-larsxschneider@gmail.com","subject":"[PATCH v11 02/14] convert: modernize tests","fromName":"","fromEmail":"larsxschneider@gmail.com","sentAt":"2016-10-16T23:20:26Z","receivedAt":"2016-10-16T23:20:59Z","isPatch":true,"sender":{"key":"larsxschneider@gmail.com","avatar":"https://avatars.githubusercontent.com/u/477434?v=4"},"body":"From: Lars Schneider \n\nUse `test_config` to set the config, check that files are empty with\n`test_must_be_empty`, compare files with `test_cmp`, and remove spaces\nafter \">\" and \"<\".\n\nPlease note that the \"rot13\" filter configured in \"setup\" keeps using\n`git config` instead of `test_config` because subsequent tests might\ndepend on it.\n\nReviewed-by: Stefan Beller \nSigned-off-by: Lars Schneider \nSigned-off-by: Junio C Hamano \n---\n t/t0021-conversion.sh | 58 +++++++++++++++++++++++++--------------------------\n 1 file changed, 29 insertions(+), 29 deletions(-)\n\ndiff --git a/t/t0021-conversion.sh b/t/t0021-conversion.sh\nindex e799e59..dc50938 100755\n--- a/t/t0021-conversion.sh\n+++ b/t/t0021-conversion.sh\n@@ -38,8 +38,8 @@ script='s/^\\$Id: \\([0-9a-f]*\\) \\$/\\1/p'\n \n test_expect_success check '\n \n-\tcmp test.o test &&\n-\tcmp test.o test.t &&\n+\ttest_cmp test.o test &&\n+\ttest_cmp test.o test.t &&\n \n \t# ident should be stripped in the repository\n \tgit diff --raw --exit-code :test :test.i &&\n@@ -47,10 +47,10 @@ test_expect_success check '\n \tembedded=$(sed -ne \"$script\" test.i) &&\n \ttest \"z$id\" = \"z$embedded\" &&\n \n-\tgit cat-file blob :test.t > test.r &&\n+\tgit cat-file blob :test.t >test.r &&\n \n-\t./rot13.sh < test.o > test.t &&\n-\tcmp test.r test.t\n+\t./rot13.sh test.t &&\n+\ttest_cmp test.r test.t\n '\n \n # If an expanded ident ever gets into the repository, we want to make sure that\n@@ -130,7 +130,7 @@ test_expect_success 'filter shell-escaped filenames' '\n \n \t# delete the files and check them out again, using a smudge filter\n \t# that will count the args and echo the command-line back to us\n-\tgit config filter.argc.smudge \"sh ./argc.sh %f\" &&\n+\ttest_config filter.argc.smudge \"sh ./argc.sh %f\" &&\n \trm \"$normal\" \"$special\" &&\n \tgit checkout -- \"$normal\" \"$special\" &&\n \n@@ -141,7 +141,7 @@ test_expect_success 'filter shell-escaped filenames' '\n \ttest_cmp expect \"$special\" &&\n \n \t# do the same thing, but with more args in the filter expression\n-\tgit config filter.argc.smudge \"sh ./argc.sh %f --my-extra-arg\" &&\n+\ttest_config filter.argc.smudge \"sh ./argc.sh %f --my-extra-arg\" &&\n \trm \"$normal\" \"$special\" &&\n \tgit checkout -- \"$normal\" \"$special\" &&\n \n@@ -154,9 +154,9 @@ test_expect_success 'filter shell-escaped filenames' '\n '\n \n test_expect_success 'required filter should filter data' '\n-\tgit config filter.required.smudge ./rot13.sh &&\n-\tgit config filter.required.clean ./rot13.sh &&\n-\tgit config filter.required.required true &&\n+\ttest_config filter.required.smudge ./rot13.sh &&\n+\ttest_config filter.required.clean ./rot13.sh &&\n+\ttest_config filter.required.required true &&\n \n \techo \"*.r filter=required\" >.gitattributes &&\n \n@@ -165,17 +165,17 @@ test_expect_success 'required filter should filter data' '\n \n \trm -f test.r &&\n \tgit checkout -- test.r &&\n-\tcmp test.o test.r &&\n+\ttest_cmp test.o test.r &&\n \n \t./rot13.sh expected &&\n \tgit cat-file blob :test.r >actual &&\n-\tcmp expected actual\n+\ttest_cmp expected actual\n '\n \n test_expect_success 'required filter smudge failure' '\n-\tgit config filter.failsmudge.smudge false &&\n-\tgit config filter.failsmudge.clean cat &&\n-\tgit config filter.failsmudge.required true &&\n+\ttest_config filter.failsmudge.smudge false &&\n+\ttest_config filter.failsmudge.clean cat &&\n+\ttest_config filter.failsmudge.required true &&\n \n \techo \"*.fs filter=failsmudge\" >.gitattributes &&\n \n@@ -186,9 +186,9 @@ test_expect_success 'required filter smudge failure' '\n '\n \n test_expect_success 'required filter clean failure' '\n-\tgit config filter.failclean.smudge cat &&\n-\tgit config filter.failclean.clean false &&\n-\tgit config filter.failclean.required true &&\n+\ttest_config filter.failclean.smudge cat &&\n+\ttest_config filter.failclean.clean false &&\n+\ttest_config filter.failclean.required true &&\n \n \techo \"*.fc filter=failclean\" >.gitattributes &&\n \n@@ -197,8 +197,8 @@ test_expect_success 'required filter clean failure' '\n '\n \n test_expect_success 'filtering large input to small output should use little memory' '\n-\tgit config filter.devnull.clean \"cat >/dev/null\" &&\n-\tgit config filter.devnull.required true &&\n+\ttest_config filter.devnull.clean \"cat >/dev/null\" &&\n+\ttest_config filter.devnull.required true &&\n \tfor i in $(test_seq 1 30); do printf \"%1048576d\" 1; done >30MB &&\n \techo \"30MB filter=devnull\" >.gitattributes &&\n \tGIT_MMAP_LIMIT=1m GIT_ALLOC_LIMIT=1m git add 30MB\n@@ -207,7 +207,7 @@ test_expect_success 'filtering large input to small output should use little mem\n test_expect_success 'filter that does not read is fine' '\n \ttest-genrandom foo $((128 * 1024 + 1)) >big &&\n \techo \"big filter=epipe\" >.gitattributes &&\n-\tgit config filter.epipe.clean \"echo xyzzy\" &&\n+\ttest_config filter.epipe.clean \"echo xyzzy\" &&\n \tgit add big &&\n \tgit cat-file blob :big >actual &&\n \techo xyzzy >expect &&\n@@ -215,20 +215,20 @@ test_expect_success 'filter that does not read is fine' '\n '\n \n test_expect_success EXPENSIVE 'filter large file' '\n-\tgit config filter.largefile.smudge cat &&\n-\tgit config filter.largefile.clean cat &&\n+\ttest_config filter.largefile.smudge cat &&\n+\ttest_config filter.largefile.clean cat &&\n \tfor i in $(test_seq 1 2048); do printf \"%1048576d\" 1; done >2GB &&\n \techo \"2GB filter=largefile\" >.gitattributes &&\n \tgit add 2GB 2>err &&\n-\t! test -s err &&\n+\ttest_must_be_empty err &&\n \trm -f 2GB &&\n \tgit checkout -- 2GB 2>err &&\n-\t! test -s err\n+\ttest_must_be_empty err\n '\n \n test_expect_success \"filter: clean empty file\" '\n-\tgit config filter.in-repo-header.clean \"echo cleaned && cat\" &&\n-\tgit config filter.in-repo-header.smudge \"sed 1d\" &&\n+\ttest_config filter.in-repo-header.clean \"echo cleaned && cat\" &&\n+\ttest_config filter.in-repo-header.smudge \"sed 1d\" &&\n \n \techo \"empty-in-worktree filter=in-repo-header\" >>.gitattributes &&\n \t>empty-in-worktree &&\n@@ -240,8 +240,8 @@ test_expect_success \"filter: clean empty file\" '\n '\n \n test_expect_success \"filter: smudge empty file\" '\n-\tgit config filter.empty-in-repo.clean \"cat >/dev/null\" &&\n-\tgit config filter.empty-in-repo.smudge \"echo smudged && cat\" &&\n+\ttest_config filter.empty-in-repo.clean \"cat >/dev/null\" &&\n+\ttest_config filter.empty-in-repo.smudge \"echo smudged && cat\" &&\n \n \techo \"empty-in-repo filter=empty-in-repo\" >>.gitattributes &&\n \techo dead data walking >empty-in-repo &&\n-- \n2.10.0\n\n"},{"id":"304238","messageId":"20161016232038.84951-4-larsxschneider@gmail.com","threadId":"44310","inReplyTo":"20161016232038.84951-1-larsxschneider@gmail.com","subject":"[PATCH v11 03/14] run-command: move check_pipe() from write_or_die to run_command","fromName":"","fromEmail":"larsxschneider@gmail.com","sentAt":"2016-10-16T23:20:27Z","receivedAt":"2016-10-16T23:21:02Z","isPatch":true,"sender":{"key":"larsxschneider@gmail.com","avatar":"https://avatars.githubusercontent.com/u/477434?v=4"},"body":"From: Lars Schneider \n\nMove check_pipe() to run_command and make it public. This is necessary\nto call the function from pkt-line in a subsequent patch.\n\nWhile at it, make async_exit() static to run_command.c as it is no\nlonger used from outside.\n\nSigned-off-by: Lars Schneider \nSigned-off-by: Ramsay Jones \nSigned-off-by: Junio C Hamano \n---\n run-command.c | 17 +++++++++++++++--\n run-command.h | 2 +-\n write_or_die.c | 13 -------------\n 3 files changed, 16 insertions(+), 16 deletions(-)\n\ndiff --git a/run-command.c b/run-command.c\nindex 5a4dbb6..3269362 100644\n--- a/run-command.c\n+++ b/run-command.c\n@@ -634,7 +634,7 @@ int in_async(void)\n \treturn !pthread_equal(main_thread, pthread_self());\n }\n \n-void NORETURN async_exit(int code)\n+static void NORETURN async_exit(int code)\n {\n \tpthread_exit((void *)(intptr_t)code);\n }\n@@ -684,13 +684,26 @@ int in_async(void)\n \treturn process_is_async;\n }\n \n-void NORETURN async_exit(int code)\n+static void NORETURN async_exit(int code)\n {\n \texit(code);\n }\n \n #endif\n \n+void check_pipe(int err)\n+{\n+\tif (err == EPIPE) {\n+\t\tif (in_async())\n+\t\t\tasync_exit(141);\n+\n+\t\tsignal(SIGPIPE, SIG_DFL);\n+\t\traise(SIGPIPE);\n+\t\t/* Should never happen, but just in case... */\n+\t\texit(141);\n+\t}\n+}\n+\n int start_async(struct async *async)\n {\n \tint need_in, need_out;\ndiff --git a/run-command.h b/run-command.h\nindex 5066649..cf29a31 100644\n--- a/run-command.h\n+++ b/run-command.h\n@@ -139,7 +139,7 @@ struct async {\n int start_async(struct async *async);\n int finish_async(struct async *async);\n int in_async(void);\n-void NORETURN async_exit(int code);\n+void check_pipe(int err);\n \n /**\n * This callback should initialize the child process and preload the\ndiff --git a/write_or_die.c b/write_or_die.c\nindex 0734432..eab8c8d 100644\n--- a/write_or_die.c\n+++ b/write_or_die.c\n@@ -1,19 +1,6 @@\n #include \"cache.h\"\n #include \"run-command.h\"\n \n-static void check_pipe(int err)\n-{\n-\tif (err == EPIPE) {\n-\t\tif (in_async())\n-\t\t\tasync_exit(141);\n-\n-\t\tsignal(SIGPIPE, SIG_DFL);\n-\t\traise(SIGPIPE);\n-\t\t/* Should never happen, but just in case... */\n-\t\texit(141);\n-\t}\n-}\n-\n /*\n * Some cases use stdio, but want to flush after the write\n * to get error handling (and to get better interactive\n-- \n2.10.0\n\n"},{"id":"304239","messageId":"20161016232038.84951-5-larsxschneider@gmail.com","threadId":"44310","inReplyTo":"20161016232038.84951-1-larsxschneider@gmail.com","subject":"[PATCH v11 04/14] run-command: add clean_on_exit_handler","fromName":"","fromEmail":"larsxschneider@gmail.com","sentAt":"2016-10-16T23:20:28Z","receivedAt":"2016-10-16T23:21:05Z","isPatch":true,"sender":{"key":"larsxschneider@gmail.com","avatar":"https://avatars.githubusercontent.com/u/477434?v=4"},"body":"From: Lars Schneider \n\nSome processes might want to perform cleanup tasks before Git kills them\ndue to the 'clean_on_exit' flag. Let's give them an interface for doing\nthis. The feature is used in a subsequent patch.\n\nPlease note, that the cleanup callback is not executed if Git dies of a\nsignal. The reason is that only \"async-signal-safe\" functions would be\nallowed to be call in that case. Since we cannot control what functions\nthe callback will use, we will not support the case. See 507d7804 for\nmore details.\n\nHelped-by: Johannes Sixt \nSigned-off-by: Lars Schneider \n---\n run-command.c | 22 ++++++++++++++++++----\n run-command.h | 2 ++\n 2 files changed, 20 insertions(+), 4 deletions(-)\n\ndiff --git a/run-command.c b/run-command.c\nindex 3269362..ca905a9 100644\n--- a/run-command.c\n+++ b/run-command.c\n@@ -21,6 +21,7 @@ void child_process_clear(struct child_process *child)\n \n struct child_to_clean {\n \tpid_t pid;\n+\tstruct child_process *process;\n \tstruct child_to_clean *next;\n };\n static struct child_to_clean *children_to_clean;\n@@ -31,6 +32,18 @@ static void cleanup_children(int sig, int in_signal)\n \twhile (children_to_clean) {\n \t\tstruct child_to_clean *p = children_to_clean;\n \t\tchildren_to_clean = p->next;\n+\n+\t\tif (p->process && !in_signal) {\n+\t\t\tstruct child_process *process = p->process;\n+\t\t\tif (process->clean_on_exit_handler) {\n+\t\t\t\ttrace_printf(\n+\t\t\t\t\t\"trace: run_command: running exit handler for pid %\"\n+\t\t\t\t\tPRIuMAX, (uintmax_t)p->pid\n+\t\t\t\t);\n+\t\t\t\tprocess->clean_on_exit_handler(process);\n+\t\t\t}\n+\t\t}\n+\n \t\tkill(p->pid, sig);\n \t\tif (!in_signal)\n \t\t\tfree(p);\n@@ -49,10 +62,11 @@ static void cleanup_children_on_exit(void)\n \tcleanup_children(SIGTERM, 0);\n }\n \n-static void mark_child_for_cleanup(pid_t pid)\n+static void mark_child_for_cleanup(pid_t pid, struct child_process *process)\n {\n \tstruct child_to_clean *p = xmalloc(sizeof(*p));\n \tp->pid = pid;\n+\tp->process = process;\n \tp->next = children_to_clean;\n \tchildren_to_clean = p;\n \n@@ -422,7 +436,7 @@ int start_command(struct child_process *cmd)\n \tif (cmd->pid < 0)\n \t\terror_errno(\"cannot fork() for %s\", cmd->argv[0]);\n \telse if (cmd->clean_on_exit)\n-\t\tmark_child_for_cleanup(cmd->pid);\n+\t\tmark_child_for_cleanup(cmd->pid, cmd);\n \n \t/*\n \t * Wait for child's execvp. If the execvp succeeds (or if fork()\n@@ -483,7 +497,7 @@ int start_command(struct child_process *cmd)\n \tif (cmd->pid < 0 && (!cmd->silent_exec_failure || errno != ENOENT))\n \t\terror_errno(\"cannot spawn %s\", cmd->argv[0]);\n \tif (cmd->clean_on_exit && cmd->pid >= 0)\n-\t\tmark_child_for_cleanup(cmd->pid);\n+\t\tmark_child_for_cleanup(cmd->pid, cmd);\n \n \targv_array_clear(&nargv);\n \tcmd->argv = sargv;\n@@ -765,7 +779,7 @@ int start_async(struct async *async)\n \t\texit(!!async->proc(proc_in, proc_out, async->data));\n \t}\n \n-\tmark_child_for_cleanup(async->pid);\n+\tmark_child_for_cleanup(async->pid, NULL);\n \n \tif (need_in)\n \t\tclose(fdin[0]);\ndiff --git a/run-command.h b/run-command.h\nindex cf29a31..dd1c78c 100644\n--- a/run-command.h\n+++ b/run-command.h\n@@ -43,6 +43,8 @@ struct child_process {\n \tunsigned stdout_to_stderr:1;\n \tunsigned use_shell:1;\n \tunsigned clean_on_exit:1;\n+\tvoid (*clean_on_exit_handler)(struct child_process *process);\n+\tvoid *clean_on_exit_handler_cbdata;\n };\n \n #define CHILD_PROCESS_INIT { NULL, ARGV_ARRAY_INIT, ARGV_ARRAY_INIT }\n-- \n2.10.0\n\n"},{"id":"304240","messageId":"20161016232038.84951-6-larsxschneider@gmail.com","threadId":"44310","inReplyTo":"20161016232038.84951-1-larsxschneider@gmail.com","subject":"[PATCH v11 05/14] pkt-line: rename packet_write() to packet_write_fmt()","fromName":"","fromEmail":"larsxschneider@gmail.com","sentAt":"2016-10-16T23:20:29Z","receivedAt":"2016-10-16T23:21:10Z","isPatch":true,"sender":{"key":"larsxschneider@gmail.com","avatar":"https://avatars.githubusercontent.com/u/477434?v=4"},"body":"From: Lars Schneider \n\npacket_write() should be called packet_write_fmt() because it is a\nprintf-like function that takes a format string as first parameter.\n\npacket_write_fmt() should be used for text strings only. Arbitrary\nbinary data should use a new packet_write() function that is introduced\nin a subsequent patch.\n\nSuggested-by: Junio C Hamano \nSigned-off-by: Lars Schneider \nSigned-off-by: Junio C Hamano \n---\n builtin/archive.c | 4 ++--\n builtin/receive-pack.c | 4 ++--\n builtin/remote-ext.c | 4 ++--\n builtin/upload-archive.c | 4 ++--\n connect.c | 2 +-\n daemon.c | 2 +-\n http-backend.c | 2 +-\n pkt-line.c | 2 +-\n pkt-line.h | 2 +-\n shallow.c | 2 +-\n upload-pack.c | 30 +++++++++++++++---------------\n 11 files changed, 29 insertions(+), 29 deletions(-)\n\ndiff --git a/builtin/archive.c b/builtin/archive.c\nindex a1e3b94..49f4914 100644\n--- a/builtin/archive.c\n+++ b/builtin/archive.c\n@@ -47,10 +47,10 @@ static int run_remote_archiver(int argc, const char **argv,\n \tif (name_hint) {\n \t\tconst char *format = archive_format_from_filename(name_hint);\n \t\tif (format)\n-\t\t\tpacket_write(fd[1], \"argument --format=%s\\n\", format);\n+\t\t\tpacket_write_fmt(fd[1], \"argument --format=%s\\n\", format);\n \t}\n \tfor (i = 1; i < argc; i++)\n-\t\tpacket_write(fd[1], \"argument %s\\n\", argv[i]);\n+\t\tpacket_write_fmt(fd[1], \"argument %s\\n\", argv[i]);\n \tpacket_flush(fd[1]);\n \n \tbuf = packet_read_line(fd[0], NULL);\ndiff --git a/builtin/receive-pack.c b/builtin/receive-pack.c\nindex 011db00..1ce7682 100644\n--- a/builtin/receive-pack.c\n+++ b/builtin/receive-pack.c\n@@ -218,7 +218,7 @@ static int receive_pack_config(const char *var, const char *value, void *cb)\n static void show_ref(const char *path, const unsigned char *sha1)\n {\n \tif (sent_capabilities) {\n-\t\tpacket_write(1, \"%s %s\\n\", sha1_to_hex(sha1), path);\n+\t\tpacket_write_fmt(1, \"%s %s\\n\", sha1_to_hex(sha1), path);\n \t} else {\n \t\tstruct strbuf cap = STRBUF_INIT;\n \n@@ -233,7 +233,7 @@ static void show_ref(const char *path, const unsigned char *sha1)\n \t\tif (advertise_push_options)\n \t\t\tstrbuf_addstr(&cap, \" push-options\");\n \t\tstrbuf_addf(&cap, \" agent=%s\", git_user_agent_sanitized());\n-\t\tpacket_write(1, \"%s %s%c%s\\n\",\n+\t\tpacket_write_fmt(1, \"%s %s%c%s\\n\",\n \t\t\t sha1_to_hex(sha1), path, 0, cap.buf);\n \t\tstrbuf_release(&cap);\n \t\tsent_capabilities = 1;\ndiff --git a/builtin/remote-ext.c b/builtin/remote-ext.c\nindex 88eb8f9..11b48bf 100644\n--- a/builtin/remote-ext.c\n+++ b/builtin/remote-ext.c\n@@ -128,9 +128,9 @@ static void send_git_request(int stdin_fd, const char *serv, const char *repo,\n \tconst char *vhost)\n {\n \tif (!vhost)\n-\t\tpacket_write(stdin_fd, \"%s %s%c\", serv, repo, 0);\n+\t\tpacket_write_fmt(stdin_fd, \"%s %s%c\", serv, repo, 0);\n \telse\n-\t\tpacket_write(stdin_fd, \"%s %s%chost=%s%c\", serv, repo, 0,\n+\t\tpacket_write_fmt(stdin_fd, \"%s %s%chost=%s%c\", serv, repo, 0,\n \t\t\t vhost, 0);\n }\n \ndiff --git a/builtin/upload-archive.c b/builtin/upload-archive.c\nindex 2caedf1..dc872f6 100644\n--- a/builtin/upload-archive.c\n+++ b/builtin/upload-archive.c\n@@ -88,11 +88,11 @@ int cmd_upload_archive(int argc, const char **argv, const char *prefix)\n \twriter.git_cmd = 1;\n \tif (start_command(&writer)) {\n \t\tint err = errno;\n-\t\tpacket_write(1, \"NACK unable to spawn subprocess\\n\");\n+\t\tpacket_write_fmt(1, \"NACK unable to spawn subprocess\\n\");\n \t\tdie(\"upload-archive: %s\", strerror(err));\n \t}\n \n-\tpacket_write(1, \"ACK\\n\");\n+\tpacket_write_fmt(1, \"ACK\\n\");\n \tpacket_flush(1);\n \n \twhile (1) {\ndiff --git a/connect.c b/connect.c\nindex 722dc3f..5330d9c 100644\n--- a/connect.c\n+++ b/connect.c\n@@ -730,7 +730,7 @@ struct child_process *git_connect(int fd[2], const char *url,\n \t\t * Note: Do not add any other headers here! Doing so\n \t\t * will cause older git-daemon servers to crash.\n \t\t */\n-\t\tpacket_write(fd[1],\n+\t\tpacket_write_fmt(fd[1],\n \t\t\t \"%s %s%chost=%s%c\",\n \t\t\t prog, path, 0,\n \t\t\t target_host, 0);\ndiff --git a/daemon.c b/daemon.c\nindex 425aad0..afce1b9 100644\n--- a/daemon.c\n+++ b/daemon.c\n@@ -281,7 +281,7 @@ static int daemon_error(const char *dir, const char *msg)\n {\n \tif (!informative_errors)\n \t\tmsg = \"access denied or repository not exported\";\n-\tpacket_write(1, \"ERR %s: %s\", msg, dir);\n+\tpacket_write_fmt(1, \"ERR %s: %s\", msg, dir);\n \treturn -1;\n }\n \ndiff --git a/http-backend.c b/http-backend.c\nindex adc8c8c..eef0a36 100644\n--- a/http-backend.c\n+++ b/http-backend.c\n@@ -464,7 +464,7 @@ static void get_info_refs(struct strbuf *hdr, char *arg)\n \t\thdr_str(hdr, content_type, buf.buf);\n \t\tend_headers(hdr);\n \n-\t\tpacket_write(1, \"# service=git-%s\\n\", svc->name);\n+\t\tpacket_write_fmt(1, \"# service=git-%s\\n\", svc->name);\n \t\tpacket_flush(1);\n \n \t\targv[0] = svc->name;\ndiff --git a/pkt-line.c b/pkt-line.c\nindex 62fdb37..0a9b61c 100644\n--- a/pkt-line.c\n+++ b/pkt-line.c\n@@ -118,7 +118,7 @@ static void format_packet(struct strbuf *out, const char *fmt, va_list args)\n \tpacket_trace(out->buf + orig_len + 4, n - 4, 1);\n }\n \n-void packet_write(int fd, const char *fmt, ...)\n+void packet_write_fmt(int fd, const char *fmt, ...)\n {\n \tstatic struct strbuf buf = STRBUF_INIT;\n \tva_list args;\ndiff --git a/pkt-line.h b/pkt-line.h\nindex 3cb9d91..1902fb3 100644\n--- a/pkt-line.h\n+++ b/pkt-line.h\n@@ -20,7 +20,7 @@\n * side can't, we stay with pure read/write interfaces.\n */\n void packet_flush(int fd);\n-void packet_write(int fd, const char *fmt, ...) __attribute__((format (printf, 2, 3)));\n+void packet_write_fmt(int fd, const char *fmt, ...) __attribute__((format (printf, 2, 3)));\n void packet_buf_flush(struct strbuf *buf);\n void packet_buf_write(struct strbuf *buf, const char *fmt, ...) __attribute__((format (printf, 2, 3)));\n \ndiff --git a/shallow.c b/shallow.c\nindex 54e2db7..d666e24 100644\n--- a/shallow.c\n+++ b/shallow.c\n@@ -260,7 +260,7 @@ static int advertise_shallow_grafts_cb(const struct commit_graft *graft, void *c\n {\n \tint fd = *(int *)cb;\n \tif (graft->nr_parent == -1)\n-\t\tpacket_write(fd, \"shallow %s\\n\", oid_to_hex(&graft->oid));\n+\t\tpacket_write_fmt(fd, \"shallow %s\\n\", oid_to_hex(&graft->oid));\n \treturn 0;\n }\n \ndiff --git a/upload-pack.c b/upload-pack.c\nindex ca7f941..cd47de6 100644\n--- a/upload-pack.c\n+++ b/upload-pack.c\n@@ -393,13 +393,13 @@ static int get_common_commits(void)\n \t\t\tif (multi_ack == 2 && got_common\n \t\t\t && !got_other && ok_to_give_up()) {\n \t\t\t\tsent_ready = 1;\n-\t\t\t\tpacket_write(1, \"ACK %s ready\\n\", last_hex);\n+\t\t\t\tpacket_write_fmt(1, \"ACK %s ready\\n\", last_hex);\n \t\t\t}\n \t\t\tif (have_obj.nr == 0 || multi_ack)\n-\t\t\t\tpacket_write(1, \"NAK\\n\");\n+\t\t\t\tpacket_write_fmt(1, \"NAK\\n\");\n \n \t\t\tif (no_done && sent_ready) {\n-\t\t\t\tpacket_write(1, \"ACK %s\\n\", last_hex);\n+\t\t\t\tpacket_write_fmt(1, \"ACK %s\\n\", last_hex);\n \t\t\t\treturn 0;\n \t\t\t}\n \t\t\tif (stateless_rpc)\n@@ -416,20 +416,20 @@ static int get_common_commits(void)\n \t\t\t\t\tconst char *hex = sha1_to_hex(sha1);\n \t\t\t\t\tif (multi_ack == 2) {\n \t\t\t\t\t\tsent_ready = 1;\n-\t\t\t\t\t\tpacket_write(1, \"ACK %s ready\\n\", hex);\n+\t\t\t\t\t\tpacket_write_fmt(1, \"ACK %s ready\\n\", hex);\n \t\t\t\t\t} else\n-\t\t\t\t\t\tpacket_write(1, \"ACK %s continue\\n\", hex);\n+\t\t\t\t\t\tpacket_write_fmt(1, \"ACK %s continue\\n\", hex);\n \t\t\t\t}\n \t\t\t\tbreak;\n \t\t\tdefault:\n \t\t\t\tgot_common = 1;\n \t\t\t\tmemcpy(last_hex, sha1_to_hex(sha1), 41);\n \t\t\t\tif (multi_ack == 2)\n-\t\t\t\t\tpacket_write(1, \"ACK %s common\\n\", last_hex);\n+\t\t\t\t\tpacket_write_fmt(1, \"ACK %s common\\n\", last_hex);\n \t\t\t\telse if (multi_ack)\n-\t\t\t\t\tpacket_write(1, \"ACK %s continue\\n\", last_hex);\n+\t\t\t\t\tpacket_write_fmt(1, \"ACK %s continue\\n\", last_hex);\n \t\t\t\telse if (have_obj.nr == 1)\n-\t\t\t\t\tpacket_write(1, \"ACK %s\\n\", last_hex);\n+\t\t\t\t\tpacket_write_fmt(1, \"ACK %s\\n\", last_hex);\n \t\t\t\tbreak;\n \t\t\t}\n \t\t\tcontinue;\n@@ -437,10 +437,10 @@ static int get_common_commits(void)\n \t\tif (!strcmp(line, \"done\")) {\n \t\t\tif (have_obj.nr > 0) {\n \t\t\t\tif (multi_ack)\n-\t\t\t\t\tpacket_write(1, \"ACK %s\\n\", last_hex);\n+\t\t\t\t\tpacket_write_fmt(1, \"ACK %s\\n\", last_hex);\n \t\t\t\treturn 0;\n \t\t\t}\n-\t\t\tpacket_write(1, \"NAK\\n\");\n+\t\t\tpacket_write_fmt(1, \"NAK\\n\");\n \t\t\treturn -1;\n \t\t}\n \t\tdie(\"git upload-pack: expected SHA1 list, got '%s'\", line);\n@@ -650,7 +650,7 @@ static void receive_needs(void)\n \t\twhile (result) {\n \t\t\tstruct object *object = &result->item->object;\n \t\t\tif (!(object->flags & (CLIENT_SHALLOW|NOT_SHALLOW))) {\n-\t\t\t\tpacket_write(1, \"shallow %s\",\n+\t\t\t\tpacket_write_fmt(1, \"shallow %s\",\n \t\t\t\t\t\toid_to_hex(&object->oid));\n \t\t\t\tregister_shallow(object->oid.hash);\n \t\t\t\tshallow_nr++;\n@@ -662,7 +662,7 @@ static void receive_needs(void)\n \t\t\tstruct object *object = shallows.objects[i].item;\n \t\t\tif (object->flags & NOT_SHALLOW) {\n \t\t\t\tstruct commit_list *parents;\n-\t\t\t\tpacket_write(1, \"unshallow %s\",\n+\t\t\t\tpacket_write_fmt(1, \"unshallow %s\",\n \t\t\t\t\toid_to_hex(&object->oid));\n \t\t\t\tobject->flags &= ~CLIENT_SHALLOW;\n \t\t\t\t/* make sure the real parents are parsed */\n@@ -741,7 +741,7 @@ static int send_ref(const char *refname, const struct object_id *oid,\n \t\tstruct strbuf symref_info = STRBUF_INIT;\n \n \t\tformat_symref_info(&symref_info, cb_data);\n-\t\tpacket_write(1, \"%s %s%c%s%s%s%s%s agent=%s\\n\",\n+\t\tpacket_write_fmt(1, \"%s %s%c%s%s%s%s%s agent=%s\\n\",\n \t\t\t oid_to_hex(oid), refname_nons,\n \t\t\t 0, capabilities,\n \t\t\t (allow_unadvertised_object_request & ALLOW_TIP_SHA1) ?\n@@ -753,11 +753,11 @@ static int send_ref(const char *refname, const struct object_id *oid,\n \t\t\t git_user_agent_sanitized());\n \t\tstrbuf_release(&symref_info);\n \t} else {\n-\t\tpacket_write(1, \"%s %s\\n\", oid_to_hex(oid), refname_nons);\n+\t\tpacket_write_fmt(1, \"%s %s\\n\", oid_to_hex(oid), refname_nons);\n \t}\n \tcapabilities = NULL;\n \tif (!peel_ref(refname, peeled.hash))\n-\t\tpacket_write(1, \"%s %s^{}\\n\", oid_to_hex(&peeled), refname_nons);\n+\t\tpacket_write_fmt(1, \"%s %s^{}\\n\", oid_to_hex(&peeled), refname_nons);\n \treturn 0;\n }\n \n-- \n2.10.0\n\n"},{"id":"304241","messageId":"20161016232038.84951-9-larsxschneider@gmail.com","threadId":"44310","inReplyTo":"20161016232038.84951-1-larsxschneider@gmail.com","subject":"[PATCH v11 08/14] pkt-line: add packet_flush_gently()","fromName":"","fromEmail":"larsxschneider@gmail.com","sentAt":"2016-10-16T23:20:32Z","receivedAt":"2016-10-16T23:21:30Z","isPatch":true,"sender":{"key":"larsxschneider@gmail.com","avatar":"https://avatars.githubusercontent.com/u/477434?v=4"},"body":"From: Lars Schneider \n\npacket_flush() would die in case of a write error even though for some\ncallers an error would be acceptable. Add packet_flush_gently() which\nwrites a pkt-line flush packet like packet_flush() but does not die in\ncase of an error. The function is used in a subsequent patch.\n\nSigned-off-by: Lars Schneider \nSigned-off-by: Junio C Hamano \n---\n pkt-line.c | 8 ++++++++\n pkt-line.h | 1 +\n 2 files changed, 9 insertions(+)\n\ndiff --git a/pkt-line.c b/pkt-line.c\nindex 56915f0..286eb09 100644\n--- a/pkt-line.c\n+++ b/pkt-line.c\n@@ -91,6 +91,14 @@ void packet_flush(int fd)\n \twrite_or_die(fd, \"0000\", 4);\n }\n \n+int packet_flush_gently(int fd)\n+{\n+\tpacket_trace(\"0000\", 4, 1);\n+\tif (write_in_full(fd, \"0000\", 4) == 4)\n+\t\treturn 0;\n+\treturn error(\"flush packet write failed\");\n+}\n+\n void packet_buf_flush(struct strbuf *buf)\n {\n \tpacket_trace(\"0000\", 4, 1);\ndiff --git a/pkt-line.h b/pkt-line.h\nindex 3caea77..3fa0899 100644\n--- a/pkt-line.h\n+++ b/pkt-line.h\n@@ -23,6 +23,7 @@ void packet_flush(int fd);\n void packet_write_fmt(int fd, const char *fmt, ...) __attribute__((format (printf, 2, 3)));\n void packet_buf_flush(struct strbuf *buf);\n void packet_buf_write(struct strbuf *buf, const char *fmt, ...) __attribute__((format (printf, 2, 3)));\n+int packet_flush_gently(int fd);\n int packet_write_fmt_gently(int fd, const char *fmt, ...) __attribute__((format (printf, 2, 3)));\n \n /*\n-- \n2.10.0\n\n"},{"id":"304242","messageId":"20161016232038.84951-8-larsxschneider@gmail.com","threadId":"44310","inReplyTo":"20161016232038.84951-1-larsxschneider@gmail.com","subject":"[PATCH v11 07/14] pkt-line: add packet_write_fmt_gently()","fromName":"","fromEmail":"larsxschneider@gmail.com","sentAt":"2016-10-16T23:20:31Z","receivedAt":"2016-10-16T23:21:33Z","isPatch":true,"sender":{"key":"larsxschneider@gmail.com","avatar":"https://avatars.githubusercontent.com/u/477434?v=4"},"body":"From: Lars Schneider \n\npacket_write_fmt() would die in case of a write error even though for\nsome callers an error would be acceptable. Add packet_write_fmt_gently()\nwhich writes a formatted pkt-line like packet_write_fmt() but does not\ndie in case of an error. The function is used in a subsequent patch.\n\nSigned-off-by: Lars Schneider \nSigned-off-by: Junio C Hamano \n---\n pkt-line.c | 34 ++++++++++++++++++++++++++++++----\n pkt-line.h | 1 +\n 2 files changed, 31 insertions(+), 4 deletions(-)\n\ndiff --git a/pkt-line.c b/pkt-line.c\nindex e8adc0f..56915f0 100644\n--- a/pkt-line.c\n+++ b/pkt-line.c\n@@ -125,16 +125,42 @@ static void format_packet(struct strbuf *out, const char *fmt, va_list args)\n \tpacket_trace(out->buf + orig_len + 4, n - 4, 1);\n }\n \n+static int packet_write_fmt_1(int fd, int gently,\n+\t\t\t const char *fmt, va_list args)\n+{\n+\tstruct strbuf buf = STRBUF_INIT;\n+\tssize_t count;\n+\n+\tformat_packet(&buf, fmt, args);\n+\tcount = write_in_full(fd, buf.buf, buf.len);\n+\tif (count == buf.len)\n+\t\treturn 0;\n+\n+\tif (!gently) {\n+\t\tcheck_pipe(errno);\n+\t\tdie_errno(\"packet write with format failed\");\n+\t}\n+\treturn error(\"packet write with format failed\");\n+}\n+\n void packet_write_fmt(int fd, const char *fmt, ...)\n {\n-\tstatic struct strbuf buf = STRBUF_INIT;\n \tva_list args;\n \n-\tstrbuf_reset(&buf);\n \tva_start(args, fmt);\n-\tformat_packet(&buf, fmt, args);\n+\tpacket_write_fmt_1(fd, 0, fmt, args);\n+\tva_end(args);\n+}\n+\n+int packet_write_fmt_gently(int fd, const char *fmt, ...)\n+{\n+\tint status;\n+\tva_list args;\n+\n+\tva_start(args, fmt);\n+\tstatus = packet_write_fmt_1(fd, 1, fmt, args);\n \tva_end(args);\n-\twrite_or_die(fd, buf.buf, buf.len);\n+\treturn status;\n }\n \n void packet_buf_write(struct strbuf *buf, const char *fmt, ...)\ndiff --git a/pkt-line.h b/pkt-line.h\nindex 1902fb3..3caea77 100644\n--- a/pkt-line.h\n+++ b/pkt-line.h\n@@ -23,6 +23,7 @@ void packet_flush(int fd);\n void packet_write_fmt(int fd, const char *fmt, ...) __attribute__((format (printf, 2, 3)));\n void packet_buf_flush(struct strbuf *buf);\n void packet_buf_write(struct strbuf *buf, const char *fmt, ...) __attribute__((format (printf, 2, 3)));\n+int packet_write_fmt_gently(int fd, const char *fmt, ...) __attribute__((format (printf, 2, 3)));\n \n /*\n * Read a packetized line into the buffer, which must be at least size bytes\n-- \n2.10.0\n\n"},{"id":"304243","messageId":"20161016232038.84951-7-larsxschneider@gmail.com","threadId":"44310","inReplyTo":"20161016232038.84951-1-larsxschneider@gmail.com","subject":"[PATCH v11 06/14] pkt-line: extract set_packet_header()","fromName":"","fromEmail":"larsxschneider@gmail.com","sentAt":"2016-10-16T23:20:30Z","receivedAt":"2016-10-16T23:21:34Z","isPatch":true,"sender":{"key":"larsxschneider@gmail.com","avatar":"https://avatars.githubusercontent.com/u/477434?v=4"},"body":"From: Lars Schneider \n\nExtracted set_packet_header() function converts an integer to a 4 byte\nhex string. Make this function locally available so that other pkt-line\nfunctions could use it.\n\nSigned-off-by: Lars Schneider \nSigned-off-by: Junio C Hamano \n---\n pkt-line.c | 19 +++++++++++++------\n 1 file changed, 13 insertions(+), 6 deletions(-)\n\ndiff --git a/pkt-line.c b/pkt-line.c\nindex 0a9b61c..e8adc0f 100644\n--- a/pkt-line.c\n+++ b/pkt-line.c\n@@ -97,10 +97,20 @@ void packet_buf_flush(struct strbuf *buf)\n \tstrbuf_add(buf, \"0000\", 4);\n }\n \n-#define hex(a) (hexchar[(a) & 15])\n-static void format_packet(struct strbuf *out, const char *fmt, va_list args)\n+static void set_packet_header(char *buf, const int size)\n {\n \tstatic char hexchar[] = \"0123456789abcdef\";\n+\n+\t#define hex(a) (hexchar[(a) & 15])\n+\tbuf[0] = hex(size >> 12);\n+\tbuf[1] = hex(size >> 8);\n+\tbuf[2] = hex(size >> 4);\n+\tbuf[3] = hex(size);\n+\t#undef hex\n+}\n+\n+static void format_packet(struct strbuf *out, const char *fmt, va_list args)\n+{\n \tsize_t orig_len, n;\n \n \torig_len = out->len;\n@@ -111,10 +121,7 @@ static void format_packet(struct strbuf *out, const char *fmt, va_list args)\n \tif (n > LARGE_PACKET_MAX)\n \t\tdie(\"protocol error: impossibly long line\");\n \n-\tout->buf[orig_len + 0] = hex(n >> 12);\n-\tout->buf[orig_len + 1] = hex(n >> 8);\n-\tout->buf[orig_len + 2] = hex(n >> 4);\n-\tout->buf[orig_len + 3] = hex(n);\n+\tset_packet_header(&out->buf[orig_len], n);\n \tpacket_trace(out->buf + orig_len + 4, n - 4, 1);\n }\n \n-- \n2.10.0\n\n"},{"id":"304244","messageId":"20161016232038.84951-10-larsxschneider@gmail.com","threadId":"44310","inReplyTo":"20161016232038.84951-1-larsxschneider@gmail.com","subject":"[PATCH v11 09/14] pkt-line: add packet_write_gently()","fromName":"","fromEmail":"larsxschneider@gmail.com","sentAt":"2016-10-16T23:20:33Z","receivedAt":"2016-10-16T23:21:36Z","isPatch":true,"sender":{"key":"larsxschneider@gmail.com","avatar":"https://avatars.githubusercontent.com/u/477434?v=4"},"body":"From: Lars Schneider \n\npacket_write_fmt_gently() uses format_packet() which lets the caller\nonly send string data via \"%s\". That means it cannot be used for\narbitrary data that may contain NULs.\n\nAdd packet_write_gently() which writes arbitrary data and does not die\nin case of an error. The function is used by other pkt-line functions in\na subsequent patch.\n\nSigned-off-by: Lars Schneider \nSigned-off-by: Junio C Hamano \n---\n pkt-line.c | 17 +++++++++++++++++\n 1 file changed, 17 insertions(+)\n\ndiff --git a/pkt-line.c b/pkt-line.c\nindex 286eb09..dca5a64 100644\n--- a/pkt-line.c\n+++ b/pkt-line.c\n@@ -171,6 +171,23 @@ int packet_write_fmt_gently(int fd, const char *fmt, ...)\n \treturn status;\n }\n \n+static int packet_write_gently(const int fd_out, const char *buf, size_t size)\n+{\n+\tstatic char packet_write_buffer[LARGE_PACKET_MAX];\n+\tsize_t packet_size;\n+\n+\tif (size > sizeof(packet_write_buffer) - 4)\n+\t\treturn error(\"packet write failed - data exceeds max packet size\");\n+\n+\tpacket_trace(buf, size, 1);\n+\tpacket_size = size + 4;\n+\tset_packet_header(packet_write_buffer, packet_size);\n+\tmemcpy(packet_write_buffer + 4, buf, size);\n+\tif (write_in_full(fd_out, packet_write_buffer, packet_size) == packet_size)\n+\t\treturn 0;\n+\treturn error(\"packet write failed\");\n+}\n+\n void packet_buf_write(struct strbuf *buf, const char *fmt, ...)\n {\n \tva_list args;\n-- \n2.10.0\n\n"},{"id":"304245","messageId":"20161016232038.84951-11-larsxschneider@gmail.com","threadId":"44310","inReplyTo":"20161016232038.84951-1-larsxschneider@gmail.com","subject":"[PATCH v11 10/14] pkt-line: add functions to read/write flush terminated packet streams","fromName":"","fromEmail":"larsxschneider@gmail.com","sentAt":"2016-10-16T23:20:34Z","receivedAt":"2016-10-16T23:21:39Z","isPatch":true,"sender":{"key":"larsxschneider@gmail.com","avatar":"https://avatars.githubusercontent.com/u/477434?v=4"},"body":"From: Lars Schneider \n\nwrite_packetized_from_fd() and write_packetized_from_buf() write a\nstream of packets. All content packets use the maximal packet size\nexcept for the last one. After the last content packet a `flush` control\npacket is written.\n\nread_packetized_to_strbuf() reads arbitrary sized packets until it\ndetects a `flush` packet.\n\nSigned-off-by: Lars Schneider \nSigned-off-by: Junio C Hamano \n---\n pkt-line.c | 72 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++\n pkt-line.h | 8 +++++++\n 2 files changed, 80 insertions(+)\n\ndiff --git a/pkt-line.c b/pkt-line.c\nindex dca5a64..0b5125f 100644\n--- a/pkt-line.c\n+++ b/pkt-line.c\n@@ -197,6 +197,46 @@ void packet_buf_write(struct strbuf *buf, const char *fmt, ...)\n \tva_end(args);\n }\n \n+int write_packetized_from_fd(int fd_in, int fd_out)\n+{\n+\tstatic char buf[LARGE_PACKET_DATA_MAX];\n+\tint err = 0;\n+\tssize_t bytes_to_write;\n+\n+\twhile (!err) {\n+\t\tbytes_to_write = xread(fd_in, buf, sizeof(buf));\n+\t\tif (bytes_to_write < 0)\n+\t\t\treturn COPY_READ_ERROR;\n+\t\tif (bytes_to_write == 0)\n+\t\t\tbreak;\n+\t\terr = packet_write_gently(fd_out, buf, bytes_to_write);\n+\t}\n+\tif (!err)\n+\t\terr = packet_flush_gently(fd_out);\n+\treturn err;\n+}\n+\n+int write_packetized_from_buf(const char *src_in, size_t len, int fd_out)\n+{\n+\tint err = 0;\n+\tsize_t bytes_written = 0;\n+\tsize_t bytes_to_write;\n+\n+\twhile (!err) {\n+\t\tif ((len - bytes_written) > LARGE_PACKET_DATA_MAX)\n+\t\t\tbytes_to_write = LARGE_PACKET_DATA_MAX;\n+\t\telse\n+\t\t\tbytes_to_write = len - bytes_written;\n+\t\tif (bytes_to_write == 0)\n+\t\t\tbreak;\n+\t\terr = packet_write_gently(fd_out, src_in + bytes_written, bytes_to_write);\n+\t\tbytes_written += bytes_to_write;\n+\t}\n+\tif (!err)\n+\t\terr = packet_flush_gently(fd_out);\n+\treturn err;\n+}\n+\n static int get_packet_data(int fd, char **src_buf, size_t *src_size,\n \t\t\t void *dst, unsigned size, int options)\n {\n@@ -306,3 +346,35 @@ char *packet_read_line_buf(char **src, size_t *src_len, int *dst_len)\n {\n \treturn packet_read_line_generic(-1, src, src_len, dst_len);\n }\n+\n+ssize_t read_packetized_to_strbuf(int fd_in, struct strbuf *sb_out)\n+{\n+\tint packet_len;\n+\n+\tsize_t orig_len = sb_out->len;\n+\tsize_t orig_alloc = sb_out->alloc;\n+\n+\tfor (;;) {\n+\t\tstrbuf_grow(sb_out, LARGE_PACKET_DATA_MAX);\n+\t\tpacket_len = packet_read(fd_in, NULL, NULL,\n+\t\t\t/* strbuf_grow() above always allocates one extra byte to\n+\t\t\t * store a '\\0' at the end of the string. packet_read()\n+\t\t\t * writes a '\\0' extra byte at the end, too. Let it know\n+\t\t\t * that there is already room for the extra byte.\n+\t\t\t */\n+\t\t\tsb_out->buf + sb_out->len, LARGE_PACKET_DATA_MAX+1,\n+\t\t\tPACKET_READ_GENTLE_ON_EOF);\n+\t\tif (packet_len <= 0)\n+\t\t\tbreak;\n+\t\tsb_out->len += packet_len;\n+\t}\n+\n+\tif (packet_len < 0) {\n+\t\tif (orig_alloc == 0)\n+\t\t\tstrbuf_release(sb_out);\n+\t\telse\n+\t\t\tstrbuf_setlen(sb_out, orig_len);\n+\t\treturn packet_len;\n+\t}\n+\treturn sb_out->len - orig_len;\n+}\ndiff --git a/pkt-line.h b/pkt-line.h\nindex 3fa0899..18eac64 100644\n--- a/pkt-line.h\n+++ b/pkt-line.h\n@@ -25,6 +25,8 @@ void packet_buf_flush(struct strbuf *buf);\n void packet_buf_write(struct strbuf *buf, const char *fmt, ...) __attribute__((format (printf, 2, 3)));\n int packet_flush_gently(int fd);\n int packet_write_fmt_gently(int fd, const char *fmt, ...) __attribute__((format (printf, 2, 3)));\n+int write_packetized_from_fd(int fd_in, int fd_out);\n+int write_packetized_from_buf(const char *src_in, size_t len, int fd_out);\n \n /*\n * Read a packetized line into the buffer, which must be at least size bytes\n@@ -77,8 +79,14 @@ char *packet_read_line(int fd, int *size);\n */\n char *packet_read_line_buf(char **src_buf, size_t *src_len, int *size);\n \n+/*\n+ * Reads a stream of variable sized packets until a flush packet is detected.\n+ */\n+ssize_t read_packetized_to_strbuf(int fd_in, struct strbuf *sb_out);\n+\n #define DEFAULT_PACKET_MAX 1000\n #define LARGE_PACKET_MAX 65520\n+#define LARGE_PACKET_DATA_MAX (LARGE_PACKET_MAX - 4)\n extern char packet_buffer[LARGE_PACKET_MAX];\n \n #endif\n-- \n2.10.0\n\n"},{"id":"304246","messageId":"20161016232038.84951-12-larsxschneider@gmail.com","threadId":"44310","inReplyTo":"20161016232038.84951-1-larsxschneider@gmail.com","subject":"[PATCH v11 11/14] convert: make apply_filter() adhere to standard Git error handling","fromName":"","fromEmail":"larsxschneider@gmail.com","sentAt":"2016-10-16T23:20:35Z","receivedAt":"2016-10-16T23:21:42Z","isPatch":true,"sender":{"key":"larsxschneider@gmail.com","avatar":"https://avatars.githubusercontent.com/u/477434?v=4"},"body":"From: Lars Schneider \n\napply_filter() returns a boolean that tells the caller if it\n\"did convert or did not convert\". The variable `ret` was used throughout\nthe function to track errors whereas `1` denoted success and `0`\nfailure. This is unusual for the Git source where `0` denotes success.\n\nRename the variable and flip its value to make the function easier\nreadable for Git developers.\n\nSigned-off-by: Lars Schneider \nSigned-off-by: Junio C Hamano \n---\n convert.c | 15 ++++++---------\n 1 file changed, 6 insertions(+), 9 deletions(-)\n\ndiff --git a/convert.c b/convert.c\nindex 986c239..597f561 100644\n--- a/convert.c\n+++ b/convert.c\n@@ -451,7 +451,7 @@ static int apply_filter(const char *path, const char *src, size_t len, int fd,\n \t *\n \t * (child --> cmd) --> us\n \t */\n-\tint ret = 1;\n+\tint err = 0;\n \tstruct strbuf nbuf = STRBUF_INIT;\n \tstruct async async;\n \tstruct filter_params params;\n@@ -477,23 +477,20 @@ static int apply_filter(const char *path, const char *src, size_t len, int fd,\n \t\treturn 0;\t/* error was already reported */\n \n \tif (strbuf_read(&nbuf, async.out, len) < 0) {\n-\t\terror(\"read from external filter '%s' failed\", cmd);\n-\t\tret = 0;\n+\t\terr = error(\"read from external filter '%s' failed\", cmd);\n \t}\n \tif (close(async.out)) {\n-\t\terror(\"read from external filter '%s' failed\", cmd);\n-\t\tret = 0;\n+\t\terr = error(\"read from external filter '%s' failed\", cmd);\n \t}\n \tif (finish_async(&async)) {\n-\t\terror(\"external filter '%s' failed\", cmd);\n-\t\tret = 0;\n+\t\terr = error(\"external filter '%s' failed\", cmd);\n \t}\n \n-\tif (ret) {\n+\tif (!err) {\n \t\tstrbuf_swap(dst, &nbuf);\n \t}\n \tstrbuf_release(&nbuf);\n-\treturn ret;\n+\treturn !err;\n }\n \n static struct convert_driver {\n-- \n2.10.0\n\n"},{"id":"304247","messageId":"20161016232038.84951-13-larsxschneider@gmail.com","threadId":"44310","inReplyTo":"20161016232038.84951-1-larsxschneider@gmail.com","subject":"[PATCH v11 12/14] convert: prepare filter..process option","fromName":"","fromEmail":"larsxschneider@gmail.com","sentAt":"2016-10-16T23:20:36Z","receivedAt":"2016-10-16T23:21:45Z","isPatch":true,"sender":{"key":"larsxschneider@gmail.com","avatar":"https://avatars.githubusercontent.com/u/477434?v=4"},"body":"From: Lars Schneider \n\nRefactor the existing 'single shot filter mechanism' and prepare the\nnew 'long running filter mechanism'.\n\nSigned-off-by: Lars Schneider \n---\n convert.c | 60 ++++++++++++++++++++++++++++++++++--------------------------\n 1 file changed, 34 insertions(+), 26 deletions(-)\n\ndiff --git a/convert.c b/convert.c\nindex 597f561..71e11ff 100644\n--- a/convert.c\n+++ b/convert.c\n@@ -442,7 +442,7 @@ static int filter_buffer_or_fd(int in, int out, void *data)\n \treturn (write_err || status);\n }\n \n-static int apply_filter(const char *path, const char *src, size_t len, int fd,\n+static int apply_single_file_filter(const char *path, const char *src, size_t len, int fd,\n struct strbuf *dst, const char *cmd)\n {\n \t/*\n@@ -456,12 +456,6 @@ static int apply_filter(const char *path, const char *src, size_t len, int fd,\n \tstruct async async;\n \tstruct filter_params params;\n \n-\tif (!cmd || !*cmd)\n-\t\treturn 0;\n-\n-\tif (!dst)\n-\t\treturn 1;\n-\n \tmemset(&async, 0, sizeof(async));\n \tasync.proc = filter_buffer_or_fd;\n \tasync.data = ¶ms;\n@@ -493,6 +487,9 @@ static int apply_filter(const char *path, const char *src, size_t len, int fd,\n \treturn !err;\n }\n \n+#define CAP_CLEAN (1u<<0)\n+#define CAP_SMUDGE (1u<<1)\n+\n static struct convert_driver {\n \tconst char *name;\n \tstruct convert_driver *next;\n@@ -501,6 +498,29 @@ static struct convert_driver {\n \tint required;\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+{\n+\tconst char *cmd = NULL;\n+\n+\tif (!drv)\n+\t\treturn 0;\n+\n+\tif (!dst)\n+\t\treturn 1;\n+\n+\tif ((CAP_CLEAN & wanted_capability) && drv->clean)\n+\t\tcmd = drv->clean;\n+\telse if ((CAP_SMUDGE & wanted_capability) && drv->smudge)\n+\t\tcmd = drv->smudge;\n+\n+\tif (cmd && *cmd)\n+\t\treturn apply_single_file_filter(path, src, len, fd, dst, cmd);\n+\n+\treturn 0;\n+}\n+\n static int read_convert_config(const char *var, const char *value, void *cb)\n {\n \tconst char *key, *name;\n@@ -839,7 +859,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->clean);\n+\treturn apply_filter(path, NULL, 0, -1, NULL, ca.drv, CAP_CLEAN);\n }\n \n const char *get_convert_attr_ascii(const char *path)\n@@ -872,18 +892,12 @@ int convert_to_git(const char *path, const char *src, size_t len,\n struct strbuf *dst, enum safe_crlf checksafe)\n {\n \tint ret = 0;\n-\tconst char *filter = NULL;\n-\tint required = 0;\n \tstruct conv_attrs ca;\n \n \tconvert_attrs(&ca, path);\n-\tif (ca.drv) {\n-\t\tfilter = ca.drv->clean;\n-\t\trequired = ca.drv->required;\n-\t}\n \n-\tret |= apply_filter(path, src, len, -1, dst, filter);\n-\tif (!ret && required)\n+\tret |= apply_filter(path, src, len, -1, dst, 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 \tif (ret && dst) {\n@@ -907,7 +921,7 @@ void convert_to_git_filter_fd(const char *path, int fd, struct strbuf *dst,\n \tassert(ca.drv);\n \tassert(ca.drv->clean);\n \n-\tif (!apply_filter(path, NULL, 0, fd, dst, ca.drv->clean))\n+\tif (!apply_filter(path, NULL, 0, fd, dst, 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@@ -919,15 +933,9 @@ static int convert_to_working_tree_internal(const char *path, const char *src,\n \t\t\t\t\t int normalizing)\n {\n \tint ret = 0, ret_filter = 0;\n-\tconst char *filter = NULL;\n-\tint required = 0;\n \tstruct conv_attrs ca;\n \n \tconvert_attrs(&ca, path);\n-\tif (ca.drv) {\n-\t\tfilter = ca.drv->smudge;\n-\t\trequired = ca.drv->required;\n-\t}\n \n \tret |= ident_to_worktree(path, src, len, dst, ca.ident);\n \tif (ret) {\n@@ -938,7 +946,7 @@ static int convert_to_working_tree_internal(const char *path, const char *src,\n \t * CRLF conversion can be skipped if normalizing, unless there\n \t * is a smudge filter. The filter might expect CRLFs.\n \t */\n-\tif (filter || !normalizing) {\n+\tif ((ca.drv && ca.drv->smudge) || !normalizing) {\n \t\tret |= crlf_to_worktree(path, src, len, dst, ca.crlf_action);\n \t\tif (ret) {\n \t\t\tsrc = dst->buf;\n@@ -946,8 +954,8 @@ 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, filter);\n-\tif (!ret_filter && required)\n+\tret_filter = apply_filter(path, src, len, -1, dst, 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-- \n2.10.0\n\n"},{"id":"304248","messageId":"20161016232038.84951-14-larsxschneider@gmail.com","threadId":"44310","inReplyTo":"20161016232038.84951-1-larsxschneider@gmail.com","subject":"[PATCH v11 13/14] convert: add filter..process option","fromName":"","fromEmail":"larsxschneider@gmail.com","sentAt":"2016-10-16T23:20:37Z","receivedAt":"2016-10-16T23:21:55Z","isPatch":true,"sender":{"key":"larsxschneider@gmail.com","avatar":"https://avatars.githubusercontent.com/u/477434?v=4"},"body":"From: Lars Schneider \n\nGit's clean/smudge mechanism invokes an external filter process for\nevery single blob that is affected by a filter. If Git filters a lot of\nblobs then the startup time of the external filter processes can become\na significant part of the overall Git execution time.\n\nIn a preliminary performance test this developer used a clean/smudge\nfilter written in golang to filter 12,000 files. This process took 364s\nwith the existing filter mechanism and 5s with the new mechanism. See\ndetails here: https://github.com/github/git-lfs/pull/1382\n\nThis patch adds the `filter..process` string option which, if\nused, keeps the external filter process running and processes all blobs\nwith the packet format (pkt-line) based protocol over standard input and\nstandard output. The full protocol is explained in detail in\n`Documentation/gitattributes.txt`.\n\nA few key decisions:\n\n* The long running filter process is referred to as filter protocol\n version 2 because the existing single shot filter invocation is\n considered version 1.\n* Git sends a welcome message and expects a response right after the\n external filter process has started. This ensures that Git will not\n hang if a version 1 filter is incorrectly used with the\n filter..process option for version 2 filters. In addition,\n Git can detect this kind of error and warn the user.\n* The status of a filter operation (e.g. \"success\" or \"error) is set\n before the actual response and (if necessary!) re-set after the\n response. The advantage of this two step status response is that if\n the filter detects an error early, then the filter can communicate\n this and Git does not even need to create structures to read the\n response.\n* All status responses are pkt-line lists terminated with a flush\n packet. This allows us to send other status fields with the same\n protocol in the future.\n\nHelped-by: Martin-Louis Bright \nReviewed-by: Jakub Narebski \nSigned-off-by: Lars Schneider \nSigned-off-by: Junio C Hamano \n---\n Documentation/gitattributes.txt | 155 +++++++++++++-\n convert.c | 300 ++++++++++++++++++++++++++-\n t/t0021-conversion.sh | 444 +++++++++++++++++++++++++++++++++++++++-\n t/t0021/rot13-filter.pl | 192 +++++++++++++++++\n 4 files changed, 1081 insertions(+), 10 deletions(-)\n create mode 100755 t/t0021/rot13-filter.pl\n\ndiff --git a/Documentation/gitattributes.txt b/Documentation/gitattributes.txt\nindex 7aff940..3f4d1ed 100644\n--- a/Documentation/gitattributes.txt\n+++ b/Documentation/gitattributes.txt\n@@ -293,7 +293,15 @@ checkout, when the `smudge` command is specified, the command is\n fed the blob object from its standard input, and its standard\n output is used to update the worktree file. Similarly, the\n `clean` command is used to convert the contents of worktree file\n-upon checkin.\n+upon checkin. By default these commands process only a single\n+blob and terminate. If a long running `process` filter is used\n+in place of `clean` and/or `smudge` filters, then Git can process\n+all blobs with a single filter command invocation for the entire\n+life of a single Git command, for example `git add --all`. If a\n+long running `process` filter is configured then it always takes\n+precedence over a configured single blob filter. See section\n+below for the description of the protocol used to communicate with\n+a `process` filter.\n \n One use of the content filtering is to massage the content into a shape\n that is more convenient for the platform, filesystem, and the user to use.\n@@ -373,6 +381,151 @@ not exist, or may have different contents. So, smudge and clean commands\n should not try to access the file on disk, but only act as filters on the\n content provided to them on standard input.\n \n+Long Running Filter Process\n+^^^^^^^^^^^^^^^^^^^^^^^^^^^\n+\n+If the filter command (a string value) is defined via\n+`filter..process` then Git can process all blobs with a\n+single filter invocation for the entire life of a single Git\n+command. This is achieved by using a packet format (pkt-line,\n+see technical/protocol-common.txt) based protocol over standard\n+input and standard output as follows. All packets, except for the\n+\"*CONTENT\" packets and the \"0000\" flush packet, are considered\n+text and therefore are terminated by a LF.\n+\n+Git starts the filter when it encounters the first file\n+that needs to be cleaned or smudged. After the filter started\n+Git sends a welcome message (\"git-filter-client\"), a list of supported\n+protocol version numbers, and a flush packet. Git expects to read a welcome\n+response message (\"git-filter-server\"), exactly one protocol version number\n+from the previously sent list, and a flush packet. All further\n+communication will be based on the selected version. The remaining\n+protocol description below documents \"version=2\". Please note that\n+\"version=42\" in the example below does not exist and is only there\n+to illustrate how the protocol would look like with more than one\n+version.\n+\n+After the version negotiation Git sends a list of all capabilities that\n+it supports and a flush packet. Git expects to read a list of desired\n+capabilities, which must be a subset of the supported capabilities list,\n+and a flush packet as response:\n+------------------------\n+packet: git> git-filter-client\n+packet: git> version=2\n+packet: git> version=42\n+packet: git> 0000\n+packet: git< git-filter-server\n+packet: git< version=2\n+packet: git< 0000\n+packet: git> capability=clean\n+packet: git> capability=smudge\n+packet: git> capability=not-yet-invented\n+packet: git> 0000\n+packet: git< capability=clean\n+packet: git< capability=smudge\n+packet: git< 0000\n+------------------------\n+Supported filter capabilities in version 2 are \"clean\" and\n+\"smudge\".\n+\n+Afterwards Git sends a list of \"key=value\" pairs terminated with\n+a flush packet. The list will contain at least the filter command\n+(based on the supported capabilities) and the pathname of the file\n+to filter relative to the repository root. Right after the flush packet\n+Git sends the content split in zero or more pkt-line packets and a\n+flush packet to terminate content. Please note, that the filter\n+must not send any response before it received the content and the\n+final flush packet.\n+------------------------\n+packet: git> command=smudge\n+packet: git> pathname=path/testfile.dat\n+packet: git> 0000\n+packet: git> CONTENT\n+packet: git> 0000\n+------------------------\n+\n+The filter is expected to respond with a list of \"key=value\" pairs\n+terminated with a flush packet. If the filter does not experience\n+problems then the list must contain a \"success\" status. Right after\n+these packets the filter is expected to send the content in zero\n+or more pkt-line packets and a flush packet at the end. Finally, a\n+second list of \"key=value\" pairs terminated with a flush packet\n+is expected. The filter can change the status in the second list\n+or keep the status as is with an empty list. Please note that the\n+empty list must be terminated with a flush packet regardless.\n+\n+------------------------\n+packet: git< status=success\n+packet: git< 0000\n+packet: git< SMUDGED_CONTENT\n+packet: git< 0000\n+packet: git< 0000 # empty list, keep \"status=success\" unchanged!\n+------------------------\n+\n+If the result content is empty then the filter is expected to respond\n+with a \"success\" status and a flush packet to signal the empty content.\n+------------------------\n+packet: git< status=success\n+packet: git< 0000\n+packet: git< 0000 # empty content!\n+packet: git< 0000 # empty list, keep \"status=success\" unchanged!\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+------------------------\n+packet: git< status=error\n+packet: git< 0000\n+------------------------\n+\n+If the filter experiences an error during processing, then it can\n+send the status \"error\" after the content was (partially or\n+completely) sent.\n+------------------------\n+packet: git< status=success\n+packet: git< 0000\n+packet: git< HALF_WRITTEN_ERRONEOUS_CONTENT\n+packet: git< 0000\n+packet: git< status=error\n+packet: git< 0000\n+------------------------\n+\n+In case the filter cannot or does not want to process the content\n+as well as any future content for the lifetime of the Git process,\n+then it is expected to respond with an \"abort\" status at any point\n+in the protocol.\n+------------------------\n+packet: git< status=abort\n+packet: git< 0000\n+------------------------\n+\n+Git neither stops nor restarts the filter process in case the\n+\"error\"/\"abort\" status is set. However, Git sets its exit code\n+according to the `filter..required` flag, mimicking the\n+behavior of the `filter..clean` / `filter..smudge`\n+mechanism.\n+\n+If the filter dies during the communication or does not adhere to\n+the protocol then Git will stop the filter process and restart it\n+with the next file that needs to be processed. Depending on the\n+`filter..required` flag Git will interpret that as error.\n+\n+After the filter has processed a blob it is expected to wait for\n+the next \"key=value\" list containing a command. Git will close\n+the command pipe on exit. The filter is expected to detect EOF\n+and exit gracefully on its own. Git will wait until the filter\n+process has stopped.\n+\n+If you develop your own long running filter\n+process then the `GIT_TRACE_PACKET` environment variables can be\n+very helpful for debugging (see linkgit:git[1]).\n+\n+Please note that you cannot use an existing `filter..clean`\n+or `filter..smudge` command with `filter..process`\n+because the former two use a different inter process communication\n+protocol than the latter one.\n+\n+\n Interaction between checkin/checkout attributes\n ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^\n \ndiff --git a/convert.c b/convert.c\nindex 71e11ff..9d2aa68 100644\n--- a/convert.c\n+++ b/convert.c\n@@ -3,6 +3,7 @@\n #include \"run-command.h\"\n #include \"quote.h\"\n #include \"sigchain.h\"\n+#include \"pkt-line.h\"\n \n /*\n * convert.c - convert a file when checking it out and checking it in.\n@@ -490,11 +491,292 @@ static int apply_single_file_filter(const char *path, const char *src, size_t le\n #define CAP_CLEAN (1u<<0)\n #define CAP_SMUDGE (1u<<1)\n \n+struct cmd2process {\n+\tstruct hashmap_entry ent; /* must be the first member! */\n+\tunsigned int supported_capabilities;\n+\tconst char *cmd;\n+\tstruct child_process process;\n+};\n+\n+static int cmd_process_map_initialized;\n+static struct hashmap cmd_process_map;\n+\n+static int cmd2process_cmp(const struct cmd2process *e1,\n+\t\t\t const struct cmd2process *e2,\n+\t\t\t const void *unused)\n+{\n+\treturn strcmp(e1->cmd, e2->cmd);\n+}\n+\n+static struct cmd2process *find_multi_file_filter_entry(struct hashmap *hashmap, const char *cmd)\n+{\n+\tstruct cmd2process key;\n+\thashmap_entry_init(&key, strhash(cmd));\n+\tkey.cmd = cmd;\n+\treturn hashmap_get(hashmap, &key, NULL);\n+}\n+\n+static int packet_write_list(int fd, const char *line, ...)\n+{\n+\tva_list args;\n+\tint err;\n+\tva_start(args, line);\n+\tfor (;;) {\n+\t\tif (!line)\n+\t\t\tbreak;\n+\t\tif (strlen(line) > LARGE_PACKET_DATA_MAX)\n+\t\t\treturn -1;\n+\t\terr = packet_write_fmt_gently(fd, \"%s\\n\", line);\n+\t\tif (err)\n+\t\t\treturn err;\n+\t\tline = va_arg(args, const char*);\n+\t}\n+\tva_end(args);\n+\treturn packet_flush_gently(fd);\n+}\n+\n+static void read_multi_file_filter_status(int fd, struct strbuf *status) {\n+\tstruct strbuf **pair;\n+\tchar *line;\n+\tfor (;;) {\n+\t\tline = packet_read_line(fd, NULL);\n+\t\tif (!line)\n+\t\t\tbreak;\n+\t\tpair = strbuf_split_str(line, '=', 2);\n+\t\tif (pair[0] && pair[0]->len && pair[1]) {\n+\t\t\t/* the last \"status=\" line wins */\n+\t\t\tif (!strcmp(pair[0]->buf, \"status=\")) {\n+\t\t\t\tstrbuf_reset(status);\n+\t\t\t\tstrbuf_addbuf(status, pair[1]);\n+\t\t\t}\n+\t\t}\n+\t\tstrbuf_list_free(pair);\n+\t}\n+}\n+\n+static void kill_multi_file_filter(struct hashmap *hashmap, struct cmd2process *entry)\n+{\n+\tif (!entry)\n+\t\treturn;\n+\n+\tentry->process.clean_on_exit = 0;\n+\tkill(entry->process.pid, SIGTERM);\n+\tfinish_command(&entry->process);\n+\n+\thashmap_remove(hashmap, entry, NULL);\n+\tfree(entry);\n+}\n+\n+static void stop_multi_file_filter(struct child_process *process)\n+{\n+\tsigchain_push(SIGPIPE, SIG_IGN);\n+\t/* Closing the pipe signals the filter to initiate a shutdown. */\n+\tclose(process->in);\n+\tclose(process->out);\n+\tsigchain_pop(SIGPIPE);\n+\t/* Finish command will wait until the shutdown is complete. */\n+\tfinish_command(process);\n+}\n+\n+static struct cmd2process *start_multi_file_filter(struct hashmap *hashmap, const char *cmd)\n+{\n+\tint err;\n+\tstruct cmd2process *entry;\n+\tstruct child_process *process;\n+\tconst char *argv[] = { cmd, NULL };\n+\tstruct string_list cap_list = STRING_LIST_INIT_NODUP;\n+\tchar *cap_buf;\n+\tconst char *cap_name;\n+\n+\tentry = xmalloc(sizeof(*entry));\n+\tentry->cmd = cmd;\n+\tentry->supported_capabilities = 0;\n+\tprocess = &entry->process;\n+\n+\tchild_process_init(process);\n+\tprocess->argv = argv;\n+\tprocess->use_shell = 1;\n+\tprocess->in = -1;\n+\tprocess->out = -1;\n+\tprocess->clean_on_exit = 1;\n+\tprocess->clean_on_exit_handler = stop_multi_file_filter;\n+\n+\tif (start_command(process)) {\n+\t\terror(\"cannot fork to run external filter '%s'\", cmd);\n+\t\treturn NULL;\n+\t}\n+\n+\thashmap_entry_init(entry, strhash(cmd));\n+\n+\tsigchain_push(SIGPIPE, SIG_IGN);\n+\n+\terr = packet_write_list(process->in, \"git-filter-client\", \"version=2\", NULL);\n+\tif (err)\n+\t\tgoto done;\n+\n+\terr = strcmp(packet_read_line(process->out, NULL), \"git-filter-server\");\n+\tif (err) {\n+\t\terror(\"external filter '%s' does not support filter protocol version 2\", cmd);\n+\t\tgoto done;\n+\t}\n+\terr = strcmp(packet_read_line(process->out, NULL), \"version=2\");\n+\tif (err)\n+\t\tgoto done;\n+\terr = packet_read_line(process->out, NULL) != NULL;\n+\tif (err)\n+\t\tgoto done;\n+\n+\terr = packet_write_list(process->in, \"capability=clean\", \"capability=smudge\", NULL);\n+\n+\tfor (;;) {\n+\t\tcap_buf = packet_read_line(process->out, NULL);\n+\t\tif (!cap_buf)\n+\t\t\tbreak;\n+\t\tstring_list_split_in_place(&cap_list, cap_buf, '=', 1);\n+\n+\t\tif (cap_list.nr != 2 || strcmp(cap_list.items[0].string, \"capability\"))\n+\t\t\tcontinue;\n+\n+\t\tcap_name = cap_list.items[1].string;\n+\t\tif (!strcmp(cap_name, \"clean\")) {\n+\t\t\tentry->supported_capabilities |= CAP_CLEAN;\n+\t\t} else if (!strcmp(cap_name, \"smudge\")) {\n+\t\t\tentry->supported_capabilities |= CAP_SMUDGE;\n+\t\t} else {\n+\t\t\twarning(\n+\t\t\t\t\"external filter '%s' requested unsupported filter capability '%s'\",\n+\t\t\t\tcmd, cap_name\n+\t\t\t);\n+\t\t}\n+\n+\t\tstring_list_clear(&cap_list, 0);\n+\t}\n+\n+done:\n+\tsigchain_pop(SIGPIPE);\n+\n+\tif (err || errno == EPIPE) {\n+\t\terror(\"initialization for external filter '%s' failed\", cmd);\n+\t\tkill_multi_file_filter(hashmap, entry);\n+\t\treturn NULL;\n+\t}\n+\n+\thashmap_add(hashmap, entry);\n+\treturn entry;\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 const unsigned int wanted_capability)\n+{\n+\tint err;\n+\tstruct cmd2process *entry;\n+\tstruct child_process *process;\n+\tstruct strbuf nbuf = STRBUF_INIT;\n+\tstruct strbuf filter_status = STRBUF_INIT;\n+\tconst char *filter_type;\n+\n+\tif (!cmd_process_map_initialized) {\n+\t\tcmd_process_map_initialized = 1;\n+\t\thashmap_init(&cmd_process_map, (hashmap_cmp_fn) cmd2process_cmp, 0);\n+\t\tentry = NULL;\n+\t} else {\n+\t\tentry = find_multi_file_filter_entry(&cmd_process_map, cmd);\n+\t}\n+\n+\tfflush(NULL);\n+\n+\tif (!entry) {\n+\t\tentry = start_multi_file_filter(&cmd_process_map, cmd);\n+\t\tif (!entry)\n+\t\t\treturn 0;\n+\t}\n+\tprocess = &entry->process;\n+\n+\tif (!(wanted_capability & entry->supported_capabilities))\n+\t\treturn 0;\n+\n+\tif (CAP_CLEAN & wanted_capability)\n+\t\tfilter_type = \"clean\";\n+\telse if (CAP_SMUDGE & wanted_capability)\n+\t\tfilter_type = \"smudge\";\n+\telse\n+\t\tdie(\"unexpected filter type\");\n+\n+\tsigchain_push(SIGPIPE, SIG_IGN);\n+\n+\tassert(strlen(filter_type) < LARGE_PACKET_DATA_MAX - strlen(\"command=\\n\"));\n+\terr = packet_write_fmt_gently(process->in, \"command=%s\\n\", filter_type);\n+\tif (err)\n+\t\tgoto done;\n+\n+\terr = strlen(path) > LARGE_PACKET_DATA_MAX - strlen(\"pathname=\\n\");\n+\tif (err) {\n+\t\terror(\"path name too long for external filter\");\n+\t\tgoto done;\n+\t}\n+\n+\terr = packet_write_fmt_gently(process->in, \"pathname=%s\\n\", path);\n+\tif (err)\n+\t\tgoto done;\n+\n+\terr = packet_flush_gently(process->in);\n+\tif (err)\n+\t\tgoto done;\n+\n+\tif (fd >= 0)\n+\t\terr = write_packetized_from_fd(fd, process->in);\n+\telse\n+\t\terr = write_packetized_from_buf(src, len, process->in);\n+\tif (err)\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+\t\tgoto done;\n+\n+\terr = read_packetized_to_strbuf(process->out, &nbuf) < 0;\n+\tif (err)\n+\t\tgoto done;\n+\n+\tread_multi_file_filter_status(process->out, &filter_status);\n+\terr = strcmp(filter_status.buf, \"success\");\n+\n+done:\n+\tsigchain_pop(SIGPIPE);\n+\n+\tif (err || errno == EPIPE) {\n+\t\tif (!strcmp(filter_status.buf, \"error\")) {\n+\t\t\t/* The filter signaled a problem with the file. */\n+\t\t} else if (!strcmp(filter_status.buf, \"abort\")) {\n+\t\t\t/*\n+\t\t\t * The filter signaled a permanent problem. Don't try to filter\n+\t\t\t * files with the same command for the lifetime of the current\n+\t\t\t * Git process.\n+\t\t\t */\n+\t\t\t entry->supported_capabilities &= ~wanted_capability;\n+\t\t} else {\n+\t\t\t/*\n+\t\t\t * Something went wrong with the protocol filter.\n+\t\t\t * Force shutdown and restart if another blob requires filtering.\n+\t\t\t */\n+\t\t\terror(\"external filter '%s' failed\", cmd);\n+\t\t\tkill_multi_file_filter(&cmd_process_map, entry);\n+\t\t}\n+\t} else {\n+\t\tstrbuf_swap(dst, &nbuf);\n+\t}\n+\tstrbuf_release(&nbuf);\n+\treturn !err;\n+}\n+\n static struct convert_driver {\n \tconst char *name;\n \tstruct convert_driver *next;\n \tconst char *smudge;\n \tconst char *clean;\n+\tconst char *process;\n \tint required;\n } *user_convert, **user_convert_tail;\n \n@@ -510,13 +792,15 @@ static int apply_filter(const char *path, const char *src, size_t len,\n \tif (!dst)\n \t\treturn 1;\n \n-\tif ((CAP_CLEAN & wanted_capability) && drv->clean)\n+\tif ((CAP_CLEAN & wanted_capability) && !drv->process && drv->clean)\n \t\tcmd = drv->clean;\n-\telse if ((CAP_SMUDGE & wanted_capability) && drv->smudge)\n+\telse if ((CAP_SMUDGE & wanted_capability) && !drv->process && drv->smudge)\n \t\tcmd = drv->smudge;\n \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 \n \treturn 0;\n }\n@@ -558,6 +842,9 @@ static int read_convert_config(const char *var, const char *value, void *cb)\n \tif (!strcmp(\"clean\", key))\n \t\treturn git_config_string(&drv->clean, var, value);\n \n+\tif (!strcmp(\"process\", key))\n+\t\treturn git_config_string(&drv->process, var, value);\n+\n \tif (!strcmp(\"required\", key)) {\n \t\tdrv->required = git_config_bool(var, value);\n \t\treturn 0;\n@@ -919,7 +1206,7 @@ void convert_to_git_filter_fd(const char *path, int fd, struct strbuf *dst,\n \tconvert_attrs(&ca, path);\n \n \tassert(ca.drv);\n-\tassert(ca.drv->clean);\n+\tassert(ca.drv->clean || ca.drv->process);\n \n \tif (!apply_filter(path, NULL, 0, fd, dst, ca.drv, CAP_CLEAN))\n \t\tdie(\"%s: clean filter '%s' failed\", path, ca.drv->name);\n@@ -944,9 +1231,10 @@ static int convert_to_working_tree_internal(const char *path, const char *src,\n \t}\n \t/*\n \t * CRLF conversion can be skipped if normalizing, unless there\n-\t * is a smudge filter. The filter might expect CRLFs.\n+\t * is a smudge or process filter (even if the process filter doesn't\n+\t * support smudge). The filters might expect CRLFs.\n \t */\n-\tif ((ca.drv && ca.drv->smudge) || !normalizing) {\n+\tif ((ca.drv && (ca.drv->smudge || ca.drv->process)) || !normalizing) {\n \t\tret |= crlf_to_worktree(path, src, len, dst, ca.crlf_action);\n \t\tif (ret) {\n \t\t\tsrc = dst->buf;\n@@ -1407,7 +1695,7 @@ struct stream_filter *get_stream_filter(const char *path, const unsigned char *s\n \tstruct stream_filter *filter = NULL;\n \n \tconvert_attrs(&ca, path);\n-\tif (ca.drv && (ca.drv->smudge || ca.drv->clean))\n+\tif (ca.drv && (ca.drv->process || ca.drv->smudge || ca.drv->clean))\n \t\treturn NULL;\n \n \tif (ca.crlf_action == CRLF_AUTO || ca.crlf_action == CRLF_AUTO_CRLF)\ndiff --git a/t/t0021-conversion.sh b/t/t0021-conversion.sh\nindex dc50938..a20b9f5 100755\n--- a/t/t0021-conversion.sh\n+++ b/t/t0021-conversion.sh\n@@ -4,13 +4,72 @@ test_description='blob conversion via gitattributes'\n \n . ./test-lib.sh\n \n-cat <rot13.sh\n+TEST_ROOT=\"$(pwd)\"\n+\n+cat <\"$TEST_ROOT/rot13.sh\"\n #!$SHELL_PATH\n tr \\\n 'abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ' \\\n 'nopqrstuvwxyzabcdefghijklmNOPQRSTUVWXYZABCDEFGHIJKLM'\n EOF\n-chmod +x rot13.sh\n+chmod +x \"$TEST_ROOT/rot13.sh\"\n+\n+generate_random_characters () {\n+\tLEN=$1\n+\tNAME=$2\n+\ttest-genrandom some-seed $LEN |\n+\t\tperl -pe \"s/./chr((ord($&) % 26) + ord('a'))/sge\" >\"$TEST_ROOT/$NAME\"\n+}\n+\n+file_size () {\n+\tcat \"$1\" | wc -c | sed \"s/^[ ]*//\"\n+}\n+\n+filter_git () {\n+\trm -f rot13-filter.log &&\n+\tgit \"$@\" 2>git-stderr.log &&\n+\trm -f git-stderr.log\n+}\n+\n+# Compare two files and ensure that `clean` and `smudge` respectively are\n+# called at least once if specified in the `expect` file. The actual\n+# invocation count is not relevant because their number can vary.\n+# c.f. http://public-inbox.org/git/xmqqshv18i8i.fsf@gitster.mtv.corp.google.com/\n+test_cmp_count () {\n+\texpect=$1\n+\tactual=$2\n+\tfor FILE in \"$expect\" \"$actual\"\n+\tdo\n+\t\tsort \"$FILE\" | uniq -c | sed \"s/^[ ]*//\" |\n+\t\t\tsed \"s/^\\([0-9]\\) IN: clean/x IN: clean/\" |\n+\t\t\tsed \"s/^\\([0-9]\\) IN: smudge/x IN: smudge/\" >\"$FILE.tmp\" &&\n+\t\tmv \"$FILE.tmp\" \"$FILE\"\n+\tdone &&\n+\ttest_cmp \"$expect\" \"$actual\"\n+}\n+\n+# Compare two files but exclude all `clean` invocations because Git can\n+# call `clean` zero or more times.\n+# c.f. http://public-inbox.org/git/xmqqshv18i8i.fsf@gitster.mtv.corp.google.com/\n+test_cmp_exclude_clean () {\n+\texpect=$1\n+\tactual=$2\n+\tfor FILE in \"$expect\" \"$actual\"\n+\tdo\n+\t\tgrep -v \"IN: clean\" \"$FILE\" >\"$FILE.tmp\" &&\n+\t\tmv \"$FILE.tmp\" \"$FILE\"\n+\tdone &&\n+\ttest_cmp \"$expect\" \"$actual\"\n+}\n+\n+# Check that the contents of two files are equal and that their rot13 version\n+# is equal to the committed content.\n+test_cmp_committed_rot13 () {\n+\ttest_cmp \"$1\" \"$2\" &&\n+\t\"$TEST_ROOT/rot13.sh\" <\"$1\" >expected &&\n+\tgit cat-file blob :\"$2\" >actual &&\n+\ttest_cmp expected actual\n+}\n \n test_expect_success setup '\n \tgit config filter.rot13.smudge ./rot13.sh &&\n@@ -31,7 +90,10 @@ test_expect_success setup '\n \tcat test >test.i &&\n \tgit add test test.t test.i &&\n \trm -f test test.t test.i &&\n-\tgit checkout -- test test.t test.i\n+\tgit checkout -- test test.t test.i &&\n+\n+\techo \"content-test2\" >test2.o &&\n+\techo \"content-test3 - filename with special characters\" >\"test3 '\\''sq'\\'',\\$x.o\"\n '\n \n script='s/^\\$Id: \\([0-9a-f]*\\) \\$/\\1/p'\n@@ -279,4 +341,380 @@ test_expect_success 'diff does not reuse worktree files that need cleaning' '\n \ttest_line_count = 0 count\n '\n \n+test_expect_success PERL 'required process filter should filter data' '\n+\ttest_config_global filter.protocol.process \"$TEST_DIRECTORY/t0021/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+\n+\t\techo \"git-stderr.log\" >.gitignore &&\n+\t\techo \"*.r filter=protocol\" >.gitattributes &&\n+\t\tgit add . &&\n+\t\tgit commit . -m \"test commit 1\" &&\n+\t\tgit branch empty-branch &&\n+\n+\t\tcp \"$TEST_ROOT/test.o\" test.r &&\n+\t\tcp \"$TEST_ROOT/test2.o\" test2.r &&\n+\t\tmkdir testsubdir &&\n+\t\tcp \"$TEST_ROOT/test3 '\\''sq'\\'',\\$x.o\" \"testsubdir/test3 '\\''sq'\\'',\\$x.r\" &&\n+\t\t>test4-empty.r &&\n+\n+\t\tS=$(file_size test.r) &&\n+\t\tS2=$(file_size test2.r) &&\n+\t\tS3=$(file_size \"testsubdir/test3 '\\''sq'\\'',\\$x.r\") &&\n+\n+\t\tfilter_git add . &&\n+\t\tcat >expected.log <<-EOF &&\n+\t\t\tSTART\n+\t\t\tinit handshake complete\n+\t\t\tIN: clean test.r $S [OK] -- OUT: $S . [OK]\n+\t\t\tIN: clean test2.r $S2 [OK] -- OUT: $S2 . [OK]\n+\t\t\tIN: clean test4-empty.r 0 [OK] -- OUT: 0 [OK]\n+\t\t\tIN: clean testsubdir/test3 '\\''sq'\\'',\\$x.r $S3 [OK] -- OUT: $S3 . [OK]\n+\t\t\tSTOP\n+\t\tEOF\n+\t\ttest_cmp_count expected.log rot13-filter.log &&\n+\n+\t\tfilter_git commit . -m \"test commit 2\" &&\n+\t\tcat >expected.log <<-EOF &&\n+\t\t\tSTART\n+\t\t\tinit handshake complete\n+\t\t\tIN: clean test.r $S [OK] -- OUT: $S . [OK]\n+\t\t\tIN: clean test2.r $S2 [OK] -- OUT: $S2 . [OK]\n+\t\t\tIN: clean test4-empty.r 0 [OK] -- OUT: 0 [OK]\n+\t\t\tIN: clean testsubdir/test3 '\\''sq'\\'',\\$x.r $S3 [OK] -- OUT: $S3 . [OK]\n+\t\t\tIN: clean test.r $S [OK] -- OUT: $S . [OK]\n+\t\t\tIN: clean test2.r $S2 [OK] -- OUT: $S2 . [OK]\n+\t\t\tIN: clean test4-empty.r 0 [OK] -- OUT: 0 [OK]\n+\t\t\tIN: clean testsubdir/test3 '\\''sq'\\'',\\$x.r $S3 [OK] -- OUT: $S3 . [OK]\n+\t\t\tSTOP\n+\t\tEOF\n+\t\ttest_cmp_count expected.log rot13-filter.log &&\n+\n+\t\trm -f test2.r \"testsubdir/test3 '\\''sq'\\'',\\$x.r\" &&\n+\n+\t\tfilter_git checkout --quiet --no-progress . &&\n+\t\tcat >expected.log <<-EOF &&\n+\t\t\tSTART\n+\t\t\tinit handshake complete\n+\t\t\tIN: smudge test2.r $S2 [OK] -- OUT: $S2 . [OK]\n+\t\t\tIN: smudge testsubdir/test3 '\\''sq'\\'',\\$x.r $S3 [OK] -- OUT: $S3 . [OK]\n+\t\t\tSTOP\n+\t\tEOF\n+\t\ttest_cmp_exclude_clean expected.log rot13-filter.log &&\n+\n+\t\tfilter_git checkout --quiet --no-progress empty-branch &&\n+\t\tcat >expected.log <<-EOF &&\n+\t\t\tSTART\n+\t\t\tinit handshake complete\n+\t\t\tIN: clean test.r $S [OK] -- OUT: $S . [OK]\n+\t\t\tSTOP\n+\t\tEOF\n+\t\ttest_cmp_exclude_clean expected.log rot13-filter.log &&\n+\n+\t\tfilter_git checkout --quiet --no-progress master &&\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 test2.r $S2 [OK] -- OUT: $S2 . [OK]\n+\t\t\tIN: smudge test4-empty.r 0 [OK] -- OUT: 0 [OK]\n+\t\t\tIN: smudge testsubdir/test3 '\\''sq'\\'',\\$x.r $S3 [OK] -- OUT: $S3 . [OK]\n+\t\t\tSTOP\n+\t\tEOF\n+\t\ttest_cmp_exclude_clean expected.log rot13-filter.log &&\n+\n+\t\ttest_cmp_committed_rot13 \"$TEST_ROOT/test.o\" test.r &&\n+\t\ttest_cmp_committed_rot13 \"$TEST_ROOT/test2.o\" test2.r &&\n+\t\ttest_cmp_committed_rot13 \"$TEST_ROOT/test3 '\\''sq'\\'',\\$x.o\" \"testsubdir/test3 '\\''sq'\\'',\\$x.r\"\n+\t)\n+'\n+\n+test_expect_success PERL 'required process filter takes precedence' '\n+\ttest_config_global filter.protocol.clean false &&\n+\ttest_config_global filter.protocol.process \"$TEST_DIRECTORY/t0021/rot13-filter.pl clean\" &&\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+\n+\t\techo \"*.r filter=protocol\" >.gitattributes &&\n+\t\tcp \"$TEST_ROOT/test.o\" test.r &&\n+\t\tS=$(file_size test.r) &&\n+\n+\t\t# Check that the process filter is invoked here\n+\t\tfilter_git add . &&\n+\t\tcat >expected.log <<-EOF &&\n+\t\t\tSTART\n+\t\t\tinit handshake complete\n+\t\t\tIN: clean test.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_expect_success PERL 'required process filter should be used only for \"clean\" operation only' '\n+\ttest_config_global filter.protocol.process \"$TEST_DIRECTORY/t0021/rot13-filter.pl clean\" &&\n+\trm -rf repo &&\n+\tmkdir repo &&\n+\t(\n+\t\tcd repo &&\n+\t\tgit init &&\n+\n+\t\techo \"*.r filter=protocol\" >.gitattributes &&\n+\t\tcp \"$TEST_ROOT/test.o\" test.r &&\n+\t\tS=$(file_size test.r) &&\n+\n+\t\tfilter_git add . &&\n+\t\tcat >expected.log <<-EOF &&\n+\t\t\tSTART\n+\t\t\tinit handshake complete\n+\t\t\tIN: clean test.r $S [OK] -- OUT: $S . [OK]\n+\t\t\tSTOP\n+\t\tEOF\n+\t\ttest_cmp_count expected.log rot13-filter.log &&\n+\n+\t\trm test.r &&\n+\n+\t\tfilter_git checkout --quiet --no-progress . &&\n+\t\t# If the filter would be used for \"smudge\", too, we would see\n+\t\t# \"IN: smudge test.r 57 [OK] -- OUT: 57 . [OK]\" here\n+\t\tcat >expected.log <<-EOF &&\n+\t\t\tSTART\n+\t\t\tinit handshake complete\n+\t\t\tSTOP\n+\t\tEOF\n+\t\ttest_cmp_exclude_clean expected.log rot13-filter.log\n+\t)\n+'\n+\n+test_expect_success PERL 'required process filter should process multiple packets' '\n+\ttest_config_global filter.protocol.process \"$TEST_DIRECTORY/t0021/rot13-filter.pl clean smudge\" &&\n+\ttest_config_global filter.protocol.required true &&\n+\n+\trm -rf repo &&\n+\tmkdir repo &&\n+\t(\n+\t\tcd repo &&\n+\t\tgit init &&\n+\n+\t\t# Generate data requiring 1, 2, 3 packets\n+\t\tS=65516 && # PKTLINE_DATA_MAXLEN -> Maximal size of a packet\n+\t\tgenerate_random_characters $(($S )) 1pkt_1__.file &&\n+\t\tgenerate_random_characters $(($S +1)) 2pkt_1+1.file &&\n+\t\tgenerate_random_characters $(($S*2-1)) 2pkt_2-1.file &&\n+\t\tgenerate_random_characters $(($S*2 )) 2pkt_2__.file &&\n+\t\tgenerate_random_characters $(($S*2+1)) 3pkt_2+1.file &&\n+\n+\t\tfor FILE in \"$TEST_ROOT\"/*.file\n+\t\tdo\n+\t\t\tcp \"$FILE\" . &&\n+\t\t\t\"$TEST_ROOT/rot13.sh\" <\"$FILE\" >\"$FILE.rot13\"\n+\t\tdone &&\n+\n+\t\techo \"*.file filter=protocol\" >.gitattributes &&\n+\t\tfilter_git add *.file .gitattributes &&\n+\t\tcat >expected.log <<-EOF &&\n+\t\t\tSTART\n+\t\t\tinit handshake complete\n+\t\t\tIN: clean 1pkt_1__.file $(($S )) [OK] -- OUT: $(($S )) . [OK]\n+\t\t\tIN: clean 2pkt_1+1.file $(($S +1)) [OK] -- OUT: $(($S +1)) .. [OK]\n+\t\t\tIN: clean 2pkt_2-1.file $(($S*2-1)) [OK] -- OUT: $(($S*2-1)) .. [OK]\n+\t\t\tIN: clean 2pkt_2__.file $(($S*2 )) [OK] -- OUT: $(($S*2 )) .. [OK]\n+\t\t\tIN: clean 3pkt_2+1.file $(($S*2+1)) [OK] -- OUT: $(($S*2+1)) ... [OK]\n+\t\t\tSTOP\n+\t\tEOF\n+\t\ttest_cmp_count expected.log rot13-filter.log &&\n+\n+\t\trm -f *.file &&\n+\n+\t\tfilter_git checkout --quiet --no-progress -- *.file &&\n+\t\tcat >expected.log <<-EOF &&\n+\t\t\tSTART\n+\t\t\tinit handshake complete\n+\t\t\tIN: smudge 1pkt_1__.file $(($S )) [OK] -- OUT: $(($S )) . [OK]\n+\t\t\tIN: smudge 2pkt_1+1.file $(($S +1)) [OK] -- OUT: $(($S +1)) .. [OK]\n+\t\t\tIN: smudge 2pkt_2-1.file $(($S*2-1)) [OK] -- OUT: $(($S*2-1)) .. [OK]\n+\t\t\tIN: smudge 2pkt_2__.file $(($S*2 )) [OK] -- OUT: $(($S*2 )) .. [OK]\n+\t\t\tIN: smudge 3pkt_2+1.file $(($S*2+1)) [OK] -- OUT: $(($S*2+1)) ... [OK]\n+\t\t\tSTOP\n+\t\tEOF\n+\t\ttest_cmp_exclude_clean expected.log rot13-filter.log &&\n+\n+\t\tfor FILE in *.file\n+\t\tdo\n+\t\t\ttest_cmp_committed_rot13 \"$TEST_ROOT/$FILE\" $FILE\n+\t\tdone\n+\t)\n+'\n+\n+test_expect_success PERL 'required process filter with clean error should fail' '\n+\ttest_config_global filter.protocol.process \"$TEST_DIRECTORY/t0021/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+\n+\t\techo \"*.r filter=protocol\" >.gitattributes &&\n+\n+\t\tcp \"$TEST_ROOT/test.o\" test.r &&\n+\t\techo \"this is going to fail\" >clean-write-fail.r &&\n+\t\techo \"content-test3-subdir\" >test3.r &&\n+\n+\t\ttest_must_fail git add .\n+\t)\n+'\n+\n+test_expect_success PERL 'process filter should restart after unexpected write failure' '\n+\ttest_config_global filter.protocol.process \"$TEST_DIRECTORY/t0021/rot13-filter.pl clean smudge\" &&\n+\trm -rf repo &&\n+\tmkdir repo &&\n+\t(\n+\t\tcd repo &&\n+\t\tgit init &&\n+\n+\t\techo \"*.r filter=protocol\" >.gitattributes &&\n+\n+\t\tcp \"$TEST_ROOT/test.o\" test.r &&\n+\t\tcp \"$TEST_ROOT/test2.o\" test2.r &&\n+\t\techo \"this is going to fail\" >smudge-write-fail.o &&\n+\t\tcp smudge-write-fail.o smudge-write-fail.r &&\n+\n+\t\tS=$(file_size test.r) &&\n+\t\tS2=$(file_size test2.r) &&\n+\t\tSF=$(file_size smudge-write-fail.r) &&\n+\n+\t\tgit add . &&\n+\t\trm -f *.r &&\n+\n+\t\trm -f rot13-filter.log &&\n+\t\tgit checkout --quiet --no-progress . 2>git-stderr.log &&\n+\n+\t\tgrep \"smudge write error at\" git-stderr.log &&\n+\t\tgrep \"error: external filter\" git-stderr.log &&\n+\n+\t\tcat >expected.log <<-EOF &&\n+\t\t\tSTART\n+\t\t\tinit handshake complete\n+\t\t\tIN: smudge smudge-write-fail.r $SF [OK] -- OUT: $SF [WRITE FAIL]\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 test2.r $S2 [OK] -- OUT: $S2 . [OK]\n+\t\t\tSTOP\n+\t\tEOF\n+\t\ttest_cmp_exclude_clean expected.log rot13-filter.log &&\n+\n+\t\ttest_cmp_committed_rot13 \"$TEST_ROOT/test.o\" test.r &&\n+\t\ttest_cmp_committed_rot13 \"$TEST_ROOT/test2.o\" test2.r &&\n+\n+\t\t# Smudge failed\n+\t\t! test_cmp smudge-write-fail.o smudge-write-fail.r &&\n+\t\t\"$TEST_ROOT/rot13.sh\" expected &&\n+\t\tgit cat-file blob :smudge-write-fail.r >actual &&\n+\t\ttest_cmp expected actual\n+\t)\n+'\n+\n+test_expect_success PERL 'process filter should not be restarted if it signals an error' '\n+\ttest_config_global filter.protocol.process \"$TEST_DIRECTORY/t0021/rot13-filter.pl clean smudge\" &&\n+\trm -rf repo &&\n+\tmkdir repo &&\n+\t(\n+\t\tcd repo &&\n+\t\tgit init &&\n+\n+\t\techo \"*.r filter=protocol\" >.gitattributes &&\n+\n+\t\tcp \"$TEST_ROOT/test.o\" test.r &&\n+\t\tcp \"$TEST_ROOT/test2.o\" test2.r &&\n+\t\techo \"this will cause an error\" >error.o &&\n+\t\tcp error.o error.r &&\n+\n+\t\tS=$(file_size test.r) &&\n+\t\tS2=$(file_size test2.r) &&\n+\t\tSE=$(file_size error.r) &&\n+\n+\t\tgit add . &&\n+\t\trm -f *.r &&\n+\n+\t\tfilter_git checkout --quiet --no-progress . &&\n+\t\tcat >expected.log <<-EOF &&\n+\t\t\tSTART\n+\t\t\tinit handshake complete\n+\t\t\tIN: smudge error.r $SE [OK] -- OUT: 0 [ERROR]\n+\t\t\tIN: smudge test.r $S [OK] -- OUT: $S . [OK]\n+\t\t\tIN: smudge test2.r $S2 [OK] -- OUT: $S2 . [OK]\n+\t\t\tSTOP\n+\t\tEOF\n+\t\ttest_cmp_exclude_clean expected.log rot13-filter.log &&\n+\n+\t\ttest_cmp_committed_rot13 \"$TEST_ROOT/test.o\" test.r &&\n+\t\ttest_cmp_committed_rot13 \"$TEST_ROOT/test2.o\" test2.r &&\n+\t\ttest_cmp error.o error.r\n+\t)\n+'\n+\n+test_expect_success PERL 'process filter abort stops processing of all further files' '\n+\ttest_config_global filter.protocol.process \"$TEST_DIRECTORY/t0021/rot13-filter.pl clean smudge\" &&\n+\trm -rf repo &&\n+\tmkdir repo &&\n+\t(\n+\t\tcd repo &&\n+\t\tgit init &&\n+\n+\t\techo \"*.r filter=protocol\" >.gitattributes &&\n+\n+\t\tcp \"$TEST_ROOT/test.o\" test.r &&\n+\t\tcp \"$TEST_ROOT/test2.o\" test2.r &&\n+\t\techo \"error this blob and all future blobs\" >abort.o &&\n+\t\tcp abort.o abort.r &&\n+\n+\t\tSA=$(file_size abort.r) &&\n+\n+\t\tgit add . &&\n+\t\trm -f *.r &&\n+\n+\t\t# Note: This test assumes that Git filters files in alphabetical\n+\t\t# order (\"abort.r\" before \"test.r\").\n+\t\tfilter_git checkout --quiet --no-progress . &&\n+\t\tcat >expected.log <<-EOF &&\n+\t\t\tSTART\n+\t\t\tinit handshake complete\n+\t\t\tIN: smudge abort.r $SA [OK] -- OUT: 0 [ABORT]\n+\t\t\tSTOP\n+\t\tEOF\n+\t\ttest_cmp_exclude_clean expected.log rot13-filter.log &&\n+\n+\t\ttest_cmp \"$TEST_ROOT/test.o\" test.r &&\n+\t\ttest_cmp \"$TEST_ROOT/test2.o\" test2.r &&\n+\t\ttest_cmp abort.o abort.r\n+\t)\n+'\n+\n+test_expect_success PERL 'invalid process filter must fail (and not hang!)' '\n+\ttest_config_global filter.protocol.process cat &&\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+\n+\t\techo \"*.r filter=protocol\" >.gitattributes &&\n+\n+\t\tcp \"$TEST_ROOT/test.o\" test.r &&\n+\t\ttest_must_fail git add . 2>git-stderr.log &&\n+\t\tgrep \"does not support filter protocol version\" git-stderr.log\n+\t)\n+'\n+\n test_done\ndiff --git a/t/t0021/rot13-filter.pl b/t/t0021/rot13-filter.pl\nnew file mode 100755\nindex 0000000..ae4c50f\n--- /dev/null\n+++ b/t/t0021/rot13-filter.pl\n@@ -0,0 +1,192 @@\n+#!/usr/bin/perl\n+#\n+# Example implementation for the Git filter protocol version 2\n+# See Documentation/gitattributes.txt, section \"Filter Protocol\"\n+#\n+# The script takes the list of supported protocol capabilities as\n+# arguments (\"clean\", \"smudge\", etc).\n+#\n+# This implementation supports special test cases:\n+# (1) If data with the pathname \"clean-write-fail.r\" is processed with\n+# a \"clean\" operation then the write operation will die.\n+# (2) If data with the pathname \"smudge-write-fail.r\" is processed with\n+# a \"smudge\" operation then the write operation will die.\n+# (3) If data with the pathname \"error.r\" is processed with any\n+# operation then the filter signals that it cannot or does not want\n+# to process the file.\n+# (4) If data with the pathname \"abort.r\" is processed with any\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+#\n+\n+use strict;\n+use warnings;\n+\n+my $MAX_PACKET_CONTENT_SIZE = 65516;\n+my @capabilities = @ARGV;\n+\n+open my $debug, \">>\", \"rot13-filter.log\" or die \"cannot open log file: $!\";\n+\n+sub rot13 {\n+\tmy $str = shift;\n+\t$str =~ y/A-Za-z/N-ZA-Mn-za-m/;\n+\treturn $str;\n+}\n+\n+sub packet_bin_read {\n+\tmy $buffer;\n+\tmy $bytes_read = read STDIN, $buffer, 4;\n+\tif ( $bytes_read == 0 ) {\n+\t\t# EOF - Git stopped talking to us!\n+\t\tprint $debug \"STOP\\n\";\n+\t\texit();\n+\t}\n+\telsif ( $bytes_read != 4 ) {\n+\t\tdie \"invalid packet: '$buffer'\";\n+\t}\n+\tmy $pkt_size = hex($buffer);\n+\tif ( $pkt_size == 0 ) {\n+\t\treturn ( 1, \"\" );\n+\t}\n+\telsif ( $pkt_size > 4 ) {\n+\t\tmy $content_size = $pkt_size - 4;\n+\t\t$bytes_read = read STDIN, $buffer, $content_size;\n+\t\tif ( $bytes_read != $content_size ) {\n+\t\t\tdie \"invalid packet ($content_size bytes expected; $bytes_read bytes read)\";\n+\t\t}\n+\t\treturn ( 0, $buffer );\n+\t}\n+\telse {\n+\t\tdie \"invalid packet size: $pkt_size\";\n+\t}\n+}\n+\n+sub packet_txt_read {\n+\tmy ( $res, $buf ) = packet_bin_read();\n+\tunless ( $buf =~ s/\\n$// ) {\n+\t\tdie \"A non-binary line MUST be terminated by an LF.\";\n+\t}\n+\treturn ( $res, $buf );\n+}\n+\n+sub packet_bin_write {\n+\tmy $buf = shift;\n+\tprint STDOUT sprintf( \"%04x\", length($buf) + 4 );\n+\tprint STDOUT $buf;\n+\tSTDOUT->flush();\n+}\n+\n+sub packet_txt_write {\n+\tpacket_bin_write( $_[0] . \"\\n\" );\n+}\n+\n+sub packet_flush {\n+\tprint STDOUT sprintf( \"%04x\", 0 );\n+\tSTDOUT->flush();\n+}\n+\n+print $debug \"START\\n\";\n+$debug->flush();\n+\n+( packet_txt_read() eq ( 0, \"git-filter-client\" ) ) || die \"bad initialize\";\n+( packet_txt_read() eq ( 0, \"version=2\" ) ) || die \"bad version\";\n+( packet_bin_read() eq ( 1, \"\" ) ) || die \"bad version end\";\n+\n+packet_txt_write(\"git-filter-server\");\n+packet_txt_write(\"version=2\");\n+packet_flush();\n+\n+( packet_txt_read() eq ( 0, \"capability=clean\" ) ) || die \"bad capability\";\n+( packet_txt_read() eq ( 0, \"capability=smudge\" ) ) || die \"bad capability\";\n+( packet_bin_read() eq ( 1, \"\" ) ) || die \"bad capability end\";\n+\n+foreach (@capabilities) {\n+\tpacket_txt_write( \"capability=\" . $_ );\n+}\n+packet_flush();\n+print $debug \"init handshake complete\\n\";\n+$debug->flush();\n+\n+while (1) {\n+\tmy ($command) = packet_txt_read() =~ /^command=([^=]+)$/;\n+\tprint $debug \"IN: $command\";\n+\t$debug->flush();\n+\n+\tmy ($pathname) = packet_txt_read() =~ /^pathname=([^=]+)$/;\n+\tprint $debug \" $pathname\";\n+\t$debug->flush();\n+\n+\t# Flush\n+\tpacket_bin_read();\n+\n+\tmy $input = \"\";\n+\t{\n+\t\tbinmode(STDIN);\n+\t\tmy $buffer;\n+\t\tmy $done = 0;\n+\t\twhile ( !$done ) {\n+\t\t\t( $done, $buffer ) = packet_bin_read();\n+\t\t\t$input .= $buffer;\n+\t\t}\n+\t\tprint $debug \" \" . length($input) . \" [OK] -- \";\n+\t\t$debug->flush();\n+\t}\n+\n+\tmy $output;\n+\tif ( $pathname eq \"error.r\" or $pathname eq \"abort.r\" ) {\n+\t\t$output = \"\";\n+\t}\n+\telsif ( $command eq \"clean\" and grep( /^clean$/, @capabilities ) ) {\n+\t\t$output = rot13($input);\n+\t}\n+\telsif ( $command eq \"smudge\" and grep( /^smudge$/, @capabilities ) ) {\n+\t\t$output = rot13($input);\n+\t}\n+\telse {\n+\t\tdie \"bad command '$command'\";\n+\t}\n+\n+\tprint $debug \"OUT: \" . length($output) . \" \";\n+\t$debug->flush();\n+\n+\tif ( $pathname eq \"error.r\" ) {\n+\t\tprint $debug \"[ERROR]\\n\";\n+\t\t$debug->flush();\n+\t\tpacket_txt_write(\"status=error\");\n+\t\tpacket_flush();\n+\t}\n+\telsif ( $pathname eq \"abort.r\" ) {\n+\t\tprint $debug \"[ABORT]\\n\";\n+\t\t$debug->flush();\n+\t\tpacket_txt_write(\"status=abort\");\n+\t\tpacket_flush();\n+\t}\n+\telse {\n+\t\tpacket_txt_write(\"status=success\");\n+\t\tpacket_flush();\n+\n+\t\tif ( $pathname eq \"${command}-write-fail.r\" ) {\n+\t\t\tprint $debug \"[WRITE FAIL]\\n\";\n+\t\t\t$debug->flush();\n+\t\t\tdie \"${command} write error\";\n+\t\t}\n+\n+\t\twhile ( length($output) > 0 ) {\n+\t\t\tmy $packet = substr( $output, 0, $MAX_PACKET_CONTENT_SIZE );\n+\t\t\tpacket_bin_write($packet);\n+\t\t\t# dots represent the number of packets\n+\t\t\tprint $debug \".\";\n+\t\t\tif ( length($output) > $MAX_PACKET_CONTENT_SIZE ) {\n+\t\t\t\t$output = substr( $output, $MAX_PACKET_CONTENT_SIZE );\n+\t\t\t}\n+\t\t\telse {\n+\t\t\t\t$output = \"\";\n+\t\t\t}\n+\t\t}\n+\t\tpacket_flush();\n+\t\tprint $debug \" [OK]\\n\";\n+\t\t$debug->flush();\n+\t\tpacket_flush();\n+\t}\n+}\n-- \n2.10.0\n\n"},{"id":"304249","messageId":"20161016232038.84951-15-larsxschneider@gmail.com","threadId":"44310","inReplyTo":"20161016232038.84951-1-larsxschneider@gmail.com","subject":"[PATCH v11 14/14] contrib/long-running-filter: add long running filter example","fromName":"","fromEmail":"larsxschneider@gmail.com","sentAt":"2016-10-16T23:20:38Z","receivedAt":"2016-10-16T23:21:59Z","isPatch":true,"sender":{"key":"larsxschneider@gmail.com","avatar":"https://avatars.githubusercontent.com/u/477434?v=4"},"body":"From: Lars Schneider \n\nAdd a simple pass-thru filter as example implementation for the Git\nfilter protocol version 2. See Documentation/gitattributes.txt, section\n\"Filter Protocol\" for more info.\n\nSigned-off-by: Lars Schneider \n---\n Documentation/gitattributes.txt | 4 +-\n contrib/long-running-filter/example.pl | 128 +++++++++++++++++++++++++++++++++\n 2 files changed, 131 insertions(+), 1 deletion(-)\n create mode 100755 contrib/long-running-filter/example.pl\n\ndiff --git a/Documentation/gitattributes.txt b/Documentation/gitattributes.txt\nindex 3f4d1ed..976243a 100644\n--- a/Documentation/gitattributes.txt\n+++ b/Documentation/gitattributes.txt\n@@ -516,7 +516,9 @@ the command pipe on exit. The filter is expected to detect EOF\n and exit gracefully on its own. Git will wait until the filter\n process has stopped.\n \n-If you develop your own long running filter\n+A long running filter demo implementation can be found in\n+`contrib/long-running-filter/example.pl` located in the Git\n+core repository. If you develop your own long running filter\n process then the `GIT_TRACE_PACKET` environment variables can be\n very helpful for debugging (see linkgit:git[1]).\n \ndiff --git a/contrib/long-running-filter/example.pl b/contrib/long-running-filter/example.pl\nnew file mode 100755\nindex 0000000..3945705\n--- /dev/null\n+++ b/contrib/long-running-filter/example.pl\n@@ -0,0 +1,128 @@\n+#!/usr/bin/perl\n+#\n+# Example implementation for the Git filter protocol version 2\n+# See Documentation/gitattributes.txt, section \"Filter Protocol\"\n+#\n+# Please note, this pass-thru filter is a minimal skeleton. No proper\n+# error handling was implemented.\n+#\n+\n+use strict;\n+use warnings;\n+\n+my $MAX_PACKET_CONTENT_SIZE = 65516;\n+\n+sub packet_bin_read {\n+\tmy $buffer;\n+\tmy $bytes_read = read STDIN, $buffer, 4;\n+\tif ( $bytes_read == 0 ) {\n+\n+\t\t# EOF - Git stopped talking to us!\n+\t\texit();\n+\t}\n+\telsif ( $bytes_read != 4 ) {\n+\t\tdie \"invalid packet: '$buffer'\";\n+\t}\n+\tmy $pkt_size = hex($buffer);\n+\tif ( $pkt_size == 0 ) {\n+\t\treturn ( 1, \"\" );\n+\t}\n+\telsif ( $pkt_size > 4 ) {\n+\t\tmy $content_size = $pkt_size - 4;\n+\t\t$bytes_read = read STDIN, $buffer, $content_size;\n+\t\tif ( $bytes_read != $content_size ) {\n+\t\t\tdie \"invalid packet ($content_size bytes expected; $bytes_read bytes read)\";\n+\t\t}\n+\t\treturn ( 0, $buffer );\n+\t}\n+\telse {\n+\t\tdie \"invalid packet size: $pkt_size\";\n+\t}\n+}\n+\n+sub packet_txt_read {\n+\tmy ( $res, $buf ) = packet_bin_read();\n+\tunless ( $buf =~ s/\\n$// ) {\n+\t\tdie \"A non-binary line MUST be terminated by an LF.\";\n+\t}\n+\treturn ( $res, $buf );\n+}\n+\n+sub packet_bin_write {\n+\tmy $buf = shift;\n+\tprint STDOUT sprintf( \"%04x\", length($buf) + 4 );\n+\tprint STDOUT $buf;\n+\tSTDOUT->flush();\n+}\n+\n+sub packet_txt_write {\n+\tpacket_bin_write( $_[0] . \"\\n\" );\n+}\n+\n+sub packet_flush {\n+\tprint STDOUT sprintf( \"%04x\", 0 );\n+\tSTDOUT->flush();\n+}\n+\n+( packet_txt_read() eq ( 0, \"git-filter-client\" ) ) || die \"bad initialize\";\n+( packet_txt_read() eq ( 0, \"version=2\" ) ) || die \"bad version\";\n+( packet_bin_read() eq ( 1, \"\" ) ) || die \"bad version end\";\n+\n+packet_txt_write(\"git-filter-server\");\n+packet_txt_write(\"version=2\");\n+packet_flush();\n+\n+( packet_txt_read() eq ( 0, \"capability=clean\" ) ) || die \"bad capability\";\n+( packet_txt_read() eq ( 0, \"capability=smudge\" ) ) || die \"bad capability\";\n+( packet_bin_read() eq ( 1, \"\" ) ) || die \"bad capability end\";\n+\n+packet_txt_write(\"capability=clean\");\n+packet_txt_write(\"capability=smudge\");\n+packet_flush();\n+\n+while (1) {\n+\tmy ($command) = packet_txt_read() =~ /^command=([^=]+)$/;\n+\tmy ($pathname) = packet_txt_read() =~ /^pathname=([^=]+)$/;\n+\n+\tpacket_bin_read();\n+\n+\tmy $input = \"\";\n+\t{\n+\t\tbinmode(STDIN);\n+\t\tmy $buffer;\n+\t\tmy $done = 0;\n+\t\twhile ( !$done ) {\n+\t\t\t( $done, $buffer ) = packet_bin_read();\n+\t\t\t$input .= $buffer;\n+\t\t}\n+\t}\n+\n+\tmy $output;\n+\tif ( $command eq \"clean\" ) {\n+\t\t### Perform clean here ###\n+\t\t$output = $input;\n+\t}\n+\telsif ( $command eq \"smudge\" ) {\n+\t\t### Perform smudge here ###\n+\t\t$output = $input;\n+\t}\n+\telse {\n+\t\tdie \"bad command '$command'\";\n+\t}\n+\n+\tpacket_txt_write(\"status=success\");\n+\tpacket_flush();\n+\twhile ( length($output) > 0 ) {\n+\t\tmy $packet = substr( $output, 0, $MAX_PACKET_CONTENT_SIZE );\n+\t\tpacket_bin_write($packet);\n+\t\tif ( length($output) > $MAX_PACKET_CONTENT_SIZE ) {\n+\t\t\t$output = substr( $output, $MAX_PACKET_CONTENT_SIZE );\n+\t\t}\n+\t\telse {\n+\t\t\t$output = \"\";\n+\t\t}\n+\t}\n+\tpacket_flush(); # flush content!\n+\tpacket_flush(); # empty list, keep \"status=success\" unchanged!\n+\n+}\n-- \n2.10.0\n\n"},{"id":"304296","messageId":"xmqqd1iyvmmd.fsf@gitster.mtv.corp.google.com","threadId":"44310","inReplyTo":"20161016232038.84951-1-larsxschneider@gmail.com","subject":"Re: [PATCH v11 00/14] Git filter protocol","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-10-17T18:47:22Z","receivedAt":"2016-10-17T18:47:42Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"larsxschneider@gmail.com writes:\n\n> The goal of this series is to avoid launching a new clean/smudge filter\n> process for each file that is filtered.\n>\n> A short summary about v1 to v5 can be found here:\n> https://git.github.io/rev_news/2016/08/17/edition-18/\n>\n> This series is also published on web:\n> https://github.com/larsxschneider/git/pull/15\n>\n> Patches 1 and 2 are cleanups and not strictly necessary for the series.\n> Patches 3 to 12 are required preparation. Patch 13 is the main patch.\n> Patch 14 adds an example how to use the Git filter protocol in contrib.\n\nWill replace. If you ever need tor reroll 13, please squash the\nfollowing in (which I already did locally so there is no need to\nresend only to correct it).\n\nThanks.\n\n convert.c | 3 ++-\n 1 file changed, 2 insertions(+), 1 deletion(-)\n\ndiff --git a/convert.c b/convert.c\nindex 9d2aa68df9..bc242276ff 100644\n--- a/convert.c\n+++ b/convert.c\n@@ -535,7 +535,8 @@ static int packet_write_list(int fd, const char *line, ...)\n \treturn packet_flush_gently(fd);\n }\n \n-static void read_multi_file_filter_status(int fd, struct strbuf *status) {\n+static void read_multi_file_filter_status(int fd, struct strbuf *status)\n+{\n \tstruct strbuf **pair;\n \tchar *line;\n \tfor (;;) {\n-- \n2.10.1-613-g6ad57fc60c\n\n\n"},{"id":"305307","messageId":"3b09d218-33bd-dc7c-235c-8954a46afc41@kdbg.org","threadId":"44310","inReplyTo":"20161016232038.84951-14-larsxschneider@gmail.com","subject":"Re: [PATCH v11 13/14] convert: add filter..process option","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2016-11-02T18:03:34Z","receivedAt":"2016-11-02T18:03:42Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 17.10.2016 um 01:20 schrieb larsxschneider@gmail.com:\n> +# Compare two files and ensure that `clean` and `smudge` respectively are\n> +# called at least once if specified in the `expect` file. The actual\n> +# invocation count is not relevant because their number can vary.\n> +# c.f. http://public-inbox.org/git/xmqqshv18i8i.fsf@gitster.mtv.corp.google.com/\n> +test_cmp_count () {\n> +\texpect=$1\n> +\tactual=$2\n> +\tfor FILE in \"$expect\" \"$actual\"\n> +\tdo\n> +\t\tsort \"$FILE\" | uniq -c | sed \"s/^[ ]*//\" |\n> +\t\t\tsed \"s/^\\([0-9]\\) IN: clean/x IN: clean/\" |\n> +\t\t\tsed \"s/^\\([0-9]\\) IN: smudge/x IN: smudge/\" >\"$FILE.tmp\" &&\n\nThis is not sufficiently portable. Some versions of uniq write the\ncount left-adjusted, not right-adjusted. How about this on top:\n\ndiff --git a/t/t0021-conversion.sh b/t/t0021-conversion.sh\nindex a20b9f58e3..f60858c517 100755\n--- a/t/t0021-conversion.sh\n+++ b/t/t0021-conversion.sh\n@@ -40,10 +40,9 @@ test_cmp_count () {\n \tactual=$2\n \tfor FILE in \"$expect\" \"$actual\"\n \tdo\n-\t\tsort \"$FILE\" | uniq -c | sed \"s/^[ ]*//\" |\n-\t\t\tsed \"s/^\\([0-9]\\) IN: clean/x IN: clean/\" |\n-\t\t\tsed \"s/^\\([0-9]\\) IN: smudge/x IN: smudge/\" >\"$FILE.tmp\" &&\n-\t\tmv \"$FILE.tmp\" \"$FILE\"\n+\t\tsort \"$FILE\" | uniq -c |\n+\t\tsed -e \"s/^ *[0-9][0-9]* *IN: /x IN: /\" >\"$FILE.tmp\" &&\n+\t\tmv \"$FILE.tmp\" \"$FILE\" || return\n \tdone &&\n \ttest_cmp \"$expect\" \"$actual\"\n }\n\n-- Hannes\n\n"},{"id":"305339","messageId":"3763DDDB-9D53-4877-8399-32DF1780CAB7@gmail.com","threadId":"44310","inReplyTo":"3b09d218-33bd-dc7c-235c-8954a46afc41@kdbg.org","subject":"Re: [PATCH v11 13/14] convert: add filter..process option","fromName":"Lars Schneider","fromEmail":"larsxschneider@gmail.com","sentAt":"2016-11-03T00:41:02Z","receivedAt":"2016-11-03T00:41:17Z","isPatch":true,"sender":{"key":"larsxschneider@gmail.com","avatar":"https://avatars.githubusercontent.com/u/477434?v=4"},"body":"\n> On 2 Nov 2016, at 18:03, Johannes Sixt wrote:\n> \n>> Am 17.10.2016 um 01:20 schrieb larsxschneider@gmail.com:\n>> +# Compare two files and ensure that `clean` and `smudge` respectively are\n>> +# called at least once if specified in the `expect` file. The actual\n>> +# invocation count is not relevant because their number can vary.\n>> +# c.f. http://public-inbox.org/git/xmqqshv18i8i.fsf@gitster.mtv.corp.google.com/\n>> +test_cmp_count () {\n>> + expect=$1\n>> + actual=$2\n>> + for FILE in \"$expect\" \"$actual\"\n>> + do\n>> + sort \"$FILE\" | uniq -c | sed \"s/^[ ]*//\" |\n>> + sed \"s/^\\([0-9]\\) IN: clean/x IN: clean/\" |\n>> + sed \"s/^\\([0-9]\\) IN: smudge/x IN: smudge/\" >\"$FILE.tmp\" &&\n> \n> This is not sufficiently portable. Some versions of uniq write the\n> count left-adjusted, not right-adjusted. How about this on top:\n> \n> diff --git a/t/t0021-conversion.sh b/t/t0021-conversion.sh\n> index a20b9f58e3..f60858c517 100755\n> --- a/t/t0021-conversion.sh\n> +++ b/t/t0021-conversion.sh\n> @@ -40,10 +40,9 @@ test_cmp_count () {\n> actual=$2\n> for FILE in \"$expect\" \"$actual\"\n> do\n> - sort \"$FILE\" | uniq -c | sed \"s/^[ ]*//\" |\n> - sed \"s/^\\([0-9]\\) IN: clean/x IN: clean/\" |\n> - sed \"s/^\\([0-9]\\) IN: smudge/x IN: smudge/\" >\"$FILE.tmp\" &&\n> - mv \"$FILE.tmp\" \"$FILE\"\n> + sort \"$FILE\" | uniq -c |\n> + sed -e \"s/^ *[0-9][0-9]* *IN: /x IN: /\" >\"$FILE.tmp\" &&\n\nThis looks good (thanks for cleaning up the redundant clean/smudge stuff - that was a refactoring artifact!). One minor nit: doesn't sed understand '[0-9]+' ?\n\n> + mv \"$FILE.tmp\" \"$FILE\" || return\n\nWhy '|| return' here?\n\n> done &&\n> test_cmp \"$expect\" \"$actual\"\n> }\n\nThank you,\nLars"},{"id":"305381","messageId":"fb4d62de-fbb5-a2b4-8eba-b135125dafa9@kdbg.org","threadId":"44310","inReplyTo":"3763DDDB-9D53-4877-8399-32DF1780CAB7@gmail.com","subject":"[PATCH] t0021: expect more variations in the output of uniq -c","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2016-11-03T20:12:13Z","receivedAt":"2016-11-03T20:12:21Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Some versions of uniq -c write the count left-justified, other version\nwrite it right-justified. Be prepared for both kinds.\n\nSigned-off-by: Johannes Sixt \n---\nHere it is as a proper patch.\n\nAm 03.11.2016 um 01:41 schrieb Lars Schneider:\n>> On 2 Nov 2016, at 18:03, Johannes Sixt wrote:\n>> + sort \"$FILE\" | uniq -c |\n>> + sed -e \"s/^ *[0-9][0-9]* *IN: /x IN: /\" >\"$FILE.tmp\" &&\n> \n> This looks good (thanks for cleaning up the redundant clean/smudge\n> stuff - that was a refactoring artifact!). One minor nit: doesn't sed\n> understand '[0-9]+' ?\n\nI don't think so.\n\n>> + mv \"$FILE.tmp\" \"$FILE\" || return\n> \n> Why '|| return' here?\n\nIf there is an error in the pipeline or in the mv command, the for loop\nwould not exit otherwise. The subsequent test_cmp most likely fails,\nbut the || return is more correct.\n\n t/t0021-conversion.sh | 7 +++----\n 1 file changed, 3 insertions(+), 4 deletions(-)\n\ndiff --git a/t/t0021-conversion.sh b/t/t0021-conversion.sh\nindex a20b9f58e3..db71acacb3 100755\n--- a/t/t0021-conversion.sh\n+++ b/t/t0021-conversion.sh\n@@ -40,10 +40,9 @@ test_cmp_count () {\n \tactual=$2\n \tfor FILE in \"$expect\" \"$actual\"\n \tdo\n-\t\tsort \"$FILE\" | uniq -c | sed \"s/^[ ]*//\" |\n-\t\t\tsed \"s/^\\([0-9]\\) IN: clean/x IN: clean/\" |\n-\t\t\tsed \"s/^\\([0-9]\\) IN: smudge/x IN: smudge/\" >\"$FILE.tmp\" &&\n-\t\tmv \"$FILE.tmp\" \"$FILE\"\n+\t\tsort \"$FILE\" | uniq -c |\n+\t\tsed -e \"s/^ *[0-9][0-9]*[ \t]*IN: /x IN: /\" >\"$FILE.tmp\" &&\n+\t\tmv \"$FILE.tmp\" \"$FILE\" || return\n \tdone &&\n \ttest_cmp \"$expect\" \"$actual\"\n }\n-- \n2.11.0.rc0.55.gd967357\n\n"},{"id":"305382","messageId":"3b58b90d-5435-1503-d052-413a947a5ab5@kdbg.org","threadId":"44310","inReplyTo":"fb4d62de-fbb5-a2b4-8eba-b135125dafa9@kdbg.org","subject":"[PATCH (optional)] t0021: use arithmetic expansion to trim whitespace from wc -c output","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2016-11-03T20:22:26Z","receivedAt":"2016-11-03T20:22:34Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Instead of a pipeline with sed and a useless use of cat, return the\nunmodified text of wc -c from function file_size, but substitute the\nresult with arithmetic expansion to get rid of the leading whitespace\nthat some version of wc -c print.\n\nSigned-off-by: Johannes Sixt \n---\nThis is a pure optimization that reduces the number of forks, which\nhelps a bit on Windows.\n\nThere would be a solution with perl that does not require trimming\nof whitespace, but perl startup times are unbearable on Windows.\nwc -c is better.\n\n t/t0021-conversion.sh | 50 +++++++++++++++++++++++++-------------------------\n 1 file changed, 25 insertions(+), 25 deletions(-)\n\ndiff --git a/t/t0021-conversion.sh b/t/t0021-conversion.sh\nindex db71acacb3..42b529f615 100755\n--- a/t/t0021-conversion.sh\n+++ b/t/t0021-conversion.sh\n@@ -22,7 +22,7 @@ generate_random_characters () {\n }\n \n file_size () {\n-\tcat \"$1\" | wc -c | sed \"s/^[ ]*//\"\n+\twc -c <\"$1\"\n }\n \n filter_git () {\n@@ -369,10 +369,10 @@ test_expect_success PERL 'required process filter should filter data' '\n \t\tcat >expected.log <<-EOF &&\n \t\t\tSTART\n \t\t\tinit handshake complete\n-\t\t\tIN: clean test.r $S [OK] -- OUT: $S . [OK]\n-\t\t\tIN: clean test2.r $S2 [OK] -- OUT: $S2 . [OK]\n+\t\t\tIN: clean test.r $(($S)) [OK] -- OUT: $(($S)) . [OK]\n+\t\t\tIN: clean test2.r $(($S2)) [OK] -- OUT: $(($S2)) . [OK]\n \t\t\tIN: clean test4-empty.r 0 [OK] -- OUT: 0 [OK]\n-\t\t\tIN: clean testsubdir/test3 '\\''sq'\\'',\\$x.r $S3 [OK] -- OUT: $S3 . [OK]\n+\t\t\tIN: clean testsubdir/test3 '\\''sq'\\'',\\$x.r $(($S3)) [OK] -- OUT: $(($S3)) . [OK]\n \t\t\tSTOP\n \t\tEOF\n \t\ttest_cmp_count expected.log rot13-filter.log &&\n@@ -381,14 +381,14 @@ test_expect_success PERL 'required process filter should filter data' '\n \t\tcat >expected.log <<-EOF &&\n \t\t\tSTART\n \t\t\tinit handshake complete\n-\t\t\tIN: clean test.r $S [OK] -- OUT: $S . [OK]\n-\t\t\tIN: clean test2.r $S2 [OK] -- OUT: $S2 . [OK]\n+\t\t\tIN: clean test.r $(($S)) [OK] -- OUT: $(($S)) . [OK]\n+\t\t\tIN: clean test2.r $(($S2)) [OK] -- OUT: $(($S2)) . [OK]\n \t\t\tIN: clean test4-empty.r 0 [OK] -- OUT: 0 [OK]\n-\t\t\tIN: clean testsubdir/test3 '\\''sq'\\'',\\$x.r $S3 [OK] -- OUT: $S3 . [OK]\n-\t\t\tIN: clean test.r $S [OK] -- OUT: $S . [OK]\n-\t\t\tIN: clean test2.r $S2 [OK] -- OUT: $S2 . [OK]\n+\t\t\tIN: clean testsubdir/test3 '\\''sq'\\'',\\$x.r $(($S3)) [OK] -- OUT: $(($S3)) . [OK]\n+\t\t\tIN: clean test.r $(($S)) [OK] -- OUT: $(($S)) . [OK]\n+\t\t\tIN: clean test2.r $(($S2)) [OK] -- OUT: $(($S2)) . [OK]\n \t\t\tIN: clean test4-empty.r 0 [OK] -- OUT: 0 [OK]\n-\t\t\tIN: clean testsubdir/test3 '\\''sq'\\'',\\$x.r $S3 [OK] -- OUT: $S3 . [OK]\n+\t\t\tIN: clean testsubdir/test3 '\\''sq'\\'',\\$x.r $(($S3)) [OK] -- OUT: $(($S3)) . [OK]\n \t\t\tSTOP\n \t\tEOF\n \t\ttest_cmp_count expected.log rot13-filter.log &&\n@@ -399,8 +399,8 @@ test_expect_success PERL 'required process filter should filter data' '\n \t\tcat >expected.log <<-EOF &&\n \t\t\tSTART\n \t\t\tinit handshake complete\n-\t\t\tIN: smudge test2.r $S2 [OK] -- OUT: $S2 . [OK]\n-\t\t\tIN: smudge testsubdir/test3 '\\''sq'\\'',\\$x.r $S3 [OK] -- OUT: $S3 . [OK]\n+\t\t\tIN: smudge test2.r $(($S2)) [OK] -- OUT: $(($S2)) . [OK]\n+\t\t\tIN: smudge testsubdir/test3 '\\''sq'\\'',\\$x.r $(($S3)) [OK] -- OUT: $(($S3)) . [OK]\n \t\t\tSTOP\n \t\tEOF\n \t\ttest_cmp_exclude_clean expected.log rot13-filter.log &&\n@@ -409,7 +409,7 @@ test_expect_success PERL 'required process filter should filter data' '\n \t\tcat >expected.log <<-EOF &&\n \t\t\tSTART\n \t\t\tinit handshake complete\n-\t\t\tIN: clean test.r $S [OK] -- OUT: $S . [OK]\n+\t\t\tIN: clean test.r $(($S)) [OK] -- OUT: $(($S)) . [OK]\n \t\t\tSTOP\n \t\tEOF\n \t\ttest_cmp_exclude_clean expected.log rot13-filter.log &&\n@@ -418,10 +418,10 @@ test_expect_success PERL 'required process filter should filter data' '\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 test2.r $S2 [OK] -- OUT: $S2 . [OK]\n+\t\t\tIN: smudge test.r $(($S)) [OK] -- OUT: $(($S)) . [OK]\n+\t\t\tIN: smudge test2.r $(($S2)) [OK] -- OUT: $(($S2)) . [OK]\n \t\t\tIN: smudge test4-empty.r 0 [OK] -- OUT: 0 [OK]\n-\t\t\tIN: smudge testsubdir/test3 '\\''sq'\\'',\\$x.r $S3 [OK] -- OUT: $S3 . [OK]\n+\t\t\tIN: smudge testsubdir/test3 '\\''sq'\\'',\\$x.r $(($S3)) [OK] -- OUT: $(($S3)) . [OK]\n \t\t\tSTOP\n \t\tEOF\n \t\ttest_cmp_exclude_clean expected.log rot13-filter.log &&\n@@ -451,7 +451,7 @@ test_expect_success PERL 'required process filter takes precedence' '\n \t\tcat >expected.log <<-EOF &&\n \t\t\tSTART\n \t\t\tinit handshake complete\n-\t\t\tIN: clean test.r $S [OK] -- OUT: $S . [OK]\n+\t\t\tIN: clean test.r $(($S)) [OK] -- OUT: $(($S)) . [OK]\n \t\t\tSTOP\n \t\tEOF\n \t\ttest_cmp_count expected.log rot13-filter.log\n@@ -474,7 +474,7 @@ test_expect_success PERL 'required process filter should be used only for \"clean\n \t\tcat >expected.log <<-EOF &&\n \t\t\tSTART\n \t\t\tinit handshake complete\n-\t\t\tIN: clean test.r $S [OK] -- OUT: $S . [OK]\n+\t\t\tIN: clean test.r $(($S)) [OK] -- OUT: $(($S)) . [OK]\n \t\t\tSTOP\n \t\tEOF\n \t\ttest_cmp_count expected.log rot13-filter.log &&\n@@ -603,11 +603,11 @@ test_expect_success PERL 'process filter should restart after unexpected write f\n \t\tcat >expected.log <<-EOF &&\n \t\t\tSTART\n \t\t\tinit handshake complete\n-\t\t\tIN: smudge smudge-write-fail.r $SF [OK] -- OUT: $SF [WRITE FAIL]\n+\t\t\tIN: smudge smudge-write-fail.r $(($SF)) [OK] -- OUT: $(($SF)) [WRITE FAIL]\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 test2.r $S2 [OK] -- OUT: $S2 . [OK]\n+\t\t\tIN: smudge test.r $(($S)) [OK] -- OUT: $(($S)) . [OK]\n+\t\t\tIN: smudge test2.r $(($S2)) [OK] -- OUT: $(($S2)) . [OK]\n \t\t\tSTOP\n \t\tEOF\n \t\ttest_cmp_exclude_clean expected.log rot13-filter.log &&\n@@ -649,9 +649,9 @@ test_expect_success PERL 'process filter should not be restarted if it signals a\n \t\tcat >expected.log <<-EOF &&\n \t\t\tSTART\n \t\t\tinit handshake complete\n-\t\t\tIN: smudge error.r $SE [OK] -- OUT: 0 [ERROR]\n-\t\t\tIN: smudge test.r $S [OK] -- OUT: $S . [OK]\n-\t\t\tIN: smudge test2.r $S2 [OK] -- OUT: $S2 . [OK]\n+\t\t\tIN: smudge error.r $(($SE)) [OK] -- OUT: 0 [ERROR]\n+\t\t\tIN: smudge test.r $(($S)) [OK] -- OUT: $(($S)) . [OK]\n+\t\t\tIN: smudge test2.r $(($S2)) [OK] -- OUT: $(($S2)) . [OK]\n \t\t\tSTOP\n \t\tEOF\n \t\ttest_cmp_exclude_clean expected.log rot13-filter.log &&\n@@ -688,7 +688,7 @@ test_expect_success PERL 'process filter abort stops processing of all further f\n \t\tcat >expected.log <<-EOF &&\n \t\t\tSTART\n \t\t\tinit handshake complete\n-\t\t\tIN: smudge abort.r $SA [OK] -- OUT: 0 [ABORT]\n+\t\t\tIN: smudge abort.r $(($SA)) [OK] -- OUT: 0 [ABORT]\n \t\t\tSTOP\n \t\tEOF\n \t\ttest_cmp_exclude_clean expected.log rot13-filter.log &&\n-- \n2.11.0.rc0.55.gd967357\n\n"},{"id":"305460","messageId":"0B7DE8F1-34DF-4271-8936-B9B596EF5F9E@gmail.com","threadId":"44310","inReplyTo":"fb4d62de-fbb5-a2b4-8eba-b135125dafa9@kdbg.org","subject":"Re: [PATCH] t0021: expect more variations in the output of uniq -c","fromName":"Lars Schneider","fromEmail":"larsxschneider@gmail.com","sentAt":"2016-11-06T15:31:45Z","receivedAt":"2016-11-06T15:31:53Z","isPatch":true,"sender":{"key":"larsxschneider@gmail.com","avatar":"https://avatars.githubusercontent.com/u/477434?v=4"},"body":"\n> On 03 Nov 2016, at 21:12, Johannes Sixt wrote:\n> \n> Some versions of uniq -c write the count left-justified, other version\n> write it right-justified. Be prepared for both kinds.\n> \n> Signed-off-by: Johannes Sixt \n> ---\n> Here it is as a proper patch.\n> \n> Am 03.11.2016 um 01:41 schrieb Lars Schneider:\n>>> On 2 Nov 2016, at 18:03, Johannes Sixt wrote:\n>>> + sort \"$FILE\" | uniq -c |\n>>> + sed -e \"s/^ *[0-9][0-9]* *IN: /x IN: /\" >\"$FILE.tmp\" &&\n>> \n>> This looks good (thanks for cleaning up the redundant clean/smudge\n>> stuff - that was a refactoring artifact!). One minor nit: doesn't sed\n>> understand '[0-9]+' ?\n> \n> I don't think so.\n> \n>>> + mv \"$FILE.tmp\" \"$FILE\" || return\n>> \n>> Why '|| return' here?\n> \n> If there is an error in the pipeline or in the mv command, the for loop\n> would not exit otherwise. The subsequent test_cmp most likely fails,\n> but the || return is more correct.\n> \n> t/t0021-conversion.sh | 7 +++----\n> 1 file changed, 3 insertions(+), 4 deletions(-)\n> \n> diff --git a/t/t0021-conversion.sh b/t/t0021-conversion.sh\n> index a20b9f58e3..db71acacb3 100755\n> --- a/t/t0021-conversion.sh\n> +++ b/t/t0021-conversion.sh\n> @@ -40,10 +40,9 @@ test_cmp_count () {\n> \tactual=$2\n> \tfor FILE in \"$expect\" \"$actual\"\n> \tdo\n> -\t\tsort \"$FILE\" | uniq -c | sed \"s/^[ ]*//\" |\n> -\t\t\tsed \"s/^\\([0-9]\\) IN: clean/x IN: clean/\" |\n> -\t\t\tsed \"s/^\\([0-9]\\) IN: smudge/x IN: smudge/\" >\"$FILE.tmp\" &&\n> -\t\tmv \"$FILE.tmp\" \"$FILE\"\n> +\t\tsort \"$FILE\" | uniq -c |\n> +\t\tsed -e \"s/^ *[0-9][0-9]*[ \t]*IN: /x IN: /\" >\"$FILE.tmp\" &&\n> +\t\tmv \"$FILE.tmp\" \"$FILE\" || return\n> \tdone &&\n> \ttest_cmp \"$expect\" \"$actual\"\n> }\n> -- \n> 2.11.0.rc0.55.gd967357\n> \n\nThis looks good to me. I wonder if I should post a patch \nto add the \"|| return\" trick to the following function\n\"test_cmp_exclude_clean\", too?!\n\nThanks,\nLars\n\n"},{"id":"305461","messageId":"C61D1F80-7B7A-4111-9418-75D178AF4353@gmail.com","threadId":"44310","inReplyTo":"3b58b90d-5435-1503-d052-413a947a5ab5@kdbg.org","subject":"Re: [PATCH (optional)] t0021: use arithmetic expansion to trim whitespace from wc -c output","fromName":"Lars Schneider","fromEmail":"larsxschneider@gmail.com","sentAt":"2016-11-06T15:45:50Z","receivedAt":"2016-11-06T15:45:59Z","isPatch":true,"sender":{"key":"larsxschneider@gmail.com","avatar":"https://avatars.githubusercontent.com/u/477434?v=4"},"body":"\n> On 03 Nov 2016, at 21:22, Johannes Sixt wrote:\n> \n> Instead of a pipeline with sed and a useless use of cat, return the\n> unmodified text of wc -c from function file_size, but substitute the\n> result with arithmetic expansion to get rid of the leading whitespace\n> that some version of wc -c print.\n> \n> Signed-off-by: Johannes Sixt \n> ---\n> This is a pure optimization that reduces the number of forks, which\n> helps a bit on Windows.\n> \n> There would be a solution with perl that does not require trimming\n> of whitespace, but perl startup times are unbearable on Windows.\n> wc -c is better.\n> \n> t/t0021-conversion.sh | 50 +++++++++++++++++++++++++-------------------------\n> 1 file changed, 25 insertions(+), 25 deletions(-)\n> \n> diff --git a/t/t0021-conversion.sh b/t/t0021-conversion.sh\n> index db71acacb3..42b529f615 100755\n> --- a/t/t0021-conversion.sh\n> +++ b/t/t0021-conversion.sh\n> @@ -22,7 +22,7 @@ generate_random_characters () {\n> }\n> \n> file_size () {\n> -\tcat \"$1\" | wc -c | sed \"s/^[ ]*//\"\n> +\twc -c <\"$1\"\n> }\n> \n> filter_git () {\n> @@ -369,10 +369,10 @@ test_expect_success PERL 'required process filter should filter data' '\n> \t\tcat >expected.log <<-EOF &&\n> \t\t\tSTART\n> \t\t\tinit handshake complete\n> -\t\t\tIN: clean test.r $S [OK] -- OUT: $S . [OK]\n> -\t\t\tIN: clean test2.r $S2 [OK] -- OUT: $S2 . [OK]\n> +\t\t\tIN: clean test.r $(($S)) [OK] -- OUT: $(($S)) . [OK]\n> +\t\t\tIN: clean test2.r $(($S2)) [OK] -- OUT: $(($S2)) . [OK]\n> \t\t\tIN: clean test4-empty.r 0 [OK] -- OUT: 0 [OK]\n> -\t\t\tIN: clean testsubdir/test3 '\\''sq'\\'',\\$x.r $S3 [OK] -- OUT: $S3 . [OK]\n> +\t\t\tIN: clean testsubdir/test3 '\\''sq'\\'',\\$x.r $(($S3)) [OK] -- OUT: $(($S3)) . [OK]\n> \t\t\tSTOP\n> \t\tEOF\n> \t\ttest_cmp_count expected.log rot13-filter.log &&\n> @@ -381,14 +381,14 @@ test_expect_success PERL 'required process filter should filter data' '\n> \t\tcat >expected.log <<-EOF &&\n> \t\t\tSTART\n> \t\t\tinit handshake complete\n> -\t\t\tIN: clean test.r $S [OK] -- OUT: $S . [OK]\n> -\t\t\tIN: clean test2.r $S2 [OK] -- OUT: $S2 . [OK]\n> +\t\t\tIN: clean test.r $(($S)) [OK] -- OUT: $(($S)) . [OK]\n> +\t\t\tIN: clean test2.r $(($S2)) [OK] -- OUT: $(($S2)) . [OK]\n> \t\t\tIN: clean test4-empty.r 0 [OK] -- OUT: 0 [OK]\n> -\t\t\tIN: clean testsubdir/test3 '\\''sq'\\'',\\$x.r $S3 [OK] -- OUT: $S3 . [OK]\n> -\t\t\tIN: clean test.r $S [OK] -- OUT: $S . [OK]\n> -\t\t\tIN: clean test2.r $S2 [OK] -- OUT: $S2 . [OK]\n> +\t\t\tIN: clean testsubdir/test3 '\\''sq'\\'',\\$x.r $(($S3)) [OK] -- OUT: $(($S3)) . [OK]\n> +\t\t\tIN: clean test.r $(($S)) [OK] -- OUT: $(($S)) . [OK]\n> +\t\t\tIN: clean test2.r $(($S2)) [OK] -- OUT: $(($S2)) . [OK]\n> \t\t\tIN: clean test4-empty.r 0 [OK] -- OUT: 0 [OK]\n> -\t\t\tIN: clean testsubdir/test3 '\\''sq'\\'',\\$x.r $S3 [OK] -- OUT: $S3 . [OK]\n> +\t\t\tIN: clean testsubdir/test3 '\\''sq'\\'',\\$x.r $(($S3)) [OK] -- OUT: $(($S3)) . [OK]\n> \t\t\tSTOP\n> \t\tEOF\n> \t\ttest_cmp_count expected.log rot13-filter.log &&\n> @@ -399,8 +399,8 @@ test_expect_success PERL 'required process filter should filter data' '\n> \t\tcat >expected.log <<-EOF &&\n> \t\t\tSTART\n> \t\t\tinit handshake complete\n> -\t\t\tIN: smudge test2.r $S2 [OK] -- OUT: $S2 . [OK]\n> -\t\t\tIN: smudge testsubdir/test3 '\\''sq'\\'',\\$x.r $S3 [OK] -- OUT: $S3 . [OK]\n> +\t\t\tIN: smudge test2.r $(($S2)) [OK] -- OUT: $(($S2)) . [OK]\n> +\t\t\tIN: smudge testsubdir/test3 '\\''sq'\\'',\\$x.r $(($S3)) [OK] -- OUT: $(($S3)) . [OK]\n> \t\t\tSTOP\n> \t\tEOF\n> \t\ttest_cmp_exclude_clean expected.log rot13-filter.log &&\n> @@ -409,7 +409,7 @@ test_expect_success PERL 'required process filter should filter data' '\n> \t\tcat >expected.log <<-EOF &&\n> \t\t\tSTART\n> \t\t\tinit handshake complete\n> -\t\t\tIN: clean test.r $S [OK] -- OUT: $S . [OK]\n> +\t\t\tIN: clean test.r $(($S)) [OK] -- OUT: $(($S)) . [OK]\n> \t\t\tSTOP\n> \t\tEOF\n> \t\ttest_cmp_exclude_clean expected.log rot13-filter.log &&\n> @@ -418,10 +418,10 @@ test_expect_success PERL 'required process filter should filter data' '\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 test2.r $S2 [OK] -- OUT: $S2 . [OK]\n> +\t\t\tIN: smudge test.r $(($S)) [OK] -- OUT: $(($S)) . [OK]\n> +\t\t\tIN: smudge test2.r $(($S2)) [OK] -- OUT: $(($S2)) . [OK]\n> \t\t\tIN: smudge test4-empty.r 0 [OK] -- OUT: 0 [OK]\n> -\t\t\tIN: smudge testsubdir/test3 '\\''sq'\\'',\\$x.r $S3 [OK] -- OUT: $S3 . [OK]\n> +\t\t\tIN: smudge testsubdir/test3 '\\''sq'\\'',\\$x.r $(($S3)) [OK] -- OUT: $(($S3)) . [OK]\n> \t\t\tSTOP\n> \t\tEOF\n> \t\ttest_cmp_exclude_clean expected.log rot13-filter.log &&\n> @@ -451,7 +451,7 @@ test_expect_success PERL 'required process filter takes precedence' '\n> \t\tcat >expected.log <<-EOF &&\n> \t\t\tSTART\n> \t\t\tinit handshake complete\n> -\t\t\tIN: clean test.r $S [OK] -- OUT: $S . [OK]\n> +\t\t\tIN: clean test.r $(($S)) [OK] -- OUT: $(($S)) . [OK]\n> \t\t\tSTOP\n> \t\tEOF\n> \t\ttest_cmp_count expected.log rot13-filter.log\n> @@ -474,7 +474,7 @@ test_expect_success PERL 'required process filter should be used only for \"clean\n> \t\tcat >expected.log <<-EOF &&\n> \t\t\tSTART\n> \t\t\tinit handshake complete\n> -\t\t\tIN: clean test.r $S [OK] -- OUT: $S . [OK]\n> +\t\t\tIN: clean test.r $(($S)) [OK] -- OUT: $(($S)) . [OK]\n> \t\t\tSTOP\n> \t\tEOF\n> \t\ttest_cmp_count expected.log rot13-filter.log &&\n> @@ -603,11 +603,11 @@ test_expect_success PERL 'process filter should restart after unexpected write f\n> \t\tcat >expected.log <<-EOF &&\n> \t\t\tSTART\n> \t\t\tinit handshake complete\n> -\t\t\tIN: smudge smudge-write-fail.r $SF [OK] -- OUT: $SF [WRITE FAIL]\n> +\t\t\tIN: smudge smudge-write-fail.r $(($SF)) [OK] -- OUT: $(($SF)) [WRITE FAIL]\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 test2.r $S2 [OK] -- OUT: $S2 . [OK]\n> +\t\t\tIN: smudge test.r $(($S)) [OK] -- OUT: $(($S)) . [OK]\n> +\t\t\tIN: smudge test2.r $(($S2)) [OK] -- OUT: $(($S2)) . [OK]\n> \t\t\tSTOP\n> \t\tEOF\n> \t\ttest_cmp_exclude_clean expected.log rot13-filter.log &&\n> @@ -649,9 +649,9 @@ test_expect_success PERL 'process filter should not be restarted if it signals a\n> \t\tcat >expected.log <<-EOF &&\n> \t\t\tSTART\n> \t\t\tinit handshake complete\n> -\t\t\tIN: smudge error.r $SE [OK] -- OUT: 0 [ERROR]\n> -\t\t\tIN: smudge test.r $S [OK] -- OUT: $S . [OK]\n> -\t\t\tIN: smudge test2.r $S2 [OK] -- OUT: $S2 . [OK]\n> +\t\t\tIN: smudge error.r $(($SE)) [OK] -- OUT: 0 [ERROR]\n> +\t\t\tIN: smudge test.r $(($S)) [OK] -- OUT: $(($S)) . [OK]\n> +\t\t\tIN: smudge test2.r $(($S2)) [OK] -- OUT: $(($S2)) . [OK]\n> \t\t\tSTOP\n> \t\tEOF\n> \t\ttest_cmp_exclude_clean expected.log rot13-filter.log &&\n> @@ -688,7 +688,7 @@ test_expect_success PERL 'process filter abort stops processing of all further f\n> \t\tcat >expected.log <<-EOF &&\n> \t\t\tSTART\n> \t\t\tinit handshake complete\n> -\t\t\tIN: smudge abort.r $SA [OK] -- OUT: 0 [ABORT]\n> +\t\t\tIN: smudge abort.r $(($SA)) [OK] -- OUT: 0 [ABORT]\n> \t\t\tSTOP\n> \t\tEOF\n> \t\ttest_cmp_exclude_clean expected.log rot13-filter.log &&\n> -- \n> 2.11.0.rc0.55.gd967357\n> \n\nThis looks good to me! As I run the Windows tests once in a while, too, \nI am in full support to make them faster :-)\n\nSince the file size function became very simple with your patch,\nshouldn't we get rid of it? If you agree, then we could squash the \npatch below into your patch.\n\nI also would like to move the arithmetic expansion to the variable\nto make the `expected.log` better readable. That means I would like\nto have this:\n\nS=$(($(wc -c \"$TEST_ROOT/$NAME\"\n }\n \n-file_size () {\n-\twc -c <\"$1\"\n-}\n-\n filter_git () {\n \trm -f rot13-filter.log &&\n \tgit \"$@\" 2>git-stderr.log &&\n@@ -363,9 +359,9 @@ test_expect_success PERL 'required process filter should filter data' '\n \t\tcp \"$TEST_ROOT/test3 '\\''sq'\\'',\\$x.o\" \"testsubdir/test3 '\\''sq'\\'',\\$x.r\" &&\n \t\t>test4-empty.r &&\n \n-\t\tS=$(file_size test.r) &&\n-\t\tS2=$(file_size test2.r) &&\n-\t\tS3=$(file_size \"testsubdir/test3 '\\''sq'\\'',\\$x.r\") &&\n+\t\tS=$(wc -c expected.log <<-EOF &&\n@@ -446,7 +442,7 @@ test_expect_success PERL 'required process filter takes precedence' '\n \n \t\techo \"*.r filter=protocol\" >.gitattributes &&\n \t\tcp \"$TEST_ROOT/test.o\" test.r &&\n-\t\tS=$(file_size test.r) &&\n+\t\tS=$(wc -c .gitattributes &&\n \t\tcp \"$TEST_ROOT/test.o\" test.r &&\n-\t\tS=$(file_size test.r) &&\n+\t\tS=$(wc -c expected.log <<-EOF &&\n@@ -589,9 +585,9 @@ test_expect_success PERL 'process filter should restart after unexpected write f\n \t\techo \"this is going to fail\" >smudge-write-fail.o &&\n \t\tcp smudge-write-fail.o smudge-write-fail.r &&\n \n-\t\tS=$(file_size test.r) &&\n-\t\tS2=$(file_size test2.r) &&\n-\t\tSF=$(file_size smudge-write-fail.r) &&\n+\t\tS=$(wc -c error.o &&\n \t\tcp error.o error.r &&\n \n-\t\tS=$(file_size test.r) &&\n-\t\tS2=$(file_size test2.r) &&\n-\t\tSE=$(file_size error.r) &&\n+\t\tS=$(wc -c abort.o &&\n \t\tcp abort.o abort.r &&\n \n-\t\tSA=$(file_size abort.r) &&\n+\t\tSA=$(wc -c \n>> On 03 Nov 2016, at 21:22, Johannes Sixt wrote:\n>> This is a pure optimization that reduces the number of forks, which\n>> helps a bit on Windows.\n>>\n>> There would be a solution with perl that does not require trimming\n>> of whitespace, but perl startup times are unbearable on Windows.\n>> wc -c is better.\n\nI was wrong here. I had looked at the perl invocations due to\ngit-sendemail, and they are awfully slow. A do-almost-nothing perl\ninvocation is in the same ballpark as wc. Therefore I changed my mind\nand suggest the patch below instead.\n\n> Since the file size function became very simple with your patch,\n> shouldn't we get rid of it? If you agree, then we could squash the \n> patch below into your patch.\n\nIn the new patch, the function is not that trivial (it uses perl), and\nthe call sites can remain as they are (simple shell variables and\nsubstitutions).\n\n---- 8< ----\n[PATCH] t0021: compute file size with a single process instead of a pipeline\n\nAvoid unwanted coding patterns (prodigal use of pipelines), and in\nparticular a useless use of cat.\n\nSigned-off-by: Johannes Sixt \n---\n t/t0021-conversion.sh | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/t/t0021-conversion.sh b/t/t0021-conversion.sh\nindex db71acacb3..cb72fa49de 100755\n--- a/t/t0021-conversion.sh\n+++ b/t/t0021-conversion.sh\n@@ -22,7 +22,7 @@ generate_random_characters () {\n }\n \n file_size () {\n-\tcat \"$1\" | wc -c | sed \"s/^[ ]*//\"\n+\tperl -e 'print -s $ARGV[0]' \"$1\"\n }\n \n filter_git () {\n-- \n2.11.0.rc0.55.gd967357\n\n"},{"id":"305470","messageId":"c842e0a7-b032-e0c4-0995-f11d93c17c0a@kdbg.org","threadId":"44310","inReplyTo":"0B7DE8F1-34DF-4271-8936-B9B596EF5F9E@gmail.com","subject":"Re: [PATCH] t0021: expect more variations in the output of uniq -c","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2016-11-06T21:40:02Z","receivedAt":"2016-11-06T21:40:10Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 06.11.2016 um 16:31 schrieb Lars Schneider:\n> This looks good to me. I wonder if I should post a patch\n> to add the \"|| return\" trick to the following function\n> \"test_cmp_exclude_clean\", too?!\n\nThe function does this:\n\n for FILE in \"$expect\" \"$actual\"\n do\n grep -v \"IN: clean\" \"$FILE\" >\"$FILE.tmp\" &&\n mv \"$FILE.tmp\" \"$FILE\"\n done &&\n\nIt is an interesting case: If only matching lines are in the file, \nnothing would be printed, and grep returns failure. But we shouldn't \ncare, because, strictly speaking, the function only wants to remove \nlines with a certain pattern. Here, it actually hurts to &&-chain grep!\n\nIt wouldn't hurt to write 'mv ... || return', but at the same time, && \nafter grep should be removed.\n\n-- Hannes\n\n"},{"id":"305474","messageId":"283FAD19-35C3-4167-8946-A0F2A9CEDC57@gmail.com","threadId":"44310","inReplyTo":"b87ddffd-3de1-4481-b484-9f03a73b6ad1@kdbg.org","subject":"Re: [PATCH (optional)] t0021: use arithmetic expansion to trim whitespace from wc -c output","fromName":"Lars Schneider","fromEmail":"larsxschneider@gmail.com","sentAt":"2016-11-06T19:43:31Z","receivedAt":"2016-11-07T02:35:45Z","isPatch":true,"sender":{"key":"larsxschneider@gmail.com","avatar":"https://avatars.githubusercontent.com/u/477434?v=4"},"body":"\n> On 06 Nov 2016, at 20:31, Johannes Sixt wrote:\n> \n> Am 06.11.2016 um 16:45 schrieb Lars Schneider:\n>> \n>>> On 03 Nov 2016, at 21:22, Johannes Sixt wrote:\n>>> This is a pure optimization that reduces the number of forks, which\n>>> helps a bit on Windows.\n>>> \n>>> There would be a solution with perl that does not require trimming\n>>> of whitespace, but perl startup times are unbearable on Windows.\n>>> wc -c is better.\n> \n> I was wrong here. I had looked at the perl invocations due to\n> git-sendemail, and they are awfully slow. A do-almost-nothing perl\n> invocation is in the same ballpark as wc. Therefore I changed my mind\n> and suggest the patch below instead.\n> \n>> Since the file size function became very simple with your patch,\n>> shouldn't we get rid of it? If you agree, then we could squash the \n>> patch below into your patch.\n> \n> In the new patch, the function is not that trivial (it uses perl), and\n> the call sites can remain as they are (simple shell variables and\n> substitutions).\n> \n> ---- 8< ----\n> [PATCH] t0021: compute file size with a single process instead of a pipeline\n> \n> Avoid unwanted coding patterns (prodigal use of pipelines), and in\n> particular a useless use of cat.\n> \n> Signed-off-by: Johannes Sixt \n> ---\n> t/t0021-conversion.sh | 2 +-\n> 1 file changed, 1 insertion(+), 1 deletion(-)\n> \n> diff --git a/t/t0021-conversion.sh b/t/t0021-conversion.sh\n> index db71acacb3..cb72fa49de 100755\n> --- a/t/t0021-conversion.sh\n> +++ b/t/t0021-conversion.sh\n> @@ -22,7 +22,7 @@ generate_random_characters () {\n> }\n> \n> file_size () {\n> -\tcat \"$1\" | wc -c | sed \"s/^[ ]*//\"\n> +\tperl -e 'print -s $ARGV[0]' \"$1\"\n> }\n> \n> filter_git () {\n> -- \n> 2.11.0.rc0.55.gd967357\n> \n\nI like this better, too. Looks good to me and works on macOS.\n\nThanks,\nLars\n\n"}]}