{fetch,upload}-pack: sideband v2 fetch response

Currently, a response to a fetch request has sideband support only while the packfile is being sent, meaning that the server cannot send notices until the start of the packfile. Extend sideband support in protocol v2 fetch responses to the whole response. upload-pack will advertise it if the uploadpack.allowsidebandall configuration variable is set, and fetch-pack will automatically request it if advertised. If the sideband is to be used throughout the whole response, upload-pack will use it to send errors instead of prefixing a PKT-LINE payload with "ERR ". This will be tested in a subsequent patch. Signed-off-by: Jonathan Tan <jonathantanmy@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Jonathan Tan committed Jan 16, 2019 at 11:28 UTC 0bbc0bc5745ab8b294a5faf8c3b1d939ae8b6d10
7 files changed +78 -13
Documentation/technical/protocol-v2.txt
+10
@@ -307,6 +307,16 @@ the 'wanted-refs' section in the server's response as explained below.
307 particular ref, where <ref> is the full name of a ref on the
308 server.
309
310 +If the 'sideband-all' feature is advertised, the following argument can be
311 +included in the client's request:
312 +
313 + sideband-all
314 + Instruct the server to send the whole response multiplexed, not just
315 + the packfile section. All non-flush and non-delim PKT-LINE in the
316 + response (not only in the packfile section) will then start with a byte
317 + indicating its sideband (1, 2, or 3), and the server may send "0005\2"
318 + (a PKT-LINE of sideband 2 with no payload) as a keepalive packet.
319 +
320 The response of `fetch` is broken into a number of sections separated by
321 delimiter packets (0001), with each section beginning with its section
322 header.
fetch-pack.c
+10 -2
@@ -1090,7 +1090,8 @@ static int add_haves(struct fetch_negotiator *negotiator,
1090 static int send_fetch_request(struct fetch_negotiator *negotiator, int fd_out,
1091 const struct fetch_pack_args *args,
1092 const struct ref *wants, struct oidset *common,
1093 - int *haves_to_send, int *in_vain)
1093 + int *haves_to_send, int *in_vain,
1094 + int sideband_all)
1095 {
1096 int ret = 0;
1097 struct strbuf req_buf = STRBUF_INIT;
@@ -1116,6 +1117,8 @@ static int send_fetch_request(struct fetch_negotiator *negotiator, int fd_out,
1117 packet_buf_write(&req_buf, "include-tag");
1118 if (prefer_ofs_delta)
1119 packet_buf_write(&req_buf, "ofs-delta");
1120 + if (sideband_all)
1121 + packet_buf_write(&req_buf, "sideband-all");
1122
1123 /* Add shallow-info and deepen request */
1124 if (server_supports_feature("fetch", "shallow", 0))
@@ -1324,6 +1327,10 @@ static struct ref *do_fetch_pack_v2(struct fetch_pack_args *args,
1327 packet_reader_init(&reader, fd[0], NULL, 0,
1328 PACKET_READ_CHOMP_NEWLINE |
1329 PACKET_READ_DIE_ON_ERR_PACKET);
1330 + if (server_supports_feature("fetch", "sideband-all", 0)) {
1331 + reader.use_sideband = 1;
1332 + reader.me = "fetch-pack";
1333 + }
1334
1335 while (state != FETCH_DONE) {
1336 switch (state) {
@@ -1357,7 +1364,8 @@ static struct ref *do_fetch_pack_v2(struct fetch_pack_args *args,
1364 case FETCH_SEND_REQUEST:
1365 if (send_fetch_request(&negotiator, fd[1], args, ref,
1366 &common,
1360 - &haves_to_send, &in_vain))
1367 + &haves_to_send, &in_vain,
1368 + reader.use_sideband))
1369 state = FETCH_GET_PACK;
1370 else
1371 state = FETCH_PROCESS_ACKS;
pkt-line.c
+32 -11
@@ -449,7 +449,7 @@ int recv_sideband(const char *me, int in_stream, int out)
449 while (1) {
450 len = packet_read(in_stream, NULL, NULL, buf, LARGE_PACKET_MAX,
451 0);
452 - if (!demultiplex_sideband(me, buf, len, &scratch,
452 + if (!demultiplex_sideband(me, buf, len, 0, &scratch,
453 &sideband_type))
454 continue;
455 switch (sideband_type) {
@@ -475,25 +475,43 @@ void packet_reader_init(struct packet_reader *reader, int fd,
475 reader->buffer = packet_buffer;
476 reader->buffer_size = sizeof(packet_buffer);
477 reader->options = options;
478 + reader->me = "git";
479 }
480
481 enum packet_read_status packet_reader_read(struct packet_reader *reader)
482 {
483 + struct strbuf scratch = STRBUF_INIT;
484 +
485 if (reader->line_peeked) {
486 reader->line_peeked = 0;
487 return reader->status;
488 }
489
487 - reader->status = packet_read_with_status(reader->fd,
488 - &reader->src_buffer,
489 - &reader->src_len,
490 - reader->buffer,
491 - reader->buffer_size,
492 - &reader->pktlen,
493 - reader->options);
490 + /*
491 + * Consume all progress packets until a primary payload packet is
492 + * received
493 + */
494 + while (1) {
495 + enum sideband_type sideband_type;
496 + reader->status = packet_read_with_status(reader->fd,
497 + &reader->src_buffer,
498 + &reader->src_len,
499 + reader->buffer,
500 + reader->buffer_size,
501 + &reader->pktlen,
502 + reader->options);
503 + if (!reader->use_sideband)
504 + break;
505 + if (demultiplex_sideband(reader->me, reader->buffer,
506 + reader->pktlen, 1, &scratch,
507 + &sideband_type))
508 + break;
509 + }
510
511 if (reader->status == PACKET_READ_NORMAL)
496 - reader->line = reader->buffer;
512 + /* Skip the sideband designator if sideband is used */
513 + reader->line = reader->use_sideband ?
514 + reader->buffer + 1 : reader->buffer;
515 else
516 reader->line = NULL;
517
@@ -515,6 +533,7 @@ enum packet_read_status packet_reader_peek(struct packet_reader *reader)
533 void packet_writer_init(struct packet_writer *writer, int dest_fd)
534 {
535 writer->dest_fd = dest_fd;
536 + writer->use_sideband = 0;
537 }
538
539 void packet_writer_write(struct packet_writer *writer, const char *fmt, ...)
@@ -522,7 +541,8 @@ void packet_writer_write(struct packet_writer *writer, const char *fmt, ...)
541 va_list args;
542
543 va_start(args, fmt);
525 - packet_write_fmt_1(writer->dest_fd, 0, "", fmt, args);
544 + packet_write_fmt_1(writer->dest_fd, 0,
545 + writer->use_sideband ? "\001" : "", fmt, args);
546 va_end(args);
547 }
548
@@ -531,7 +551,8 @@ void packet_writer_error(struct packet_writer *writer, const char *fmt, ...)
551 va_list args;
552
553 va_start(args, fmt);
534 - packet_write_fmt_1(writer->dest_fd, 0, "ERR ", fmt, args);
554 + packet_write_fmt_1(writer->dest_fd, 0,
555 + writer->use_sideband ? "\003" : "ERR ", fmt, args);
556 va_end(args);
557 }
558
pkt-line.h
+4
@@ -162,6 +162,9 @@ struct packet_reader {
162
163 /* indicates if a line has been peeked */
164 int line_peeked;
165 +
166 + unsigned use_sideband : 1;
167 + const char *me;
168 };
169
170 /*
@@ -201,6 +204,7 @@ extern char packet_buffer[LARGE_PACKET_MAX];
204
205 struct packet_writer {
206 int dest_fd;
207 + unsigned use_sideband : 1;
208 };
209
210 void packet_writer_init(struct packet_writer *writer, int dest_fd);
sideband.c
+5
@@ -114,6 +114,7 @@ static void maybe_colorize_sideband(struct strbuf *dest, const char *src, int n)
114 #define DUMB_SUFFIX " "
115
116 int demultiplex_sideband(const char *me, char *buf, int len,
117 + int die_on_error,
118 struct strbuf *scratch,
119 enum sideband_type *sideband_type)
120 {
@@ -144,6 +145,8 @@ int demultiplex_sideband(const char *me, char *buf, int len,
145 len--;
146 switch (band) {
147 case 3:
148 + if (die_on_error)
149 + die("remote error: %s", buf + 1);
150 strbuf_addf(scratch, "%s%s", scratch->len ? "\n" : "",
151 DISPLAY_PREFIX);
152 maybe_colorize_sideband(scratch, buf + 1, len);
@@ -195,6 +198,8 @@ int demultiplex_sideband(const char *me, char *buf, int len,
198 }
199
200 cleanup:
201 + if (die_on_error && *sideband_type == SIDEBAND_PROTOCOL_ERROR)
202 + die("%s", scratch->buf);
203 if (scratch->len) {
204 strbuf_addch(scratch, '\n');
205 xwrite(2, scratch->buf, scratch->len);
sideband.h
+1
@@ -20,6 +20,7 @@ enum sideband_type {
20 * progress messages split across multiple packets.
21 */
22 int demultiplex_sideband(const char *me, char *buf, int len,
23 + int die_on_error,
24 struct strbuf *scratch,
25 enum sideband_type *sideband_type);
26
upload-pack.c
+16
@@ -71,6 +71,8 @@ static int allow_filter;
71 static int allow_ref_in_want;
72 static struct list_objects_filter_options filter_options;
73
74 +static int allow_sideband_all;
75 +
76 static void reset_timeout(void)
77 {
78 alarm(timeout);
@@ -1046,6 +1048,8 @@ static int upload_pack_config(const char *var, const char *value, void *unused)
1048 allow_filter = git_config_bool(var, value);
1049 } else if (!strcmp("uploadpack.allowrefinwant", var)) {
1050 allow_ref_in_want = git_config_bool(var, value);
1051 + } else if (!strcmp("uploadpack.allowsidebandall", var)) {
1052 + allow_sideband_all = git_config_bool(var, value);
1053 }
1054
1055 if (current_config_scope() != CONFIG_SCOPE_REPO) {
@@ -1284,6 +1288,11 @@ static void process_args(struct packet_reader *request,
1288 continue;
1289 }
1290
1291 + if (allow_sideband_all && !strcmp(arg, "sideband-all")) {
1292 + data->writer.use_sideband = 1;
1293 + continue;
1294 + }
1295 +
1296 /* ignore unknown lines maybe? */
1297 die("unexpected line: '%s'", arg);
1298 }
@@ -1496,6 +1505,7 @@ int upload_pack_advertise(struct repository *r,
1505 if (value) {
1506 int allow_filter_value;
1507 int allow_ref_in_want;
1508 + int allow_sideband_all_value;
1509
1510 strbuf_addstr(value, "shallow");
1511
@@ -1510,6 +1520,12 @@ int upload_pack_advertise(struct repository *r,
1520 &allow_ref_in_want) &&
1521 allow_ref_in_want)
1522 strbuf_addstr(value, " ref-in-want");
1523 +
1524 + if (!repo_config_get_bool(the_repository,
1525 + "uploadpack.allowsidebandall",
1526 + &allow_sideband_all_value) &&
1527 + allow_sideband_all_value)
1528 + strbuf_addstr(value, " sideband-all");
1529 }
1530
1531 return 1;