sub-process: refactor handshake to common function

Refactor, into a common function, the version and capability negotiation done when invoking a long-running process as a clean or smudge filter. This will be useful for other Git code that needs to interact similarly with a long-running process. As you can see in the change to t0021, this commit changes the error message reported when the long-running process does not introduce itself with the expected "server"-terminated line. Originally, the error message reports that the filter "does not support filter protocol version 2", differentiating between the old single-file filter protocol and the new multi-file filter protocol - I have updated it to something more generic and useful. Signed-off-by: Jonathan Tan <jonathantanmy@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Jonathan Tan committed Jul 26, 2017 at 11:17 UTC fa64a2fdbeedd98c5f24d1662bcc470a8449abcf
6 files changed +138 -90
convert.c
+7 -68
@@ -513,78 +513,17 @@ static struct hashmap subprocess_map;
513
514 static int start_multi_file_filter_fn(struct subprocess_entry *subprocess)
515 {
516 - int err, i;
517 - struct cmd2process *entry = (struct cmd2process *)subprocess;
518 - struct string_list cap_list = STRING_LIST_INIT_NODUP;
519 - char *cap_buf;
520 - const char *cap_name;
521 - struct child_process *process = &subprocess->process;
522 - const char *cmd = subprocess->cmd;
523 -
524 - static const struct {
525 - const char *name;
526 - unsigned int cap;
527 - } known_caps[] = {
516 + static int versions[] = {2, 0};
517 + static struct subprocess_capability capabilities[] = {
518 { "clean", CAP_CLEAN },
519 { "smudge", CAP_SMUDGE },
520 { "delay", CAP_DELAY },
521 + { NULL, 0 }
522 };
532 -
533 - sigchain_push(SIGPIPE, SIG_IGN);
534 -
535 - err = packet_writel(process->in, "git-filter-client", "version=2", NULL);
536 - if (err)
537 - goto done;
538 -
539 - err = strcmp(packet_read_line(process->out, NULL), "git-filter-server");
540 - if (err) {
541 - error("external filter '%s' does not support filter protocol version 2", cmd);
542 - goto done;
543 - }
544 - err = strcmp(packet_read_line(process->out, NULL), "version=2");
545 - if (err)
546 - goto done;
547 - err = packet_read_line(process->out, NULL) != NULL;
548 - if (err)
549 - goto done;
550 -
551 - for (i = 0; i < ARRAY_SIZE(known_caps); ++i) {
552 - err = packet_write_fmt_gently(
553 - process->in, "capability=%s\n", known_caps[i].name);
554 - if (err)
555 - goto done;
556 - }
557 - err = packet_flush_gently(process->in);
558 - if (err)
559 - goto done;
560 -
561 - for (;;) {
562 - cap_buf = packet_read_line(process->out, NULL);
563 - if (!cap_buf)
564 - break;
565 - string_list_split_in_place(&cap_list, cap_buf, '=', 1);
566 -
567 - if (cap_list.nr != 2 || strcmp(cap_list.items[0].string, "capability"))
568 - continue;
569 -
570 - cap_name = cap_list.items[1].string;
571 - i = ARRAY_SIZE(known_caps) - 1;
572 - while (i >= 0 && strcmp(cap_name, known_caps[i].name))
573 - i--;
574 -
575 - if (i >= 0)
576 - entry->supported_capabilities |= known_caps[i].cap;
577 - else
578 - warning("external filter '%s' requested unsupported filter capability '%s'",
579 - cmd, cap_name);
580 -
581 - string_list_clear(&cap_list, 0);
582 - }
583 -
584 -done:
585 - sigchain_pop(SIGPIPE);
586 -
587 - return err;
523 + struct cmd2process *entry = (struct cmd2process *)subprocess;
524 + return subprocess_handshake(subprocess, "git-filter", versions, NULL,
525 + capabilities,
526 + &entry->supported_capabilities);
527 }
528
529 static void handle_filter_error(const struct strbuf *filter_status,
pkt-line.c
-19
@@ -171,25 +171,6 @@ int packet_write_fmt_gently(int fd, const char *fmt, ...)
171 return status;
172 }
173
174 -int packet_writel(int fd, const char *line, ...)
175 -{
176 - va_list args;
177 - int err;
178 - va_start(args, line);
179 - for (;;) {
180 - if (!line)
181 - break;
182 - if (strlen(line) > LARGE_PACKET_DATA_MAX)
183 - return -1;
184 - err = packet_write_fmt_gently(fd, "%s\n", line);
185 - if (err)
186 - return err;
187 - line = va_arg(args, const char*);
188 - }
189 - va_end(args);
190 - return packet_flush_gently(fd);
191 -}
192 -
174 static int packet_write_gently(const int fd_out, const char *buf, size_t size)
175 {
176 static char packet_write_buffer[LARGE_PACKET_MAX];
pkt-line.h
-2
@@ -25,8 +25,6 @@ void packet_buf_flush(struct strbuf *buf);
25 void packet_buf_write(struct strbuf *buf, const char *fmt, ...) __attribute__((format (printf, 2, 3)));
26 int packet_flush_gently(int fd);
27 int packet_write_fmt_gently(int fd, const char *fmt, ...) __attribute__((format (printf, 2, 3)));
28 -LAST_ARG_MUST_BE_NULL
29 -int packet_writel(int fd, const char *line, ...);
28 int write_packetized_from_fd(int fd_in, int fd_out);
29 int write_packetized_from_buf(const char *src_in, size_t len, int fd_out);
30
sub-process.c
+104
@@ -105,3 +105,107 @@ int subprocess_start(struct hashmap *hashmap, struct subprocess_entry *entry, co
105 hashmap_add(hashmap, entry);
106 return 0;
107 }
108 +
109 +static int handshake_version(struct child_process *process,
110 + const char *welcome_prefix, int *versions,
111 + int *chosen_version)
112 +{
113 + int version_scratch;
114 + int i;
115 + char *line;
116 + const char *p;
117 +
118 + if (!chosen_version)
119 + chosen_version = &version_scratch;
120 +
121 + if (packet_write_fmt_gently(process->in, "%s-client\n",
122 + welcome_prefix))
123 + return error("Could not write client identification");
124 + for (i = 0; versions[i]; i++) {
125 + if (packet_write_fmt_gently(process->in, "version=%d\n",
126 + versions[i]))
127 + return error("Could not write requested version");
128 + }
129 + if (packet_flush_gently(process->in))
130 + return error("Could not write flush packet");
131 +
132 + if (!(line = packet_read_line(process->out, NULL)) ||
133 + !skip_prefix(line, welcome_prefix, &p) ||
134 + strcmp(p, "-server"))
135 + return error("Unexpected line '%s', expected %s-server",
136 + line ? line : "<flush packet>", welcome_prefix);
137 + if (!(line = packet_read_line(process->out, NULL)) ||
138 + !skip_prefix(line, "version=", &p) ||
139 + strtol_i(p, 10, chosen_version))
140 + return error("Unexpected line '%s', expected version",
141 + line ? line : "<flush packet>");
142 + if ((line = packet_read_line(process->out, NULL)))
143 + return error("Unexpected line '%s', expected flush", line);
144 +
145 + /* Check to make sure that the version received is supported */
146 + for (i = 0; versions[i]; i++) {
147 + if (versions[i] == *chosen_version)
148 + break;
149 + }
150 + if (!versions[i])
151 + return error("Version %d not supported", *chosen_version);
152 +
153 + return 0;
154 +}
155 +
156 +static int handshake_capabilities(struct child_process *process,
157 + struct subprocess_capability *capabilities,
158 + unsigned int *supported_capabilities)
159 +{
160 + int i;
161 + char *line;
162 +
163 + for (i = 0; capabilities[i].name; i++) {
164 + if (packet_write_fmt_gently(process->in, "capability=%s\n",
165 + capabilities[i].name))
166 + return error("Could not write requested capability");
167 + }
168 + if (packet_flush_gently(process->in))
169 + return error("Could not write flush packet");
170 +
171 + while ((line = packet_read_line(process->out, NULL))) {
172 + const char *p;
173 + if (!skip_prefix(line, "capability=", &p))
174 + continue;
175 +
176 + for (i = 0;
177 + capabilities[i].name && strcmp(p, capabilities[i].name);
178 + i++)
179 + ;
180 + if (capabilities[i].name) {
181 + if (supported_capabilities)
182 + *supported_capabilities |= capabilities[i].flag;
183 + } else {
184 + warning("external filter requested unsupported filter capability '%s'",
185 + p);
186 + }
187 + }
188 +
189 + return 0;
190 +}
191 +
192 +int subprocess_handshake(struct subprocess_entry *entry,
193 + const char *welcome_prefix,
194 + int *versions,
195 + int *chosen_version,
196 + struct subprocess_capability *capabilities,
197 + unsigned int *supported_capabilities)
198 +{
199 + int retval;
200 + struct child_process *process = &entry->process;
201 +
202 + sigchain_push(SIGPIPE, SIG_IGN);
203 +
204 + retval = handshake_version(process, welcome_prefix, versions,
205 + chosen_version) ||
206 + handshake_capabilities(process, capabilities,
207 + supported_capabilities);
208 +
209 + sigchain_pop(SIGPIPE);
210 + return retval;
211 +}
sub-process.h
+26
@@ -29,6 +29,16 @@ struct subprocess_entry {
29 struct child_process process;
30 };
31
32 +struct subprocess_capability {
33 + const char *name;
34 +
35 + /*
36 + * subprocess_handshake will "|=" this value to supported_capabilities
37 + * if the server reports that it supports this capability.
38 + */
39 + unsigned int flag;
40 +};
41 +
42 /* subprocess functions */
43
44 /* Function to test two subprocess hashmap entries for equality. */
@@ -62,6 +72,22 @@ static inline struct child_process *subprocess_get_child_process(
72 return &entry->process;
73 }
74
75 +/*
76 + * Perform the version and capability negotiation as described in the "Long
77 + * Running Filter Process" section of the gitattributes documentation using the
78 + * given requested versions and capabilities. The "versions" and "capabilities"
79 + * parameters are arrays terminated by a 0 or blank struct.
80 + *
81 + * This function is typically called when a subprocess is started (as part of
82 + * the "startfn" passed to subprocess_start).
83 + */
84 +int subprocess_handshake(struct subprocess_entry *entry,
85 + const char *welcome_prefix,
86 + int *versions,
87 + int *chosen_version,
88 + struct subprocess_capability *capabilities,
89 + unsigned int *supported_capabilities);
90 +
91 /*
92 * Helper function that will read packets looking for "status=<foo>"
93 * key/value pairs and return the value from the last "status" packet
t/t0021-conversion.sh
+1 -1
@@ -697,7 +697,7 @@ test_expect_success PERL 'invalid process filter must fail (and not hang!)' '
697
698 cp "$TEST_ROOT/test.o" test.r &&
699 test_must_fail git add . 2>git-stderr.log &&
700 - grep "does not support filter protocol version" git-stderr.log
700 + grep "expected git-filter-server" git-stderr.log
701 )
702 '
703