From: Ramsay Jones <ramsay@ramsayjones.plus.com>
To: larsxschneider@gmail.com, git@vger.kernel.org
Cc: peff@peff.net, jnareb@gmail.com, tboegi@web.de
Subject: Re: [PATCH v1 3/3] convert: add filter.<driver>.useProtocol option
Date: Sat, 23 Jul 2016 00:28:45 +0100 [thread overview]
Message-ID: <d32793ea-2b78-a16e-2d77-42e2b9d36ce8@ramsayjones.plus.com> (raw)
In-Reply-To: <9f47cf44-7163-a7a7-c1f0-87ebdee65b37@ramsayjones.plus.com>
Hi Lars,
On 23/07/16 00:19, Ramsay Jones wrote:
>
>
> On 22/07/16 16:49, larsxschneider@gmail.com wrote:
>> From: Lars Schneider <larsxschneider@gmail.com>
>>
>> Git's clean/smudge mechanism invokes an external filter process for every
>> single blob that is affected by a filter. If Git filters a lot of blobs
>> then the startup time of the external filter processes can become a
>> significant part of the overall Git execution time.
>>
>> This patch adds the filter.<driver>.useProtocol option which, if enabled,
>> keeps the external filter process running and processes all blobs with
>> the following protocol over stdin/stdout.
>>
>> 1. Git starts the filter on first usage and expects a welcome message
>> with protocol version number:
>> Git <-- Filter: "git-filter-protocol\n"
>> Git <-- Filter: "version 1"
>
> Hmm, I was a bit surprised to see a 'filter' talk first (but so long as the
> interaction is fully defined, I guess it doesn't matter).
>
> [If you wanted to check for a version, you could add a "version" command
> instead, just like "clean" and "smudge".]
>
> [snip]
>
>> diff --git a/convert.c b/convert.c
>> index 522e2c5..91ce86f 100644
>> --- a/convert.c
>> +++ b/convert.c
>> @@ -481,12 +481,188 @@ static int apply_filter(const char *path, const char *src, size_t len, int fd,
>> return ret;
>> }
>>
>> +static int cmd_process_map_init = 0;
>> +static struct hashmap cmd_process_map;
>> +
>> +struct cmd2process {
>> + struct hashmap_entry ent; /* must be the first member! */
>> + const char *cmd;
>> + long protocol;
>> + struct child_process process;
>> +};
>> +
>> +static int cmd2process_cmp(const struct cmd2process *e1,
>> + const struct cmd2process *e2,
>> + const void *unused)
>> +{
>> + return strcmp(e1->cmd, e2->cmd);
>> +}
>> +
>> +static struct cmd2process *find_protocol_filter_entry(const char *cmd)
>> +{
>> + struct cmd2process k;
>> + hashmap_entry_init(&k, strhash(cmd));
>> + k.cmd = cmd;
>> + return hashmap_get(&cmd_process_map, &k, NULL);
>> +}
>> +
>> +static void stop_protocol_filter(struct cmd2process *entry) {
>> + if (!entry)
>> + return;
>> + sigchain_push(SIGPIPE, SIG_IGN);
>> + close(entry->process.in);
>> + close(entry->process.out);
>> + sigchain_pop(SIGPIPE);
>> + finish_command(&entry->process);
>> + child_process_clear(&entry->process);
>> + hashmap_remove(&cmd_process_map, entry, NULL);
>> + free(entry);
>> +}
>> +
>> +static struct cmd2process *start_protocol_filter(const char *cmd)
>> +{
>> + int ret = 1;
>> + struct cmd2process *entry = NULL;
>> + struct child_process *process = NULL;
>> + struct strbuf nbuf = STRBUF_INIT;
>> + struct string_list split = STRING_LIST_INIT_NODUP;
>> + const char *argv[] = { NULL, NULL };
>> + const char *header = "git-filter-protocol\nversion";
>> +
>> + entry = xmalloc(sizeof(*entry));
>> + hashmap_entry_init(entry, strhash(cmd));
>> + entry->cmd = cmd;
>> + process = &entry->process;
>> +
>> + child_process_init(process);
>> + argv[0] = cmd;
>> + process->argv = argv;
>> + process->use_shell = 1;
>> + process->in = -1;
>> + process->out = -1;
>> +
>> + if (start_command(process)) {
>> + error("cannot fork to run external persistent filter '%s'", cmd);
>> + return NULL;
>> + }
>> + strbuf_reset(&nbuf);
>> +
>> + sigchain_push(SIGPIPE, SIG_IGN);
>> + ret &= strbuf_read_once(&nbuf, process->out, 0) > 0;
>
> Hmm, how much will be read into nbuf by this single call?
> Since strbuf_read_once() makes a single call to xread(), with
> a len argument that will probably be 8192, you can not really
> tell how much it will read, in general. (xread() does not
> guarantee how many bytes it will read.)
>
> In particular, it could be less than strlen(header).
Please ignore this email, it's late ... ;-)
Sorry for the noise.
[Off to bed now]
ATB,
Ramsay Jones
next prev parent reply other threads:[~2016-07-22 23:28 UTC|newest]
Thread overview: 77+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-07-22 15:48 [PATCH v1 0/3] Git filter protocol larsxschneider
2016-07-22 15:48 ` [PATCH v1 1/3] convert: quote filter names in error messages larsxschneider
2016-07-22 15:48 ` [PATCH v1 2/3] convert: modernize tests larsxschneider
2016-07-26 15:18 ` Remi Galan Alfonso
2016-07-26 20:40 ` Junio C Hamano
2016-07-22 15:49 ` [PATCH v1 3/3] convert: add filter.<driver>.useProtocol option larsxschneider
2016-07-22 22:32 ` Torsten Bögershausen
2016-07-24 12:09 ` Lars Schneider
2016-07-22 23:19 ` Ramsay Jones
2016-07-22 23:28 ` Ramsay Jones [this message]
2016-07-24 17:16 ` Lars Schneider
2016-07-24 22:36 ` Ramsay Jones
2016-07-24 23:22 ` Jakub Narębski
2016-07-25 20:32 ` Lars Schneider
2016-07-26 10:58 ` Jakub Narębski
2016-07-25 20:24 ` Lars Schneider
2016-07-23 0:11 ` Jakub Narębski
2016-07-23 7:27 ` Eric Wong
2016-07-26 20:00 ` Jeff King
2016-07-24 18:36 ` Lars Schneider
2016-07-24 20:14 ` Jakub Narębski
2016-07-24 21:30 ` Jakub Narębski
2016-07-25 20:16 ` Lars Schneider
2016-07-26 12:24 ` Jakub Narębski
2016-07-25 20:09 ` Lars Schneider
2016-07-26 14:18 ` Jakub Narębski
2016-07-23 8:14 ` Eric Wong
2016-07-24 19:11 ` Lars Schneider
2016-07-25 7:27 ` Eric Wong
2016-07-25 15:48 ` Duy Nguyen
2016-07-22 21:39 ` [PATCH v1 0/3] Git filter protocol Junio C Hamano
2016-07-24 11:24 ` Lars Schneider
2016-07-26 20:11 ` Jeff King
2016-07-27 0:06 ` [PATCH v2 0/5] " larsxschneider
2016-07-27 0:06 ` [PATCH v2 1/5] convert: quote filter names in error messages larsxschneider
2016-07-27 20:01 ` Jakub Narębski
2016-07-28 8:23 ` Lars Schneider
2016-07-27 0:06 ` [PATCH v2 2/5] convert: modernize tests larsxschneider
2016-07-27 0:06 ` [PATCH v2 3/5] pkt-line: extract and use `set_packet_header` function larsxschneider
2016-07-27 0:20 ` Junio C Hamano
2016-07-27 9:13 ` Lars Schneider
2016-07-27 16:31 ` Junio C Hamano
2016-07-27 0:06 ` [PATCH v2 4/5] convert: generate large test files only once larsxschneider
2016-07-27 2:35 ` Torsten Bögershausen
2016-07-27 13:32 ` Jeff King
2016-07-27 16:50 ` Lars Schneider
2016-07-27 0:06 ` [PATCH v2 5/5] convert: add filter.<driver>.process option larsxschneider
2016-07-27 1:32 ` Jeff King
2016-07-27 17:31 ` Lars Schneider
2016-07-27 18:11 ` Jeff King
2016-07-28 12:10 ` Lars Schneider
2016-07-28 13:35 ` Jeff King
2016-07-27 9:41 ` Eric Wong
2016-07-29 10:38 ` Lars Schneider
2016-07-29 11:24 ` Jakub Narębski
2016-07-29 11:31 ` Lars Schneider
2016-08-05 18:55 ` Eric Wong
2016-08-05 23:26 ` Lars Schneider
2016-08-05 23:38 ` Eric Wong
2016-07-27 23:31 ` Jakub Narębski
2016-07-29 8:04 ` Lars Schneider
2016-07-29 17:35 ` Junio C Hamano
2016-07-29 23:11 ` Jakub Narębski
2016-07-29 23:44 ` Lars Schneider
2016-07-30 9:32 ` Jakub Narębski
2016-07-28 10:32 ` Torsten Bögershausen
2016-07-27 19:08 ` [PATCH v2 0/5] Git filter protocol Jakub Narębski
2016-07-28 7:16 ` Lars Schneider
2016-07-28 10:42 ` Jakub Narębski
2016-07-28 13:29 ` Jeff King
2016-07-29 7:40 ` Jakub Narębski
2016-07-29 8:14 ` Lars Schneider
2016-07-29 15:57 ` Jeff King
2016-07-29 16:20 ` Lars Schneider
2016-07-29 16:50 ` Jeff King
2016-07-29 17:43 ` Lars Schneider
2016-07-29 18:27 ` Jeff King
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: http://vger.kernel.org/majordomo-info.html
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=d32793ea-2b78-a16e-2d77-42e2b9d36ce8@ramsayjones.plus.com \
--to=ramsay@ramsayjones.plus.com \
--cc=git@vger.kernel.org \
--cc=jnareb@gmail.com \
--cc=larsxschneider@gmail.com \
--cc=peff@peff.net \
--cc=tboegi@web.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).