Merge branch 'jk/push-progress'

"git push" and "git clone" learned to give better progress meters
to the end user who is waiting on the terminal.

* jk/push-progress:
  receive-pack: send keepalives during quiet periods
  receive-pack: turn on connectivity progress
  receive-pack: relay connectivity errors to sideband
  receive-pack: turn on index-pack resolving progress
  index-pack: add flag for showing delta-resolution progress
  clone: use a real progress meter for connectivity check
  check_connected: add progress flag
  check_connected: relay errors to alternate descriptor
  check_everything_connected: use a struct with named options
  check_everything_connected: convert to argv_array
  rev-list: add optional progress reporting
  check_everything_connected: always pass --quiet to rev-list

Documentation/config.txt

@@ -2488,6 +2488,15 @@ receive.fsck.skipList::
 	can be safely ignored such as invalid committer email addresses.
 	Note: corrupt objects cannot be skipped with this setting.
 
+receive.keepAlive::
+	After receiving the pack from the client, `receive-pack` may
+	produce no output (if `--quiet` was specified) while processing
+	the pack, causing some networks to drop the TCP connection.
+	With this option set, if `receive-pack` does not transmit
+	any data in this phase for `receive.keepAlive` seconds, it will
+	send a short keepalive packet.  The default is 5 seconds; set
+	to 0 to disable keepalives entirely.
+
 receive.unpackLimit::
 	If the number of objects received in a push is below this
 	limit then the objects will be unpacked into loose object

Documentation/rev-list-options.txt

@@ -274,6 +274,10 @@ ifdef::git-rev-list[]
 	Try to speed up the traversal using the pack bitmap index (if
 	one is available). Note that when traversing with `--objects`,
 	trees and blobs will not have their associated path printed.
+
+--progress=<header>::
+	Show progress reports on stderr as objects are considered. The
+	`<header>` text will be printed with each progress update.
 endif::git-rev-list[]
 
 --

builtin/clone.c

