Hi Lars,
On 23/07/16 00:19, Ramsay Jones wrote:
>
>
> On 22/07/16 16:49, larsxschneider@xxxxxxxxx wrote:
>> From: Lars Schneider <larsxschneider@xxxxxxxxx>
>>
>> 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
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html