@@ -624,13 +624,13 @@ static void update_remote_refs(const struct ref *refs,
 	const struct ref *rm = mapped_refs;
 
 	if (check_connectivity) {
-		if (transport->progress)
+		struct check_connected_options opt = CHECK_CONNECTED_INIT;
-			fprintf(stderr, _("Checking connectivity... "));
+
-		if (check_everything_connected_with_transport(iterate_ref_map,
+		opt.transport = transport;
-							      0, &rm, transport))
+		opt.progress = transport->progress;
+
+		if (check_connected(iterate_ref_map, &rm, &opt))
 			die(_("remote did not send all necessary objects"));
-		if (transport->progress)
-			fprintf(stderr, _("done.\n"));
 	}
 
 	if (refs) {

builtin/fetch.c

@@ -729,7 +729,7 @@ static int store_updated_refs(const char *raw_url, const char *remote_name,
 		url = xstrdup("foreign");
 
 	rm = ref_map;
-	if (check_everything_connected(iterate_ref_map, 0, &rm)) {
+	if (check_connected(iterate_ref_map, &rm, NULL)) {
 		rc = error(_("%s did not send all necessary objects\n"), url);
 		goto abort;
 	}
@@ -866,6 +866,7 @@ static int store_updated_refs(const char *raw_url, const char *remote_name,
 static int quickfetch(struct ref *ref_map)
 {
 	struct ref *rm = ref_map;
+	struct check_connected_options opt = CHECK_CONNECTED_INIT;
 
 	/*
 	 * If we are deepening a shallow clone we already have these
@@ -876,7 +877,8 @@ static int quickfetch(struct ref *ref_map)
 	 */
 	if (depth)
 		return -1;
-	return check_everything_connected(iterate_ref_map, 1, &rm);
+	opt.quiet = 1;
+	return check_connected(iterate_ref_map, &rm, &opt);
 }
 
 static int fetch_refs(struct transport *transport, struct ref *ref_map)

builtin/index-pack.c

@@ -77,6 +77,7 @@ static int strict;
 static int do_fsck_object;
 static struct fsck_options fsck_options = FSCK_OPTIONS_STRICT;
 static int verbose;
+static int show_resolving_progress;
 static int show_stat;
 static int check_self_contained_and_connected;
 
@@ -1191,7 +1192,7 @@ static void resolve_deltas(void)
 	qsort(ref_deltas, nr_ref_deltas, sizeof(struct ref_delta_entry),
 	      compare_ref_delta_entry);
 
-	if (verbose)
+	if (verbose || show_resolving_progress)
 		progress = start_progress(_("Resolving deltas"),
 					  nr_ref_deltas + nr_ofs_deltas);
 
@@ -1626,6 +1627,7 @@ int cmd_index_pack(int argc, const char **argv, const char *prefix)
 	struct pack_idx_option opts;
 	unsigned char pack_sha1[20];
 	unsigned foreign_nr = 1;	/* zero is a "good" value, assume bad */
+	int report_end_of_input = 0;
 
 	if (argc == 2 && !strcmp(argv[1], "-h"))
 		usage(index_pack_usage);
@@ -1695,6 +1697,10 @@ int cmd_index_pack(int argc, const char **argv, const char *prefix)
 				input_len = sizeof(*hdr);
 			} else if (!strcmp(arg, "-v")) {
 				verbose = 1;
+			} else if (!strcmp(arg, "--show-resolving-progress")) {
+				show_resolving_progress = 1;
+			} else if (!strcmp(arg, "--report-end-of-input")) {
+				report_end_of_input = 1;
 			} else if (!strcmp(arg, "-o")) {
 				if (index_name || (i+1) >= argc)
 					usage(index_pack_usage);
@@ -1752,6 +1758,8 @@ int cmd_index_pack(int argc, const char **argv, const char *prefix)
 		obj_stat = xcalloc(st_add(nr_objects, 1), sizeof(struct object_stat));
 	ofs_deltas = xcalloc(nr_objects, sizeof(struct ofs_delta_entry));
 	parse_pack_objects(pack_sha1);
+	if (report_end_of_input)
+		write_in_full(2, "\0", 1);
 	resolve_deltas();
 	conclude_pack(fix_thin_pack, curr_pack, pack_sha1);
 	free(ofs_deltas);

builtin/receive-pack.c

@@ -78,6 +78,13 @@ static long nonce_stamp_slop;
 static unsigned long nonce_stamp_slop_limit;
 static struct ref_transaction *transaction;
 
+static enum {
+	KEEPALIVE_NEVER = 0,
+	KEEPALIVE_AFTER_NUL,
+	KEEPALIVE_ALWAYS
+} use_keepalive;
+static int keepalive_in_sec = 5;
+
 static enum deny_action parse_deny_action(const char *var, const char *value)
 {
 	if (value) {
@@ -200,6 +207,11 @@ static int receive_pack_config(const char *var, const char *value, void *cb)
 		return 0;
 	}
 
+	if (strcmp(var, "receive.keepalive") == 0) {
+		keepalive_in_sec = git_config_int(var, value);
+		return 0;
+	}
+
 	return git_default_config(var, value, cb);
 }
 
@@ -328,10 +340,60 @@ static void rp_error(const char *err, ...)
 static int copy_to_sideband(int in, int out, void *arg)
 {
 	char data[128];
+	int keepalive_active = 0;
+
+	if (keepalive_in_sec <= 0)
+		use_keepalive = KEEPALIVE_NEVER;
+	if (use_keepalive == KEEPALIVE_ALWAYS)
+		keepalive_active = 1;
+
 	while (1) {
-		ssize_t sz = xread(in, data, sizeof(data));
+		ssize_t sz;
+
+		if (keepalive_active) {
+			struct pollfd pfd;
+			int ret;
+
+			pfd.fd = in;
+			pfd.events = POLLIN;
+			ret = poll(&pfd, 1, 1000 * keepalive_in_sec);
+
+			if (ret < 0) {
+				if (errno == EINTR)
+					continue;
+				else
+					break;
+			} else if (ret == 0) {
+				/* no data; send a keepalive packet */
+				static const char buf[] = "0005\1";
+				write_or_die(1, buf, sizeof(buf) - 1);
+				continue;
+			} /* else there is actual data to read */
+		}
+
+		sz = xread(in, data, sizeof(data));
 		if (sz <= 0)
 			break;
+
+		if (use_keepalive == KEEPALIVE_AFTER_NUL && !keepalive_active) {
+			const char *p = memchr(data, '\0', sz);
+			if (p) {
+				/*
+				 * The NUL tells us to start sending keepalives. Make
+				 * sure we send any other data we read along
+				 * with it.
+				 */
+				keepalive_active = 1;
+				send_sideband(1, 2, data, p - data, use_sideband);
+				send_sideband(1, 2, p + 1, sz - (p - data + 1), use_sideband);
+				continue;
+			}
+		}
+
+		/*
+		 * Either we're not looking for a NUL signal, or we didn't see
+		 * it yet; just pass along the data.
+		 */
 		send_sideband(1, 2, data, sz, use_sideband);
 	}
 	close(in);
@@ -761,7 +823,7 @@ static int update_shallow_ref(struct command *cmd, struct shallow_info *si)
 {
 	static struct lock_file shallow_lock;
 	struct sha1_array extra = SHA1_ARRAY_INIT;
-	const char *alt_file;
+	struct check_connected_options opt = CHECK_CONNECTED_INIT;
 	uint32_t mask = 1 << (cmd->index % 32);
 	int i;
 
@@ -773,9 +835,8 @@ static int update_shallow_ref(struct command *cmd, struct shallow_info *si)
 		    !delayed_reachability_test(si, i))
 			sha1_array_append(&extra, si->shallow->sha1[i]);
 
-	setup_alternate_shallow(&shallow_lock, &alt_file, &extra);
+	setup_alternate_shallow(&shallow_lock, &opt.shallow_file, &extra);
-	if (check_shallow_connected(command_singleton_iterator,
+	if (check_connected(command_singleton_iterator, cmd, &opt)) {
-				    0, cmd, alt_file)) {
 		rollback_lock_file(&shallow_lock);
 		sha1_array_clear(&extra);
 		return -1;
@@ -1184,8 +1245,8 @@ static void set_connectivity_errors(struct command *commands,
 		if (shallow_update && si->shallow_ref[cmd->index])
 			/* to be checked in update_shallow_ref() */
 			continue;
-		if (!check_everything_connected(command_singleton_iterator,
+		if (!check_connected(command_singleton_iterator, &singleton,
-						0, &singleton))
+				     NULL))
 			continue;
 		cmd->error_string = "missing necessary objects";
 	}
@@ -1343,9 +1404,12 @@ static void execute_commands(struct command *commands,
 			     struct shallow_info *si,
 			     const struct string_list *push_options)
 {
+	struct check_connected_options opt = CHECK_CONNECTED_INIT;
 	struct command *cmd;
 	unsigned char sha1[20];
 	struct iterate_data data;
+	struct async muxer;
+	int err_fd = 0;
 
 	if (unpacker_error) {
 		for (cmd = commands; cmd; cmd = cmd->next)
@@ -1353,11 +1417,25 @@ static void execute_commands(struct command *commands,
 		return;
 	}
 
+	if (use_sideband) {
+		memset(&muxer, 0, sizeof(muxer));
+		muxer.proc = copy_to_sideband;
+		muxer.in = -1;
+		if (!start_async(&muxer))
+			err_fd = muxer.in;
+		/* ...else, continue without relaying sideband */
+	}
+
 	data.cmds = commands;
 	data.si = si;
-	if (check_everything_connected(iterate_receive_command_list, 0, &data))
+	opt.err_fd = err_fd;
+	opt.progress = err_fd && !quiet;
+	if (check_connected(iterate_receive_command_list, &data, &opt))
 		set_connectivity_errors(commands, si);
 
+	if (use_sideband)
+		finish_async(&muxer);
+
 	reject_updates_to_hidden(commands);
 
 	if (run_receive_hook(commands, "pre-receive", 0, push_options)) {
@@ -1591,6 +1669,10 @@ static const char *unpack(int err_fd, struct shallow_info *si)
 				 (uintmax_t)getpid(),
 				 hostname);
 
+		if (!quiet && err_fd)
+			argv_array_push(&child.args, "--show-resolving-progress");
+		if (use_sideband)
+			argv_array_push(&child.args, "--report-end-of-input");
 		if (fsck_objects)
 			argv_array_pushf(&child.args, "--strict%s",
 				fsck_msg_types.buf);
@@ -1620,6 +1702,7 @@ static const char *unpack_with_sideband(struct shallow_info *si)
 	if (!use_sideband)
 		return unpack(0, si);
 
+	use_keepalive = KEEPALIVE_AFTER_NUL;
 	memset(&muxer, 0, sizeof(muxer));
 	muxer.proc = copy_to_sideband;
 	muxer.in = -1;
@@ -1811,6 +1894,7 @@ int cmd_receive_pack(int argc, const char **argv, const char *prefix)
 			unpack_status = unpack_with_sideband(&si);
 			update_shallow_info(commands, &si, &ref);
 		}
+		use_keepalive = KEEPALIVE_ALWAYS;
 		execute_commands(commands, unpack_status, &si,
 				 &push_options);
 		if (pack_lockfile)

builtin/rev-list.c

@@ -9,6 +9,7 @@
 #include "log-tree.h"
 #include "graph.h"
 #include "bisect.h"
+#include "progress.h"
 
 static const char rev_list_usage[] =
 "git rev-list [OPTION] <commit-id>... [ -- paths... ]\n"
@@ -49,12 +50,17 @@ static const char rev_list_usage[] =
 "    --bisect-all"
 ;
 
+static struct progress *progress;
+static unsigned progress_counter;
+
 static void finish_commit(struct commit *commit, void *data);
 static void show_commit(struct commit *commit, void *data)
 {
 	struct rev_list_info *info = data;
 	struct rev_info *revs = info->revs;
 
+	display_progress(progress, ++progress_counter);
+
 	if (info->flags & REV_LIST_QUIET) {
 		finish_commit(commit, data);
 		return;
@@ -190,6 +196,7 @@ static void show_object(struct object *obj, const char *name, void *cb_data)
 {
 	struct rev_list_info *info = cb_data;
 	finish_object(obj, name, cb_data);
+	display_progress(progress, ++progress_counter);
 	if (info->flags & REV_LIST_QUIET)
 		return;
 	show_object_with_name(stdout, obj, name);
@@ -276,6 +283,7 @@ int cmd_rev_list(int argc, const char **argv, const char *prefix)
 	int bisect_show_vars = 0;
 	int bisect_find_all = 0;
 	int use_bitmap_index = 0;
+	const char *show_progress = NULL;
 
 	git_config(git_default_config, NULL);
 	init_revisions(&revs, prefix);
@@ -325,6 +333,10 @@ int cmd_rev_list(int argc, const char **argv, const char *prefix)
 			test_bitmap_walk(&revs);
 			return 0;
 		}
+		if (skip_prefix(arg, "--progress=", &arg)) {
+			show_progress = arg;
+			continue;
+		}
 		usage(rev_list_usage);
 
 	}
@@ -355,6 +367,9 @@ int cmd_rev_list(int argc, const char **argv, const char *prefix)
 	if (bisect_list)
 		revs.limited = 1;
 
+	if (show_progress)
+		progress = start_progress_delay(show_progress, 0, 0, 2);
+
 	if (use_bitmap_index && !revs.prune) {
 		if (revs.count && !revs.left_right && !revs.cherry_mark) {
 			uint32_t commit_count;
@@ -392,6 +407,8 @@ int cmd_rev_list(int argc, const char **argv, const char *prefix)
 
 	traverse_commit_list(&revs, show_commit, show_object, &info);
 
+	stop_progress(&progress);
+
 	if (revs.count) {
 		if (revs.left_right && revs.cherry_mark)
 			printf("%d\t%d\t%d\n", revs.count_left, revs.count_right, revs.count_same);

connected.c

@@ -4,10 +4,6 @@
 #include "connected.h"
 #include "transport.h"
 
-int check_everything_connected(sha1_iterate_fn fn, int quiet, void *cb_data)
-{
-	return check_everything_connected_with_transport(fn, quiet, cb_data, NULL);
-}
 /*
  * If we feed all the commits we want to verify to this command
  *
@@ -19,22 +15,27 @@ int check_everything_connected(sha1_iterate_fn fn, int quiet, void *cb_data)
  *
  * Returns 0 if everything is connected, non-zero otherwise.
  */
-static int check_everything_connected_real(sha1_iterate_fn fn,
+int check_connected(sha1_iterate_fn fn, void *cb_data,
-					   int quiet,
+		    struct check_connected_options *opt)
-					   void *cb_data,
-					   struct transport *transport,
-					   const char *shallow_file)
 {
 	struct child_process rev_list = CHILD_PROCESS_INIT;
-	const char *argv[9];
+	struct check_connected_options defaults = CHECK_CONNECTED_INIT;
 	char commit[41];
 	unsigned char sha1[20];
-	int err = 0, ac = 0;
+	int err = 0;
 	struct packed_git *new_pack = NULL;
+	struct transport *transport;
 	size_t base_len;
 
-	if (fn(cb_data, sha1))
+	if (!opt)
+		opt = &defaults;
+	transport = opt->transport;
+
+	if (fn(cb_data, sha1)) {
+		if (opt->err_fd)
+			close(opt->err_fd);
 		return err;
+	}
 
 	if (transport && transport->smart_options &&
 	    transport->smart_options->self_contained_and_connected &&
@@ -47,24 +48,28 @@ static int check_everything_connected_real(sha1_iterate_fn fn,
 		strbuf_release(&idx_file);
 	}
 
-	if (shallow_file) {
+	if (opt->shallow_file) {
-		argv[ac++] = "--shallow-file";
+		argv_array_push(&rev_list.args, "--shallow-file");
-		argv[ac++] = shallow_file;
+		argv_array_push(&rev_list.args, opt->shallow_file);
 	}
-	argv[ac++] = "rev-list";
+	argv_array_push(&rev_list.args,"rev-list");
-	argv[ac++] = "--objects";
+	argv_array_push(&rev_list.args, "--objects");
-	argv[ac++] = "--stdin";
+	argv_array_push(&rev_list.args, "--stdin");
-	argv[ac++] = "--not";
+	argv_array_push(&rev_list.args, "--not");
-	argv[ac++] = "--all";
+	argv_array_push(&rev_list.args, "--all");
-	if (quiet)
+	argv_array_push(&rev_list.args, "--quiet");
-		argv[ac++] = "--quiet";
+	if (opt->progress)
-	argv[ac] = NULL;
+		argv_array_pushf(&rev_list.args, "--progress=%s",
+				 _("Checking connectivity"));
 
-	rev_list.argv = argv;
 	rev_list.git_cmd = 1;
 	rev_list.in = -1;
 	rev_list.no_stdout = 1;
-	rev_list.no_stderr = quiet;
+	if (opt->err_fd)
+		rev_list.err = opt->err_fd;
+	else
+		rev_list.no_stderr = opt->quiet;
+
 	if (start_command(&rev_list))
 		return error(_("Could not run 'git rev-list'"));
 
@@ -98,19 +103,3 @@ static int check_everything_connected_real(sha1_iterate_fn fn,
 	sigchain_pop(SIGPIPE);
 	return finish_command(&rev_list) || err;
 }
-
-int check_everything_connected_with_transport(sha1_iterate_fn fn,
-					      int quiet,
-					      void *cb_data,
-					      struct transport *transport)
-{
-	return check_everything_connected_real(fn, quiet, cb_data,
-					       transport, NULL);
-}
-
-int check_shallow_connected(sha1_iterate_fn fn, int quiet, void *cb_data,
-			    const char *shallow_file)
-{
-	return check_everything_connected_real(fn, quiet, cb_data,
-					       NULL, shallow_file);
-}

connected.h

@@ -10,18 +10,43 @@ struct transport;
  */
 typedef int (*sha1_iterate_fn)(void *, unsigned char [20]);
 
+/*
+ * Named-arguments struct for check_connected. All arguments are
+ * optional, and can be left to defaults as set by CHECK_CONNECTED_INIT.
+ */
+struct check_connected_options {
+	/* Avoid printing any errors to stderr. */
+	int quiet;
+
+	/* --shallow-file to pass to rev-list sub-process */
+	const char *shallow_file;
+
+	/* Transport whose objects we are checking, if available. */
+	struct transport *transport;
+
+	/*
+	 * If non-zero, send error messages to this descriptor rather
+	 * than stderr. The descriptor is closed before check_connected
+	 * returns.
+	 */
+	int err_fd;
+
+	/* If non-zero, show progress as we traverse the objects. */
+	int progress;
+};
+
+#define CHECK_CONNECTED_INIT { 0 }
+
 /*
  * Make sure that our object store has all the commits necessary to
  * connect the ancestry chain to some of our existing refs, and all
  * the trees and blobs that these commits use.
  *
  * Return 0 if Ok, non zero otherwise (i.e. some missing objects)
+ *
+ * If "opt" is NULL, behaves as if CHECK_CONNECTED_INIT was passed.
  */
-extern int check_everything_connected(sha1_iterate_fn, int quiet, void *cb_data);
+int check_connected(sha1_iterate_fn fn, void *cb_data,
-extern int check_shallow_connected(sha1_iterate_fn, int quiet, void *cb_data,
+		    struct check_connected_options *opt);
-				   const char *shallow_file);
-extern int check_everything_connected_with_transport(sha1_iterate_fn, int quiet,
-						     void *cb_data,
-						     struct transport *transport);
 
 #endif /* CONNECTED_H */