[PATCH] add --summary option to git-push and git-fetch

[PATCH] add --summary option to git-push and git-fetch

[Larry D'Anna]

--summary will cause git-push to output a one-line of each commit pushed.
--summary=n will display at most n commits for each ref pushed.

$ git push --dry-run --summary origin :
To /home/larry/gitsandbox/a
   80f0e50..5593a38  master -> master
    > 5593a38 foo
    > 81c03f8 bar

Fetch works the same way.

Signed-off-by: Larry D'Anna <larry@elder-gods.org>
---
        
This patch should be applied on top of 1965ff7 add --porcelain option to git-push

Differences since the last version of this patch:

* added --summary to fetch 

* commits are marked by < or > like --left-right

* fixed a bug where it died gracelessly on a forced push A...B if A is not in 
  the local repository.

* fixed a bug where cmd_log was modifying the part lists of commits.

* called get_revision and friends directly instead of using cmd_log


 Documentation/fetch-options.txt |    7 +++++++
 Documentation/git-push.txt      |    6 ++++++
 builtin-fetch.c                 |   24 ++++++++++++++++++------
 builtin-log.c                   |   35 +++++++++++++++++++++++++++++++++++
 builtin-push.c                  |   12 +++++++++---
 builtin.h                       |    2 ++
 transport.c                     |   39 +++++++++++++++++++++++++++------------
 transport.h                     |    2 +-
 8 files changed, 105 insertions(+), 22 deletions(-)

diff --git a/Documentation/fetch-options.txt b/Documentation/fetch-options.txt
index d313795..2e66d5e 100644
--- a/Documentation/fetch-options.txt
+++ b/Documentation/fetch-options.txt
@@ -27,6 +27,13 @@
 	fetches is a descendant of `<lbranch>`.  This option
 	overrides that check.
 
+--summary::
+	Print a one-line summary of each commit fetched.
+
+--summary=<n>::
+	Like --summary, but with a limit of <n> commits per ref.
+
+
 ifdef::git-pull[]
 --no-tags::
 endif::git-pull[]
diff --git a/Documentation/git-push.txt b/Documentation/git-push.txt
index 2653388..803fe36 100644
--- a/Documentation/git-push.txt
+++ b/Documentation/git-push.txt
@@ -85,6 +85,12 @@ nor in any Push line of the corresponding remotes file---see below).
 --dry-run::
 	Do everything except actually send the updates.
 
+--summary::
+	Print a one-line summary of each commit pushed.
+
+--summary=<n>::
+	Like --summary, but with a limit of <n> commits per ref.
+
 --porcelain::
 	Produce machine-readable output.  The output status line for each ref
 	will be tab-separated and sent to stdout instead of stderr.  The full
diff --git a/builtin-fetch.c b/builtin-fetch.c
index cd5eb9a..b79d870 100644
--- a/builtin-fetch.c
+++ b/builtin-fetch.c
@@ -29,6 +29,7 @@ static const char *depth;
 static const char *upload_pack;
 static struct strbuf default_rla = STRBUF_INIT;
 static struct transport *transport;
+static int summary = 0;
 
 static struct option builtin_fetch_options[] = {
 	OPT__VERBOSITY(&verbosity),
@@ -47,6 +48,9 @@ static struct option builtin_fetch_options[] = {
 		    "allow updating of HEAD ref"),
 	OPT_STRING(0, "depth", &depth, "DEPTH",
 		   "deepen history of shallow clone"),
+	{ OPTION_INTEGER, 0, "summary", &summary, "n", "print a summary of [at most n] fetched commits",
+	  PARSE_OPT_OPTARG, NULL, -1
+	},
 	OPT_END()
 };
 
@@ -197,7 +201,8 @@ static int s_update_ref(const char *action,
 
 static int update_local_ref(struct ref *ref,
 			    const char *remote,
-			    char *display)
+			    char *display,
+			    char *quickref)
 {
 	struct commit *current = NULL, *updated;
 	enum object_type type;
@@ -260,11 +265,12 @@ static int update_local_ref(struct ref *ref,
 		sprintf(display, "%c %-*s %-*s -> %s%s", r ? '!' : '*',
 			SUMMARY_WIDTH, what, REFCOL_WIDTH, remote, pretty_ref,
 			r ? "  (unable to update local ref)" : "");
+		if (!r)
+			strcpy (quickref, find_unique_abbrev(ref->new_sha1, DEFAULT_ABBREV));
 		return r;
 	}
 
 	if (in_merge_bases(current, &updated, 1)) {
-		char quickref[83];
 		int r;
 		strcpy(quickref, find_unique_abbrev(current->object.sha1, DEFAULT_ABBREV));
 		strcat(quickref, "..");
@@ -275,7 +281,6 @@ static int update_local_ref(struct ref *ref,
 			pretty_ref, r ? "  (unable to update local ref)" : "");
 		return r;
 	} else if (force || ref->force) {
-		char quickref[84];
 		int r;
 		strcpy(quickref, find_unique_abbrev(current->object.sha1, DEFAULT_ABBREV));
 		strcat(quickref, "...");
@@ -301,6 +306,7 @@ static int store_updated_refs(const char *raw_url, const char *remote_name,
 	struct commit *commit;
 	int url_len, i, note_len, shown_url = 0, rc = 0;
 	char note[1024];
+	char quickref[84];
 	const char *what, *kind;
 	struct ref *rm;
 	char *url, *filename = git_path("FETCH_HEAD");
@@ -373,12 +379,15 @@ static int store_updated_refs(const char *raw_url, const char *remote_name,
 				fputc(url[i], fp);
 		fputc('\n', fp);
 
-		if (ref)
-			rc |= update_local_ref(ref, what, note);
-		else
+		if (ref) {
+			*quickref = 0;
+			rc |= update_local_ref(ref, what, note, quickref);
+		} else {
+			strcpy (quickref, find_unique_abbrev(rm->old_sha1, DEFAULT_ABBREV));
 			sprintf(note, "* %-*s %-*s -> FETCH_HEAD",
 				SUMMARY_WIDTH, *kind ? kind : "branch",
 				 REFCOL_WIDTH, *what ? what : "HEAD");
+		}
 		if (*note) {
 			if (verbosity >= 0 && !shown_url) {
 				fprintf(stderr, "From %.*s\n",
@@ -388,6 +397,9 @@ static int store_updated_refs(const char *raw_url, const char *remote_name,
 			if (verbosity >= 0)
 				fprintf(stderr, " %s\n", note);
 		}
+		if (summary && quickref[0])
+			print_summary_for_push_or_fetch (quickref, summary);
+
 	}
 	free(url);
 	fclose(fp);
diff --git a/builtin-log.c b/builtin-log.c
index 44f9a27..cc4dc0a 100644
--- a/builtin-log.c
+++ b/builtin-log.c
@@ -1293,3 +1293,38 @@ int cmd_cherry(int argc, const char **argv, const char *prefix)
 	free_patch_ids(&ids);
 	return 0;
 }
+
+
+void print_summary_for_push_or_fetch (const char *quickref, int limit)
+{
+	struct rev_info rev;
+	FILE *temp;
+	int i, max;
+	struct object *obj;
+	struct commit *commit;
+
+	temp = stdout;
+	stdout = stderr;
+
+	max = get_max_object_index();
+	for (i = 0; i < max; i++)  {
+		obj = get_indexed_object(i);
+		if (obj)
+			obj->flags &= ~ALL_REV_FLAGS;
+	}
+
+	init_revisions(&rev, NULL);
+	if (limit > 0)
+		rev.max_count = limit;
+	rev.prune = 0;
+	rev.verbose_header = 1;
+	rev.always_show_header = 1;
+	get_commit_format("    %m %h %s", &rev);
+	assert(!handle_revision_arg(quickref, &rev, 0, 1));
+	assert(!prepare_revision_walk(&rev));
+
+	while ((commit = get_revision(&rev)) != NULL)
+		log_tree_commit(&rev, commit);
+
+	stdout = temp;
+}
diff --git a/builtin-push.c b/builtin-push.c
index 0a0297f..00cf846 100644
--- a/builtin-push.c
+++ b/builtin-push.c
@@ -113,7 +113,7 @@ static void setup_default_push_refspecs(void)
 	}
 }
 
-static int do_push(const char *repo, int flags)
+static int do_push(const char *repo, int flags, int summary)
 {
 	int i, errs;
 	struct remote *remote = remote_get(repo);
@@ -173,7 +173,7 @@ static int do_push(const char *repo, int flags)
 
 		if (flags & TRANSPORT_PUSH_VERBOSE)
 			fprintf(stderr, "Pushing to %s\n", url[i]);
-		err = transport_push(transport, refspec_nr, refspec, flags);
+		err = transport_push(transport, refspec_nr, refspec, flags, summary);
 		err |= transport_disconnect(transport);
 
 		if (!err)
@@ -192,6 +192,8 @@ int cmd_push(int argc, const char **argv, const char *prefix)
 	int rc;
 	const char *repo = NULL;	/* default repository */
 
+	int summary = 0;
+
 	struct option options[] = {
 		OPT_BIT('v', "verbose", &flags, "be verbose", TRANSPORT_PUSH_VERBOSE),
 		OPT_STRING( 0 , "repo", &repo, "repository", "repository"),
@@ -205,6 +207,10 @@ int cmd_push(int argc, const char **argv, const char *prefix)
 		OPT_BOOLEAN( 0 , "thin", &thin, "use thin pack"),
 		OPT_STRING( 0 , "receive-pack", &receivepack, "receive-pack", "receive pack program"),
 		OPT_STRING( 0 , "exec", &receivepack, "receive-pack", "receive pack program"),
+		{ OPTION_INTEGER, 0, "summary", &summary, "n", "print a summary of [at most n] pushed commits",
+		  PARSE_OPT_OPTARG, NULL, -1
+		},
+
 		OPT_END()
 	};
 
@@ -218,7 +224,7 @@ int cmd_push(int argc, const char **argv, const char *prefix)
 		set_refspecs(argv + 1, argc - 1);
 	}
 
-	rc = do_push(repo, flags);
+	rc = do_push(repo, flags, summary);
 	if (rc == -1)
 		usage_with_options(push_usage, options);
 	else
diff --git a/builtin.h b/builtin.h
index 20427d2..1991eb2 100644
--- a/builtin.h
+++ b/builtin.h
@@ -113,4 +113,6 @@ extern int cmd_verify_pack(int argc, const char **argv, const char *prefix);
 extern int cmd_show_ref(int argc, const char **argv, const char *prefix);
 extern int cmd_pack_refs(int argc, const char **argv, const char *prefix);
 
+extern void print_summary_for_push_or_fetch (const char *quickref, int limit);
+
 #endif
diff --git a/transport.c b/transport.c
index b074067..139f472 100644
--- a/transport.c
+++ b/transport.c
@@ -11,6 +11,7 @@
 #include "bundle.h"
 #include "dir.h"
 #include "refs.h"
+#include "builtin.h"
 
 /* rsync support */
 
@@ -750,17 +751,20 @@ static const char *status_abbrev(unsigned char sha1[20])
 	return find_unique_abbrev(sha1, DEFAULT_ABBREV);
 }
 
-static void print_ok_ref_status(struct ref *ref, int porcelain)
+static void print_ok_ref_status(struct ref *ref, int porcelain, int summary)
 {
+	char quickref[84];
+	int summary_impossible = 0;
+
 	if (ref->deletion)
 		print_ref_status('-', "[deleted]", ref, NULL, NULL, porcelain);
-	else if (is_null_sha1(ref->old_sha1))
+	else if (is_null_sha1(ref->old_sha1)) {
 		print_ref_status('*',
 			(!prefixcmp(ref->name, "refs/tags/") ? "[new tag]" :
 			"[new branch]"),
 			ref, ref->peer_ref, NULL, porcelain);
-	else {
-		char quickref[84];
+		strcpy (quickref, status_abbrev(ref->new_sha1));
+	} else {
 		char type;
 		const char *msg;
 
@@ -769,6 +773,8 @@ static void print_ok_ref_status(struct ref *ref, int porcelain)
 			strcat(quickref, "...");
 			type = '+';
 			msg = "forced update";
+			if (!lookup_commit_reference_gently(ref->old_sha1, 1))
+				summary_impossible = 1;
 		} else {
 			strcat(quickref, "..");
 			type = ' ';
@@ -778,9 +784,17 @@ static void print_ok_ref_status(struct ref *ref, int porcelain)
 
 		print_ref_status(type, quickref, ref, ref->peer_ref, msg, porcelain);
 	}
+
+	if (summary) {
+		if (summary_impossible) {
+			fprintf (stderr, "    %s is unavailable\n", status_abbrev(ref->old_sha1));
+		} else {
+			print_summary_for_push_or_fetch(quickref, summary);
+		}
+	}
 }
 
-static int print_one_push_status(struct ref *ref, const char *dest, int count, int porcelain)
+static int print_one_push_status(struct ref *ref, const char *dest, int count, int porcelain, int summary)
 {
 	if (!count)
 		fprintf(stderr, "To %s\n", dest);
@@ -812,7 +826,7 @@ static int print_one_push_status(struct ref *ref, const char *dest, int count, i
 						 "remote failed to report status", porcelain);
 		break;
 	case REF_STATUS_OK:
-		print_ok_ref_status(ref, porcelain);
+		print_ok_ref_status(ref, porcelain, summary);
 		break;
 	}
 
@@ -820,7 +834,7 @@ static int print_one_push_status(struct ref *ref, const char *dest, int count, i
 }
 
 static void print_push_status(const char *dest, struct ref *refs,
-							  int verbose, int porcelain)
+							  int verbose, int porcelain, int summary)
 {
 	struct ref *ref;
 	int n = 0;
@@ -828,18 +842,18 @@ static void print_push_status(const char *dest, struct ref *refs,
 	if (verbose) {
 		for (ref = refs; ref; ref = ref->next)
 			if (ref->status == REF_STATUS_UPTODATE)
-				n += print_one_push_status(ref, dest, n, porcelain);
+				n += print_one_push_status(ref, dest, n, porcelain, summary);
 	}
 
 	for (ref = refs; ref; ref = ref->next)
 		if (ref->status == REF_STATUS_OK)
-			n += print_one_push_status(ref, dest, n, porcelain);
+			n += print_one_push_status(ref, dest, n, porcelain, summary);
 
 	for (ref = refs; ref; ref = ref->next) {
 		if (ref->status != REF_STATUS_NONE &&
 		    ref->status != REF_STATUS_UPTODATE &&
 		    ref->status != REF_STATUS_OK)
-			n += print_one_push_status(ref, dest, n, porcelain);
+			n += print_one_push_status(ref, dest, n, porcelain, summary);
 	}
 }
 
@@ -997,7 +1011,8 @@ int transport_set_option(struct transport *transport,
 }
 
 int transport_push(struct transport *transport,
-		   int refspec_nr, const char **refspec, int flags)
+				   int refspec_nr, const char **refspec,
+				   int flags, int summary)
 {
 	verify_remote_names(refspec_nr, refspec);
 
@@ -1024,7 +1039,7 @@ int transport_push(struct transport *transport,
 
 		ret = transport->push_refs(transport, remote_refs, flags);
 
-		print_push_status(transport->url, remote_refs, verbose | porcelain, porcelain);
+		print_push_status(transport->url, remote_refs, verbose | porcelain, porcelain, summary);
 
 		if (!(flags & TRANSPORT_PUSH_DRY_RUN)) {
 			struct ref *ref;
diff --git a/transport.h b/transport.h
index 51b5397..360051e 100644
--- a/transport.h
+++ b/transport.h
@@ -68,7 +68,7 @@ int transport_set_option(struct transport *transport, const char *name,
 			 const char *value);
 
 int transport_push(struct transport *connection,
-		   int refspec_nr, const char **refspec, int flags);
+				   int refspec_nr, const char **refspec, int flags, int summary);
 
 const struct ref *transport_get_remote_refs(struct transport *transport);
 
-- 
1.6.3.3.403.gdac20

Re: [PATCH] add --summary option to git-push and git-fetch

[Junio C Hamano]

Larry D'Anna <larry@elder-gods.org> writes:

> --summary will cause git-push to output a one-line of each commit pushed.
> --summary=n will display at most n commits for each ref pushed.
>
> $ git push --dry-run --summary origin :
> To /home/larry/gitsandbox/a
>    80f0e50..5593a38  master -> master
>     > 5593a38 foo
>     > 81c03f8 bar
>
> Fetch works the same way.
>
> Signed-off-by: Larry D'Anna <larry@elder-gods.org>
> ---

With this rewrite not to call cmd_log() directly, it looks much better
than the previous round.  It allows us more freedom to do things slightly
differently than the stock cmd_log() lets us, such as giving "..." after
"n" commits by default, much like fmt-merge-msg does.

> diff --git a/builtin-fetch.c b/builtin-fetch.c
> index cd5eb9a..b79d870 100644
> --- a/builtin-fetch.c
> +++ b/builtin-fetch.c
> @@ -29,6 +29,7 @@ static const char *depth;
>  static const char *upload_pack;
>  static struct strbuf default_rla = STRBUF_INIT;
>  static struct transport *transport;
> +static int summary = 0;

Don't initialize statics with "= 0;".  BSS will take care of it.

>  static struct option builtin_fetch_options[] = {
>  	OPT__VERBOSITY(&verbosity),
> @@ -47,6 +48,9 @@ static struct option builtin_fetch_options[] = {
>  		    "allow updating of HEAD ref"),
>  	OPT_STRING(0, "depth", &depth, "DEPTH",
>  		   "deepen history of shallow clone"),
> +	{ OPTION_INTEGER, 0, "summary", &summary, "n", "print a summary of [at most n] fetched commits",
> +	  PARSE_OPT_OPTARG, NULL, -1
> +	},
>  	OPT_END()
>  };

I think I'd prefer some reasonable default instead of making it unlimited,
much like how fmt-merge-msg does.  We might want to make this configurable
(I think fmt-merge-msg uses a hardcoded 20), and perhaps even use them the
same configuration variable (summary.length or something).

> @@ -260,11 +265,12 @@ static int update_local_ref(struct ref *ref,
>  		sprintf(display, "%c %-*s %-*s -> %s%s", r ? '!' : '*',
>  			SUMMARY_WIDTH, what, REFCOL_WIDTH, remote, pretty_ref,
>  			r ? "  (unable to update local ref)" : "");
> +		if (!r)
> +			strcpy (quickref, find_unique_abbrev(ref->new_sha1, DEFAULT_ABBREV));

We do not leave extra whitespace between function name and open
parenthesis (you have other instances of this style violation); on the
other hand, we do keep one whitespace after keywords like if, while, and
for (this is just fyi; I do not think I saw any violation of the latter in
the patch).

> diff --git a/builtin-log.c b/builtin-log.c
> index 44f9a27..cc4dc0a 100644
> --- a/builtin-log.c
> +++ b/builtin-log.c
> @@ -1293,3 +1293,38 @@ int cmd_cherry(int argc, const char **argv, const char *prefix)
> ...
> +void print_summary_for_push_or_fetch (const char *quickref, int limit)
> +{
> +...
> +	temp = stdout;
> +	stdout = stderr;

Is this even a valid C?  stdout and friends are described in POSIX.1 as
"Normally, there are three open streams with constant pointers declared in
the <stdio.h> header and associated with the standard open files."

At least Solaris 11 Clib headers does not seem to like it.  I do not know
about Windows.

[PATCH v3] add --summary option to git-push and git-fetch

[Larry D'Anna]

--summary will cause git-push to output a one-line of each commit pushed.
--summary=n will display at most n commits for each ref pushed.

$ git push --dry-run --summary origin :
To /home/larry/gitsandbox/a
   80f0e50..5593a38  master -> master
    > 5593a38 foo
    > 81c03f8 bar

Fetch works the same way.

Signed-off-by: Larry D'Anna <larry@elder-gods.org>
---

This patch is meant to be applied on top of 1965ff7 add --porcelain option to git-push

Differences since the last version:

* it will print ... at the end of the summary if it stops because there's too many lines

* it prints an extra newline after each summary to make the output more readable

* --summary now defaults to --summary=20 

* whitespace is fixed 

* no more stdout = stderr shenanigans

* s/static int summary = 0;/static int summary;/

 Documentation/fetch-options.txt |    7 +++++++
 Documentation/git-push.txt      |    6 ++++++
 builtin-fetch.c                 |   24 ++++++++++++++++++------
 builtin-log.c                   |   34 ++++++++++++++++++++++++++++++++++
 builtin-push.c                  |   12 +++++++++---
 builtin.h                       |    2 ++
 transport.c                     |   39 +++++++++++++++++++++++++++------------
 transport.h                     |    2 +-
 8 files changed, 104 insertions(+), 22 deletions(-)

diff --git a/Documentation/fetch-options.txt b/Documentation/fetch-options.txt
index d313795..2e66d5e 100644
--- a/Documentation/fetch-options.txt
+++ b/Documentation/fetch-options.txt
@@ -27,6 +27,13 @@
 	fetches is a descendant of `<lbranch>`.  This option
 	overrides that check.
 
+--summary::
+	Print a one-line summary of each commit fetched.
+
+--summary=<n>::
+	Like --summary, but with a limit of <n> commits per ref.
+
+
 ifdef::git-pull[]
 --no-tags::
 endif::git-pull[]
diff --git a/Documentation/git-push.txt b/Documentation/git-push.txt
index 2653388..803fe36 100644
--- a/Documentation/git-push.txt
+++ b/Documentation/git-push.txt
@@ -85,6 +85,12 @@ nor in any Push line of the corresponding remotes file---see below).
 --dry-run::
 	Do everything except actually send the updates.
 
+--summary::
+	Print a one-line summary of each commit pushed.
+
+--summary=<n>::
+	Like --summary, but with a limit of <n> commits per ref.
+
 --porcelain::
 	Produce machine-readable output.  The output status line for each ref
 	will be tab-separated and sent to stdout instead of stderr.  The full
diff --git a/builtin-fetch.c b/builtin-fetch.c
index cd5eb9a..c98d06b 100644
--- a/builtin-fetch.c
+++ b/builtin-fetch.c
@@ -29,6 +29,7 @@ static const char *depth;
 static const char *upload_pack;
 static struct strbuf default_rla = STRBUF_INIT;
 static struct transport *transport;
+static int summary;
 
 static struct option builtin_fetch_options[] = {
 	OPT__VERBOSITY(&verbosity),
@@ -47,6 +48,9 @@ static struct option builtin_fetch_options[] = {
 		    "allow updating of HEAD ref"),
 	OPT_STRING(0, "depth", &depth, "DEPTH",
 		   "deepen history of shallow clone"),
+	{ OPTION_INTEGER, 0, "summary", &summary, "n", "print a summary of [at most n] fetched commits",
+	  PARSE_OPT_OPTARG, NULL, 20
+	},
 	OPT_END()
 };
 
@@ -197,7 +201,8 @@ static int s_update_ref(const char *action,
 
 static int update_local_ref(struct ref *ref,
 			    const char *remote,
-			    char *display)
+			    char *display,
+			    char *quickref)
 {
 	struct commit *current = NULL, *updated;
 	enum object_type type;
@@ -260,11 +265,12 @@ static int update_local_ref(struct ref *ref,
 		sprintf(display, "%c %-*s %-*s -> %s%s", r ? '!' : '*',
 			SUMMARY_WIDTH, what, REFCOL_WIDTH, remote, pretty_ref,
 			r ? "  (unable to update local ref)" : "");
+		if (!r)
+			strcpy(quickref, find_unique_abbrev(ref->new_sha1, DEFAULT_ABBREV));
 		return r;
 	}
 
 	if (in_merge_bases(current, &updated, 1)) {
-		char quickref[83];
 		int r;
 		strcpy(quickref, find_unique_abbrev(current->object.sha1, DEFAULT_ABBREV));
 		strcat(quickref, "..");
@@ -275,7 +281,6 @@ static int update_local_ref(struct ref *ref,
 			pretty_ref, r ? "  (unable to update local ref)" : "");
 		return r;
 	} else if (force || ref->force) {
-		char quickref[84];
 		int r;
 		strcpy(quickref, find_unique_abbrev(current->object.sha1, DEFAULT_ABBREV));
 		strcat(quickref, "...");
@@ -301,6 +306,7 @@ static int store_updated_refs(const char *raw_url, const char *remote_name,
 	struct commit *commit;
 	int url_len, i, note_len, shown_url = 0, rc = 0;
 	char note[1024];
+	char quickref[84];
 	const char *what, *kind;
 	struct ref *rm;
 	char *url, *filename = git_path("FETCH_HEAD");
@@ -373,12 +379,15 @@ static int store_updated_refs(const char *raw_url, const char *remote_name,
 				fputc(url[i], fp);
 		fputc('\n', fp);
 
-		if (ref)
-			rc |= update_local_ref(ref, what, note);
-		else
+		if (ref) {
+			*quickref = 0;
+			rc |= update_local_ref(ref, what, note, quickref);
+		} else {
+			strcpy(quickref, find_unique_abbrev(rm->old_sha1, DEFAULT_ABBREV));
 			sprintf(note, "* %-*s %-*s -> FETCH_HEAD",
 				SUMMARY_WIDTH, *kind ? kind : "branch",
 				 REFCOL_WIDTH, *what ? what : "HEAD");
+		}
 		if (*note) {
 			if (verbosity >= 0 && !shown_url) {
 				fprintf(stderr, "From %.*s\n",
@@ -388,6 +397,9 @@ static int store_updated_refs(const char *raw_url, const char *remote_name,
 			if (verbosity >= 0)
 				fprintf(stderr, " %s\n", note);
 		}
+		if (summary && quickref[0])
+			print_summary_for_push_or_fetch(quickref, summary);
+
 	}
 	free(url);
 	fclose(fp);
diff --git a/builtin-log.c b/builtin-log.c
index 0c2fa0a..e25285b 100644
--- a/builtin-log.c
+++ b/builtin-log.c
@@ -1293,3 +1293,37 @@ int cmd_cherry(int argc, const char **argv, const char *prefix)
 	free_patch_ids(&ids);
 	return 0;
 }
+
+
+void print_summary_for_push_or_fetch(const char *quickref, int limit)
+{
+	struct rev_info rev;
+	int i, max;
+	struct object *obj;
+	struct commit *commit;
+
+	max = get_max_object_index();
+	for (i = 0; i < max; i++)  {
+		obj = get_indexed_object(i);
+		if (obj)
+			obj->flags &= ~ALL_REV_FLAGS;
+	}
+
+	init_revisions(&rev, NULL);
+	rev.prune = 0;
+	assert(!handle_revision_arg(quickref, &rev, 0, 1));
+	assert(!prepare_revision_walk(&rev));
+
+	while ((commit = get_revision(&rev)) != NULL) {
+		struct strbuf buf = STRBUF_INIT;
+		if (limit == 0) {
+			fprintf(stderr, "    ...\n");
+			break;
+		}
+		format_commit_message(commit, "    %m %h %s\n", &buf, 0);
+		fputs(buf.buf, stderr);
+		strbuf_release(&buf);
+		limit--;
+	}
+	fputs("\n", stderr);
+}
diff --git a/builtin-push.c b/builtin-push.c
index 0a0297f..d38f9a8 100644
--- a/builtin-push.c
+++ b/builtin-push.c
@@ -113,7 +113,7 @@ static void setup_default_push_refspecs(void)
 	}
 }
 
-static int do_push(const char *repo, int flags)
+static int do_push(const char *repo, int flags, int summary)
 {
 	int i, errs;
 	struct remote *remote = remote_get(repo);
@@ -173,7 +173,7 @@ static int do_push(const char *repo, int flags)
 
 		if (flags & TRANSPORT_PUSH_VERBOSE)
 			fprintf(stderr, "Pushing to %s\n", url[i]);
-		err = transport_push(transport, refspec_nr, refspec, flags);
+		err = transport_push(transport, refspec_nr, refspec, flags, summary);
 		err |= transport_disconnect(transport);
 
 		if (!err)
@@ -192,6 +192,8 @@ int cmd_push(int argc, const char **argv, const char *prefix)
 	int rc;
 	const char *repo = NULL;	/* default repository */
 
+	int summary = 0;
+
 	struct option options[] = {
 		OPT_BIT('v', "verbose", &flags, "be verbose", TRANSPORT_PUSH_VERBOSE),
 		OPT_STRING( 0 , "repo", &repo, "repository", "repository"),
@@ -205,6 +207,10 @@ int cmd_push(int argc, const char **argv, const char *prefix)
 		OPT_BOOLEAN( 0 , "thin", &thin, "use thin pack"),
 		OPT_STRING( 0 , "receive-pack", &receivepack, "receive-pack", "receive pack program"),
 		OPT_STRING( 0 , "exec", &receivepack, "receive-pack", "receive pack program"),
+		{ OPTION_INTEGER, 0, "summary", &summary, "n", "print a summary of [at most n] pushed commits",
+		  PARSE_OPT_OPTARG, NULL, 20
+		},
+
 		OPT_END()
 	};
 
@@ -218,7 +224,7 @@ int cmd_push(int argc, const char **argv, const char *prefix)
 		set_refspecs(argv + 1, argc - 1);
 	}
 
-	rc = do_push(repo, flags);
+	rc = do_push(repo, flags, summary);
 	if (rc == -1)
 		usage_with_options(push_usage, options);
 	else
diff --git a/builtin.h b/builtin.h
index 20427d2..5aea3a3 100644
--- a/builtin.h
+++ b/builtin.h
@@ -113,4 +113,6 @@ extern int cmd_verify_pack(int argc, const char **argv, const char *prefix);
 extern int cmd_show_ref(int argc, const char **argv, const char *prefix);
 extern int cmd_pack_refs(int argc, const char **argv, const char *prefix);
 
+extern void print_summary_for_push_or_fetch(const char *quickref, int limit);
+
 #endif
diff --git a/transport.c b/transport.c
index de0d587..80105ae 100644
--- a/transport.c
+++ b/transport.c
@@ -11,6 +11,7 @@
 #include "bundle.h"
 #include "dir.h"
 #include "refs.h"
+#include "builtin.h"
 
 /* rsync support */
 
@@ -750,17 +751,20 @@ static const char *status_abbrev(unsigned char sha1[20])
 	return find_unique_abbrev(sha1, DEFAULT_ABBREV);
 }
 
-static void print_ok_ref_status(struct ref *ref, int porcelain)
+static void print_ok_ref_status(struct ref *ref, int porcelain, int summary)
 {
+	char quickref[84];
+	int summary_impossible = 0;
+
 	if (ref->deletion)
 		print_ref_status('-', "[deleted]", ref, NULL, NULL, porcelain);
-	else if (is_null_sha1(ref->old_sha1))
+	else if (is_null_sha1(ref->old_sha1)) {
 		print_ref_status('*',
 			(!prefixcmp(ref->name, "refs/tags/") ? "[new tag]" :
 			"[new branch]"),
 			ref, ref->peer_ref, NULL, porcelain);
-	else {
-		char quickref[84];
+		strcpy(quickref, status_abbrev(ref->new_sha1));
+	} else {
 		char type;
 		const char *msg;
 
@@ -769,6 +773,8 @@ static void print_ok_ref_status(struct ref *ref, int porcelain)
 			strcat(quickref, "...");
 			type = '+';
 			msg = "forced update";
+			if (!lookup_commit_reference_gently(ref->old_sha1, 1))
+				summary_impossible = 1;
 		} else {
 			strcat(quickref, "..");
 			type = ' ';
@@ -778,9 +784,17 @@ static void print_ok_ref_status(struct ref *ref, int porcelain)
 
 		print_ref_status(type, quickref, ref, ref->peer_ref, msg, porcelain);
 	}
+
+	if (summary) {
+		if (summary_impossible) {
+			fprintf(stderr, "    %s is unavailable\n", status_abbrev(ref->old_sha1));
+		} else {
+			print_summary_for_push_or_fetch(quickref, summary);
+		}
+	}
 }
 
-static int print_one_push_status(struct ref *ref, const char *dest, int count, int porcelain)
+static int print_one_push_status(struct ref *ref, const char *dest, int count, int porcelain, int summary)
 {
 	if (!count)
 		fprintf(stderr, "To %s\n", dest);
@@ -812,7 +826,7 @@ static int print_one_push_status(struct ref *ref, const char *dest, int count, i
 						 "remote failed to report status", porcelain);
 		break;
 	case REF_STATUS_OK:
-		print_ok_ref_status(ref, porcelain);
+		print_ok_ref_status(ref, porcelain, summary);
 		break;
 	}
 
@@ -820,7 +834,7 @@ static int print_one_push_status(struct ref *ref, const char *dest, int count, i
 }
 
 static void print_push_status(const char *dest, struct ref *refs,
-							  int verbose, int porcelain)
+							  int verbose, int porcelain, int summary)
 {
 	struct ref *ref;
 	int n = 0;
@@ -828,18 +842,18 @@ static void print_push_status(const char *dest, struct ref *refs,
 	if (verbose) {
 		for (ref = refs; ref; ref = ref->next)
 			if (ref->status == REF_STATUS_UPTODATE)
-				n += print_one_push_status(ref, dest, n, porcelain);
+				n += print_one_push_status(ref, dest, n, porcelain, summary);
 	}
 
 	for (ref = refs; ref; ref = ref->next)
 		if (ref->status == REF_STATUS_OK)
-			n += print_one_push_status(ref, dest, n, porcelain);
+			n += print_one_push_status(ref, dest, n, porcelain, summary);
 
 	for (ref = refs; ref; ref = ref->next) {
 		if (ref->status != REF_STATUS_NONE &&
 		    ref->status != REF_STATUS_UPTODATE &&
 		    ref->status != REF_STATUS_OK)
-			n += print_one_push_status(ref, dest, n, porcelain);
+			n += print_one_push_status(ref, dest, n, porcelain, summary);
 	}
 }
 
@@ -997,7 +1011,8 @@ int transport_set_option(struct transport *transport,
 }
 
 int transport_push(struct transport *transport,
-		   int refspec_nr, const char **refspec, int flags)
+				   int refspec_nr, const char **refspec,
+				   int flags, int summary)
 {
 	verify_remote_names(refspec_nr, refspec);
 
@@ -1024,7 +1039,7 @@ int transport_push(struct transport *transport,
 
 		ret = transport->push_refs(transport, remote_refs, flags);
 
-		print_push_status(transport->url, remote_refs, verbose | porcelain, porcelain);
+		print_push_status(transport->url, remote_refs, verbose | porcelain, porcelain, summary);
 
 		if (!(flags & TRANSPORT_PUSH_DRY_RUN)) {
 			struct ref *ref;
diff --git a/transport.h b/transport.h
index 51b5397..360051e 100644
--- a/transport.h
+++ b/transport.h
@@ -68,7 +68,7 @@ int transport_set_option(struct transport *transport, const char *name,
 			 const char *value);
 
 int transport_push(struct transport *connection,
-		   int refspec_nr, const char **refspec, int flags);
+				   int refspec_nr, const char **refspec, int flags, int summary);
 
 const struct ref *transport_get_remote_refs(struct transport *transport);
 
-- 
1.6.3.3.409.g4a08

Re: [PATCH v3] add --summary option to git-push and git-fetch

[Larry D'Anna]

This patch has a bug in it that causes it to randomly segfault, so don't bother
with it.  I'll send in a fixed version when I figure out the cause.

     --larry

[PATCH v4] add --summary option to git-push and git-fetch

[Larry D'Anna]

--summary will cause git-push to output a one-line of each commit pushed.
--summary=n will display at most n commits for each ref pushed.

$ git push --dry-run --summary origin :
To /home/larry/gitsandbox/a
   80f0e50..5593a38  master -> master
    > 5593a38 foo
    > 81c03f8 bar

Fetch works the same way.

Signed-off-by: Larry D'Anna <larry@elder-gods.org>
---

 Changes since last version: 

 * fixed the segfalt bug.  commit->buffer was NULL.

 Documentation/fetch-options.txt |    7 ++++++
 Documentation/git-push.txt      |    6 +++++
 builtin-fetch.c                 |   24 ++++++++++++++++-----
 builtin-log.c                   |   42 +++++++++++++++++++++++++++++++++++++++
 builtin-push.c                  |   12 ++++++++--
 builtin.h                       |    2 +
 transport.c                     |   39 +++++++++++++++++++++++++-----------
 transport.h                     |    2 +-
 8 files changed, 112 insertions(+), 22 deletions(-)

diff --git a/Documentation/fetch-options.txt b/Documentation/fetch-options.txt
index d313795..2e66d5e 100644
--- a/Documentation/fetch-options.txt
+++ b/Documentation/fetch-options.txt
@@ -27,6 +27,13 @@
 	fetches is a descendant of `<lbranch>`.  This option
 	overrides that check.
 
+--summary::
+	Print a one-line summary of each commit fetched.
+
+--summary=<n>::
+	Like --summary, but with a limit of <n> commits per ref.
+
+
 ifdef::git-pull[]
 --no-tags::
 endif::git-pull[]
diff --git a/Documentation/git-push.txt b/Documentation/git-push.txt
index 2653388..803fe36 100644
--- a/Documentation/git-push.txt
+++ b/Documentation/git-push.txt
@@ -85,6 +85,12 @@ nor in any Push line of the corresponding remotes file---see below).
 --dry-run::
 	Do everything except actually send the updates.
 
+--summary::
+	Print a one-line summary of each commit pushed.
+
+--summary=<n>::
+	Like --summary, but with a limit of <n> commits per ref.
+
 --porcelain::
 	Produce machine-readable output.  The output status line for each ref
 	will be tab-separated and sent to stdout instead of stderr.  The full
diff --git a/builtin-fetch.c b/builtin-fetch.c
index cd5eb9a..c98d06b 100644
--- a/builtin-fetch.c
+++ b/builtin-fetch.c
@@ -29,6 +29,7 @@ static const char *depth;
 static const char *upload_pack;
 static struct strbuf default_rla = STRBUF_INIT;
 static struct transport *transport;
+static int summary;
 
 static struct option builtin_fetch_options[] = {
 	OPT__VERBOSITY(&verbosity),
@@ -47,6 +48,9 @@ static struct option builtin_fetch_options[] = {
 		    "allow updating of HEAD ref"),
 	OPT_STRING(0, "depth", &depth, "DEPTH",
 		   "deepen history of shallow clone"),
+	{ OPTION_INTEGER, 0, "summary", &summary, "n", "print a summary of [at most n] fetched commits",
+	  PARSE_OPT_OPTARG, NULL, 20
+	},
 	OPT_END()
 };
 
@@ -197,7 +201,8 @@ static int s_update_ref(const char *action,
 
 static int update_local_ref(struct ref *ref,
 			    const char *remote,
-			    char *display)
+			    char *display,
+			    char *quickref)
 {
 	struct commit *current = NULL, *updated;
 	enum object_type type;
@@ -260,11 +265,12 @@ static int update_local_ref(struct ref *ref,
 		sprintf(display, "%c %-*s %-*s -> %s%s", r ? '!' : '*',
 			SUMMARY_WIDTH, what, REFCOL_WIDTH, remote, pretty_ref,
 			r ? "  (unable to update local ref)" : "");
+		if (!r)
+			strcpy(quickref, find_unique_abbrev(ref->new_sha1, DEFAULT_ABBREV));
 		return r;
 	}
 
 	if (in_merge_bases(current, &updated, 1)) {
-		char quickref[83];
 		int r;
 		strcpy(quickref, find_unique_abbrev(current->object.sha1, DEFAULT_ABBREV));
 		strcat(quickref, "..");
@@ -275,7 +281,6 @@ static int update_local_ref(struct ref *ref,
 			pretty_ref, r ? "  (unable to update local ref)" : "");
 		return r;
 	} else if (force || ref->force) {
-		char quickref[84];
 		int r;
 		strcpy(quickref, find_unique_abbrev(current->object.sha1, DEFAULT_ABBREV));
 		strcat(quickref, "...");
@@ -301,6 +306,7 @@ static int store_updated_refs(const char *raw_url, const char *remote_name,
 	struct commit *commit;
 	int url_len, i, note_len, shown_url = 0, rc = 0;
 	char note[1024];
+	char quickref[84];
 	const char *what, *kind;
 	struct ref *rm;
 	char *url, *filename = git_path("FETCH_HEAD");
@@ -373,12 +379,15 @@ static int store_updated_refs(const char *raw_url, const char *remote_name,
 				fputc(url[i], fp);
 		fputc('\n', fp);
 
-		if (ref)
-			rc |= update_local_ref(ref, what, note);
-		else
+		if (ref) {
+			*quickref = 0;
+			rc |= update_local_ref(ref, what, note, quickref);
+		} else {
+			strcpy(quickref, find_unique_abbrev(rm->old_sha1, DEFAULT_ABBREV));
 			sprintf(note, "* %-*s %-*s -> FETCH_HEAD",
 				SUMMARY_WIDTH, *kind ? kind : "branch",
 				 REFCOL_WIDTH, *what ? what : "HEAD");
+		}
 		if (*note) {
 			if (verbosity >= 0 && !shown_url) {
 				fprintf(stderr, "From %.*s\n",
@@ -388,6 +397,9 @@ static int store_updated_refs(const char *raw_url, const char *remote_name,
 			if (verbosity >= 0)
 				fprintf(stderr, " %s\n", note);
 		}
+		if (summary && quickref[0])
+			print_summary_for_push_or_fetch(quickref, summary);
+
 	}
 	free(url);
 	fclose(fp);
diff --git a/builtin-log.c b/builtin-log.c
index 0c2fa0a..a09670c 100644
--- a/builtin-log.c
+++ b/builtin-log.c
@@ -1293,3 +1293,45 @@ int cmd_cherry(int argc, const char **argv, const char *prefix)
 	free_patch_ids(&ids);
 	return 0;
 }
+
+
+void print_summary_for_push_or_fetch(const char *quickref, int limit)
+{
+	struct rev_info rev;
+	int i, max;
+	struct object *obj;
+	struct commit *commit;
+
+	max = get_max_object_index();
+	for (i = 0; i < max; i++)  {
+		obj = get_indexed_object(i);
+		if (obj)
+			obj->flags &= ~ALL_REV_FLAGS;
+	}
+
+	init_revisions(&rev, NULL);
+	rev.prune = 0;
+	assert(!handle_revision_arg(quickref, &rev, 0, 1));
+	assert(!prepare_revision_walk(&rev));
+
+	while ((commit = get_revision(&rev)) != NULL) {
+		struct strbuf buf = STRBUF_INIT;
+		if (limit == 0) {
+			fprintf(stderr, "    ...\n");
+			break;
+		}
+		if (!commit->buffer) {
+			enum object_type type;
+			unsigned long size;
+			commit->buffer =
+				read_sha1_file(commit->object.sha1, &type, &size);
+			if (!commit->buffer)
+				die("Cannot read commit %s", sha1_to_hex(commit->object.sha1));
+		}
+		format_commit_message(commit, "    %m %h %s\n", &buf, 0);
+		fputs(buf.buf, stderr);
+		strbuf_release(&buf);
+		limit--;
+	}
+	fputs("\n", stderr);
+}
diff --git a/builtin-push.c b/builtin-push.c
index 0a0297f..d38f9a8 100644
--- a/builtin-push.c
+++ b/builtin-push.c
@@ -113,7 +113,7 @@ static void setup_default_push_refspecs(void)
 	}
 }
 
-static int do_push(const char *repo, int flags)
+static int do_push(const char *repo, int flags, int summary)
 {
 	int i, errs;
 	struct remote *remote = remote_get(repo);
@@ -173,7 +173,7 @@ static int do_push(const char *repo, int flags)
 
 		if (flags & TRANSPORT_PUSH_VERBOSE)
 			fprintf(stderr, "Pushing to %s\n", url[i]);
-		err = transport_push(transport, refspec_nr, refspec, flags);
+		err = transport_push(transport, refspec_nr, refspec, flags, summary);
 		err |= transport_disconnect(transport);
 
 		if (!err)
@@ -192,6 +192,8 @@ int cmd_push(int argc, const char **argv, const char *prefix)
 	int rc;
 	const char *repo = NULL;	/* default repository */
 
+	int summary = 0;
+
 	struct option options[] = {
 		OPT_BIT('v', "verbose", &flags, "be verbose", TRANSPORT_PUSH_VERBOSE),
 		OPT_STRING( 0 , "repo", &repo, "repository", "repository"),
@@ -205,6 +207,10 @@ int cmd_push(int argc, const char **argv, const char *prefix)
 		OPT_BOOLEAN( 0 , "thin", &thin, "use thin pack"),
 		OPT_STRING( 0 , "receive-pack", &receivepack, "receive-pack", "receive pack program"),
 		OPT_STRING( 0 , "exec", &receivepack, "receive-pack", "receive pack program"),
+		{ OPTION_INTEGER, 0, "summary", &summary, "n", "print a summary of [at most n] pushed commits",
+		  PARSE_OPT_OPTARG, NULL, 20
+		},
+
 		OPT_END()
 	};
 
@@ -218,7 +224,7 @@ int cmd_push(int argc, const char **argv, const char *prefix)
 		set_refspecs(argv + 1, argc - 1);
 	}
 
-	rc = do_push(repo, flags);
+	rc = do_push(repo, flags, summary);
 	if (rc == -1)
 		usage_with_options(push_usage, options);
 	else
diff --git a/builtin.h b/builtin.h
index 20427d2..5aea3a3 100644
--- a/builtin.h
+++ b/builtin.h
@@ -113,4 +113,6 @@ extern int cmd_verify_pack(int argc, const char **argv, const char *prefix);
 extern int cmd_show_ref(int argc, const char **argv, const char *prefix);
 extern int cmd_pack_refs(int argc, const char **argv, const char *prefix);
 
+extern void print_summary_for_push_or_fetch(const char *quickref, int limit);
+
 #endif
diff --git a/transport.c b/transport.c
index de0d587..80105ae 100644
--- a/transport.c
+++ b/transport.c
@@ -11,6 +11,7 @@
 #include "bundle.h"
 #include "dir.h"
 #include "refs.h"
+#include "builtin.h"
 
 /* rsync support */
 
@@ -750,17 +751,20 @@ static const char *status_abbrev(unsigned char sha1[20])
 	return find_unique_abbrev(sha1, DEFAULT_ABBREV);
 }
 
-static void print_ok_ref_status(struct ref *ref, int porcelain)
+static void print_ok_ref_status(struct ref *ref, int porcelain, int summary)
 {
+	char quickref[84];
+	int summary_impossible = 0;
+
 	if (ref->deletion)
 		print_ref_status('-', "[deleted]", ref, NULL, NULL, porcelain);
-	else if (is_null_sha1(ref->old_sha1))
+	else if (is_null_sha1(ref->old_sha1)) {
 		print_ref_status('*',
 			(!prefixcmp(ref->name, "refs/tags/") ? "[new tag]" :
 			"[new branch]"),
 			ref, ref->peer_ref, NULL, porcelain);
-	else {
-		char quickref[84];
+		strcpy(quickref, status_abbrev(ref->new_sha1));
+	} else {
 		char type;
 		const char *msg;
 
@@ -769,6 +773,8 @@ static void print_ok_ref_status(struct ref *ref, int porcelain)
 			strcat(quickref, "...");
 			type = '+';
 			msg = "forced update";
+			if (!lookup_commit_reference_gently(ref->old_sha1, 1))
+				summary_impossible = 1;
 		} else {
 			strcat(quickref, "..");
 			type = ' ';
@@ -778,9 +784,17 @@ static void print_ok_ref_status(struct ref *ref, int porcelain)
 
 		print_ref_status(type, quickref, ref, ref->peer_ref, msg, porcelain);
 	}
+
+	if (summary) {
+		if (summary_impossible) {
+			fprintf(stderr, "    %s is unavailable\n", status_abbrev(ref->old_sha1));
+		} else {
+			print_summary_for_push_or_fetch(quickref, summary);
+		}
+	}
 }
 
-static int print_one_push_status(struct ref *ref, const char *dest, int count, int porcelain)
+static int print_one_push_status(struct ref *ref, const char *dest, int count, int porcelain, int summary)
 {
 	if (!count)
 		fprintf(stderr, "To %s\n", dest);
@@ -812,7 +826,7 @@ static int print_one_push_status(struct ref *ref, const char *dest, int count, i
 						 "remote failed to report status", porcelain);
 		break;
 	case REF_STATUS_OK:
-		print_ok_ref_status(ref, porcelain);
+		print_ok_ref_status(ref, porcelain, summary);
 		break;
 	}
 
@@ -820,7 +834,7 @@ static int print_one_push_status(struct ref *ref, const char *dest, int count, i
 }
 
 static void print_push_status(const char *dest, struct ref *refs,
-							  int verbose, int porcelain)
+							  int verbose, int porcelain, int summary)
 {
 	struct ref *ref;
 	int n = 0;
@@ -828,18 +842,18 @@ static void print_push_status(const char *dest, struct ref *refs,
 	if (verbose) {
 		for (ref = refs; ref; ref = ref->next)
 			if (ref->status == REF_STATUS_UPTODATE)
-				n += print_one_push_status(ref, dest, n, porcelain);
+				n += print_one_push_status(ref, dest, n, porcelain, summary);
 	}
 
 	for (ref = refs; ref; ref = ref->next)
 		if (ref->status == REF_STATUS_OK)
-			n += print_one_push_status(ref, dest, n, porcelain);
+			n += print_one_push_status(ref, dest, n, porcelain, summary);
 
 	for (ref = refs; ref; ref = ref->next) {
 		if (ref->status != REF_STATUS_NONE &&
 		    ref->status != REF_STATUS_UPTODATE &&
 		    ref->status != REF_STATUS_OK)
-			n += print_one_push_status(ref, dest, n, porcelain);
+			n += print_one_push_status(ref, dest, n, porcelain, summary);
 	}
 }
 
@@ -997,7 +1011,8 @@ int transport_set_option(struct transport *transport,
 }
 
 int transport_push(struct transport *transport,
-		   int refspec_nr, const char **refspec, int flags)
+				   int refspec_nr, const char **refspec,
+				   int flags, int summary)
 {
 	verify_remote_names(refspec_nr, refspec);
 
@@ -1024,7 +1039,7 @@ int transport_push(struct transport *transport,
 
 		ret = transport->push_refs(transport, remote_refs, flags);
 
-		print_push_status(transport->url, remote_refs, verbose | porcelain, porcelain);
+		print_push_status(transport->url, remote_refs, verbose | porcelain, porcelain, summary);
 
 		if (!(flags & TRANSPORT_PUSH_DRY_RUN)) {
 			struct ref *ref;
diff --git a/transport.h b/transport.h
index 51b5397..360051e 100644
--- a/transport.h
+++ b/transport.h
@@ -68,7 +68,7 @@ int transport_set_option(struct transport *transport, const char *name,
 			 const char *value);
 
 int transport_push(struct transport *connection,
-		   int refspec_nr, const char **refspec, int flags);
+				   int refspec_nr, const char **refspec, int flags, int summary);
 
 const struct ref *transport_get_remote_refs(struct transport *transport);
 
-- 
1.6.3.3.415.gbe1e

Re: [PATCH v4] add --summary option to git-push and git-fetch

[Stephen Boyd]

Larry D'Anna wrote:
> --summary will cause git-push to output a one-line of each commit pushed.
> --summary=n will display at most n commits for each ref pushed.
>
> $ git push --dry-run --summary origin :
> To /home/larry/gitsandbox/a
>    80f0e50..5593a38  master -> master
>     > 5593a38 foo
>     > 81c03f8 bar
>
> Fetch works the same way.
>
> Signed-off-by: Larry D'Anna <larry@elder-gods.org>
> ---
>
>  Changes since last version: 
>
>  * fixed the segfalt bug.  commit->buffer was NULL.
>   

Maybe adding a few tests to exercise this new option will give reviewers
a better assurance you've squashed the bugs in previous rounds?

[PATCH v4] add --summary option to git-push and git-fetch

[Larry D'Anna]

--summary will cause git-push to output a one-line of each commit pushed.
--summary=n will display at most n commits for each ref pushed.

$ git push --dry-run --summary origin :
To /home/larry/gitsandbox/a
   80f0e50..5593a38  master -> master
    > 5593a38 foo
    > 81c03f8 bar

Fetch works the same way.

Signed-off-by: Larry D'Anna <larry@elder-gods.org>
---
 
 Changes sicne last version of this patch: 

    * added some tests

 Documentation/fetch-options.txt |    7 ++++
 Documentation/git-push.txt      |    6 +++
 builtin-fetch.c                 |   24 +++++++++---
 builtin-log.c                   |   42 +++++++++++++++++++++
 builtin-push.c                  |   12 +++++--
 builtin.h                       |    2 +
 t/t5516-fetch-push.sh           |   77 +++++++++++++++++++++++++++++++++++++++
 transport.c                     |   39 ++++++++++++++------
 transport.h                     |    2 +-
 9 files changed, 189 insertions(+), 22 deletions(-)

diff --git a/Documentation/fetch-options.txt b/Documentation/fetch-options.txt
index d313795..2e66d5e 100644
--- a/Documentation/fetch-options.txt
+++ b/Documentation/fetch-options.txt
@@ -27,6 +27,13 @@
 	fetches is a descendant of `<lbranch>`.  This option
 	overrides that check.
 
+--summary::
+	Print a one-line summary of each commit fetched.
+
+--summary=<n>::
+	Like --summary, but with a limit of <n> commits per ref.
+
+
 ifdef::git-pull[]
 --no-tags::
 endif::git-pull[]
diff --git a/Documentation/git-push.txt b/Documentation/git-push.txt
index 2653388..803fe36 100644
--- a/Documentation/git-push.txt
+++ b/Documentation/git-push.txt
@@ -85,6 +85,12 @@ nor in any Push line of the corresponding remotes file---see below).
 --dry-run::
 	Do everything except actually send the updates.
 
+--summary::
+	Print a one-line summary of each commit pushed.
+
+--summary=<n>::
+	Like --summary, but with a limit of <n> commits per ref.
+
 --porcelain::
 	Produce machine-readable output.  The output status line for each ref
 	will be tab-separated and sent to stdout instead of stderr.  The full
diff --git a/builtin-fetch.c b/builtin-fetch.c
index cd5eb9a..c98d06b 100644
--- a/builtin-fetch.c
+++ b/builtin-fetch.c
@@ -29,6 +29,7 @@ static const char *depth;
 static const char *upload_pack;
 static struct strbuf default_rla = STRBUF_INIT;
 static struct transport *transport;
+static int summary;
 
 static struct option builtin_fetch_options[] = {
 	OPT__VERBOSITY(&verbosity),
@@ -47,6 +48,9 @@ static struct option builtin_fetch_options[] = {
 		    "allow updating of HEAD ref"),
 	OPT_STRING(0, "depth", &depth, "DEPTH",
 		   "deepen history of shallow clone"),
+	{ OPTION_INTEGER, 0, "summary", &summary, "n", "print a summary of [at most n] fetched commits",
+	  PARSE_OPT_OPTARG, NULL, 20
+	},
 	OPT_END()
 };
 
@@ -197,7 +201,8 @@ static int s_update_ref(const char *action,
 
 static int update_local_ref(struct ref *ref,
 			    const char *remote,
-			    char *display)
+			    char *display,
+			    char *quickref)
 {
 	struct commit *current = NULL, *updated;
 	enum object_type type;
@@ -260,11 +265,12 @@ static int update_local_ref(struct ref *ref,
 		sprintf(display, "%c %-*s %-*s -> %s%s", r ? '!' : '*',
 			SUMMARY_WIDTH, what, REFCOL_WIDTH, remote, pretty_ref,
 			r ? "  (unable to update local ref)" : "");
+		if (!r)
+			strcpy(quickref, find_unique_abbrev(ref->new_sha1, DEFAULT_ABBREV));
 		return r;
 	}
 
 	if (in_merge_bases(current, &updated, 1)) {
-		char quickref[83];
 		int r;
 		strcpy(quickref, find_unique_abbrev(current->object.sha1, DEFAULT_ABBREV));
 		strcat(quickref, "..");
@@ -275,7 +281,6 @@ static int update_local_ref(struct ref *ref,
 			pretty_ref, r ? "  (unable to update local ref)" : "");
 		return r;
 	} else if (force || ref->force) {
-		char quickref[84];
 		int r;
 		strcpy(quickref, find_unique_abbrev(current->object.sha1, DEFAULT_ABBREV));
 		strcat(quickref, "...");
@@ -301,6 +306,7 @@ static int store_updated_refs(const char *raw_url, const char *remote_name,
 	struct commit *commit;
 	int url_len, i, note_len, shown_url = 0, rc = 0;
 	char note[1024];
+	char quickref[84];
 	const char *what, *kind;
 	struct ref *rm;
 	char *url, *filename = git_path("FETCH_HEAD");
@@ -373,12 +379,15 @@ static int store_updated_refs(const char *raw_url, const char *remote_name,
 				fputc(url[i], fp);
 		fputc('\n', fp);
 
-		if (ref)
-			rc |= update_local_ref(ref, what, note);
-		else
+		if (ref) {
+			*quickref = 0;
+			rc |= update_local_ref(ref, what, note, quickref);
+		} else {
+			strcpy(quickref, find_unique_abbrev(rm->old_sha1, DEFAULT_ABBREV));
 			sprintf(note, "* %-*s %-*s -> FETCH_HEAD",
 				SUMMARY_WIDTH, *kind ? kind : "branch",
 				 REFCOL_WIDTH, *what ? what : "HEAD");
+		}
 		if (*note) {
 			if (verbosity >= 0 && !shown_url) {
 				fprintf(stderr, "From %.*s\n",
@@ -388,6 +397,9 @@ static int store_updated_refs(const char *raw_url, const char *remote_name,
 			if (verbosity >= 0)
 				fprintf(stderr, " %s\n", note);
 		}
+		if (summary && quickref[0])
+			print_summary_for_push_or_fetch(quickref, summary);
+
 	}
 	free(url);
 	fclose(fp);
diff --git a/builtin-log.c b/builtin-log.c
index 0c2fa0a..a09670c 100644
--- a/builtin-log.c
+++ b/builtin-log.c
@@ -1293,3 +1293,45 @@ int cmd_cherry(int argc, const char **argv, const char *prefix)
 	free_patch_ids(&ids);
 	return 0;
 }
+
+
+void print_summary_for_push_or_fetch(const char *quickref, int limit)
+{
+	struct rev_info rev;
+	int i, max;
+	struct object *obj;
+	struct commit *commit;
+
+	max = get_max_object_index();
+	for (i = 0; i < max; i++)  {
+		obj = get_indexed_object(i);
+		if (obj)
+			obj->flags &= ~ALL_REV_FLAGS;
+	}
+
+	init_revisions(&rev, NULL);
+	rev.prune = 0;
+	assert(!handle_revision_arg(quickref, &rev, 0, 1));
+	assert(!prepare_revision_walk(&rev));
+
+	while ((commit = get_revision(&rev)) != NULL) {
+		struct strbuf buf = STRBUF_INIT;
+		if (limit == 0) {
+			fprintf(stderr, "    ...\n");
+			break;
+		}
+		if (!commit->buffer) {
+			enum object_type type;
+			unsigned long size;
+			commit->buffer =
+				read_sha1_file(commit->object.sha1, &type, &size);
+			if (!commit->buffer)
+				die("Cannot read commit %s", sha1_to_hex(commit->object.sha1));
+		}
+		format_commit_message(commit, "    %m %h %s\n", &buf, 0);
+		fputs(buf.buf, stderr);
+		strbuf_release(&buf);
+		limit--;
+	}
+	fputs("\n", stderr);
+}
diff --git a/builtin-push.c b/builtin-push.c
index 0a0297f..d38f9a8 100644
--- a/builtin-push.c
+++ b/builtin-push.c
@@ -113,7 +113,7 @@ static void setup_default_push_refspecs(void)
 	}
 }
 
-static int do_push(const char *repo, int flags)
+static int do_push(const char *repo, int flags, int summary)
 {
 	int i, errs;
 	struct remote *remote = remote_get(repo);
@@ -173,7 +173,7 @@ static int do_push(const char *repo, int flags)
 
 		if (flags & TRANSPORT_PUSH_VERBOSE)
 			fprintf(stderr, "Pushing to %s\n", url[i]);
-		err = transport_push(transport, refspec_nr, refspec, flags);
+		err = transport_push(transport, refspec_nr, refspec, flags, summary);
 		err |= transport_disconnect(transport);
 
 		if (!err)
@@ -192,6 +192,8 @@ int cmd_push(int argc, const char **argv, const char *prefix)
 	int rc;
 	const char *repo = NULL;	/* default repository */
 
+	int summary = 0;
+
 	struct option options[] = {
 		OPT_BIT('v', "verbose", &flags, "be verbose", TRANSPORT_PUSH_VERBOSE),
 		OPT_STRING( 0 , "repo", &repo, "repository", "repository"),
@@ -205,6 +207,10 @@ int cmd_push(int argc, const char **argv, const char *prefix)
 		OPT_BOOLEAN( 0 , "thin", &thin, "use thin pack"),
 		OPT_STRING( 0 , "receive-pack", &receivepack, "receive-pack", "receive pack program"),
 		OPT_STRING( 0 , "exec", &receivepack, "receive-pack", "receive pack program"),
+		{ OPTION_INTEGER, 0, "summary", &summary, "n", "print a summary of [at most n] pushed commits",
+		  PARSE_OPT_OPTARG, NULL, 20
+		},
+
 		OPT_END()
 	};
 
@@ -218,7 +224,7 @@ int cmd_push(int argc, const char **argv, const char *prefix)
 		set_refspecs(argv + 1, argc - 1);
 	}
 
-	rc = do_push(repo, flags);
+	rc = do_push(repo, flags, summary);
 	if (rc == -1)
 		usage_with_options(push_usage, options);
 	else
diff --git a/builtin.h b/builtin.h
index 20427d2..5aea3a3 100644
--- a/builtin.h
+++ b/builtin.h
@@ -113,4 +113,6 @@ extern int cmd_verify_pack(int argc, const char **argv, const char *prefix);
 extern int cmd_show_ref(int argc, const char **argv, const char *prefix);
 extern int cmd_pack_refs(int argc, const char **argv, const char *prefix);
 
+extern void print_summary_for_push_or_fetch(const char *quickref, int limit);
+
 #endif
diff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh
index 2d2633f..cae270d 100755
--- a/t/t5516-fetch-push.sh
+++ b/t/t5516-fetch-push.sh
@@ -90,6 +90,48 @@ test_expect_success 'fetch without wildcard' '
 	)
 '
 
+test_expect_success 'fetch --summary' '
+	mk_empty &&
+	(
+		cd testrepo &&
+		git fetch --summary .. refs/heads/master:refs/remotes/origin/master 2>stderr &&
+
+		r=$(git show-ref -s --verify refs/remotes/origin/master) &&
+		test "z$r" = "z$the_commit" &&
+
+		test 1 = $(git for-each-ref refs/remotes/origin | wc -l) &&
+
+		grep -E  "^    > [a-fA-F0-9]+ second$" stderr &&
+		grep -E  "^    > [a-fA-F0-9]+ repo$" stderr
+	)
+        '
+
+test_expect_success 'fetch --summary forced update' '
+	mk_empty &&
+	(
+		cd testrepo &&
+		git fetch .. refs/heads/master:refs/remotes/origin/master &&
+
+		git checkout refs/remotes/origin/master^ &&
+		: >path3 &&
+		git add path3 &&
+		test_tick &&
+		git commit -a -m third &&
+		git update-ref refs/remotes/origin/master HEAD &&
+
+		git fetch .. -f --summary refs/heads/master:refs/remotes/origin/master 2>stderr &&
+
+		r=$(git show-ref -s --verify refs/remotes/origin/master) &&
+		test "z$r" = "z$the_commit" &&
+
+		test 1 = $(git for-each-ref refs/remotes/origin | wc -l) &&
+
+		grep -E  "^    < [a-fA-F0-9]+ third$" stderr &&
+		grep -E  "^    > [a-fA-F0-9]+ second$" stderr
+	)
+
+'
+
 test_expect_success 'fetch with wildcard' '
 	mk_empty &&
 	(
@@ -135,6 +177,41 @@ test_expect_success 'push without wildcard' '
 	)
 '
 
+test_expect_success 'push --summary' '
+	mk_empty &&
+
+	git push --summary testrepo refs/heads/master:refs/remotes/origin/master 2>stderr &&
+	(
+		cd testrepo &&
+		r=$(git show-ref -s --verify refs/remotes/origin/master) &&
+		test "z$r" = "z$the_commit" &&
+
+		test 1 = $(git for-each-ref refs/remotes/origin | wc -l)
+	) &&
+
+	grep -E  "^    > [a-fA-F0-9]+ second$" stderr &&
+	grep -E  "^    > [a-fA-F0-9]+ repo$" stderr
+'
+
+test_expect_success 'push --summary forced update' '
+	mk_empty &&
+
+	git push testrepo refs/heads/master:refs/remotes/origin/master &&
+
+	git checkout master^ &&
+	: >path3 &&
+	git add path3 &&
+	test_tick &&
+	git commit -a -m third &&
+
+	git push --summary -f testrepo HEAD:refs/remotes/origin/master 2>stderr &&
+
+	grep -E  "^    < [a-fA-F0-9]+ second$" stderr &&
+	grep -E  "^    > [a-fA-F0-9]+ third$" stderr &&
+
+	git checkout master
+'
+
 test_expect_success 'push with wildcard' '
 	mk_empty &&
 
diff --git a/transport.c b/transport.c
index de0d587..80105ae 100644
--- a/transport.c
+++ b/transport.c
@@ -11,6 +11,7 @@
 #include "bundle.h"
 #include "dir.h"
 #include "refs.h"
+#include "builtin.h"
 
 /* rsync support */
 
@@ -750,17 +751,20 @@ static const char *status_abbrev(unsigned char sha1[20])
 	return find_unique_abbrev(sha1, DEFAULT_ABBREV);
 }
 
-static void print_ok_ref_status(struct ref *ref, int porcelain)
+static void print_ok_ref_status(struct ref *ref, int porcelain, int summary)
 {
+	char quickref[84];
+	int summary_impossible = 0;
+
 	if (ref->deletion)
 		print_ref_status('-', "[deleted]", ref, NULL, NULL, porcelain);
-	else if (is_null_sha1(ref->old_sha1))
+	else if (is_null_sha1(ref->old_sha1)) {
 		print_ref_status('*',
 			(!prefixcmp(ref->name, "refs/tags/") ? "[new tag]" :
 			"[new branch]"),
 			ref, ref->peer_ref, NULL, porcelain);
-	else {
-		char quickref[84];
+		strcpy(quickref, status_abbrev(ref->new_sha1));
+	} else {
 		char type;
 		const char *msg;
 
@@ -769,6 +773,8 @@ static void print_ok_ref_status(struct ref *ref, int porcelain)
 			strcat(quickref, "...");
 			type = '+';
 			msg = "forced update";
+			if (!lookup_commit_reference_gently(ref->old_sha1, 1))
+				summary_impossible = 1;
 		} else {
 			strcat(quickref, "..");
 			type = ' ';
@@ -778,9 +784,17 @@ static void print_ok_ref_status(struct ref *ref, int porcelain)
 
 		print_ref_status(type, quickref, ref, ref->peer_ref, msg, porcelain);
 	}
+
+	if (summary) {
+		if (summary_impossible) {
+			fprintf(stderr, "    %s is unavailable\n", status_abbrev(ref->old_sha1));
+		} else {
+			print_summary_for_push_or_fetch(quickref, summary);
+		}
+	}
 }
 
-static int print_one_push_status(struct ref *ref, const char *dest, int count, int porcelain)
+static int print_one_push_status(struct ref *ref, const char *dest, int count, int porcelain, int summary)
 {
 	if (!count)
 		fprintf(stderr, "To %s\n", dest);
@@ -812,7 +826,7 @@ static int print_one_push_status(struct ref *ref, const char *dest, int count, i
 						 "remote failed to report status", porcelain);
 		break;
 	case REF_STATUS_OK:
-		print_ok_ref_status(ref, porcelain);
+		print_ok_ref_status(ref, porcelain, summary);
 		break;
 	}
 
@@ -820,7 +834,7 @@ static int print_one_push_status(struct ref *ref, const char *dest, int count, i
 }
 
 static void print_push_status(const char *dest, struct ref *refs,
-							  int verbose, int porcelain)
+							  int verbose, int porcelain, int summary)
 {
 	struct ref *ref;
 	int n = 0;
@@ -828,18 +842,18 @@ static void print_push_status(const char *dest, struct ref *refs,
 	if (verbose) {
 		for (ref = refs; ref; ref = ref->next)
 			if (ref->status == REF_STATUS_UPTODATE)
-				n += print_one_push_status(ref, dest, n, porcelain);
+				n += print_one_push_status(ref, dest, n, porcelain, summary);
 	}
 
 	for (ref = refs; ref; ref = ref->next)
 		if (ref->status == REF_STATUS_OK)
-			n += print_one_push_status(ref, dest, n, porcelain);
+			n += print_one_push_status(ref, dest, n, porcelain, summary);
 
 	for (ref = refs; ref; ref = ref->next) {
 		if (ref->status != REF_STATUS_NONE &&
 		    ref->status != REF_STATUS_UPTODATE &&
 		    ref->status != REF_STATUS_OK)
-			n += print_one_push_status(ref, dest, n, porcelain);
+			n += print_one_push_status(ref, dest, n, porcelain, summary);
 	}
 }
 
@@ -997,7 +1011,8 @@ int transport_set_option(struct transport *transport,
 }
 
 int transport_push(struct transport *transport,
-		   int refspec_nr, const char **refspec, int flags)
+				   int refspec_nr, const char **refspec,
+				   int flags, int summary)
 {
 	verify_remote_names(refspec_nr, refspec);
 
@@ -1024,7 +1039,7 @@ int transport_push(struct transport *transport,
 
 		ret = transport->push_refs(transport, remote_refs, flags);
 
-		print_push_status(transport->url, remote_refs, verbose | porcelain, porcelain);
+		print_push_status(transport->url, remote_refs, verbose | porcelain, porcelain, summary);
 
 		if (!(flags & TRANSPORT_PUSH_DRY_RUN)) {
 			struct ref *ref;
diff --git a/transport.h b/transport.h
index 51b5397..360051e 100644
--- a/transport.h
+++ b/transport.h
@@ -68,7 +68,7 @@ int transport_set_option(struct transport *transport, const char *name,
 			 const char *value);
 
 int transport_push(struct transport *connection,
-		   int refspec_nr, const char **refspec, int flags);
+				   int refspec_nr, const char **refspec, int flags, int summary);
 
 const struct ref *transport_get_remote_refs(struct transport *transport);
 
-- 
1.6.3.3.415.gbe1e

Re: [PATCH v4] add --summary option to git-push and git-fetch

[Junio C Hamano]

Larry D'Anna <larry@elder-gods.org> writes:

> --summary will cause git-push to output a one-line of each commit pushed.
> --summary=n will display at most n commits for each ref pushed.
>
> $ git push --dry-run --summary origin :
> To /home/larry/gitsandbox/a
>    80f0e50..5593a38  master -> master
>     > 5593a38 foo
>     > 81c03f8 bar
> Fetch works the same way.
>
> Signed-off-by: Larry D'Anna <larry@elder-gods.org>
> ---
>  
>  Changes sicne last version of this patch: 
>
>     * added some tests

Thanks.

> @@ -373,12 +379,15 @@ static int store_updated_refs(const char *raw_url, const char *remote_name,
>  				fputc(url[i], fp);
>  		fputc('\n', fp);
>  
> -		if (ref)
> -			rc |= update_local_ref(ref, what, note);
> -		else
> +		if (ref) {
> +			*quickref = 0;
> +			rc |= update_local_ref(ref, what, note, quickref);

Makes me wonder why update_local_ref() does not put that NUL upon entry.

> +void print_summary_for_push_or_fetch(const char *quickref, int limit)
> +{
> +	struct rev_info rev;
> +	int i, max;
> +	struct object *obj;
> +	struct commit *commit;
> +
> +	max = get_max_object_index();
> +	for (i = 0; i < max; i++)  {
> +		obj = get_indexed_object(i);
> +		if (obj)
> +			obj->flags &= ~ALL_REV_FLAGS;
> +	}

Yuck; this is a horribly heavy sledgehammer.  Couldn't you at least do
clear_commit_marks() to limit the extent of the damage?

> +	init_revisions(&rev, NULL);
> +	rev.prune = 0;
> +	assert(!handle_revision_arg(quickref, &rev, 0, 1));
> +	assert(!prepare_revision_walk(&rev));
> +
> +	while ((commit = get_revision(&rev)) != NULL) {
> +		struct strbuf buf = STRBUF_INIT;
> +		if (limit == 0) {
> +			fprintf(stderr, "    ...\n");

How would you know, when you asked 20 and you showed 20 here, that there
is no more to come?

> +			break;
> +		}

> +		if (!commit->buffer) {
> +			enum object_type type;
> +			unsigned long size;
> +			commit->buffer =
> +				read_sha1_file(commit->object.sha1, &type, &size);
> +			if (!commit->buffer)
> +				die("Cannot read commit %s", sha1_to_hex(commit->object.sha1));
> +		}
> +		format_commit_message(commit, "    %m %h %s\n", &buf, 0);

Hmm, why so many spaces before %m and after %m?

> -static int do_push(const char *repo, int flags)
> +static int do_push(const char *repo, int flags, int summary)

Couldn't this be just another bit in the flag?  I didn't check but I
suspect you wouldn't have to touch the intermediate functions in the call
chain that way.

> diff --git a/builtin.h b/builtin.h
> index 20427d2..5aea3a3 100644
> --- a/builtin.h
> +++ b/builtin.h
> @@ -113,4 +113,6 @@ extern int cmd_verify_pack(int argc, const char **argv, const char *prefix);
>  extern int cmd_show_ref(int argc, const char **argv, const char *prefix);
>  extern int cmd_pack_refs(int argc, const char **argv, const char *prefix);
>  
> +extern void print_summary_for_push_or_fetch(const char *quickref, int limit);

Please; not at the end, but at the front where all the other command
helpers live.  I actually suspect that it might be better to migrate some
part of builtin-log.c, together with your new helper function, to a new
file log.c with accompanying header file log.h, but that could be a
separate patch.

> +test_expect_success 'fetch --summary' '
> +	mk_empty &&
> +	(
> +		cd testrepo &&
> +		git fetch --summary .. refs/heads/master:refs/remotes/origin/master 2>stderr &&
> +
> +		r=$(git show-ref -s --verify refs/remotes/origin/master) &&
> +		test "z$r" = "z$the_commit" &&
> +
> +		test 1 = $(git for-each-ref refs/remotes/origin | wc -l) &&
> +
> +		grep -E  "^    > [a-fA-F0-9]+ second$" stderr &&
> +		grep -E  "^    > [a-fA-F0-9]+ repo$" stderr

Look at the output from

    $ git grep -n -e 'grep .*-E' -e 'egrep '

before applying your patch.  I think we support people with grep that does
not know about -E option.

> +test_expect_success 'fetch --summary forced update' '
> +	mk_empty &&
> +	(
> ...
> +	)
> +
> +'

There are at least two missing combinations. (1) "fetch --summary" to
fetch a new branch, and (2) "fetch --summary" does not try segfaulting by
accessing unavailable information after a failed fetch.

The same comment applies to the push side of the tests.

> -static void print_ok_ref_status(struct ref *ref, int porcelain)
> +static void print_ok_ref_status(struct ref *ref, int porcelain, int summary)

The same comment on "flags" applies here and the all the functions in the
call chain that adds this extra parameter.  porcelain/summary should be a
single int with two bits used.  It may be cleaner to change "porcelain" to
"a bit inside flag" in a separate patch _before_ this one, as a cleanup of
the previous "add --porcelain option to git-push" patch.

Re: [PATCH v4] add --summary option to git-push and git-fetch

[Larry D'Anna]

I know it's been a while but.....

> > @@ -373,12 +379,15 @@ static int store_updated_refs(const char *raw_url, const char *remote_name,
> >  				fputc(url[i], fp);
> >  		fputc('\n', fp);
> >  
> > -		if (ref)
> > -			rc |= update_local_ref(ref, what, note);
> > -		else
> > +		if (ref) {
> > +			*quickref = 0;
> > +			rc |= update_local_ref(ref, what, note, quickref);
> 
> Makes me wonder why update_local_ref() does not put that NUL upon entry.

I'm not sure what you mean.  Could you elaborate?

> > +	init_revisions(&rev, NULL);
> > +	rev.prune = 0;
> > +	assert(!handle_revision_arg(quickref, &rev, 0, 1));
> > +	assert(!prepare_revision_walk(&rev));
> > +
> > +	while ((commit = get_revision(&rev)) != NULL) {
> > +		struct strbuf buf = STRBUF_INIT;
> > +		if (limit == 0) {
> > +			fprintf(stderr, "    ...\n");
> 
> How would you know, when you asked 20 and you showed 20 here, that there
> is no more to come?

If there's more it will print the "...", if there isn't then it won't.

> > +			break;
> > +		}
> 
> > +		if (!commit->buffer) {
> > +			enum object_type type;
> > +			unsigned long size;
> > +			commit->buffer =
> > +				read_sha1_file(commit->object.sha1, &type, &size);
> > +			if (!commit->buffer)
> > +				die("Cannot read commit %s", sha1_to_hex(commit->object.sha1));
> > +		}
> > +		format_commit_message(commit, "    %m %h %s\n", &buf, 0);
> 
> Hmm, why so many spaces before %m and after %m?

So the summary lines are nicely indented with respect to the other output.

> > -static int do_push(const char *repo, int flags)
> > +static int do_push(const char *repo, int flags, int summary)
> 
> Couldn't this be just another bit in the flag?  I didn't check but I
> suspect you wouldn't have to touch the intermediate functions in the call
> chain that way.

It can't just be a bit because the "summary" parameter contains number of
summary lines to print.

> > +test_expect_success 'fetch --summary forced update' '
> > +	mk_empty &&
> > +	(
> > ...
> > +	)
> > +
> > +'
> 
> There are at least two missing combinations. (1) "fetch --summary" to
> fetch a new branch, and (2) "fetch --summary" does not try segfaulting by
> accessing unavailable information after a failed fetch.
> 
> The same comment applies to the push side of the tests.

What would be a good way to induce a failed fetch for this test?


     --larry

[PATCH v5] add --summary option to git-push and git-fetch

[Larry D'Anna]

--summary will cause git-push to output a one-line of each commit pushed.
--summary=n will display at most n commits for each ref pushed.

$ git push --dry-run --summary origin :
To /home/larry/gitsandbox/a
   80f0e50..5593a38  master -> master
    > 5593a38 foo
    > 81c03f8 bar

Fetch works the same way.

Signed-off-by: Larry D'Anna <larry@elder-gods.org>
---
 Changes since last version: 

  * added more tests
  * use clear_commit_marks instead of sledgehammer
  * moved declaration of print_summary_for_push_or_fetch to the top of the header
  * use egrep instead of grep -E

 Documentation/fetch-options.txt |    6 ++
 Documentation/git-push.txt      |    6 ++
 builtin-fetch.c                 |   24 ++++++--
 builtin-log.c                   |   35 +++++++++++
 builtin-push.c                  |   17 ++++--
 builtin.h                       |    2 +
 revision.c                      |   35 ++++++++++--
 revision.h                      |    1 +
 t/t5516-fetch-push.sh           |  121 +++++++++++++++++++++++++++++++++++++++
 transport.c                     |   42 +++++++++-----
 transport.h                     |    3 +-
 11 files changed, 260 insertions(+), 32 deletions(-)

diff --git a/Documentation/fetch-options.txt b/Documentation/fetch-options.txt
index fe716b2..53bf049 100644
--- a/Documentation/fetch-options.txt
+++ b/Documentation/fetch-options.txt
@@ -25,6 +25,12 @@ endif::git-pull[]
 	fetches is a descendant of `<lbranch>`.  This option
 	overrides that check.
 
+--summary::
+	Print a one-line summary of each commit fetched.
+
+--summary=<n>::
+	Like --summary, but with a limit of <n> commits per ref.
+
 -k::
 --keep::
 	Keep downloaded pack.
diff --git a/Documentation/git-push.txt b/Documentation/git-push.txt
index 73a921c..25e0dec 100644
--- a/Documentation/git-push.txt
+++ b/Documentation/git-push.txt
@@ -86,6 +86,12 @@ nor in any Push line of the corresponding remotes file---see below).
 --dry-run::
 	Do everything except actually send the updates.
 
+--summary::
+	Print a one-line summary of each commit pushed.
+
+--summary=<n>::
+	Like --summary, but with a limit of <n> commits per ref.
+
 --porcelain::
 	Produce machine-readable output.  The output status line for each ref
 	will be tab-separated and sent to stdout instead of stderr.  The full
diff --git a/builtin-fetch.c b/builtin-fetch.c
index 8654fa7..4f56162 100644
--- a/builtin-fetch.c
+++ b/builtin-fetch.c
@@ -32,6 +32,7 @@ static const char *depth;
 static const char *upload_pack;
 static struct strbuf default_rla = STRBUF_INIT;
 static struct transport *transport;
+static int summary;
 
 static struct option builtin_fetch_options[] = {
 	OPT__VERBOSITY(&verbosity),
@@ -58,6 +59,9 @@ static struct option builtin_fetch_options[] = {
 		    "allow updating of HEAD ref"),
 	OPT_STRING(0, "depth", &depth, "DEPTH",
 		   "deepen history of shallow clone"),
+	{ OPTION_INTEGER, 0, "summary", &summary, "n", "print a summary of [at most n] fetched commits",
+	  PARSE_OPT_OPTARG, NULL, 20
+	},
 	OPT_END()
 };
 
@@ -210,7 +214,8 @@ static int s_update_ref(const char *action,
 
 static int update_local_ref(struct ref *ref,
 			    const char *remote,
-			    char *display)
+			    char *display,
+			    char *quickref)
 {
 	struct commit *current = NULL, *updated;
 	enum object_type type;
@@ -273,11 +278,12 @@ static int update_local_ref(struct ref *ref,
 		sprintf(display, "%c %-*s %-*s -> %s%s", r ? '!' : '*',
 			SUMMARY_WIDTH, what, REFCOL_WIDTH, remote, pretty_ref,
 			r ? "  (unable to update local ref)" : "");
+		if (!r)
+			strcpy(quickref, find_unique_abbrev(ref->new_sha1, DEFAULT_ABBREV));
 		return r;
 	}
 
 	if (in_merge_bases(current, &updated, 1)) {
-		char quickref[83];
 		int r;
 		strcpy(quickref, find_unique_abbrev(current->object.sha1, DEFAULT_ABBREV));
 		strcat(quickref, "..");
@@ -288,7 +294,6 @@ static int update_local_ref(struct ref *ref,
 			pretty_ref, r ? "  (unable to update local ref)" : "");
 		return r;
 	} else if (force || ref->force) {
-		char quickref[84];
 		int r;
 		strcpy(quickref, find_unique_abbrev(current->object.sha1, DEFAULT_ABBREV));
 		strcat(quickref, "...");
@@ -314,6 +319,7 @@ static int store_updated_refs(const char *raw_url, const char *remote_name,
 	struct commit *commit;
 	int url_len, i, note_len, shown_url = 0, rc = 0;
 	char note[1024];
+	char quickref[84];
 	const char *what, *kind;
 	struct ref *rm;
 	char *url, *filename = dry_run ? "/dev/null" : git_path("FETCH_HEAD");
@@ -389,12 +395,15 @@ static int store_updated_refs(const char *raw_url, const char *remote_name,
 				fputc(url[i], fp);
 		fputc('\n', fp);
 
-		if (ref)
-			rc |= update_local_ref(ref, what, note);
-		else
+		if (ref) {
+			*quickref = 0;
+			rc |= update_local_ref(ref, what, note, quickref);
+		} else {
+			strcpy(quickref, find_unique_abbrev(rm->old_sha1, DEFAULT_ABBREV));
 			sprintf(note, "* %-*s %-*s -> FETCH_HEAD",
 				SUMMARY_WIDTH, *kind ? kind : "branch",
 				 REFCOL_WIDTH, *what ? what : "HEAD");
+		}
 		if (*note) {
 			if (verbosity >= 0 && !shown_url) {
 				fprintf(stderr, "From %.*s\n",
@@ -404,6 +413,9 @@ static int store_updated_refs(const char *raw_url, const char *remote_name,
 			if (verbosity >= 0)
 				fprintf(stderr, " %s\n", note);
 		}
+		if (summary && quickref[0])
+			print_summary_for_push_or_fetch(quickref, summary);
+
 	}
 	free(url);
 	fclose(fp);
diff --git a/builtin-log.c b/builtin-log.c
index 8d16832..55ac4e4 100644
--- a/builtin-log.c
+++ b/builtin-log.c
@@ -1350,3 +1350,38 @@ int cmd_cherry(int argc, const char **argv, const char *prefix)
 	free_patch_ids(&ids);
 	return 0;
 }
+
+
+void print_summary_for_push_or_fetch(const char *quickref, int limit)
+{
+	struct rev_info rev;
+	int i, max;
+	struct object *obj;
+	struct commit *commit;
+
+	init_revisions(&rev, NULL);
+	rev.prune = 0;
+	assert(!handle_revision_arg_clearing_flags(quickref, &rev, 0, ALL_REV_FLAGS, 1));
+	assert(!prepare_revision_walk(&rev));
+
+	while ((commit = get_revision(&rev)) != NULL) {
+		struct strbuf buf = STRBUF_INIT;
+		if (limit == 0) {
+			fprintf(stderr, "    ...\n");
+			break;
+		}
+		if (!commit->buffer) {
+			enum object_type type;
+			unsigned long size;
+			commit->buffer =
+				read_sha1_file(commit->object.sha1, &type, &size);
+			if (!commit->buffer)
+				die("Cannot read commit %s", sha1_to_hex(commit->object.sha1));
+		}
+		format_commit_message(commit, "    %m %h %s\n", &buf, 0);
+		fputs(buf.buf, stderr);
+		strbuf_release(&buf);
+		limit--;
+	}
+	fputs("\n", stderr);
+}
diff --git a/builtin-push.c b/builtin-push.c
index 5df6608..30f5a61 100644
--- a/builtin-push.c
+++ b/builtin-push.c
@@ -101,7 +101,7 @@ static void setup_default_push_refspecs(void)
 	}
 }
 
-static int push_with_options(struct transport *transport, int flags)
+static int push_with_options(struct transport *transport, int flags, int summary)
 {
 	int err;
 	int nonfastforward;
@@ -114,7 +114,7 @@ static int push_with_options(struct transport *transport, int flags)
 	if (flags & TRANSPORT_PUSH_VERBOSE)
 		fprintf(stderr, "Pushing to %s\n", transport->url);
 	err = transport_push(transport, refspec_nr, refspec, flags,
-			     &nonfastforward);
+						 summary, &nonfastforward);
 	if (err != 0)
 		error("failed to push some refs to '%s'", transport->url);
 
@@ -132,7 +132,7 @@ static int push_with_options(struct transport *transport, int flags)
 	return 1;
 }
 
-static int do_push(const char *repo, int flags)
+static int do_push(const char *repo, int flags, int summary)
 {
 	int i, errs;
 	struct remote *remote = remote_get(repo);
@@ -184,14 +184,14 @@ static int do_push(const char *repo, int flags)
 		for (i = 0; i < url_nr; i++) {
 			struct transport *transport =
 				transport_get(remote, url[i]);
-			if (push_with_options(transport, flags))
+			if (push_with_options(transport, flags, summary))
 				errs++;
 		}
 	} else {
 		struct transport *transport =
 			transport_get(remote, NULL);
 
-		if (push_with_options(transport, flags))
+ 		if (push_with_options(transport, flags, summary))
 			errs++;
 	}
 	return !!errs;
@@ -203,6 +203,8 @@ int cmd_push(int argc, const char **argv, const char *prefix)
 	int tags = 0;
 	int rc;
 	const char *repo = NULL;	/* default repository */
+	int summary = 0;
+
 	struct option options[] = {
 		OPT_BIT('q', "quiet", &flags, "be quiet", TRANSPORT_PUSH_QUIET),
 		OPT_BIT('v', "verbose", &flags, "be verbose", TRANSPORT_PUSH_VERBOSE),
@@ -218,6 +220,9 @@ int cmd_push(int argc, const char **argv, const char *prefix)
 		OPT_BOOLEAN( 0 , "thin", &thin, "use thin pack"),
 		OPT_STRING( 0 , "receive-pack", &receivepack, "receive-pack", "receive pack program"),
 		OPT_STRING( 0 , "exec", &receivepack, "receive-pack", "receive pack program"),
+		{ OPTION_INTEGER, 0, "summary", &summary, "n", "print a summary of [at most n] pushed commits",
+		  PARSE_OPT_OPTARG, NULL, 20
+		},
 		OPT_BIT('u', "set-upstream", &flags, "set upstream for git pull/status",
 			TRANSPORT_PUSH_SET_UPSTREAM),
 		OPT_END()
@@ -239,7 +244,7 @@ int cmd_push(int argc, const char **argv, const char *prefix)
 		set_refspecs(argv + 1, argc - 1);
 	}
 
-	rc = do_push(repo, flags);
+	rc = do_push(repo, flags, summary);
 	if (rc == -1)
 		usage_with_options(push_usage, options);
 	else
diff --git a/builtin.h b/builtin.h
index e8202f3..ec411be 100644
--- a/builtin.h
+++ b/builtin.h
@@ -19,6 +19,7 @@ extern int commit_tree(const char *msg, unsigned char *tree,
 		struct commit_list *parents, unsigned char *ret,
 		const char *author);
 extern int check_pager_config(const char *cmd);
+extern void print_summary_for_push_or_fetch(const char *quickref, int limit);
 
 extern int cmd_add(int argc, const char **argv, const char *prefix);
 extern int cmd_annotate(int argc, const char **argv, const char *prefix);
@@ -122,4 +123,5 @@ extern int cmd_show_ref(int argc, const char **argv, const char *prefix);
 extern int cmd_pack_refs(int argc, const char **argv, const char *prefix);
 extern int cmd_replace(int argc, const char **argv, const char *prefix);
 
+
 #endif
diff --git a/revision.c b/revision.c
index 3ba6d99..cf3103a 100644
--- a/revision.c
+++ b/revision.c
@@ -768,7 +768,7 @@ static void handle_reflog(struct rev_info *revs, unsigned flags)
 	for_each_reflog(handle_one_reflog, &cb);
 }
 
-static int add_parents_only(struct rev_info *revs, const char *arg, int flags)
+static int add_parents_only(struct rev_info *revs, const char *arg, int flags, int flags_to_clear)
 {
 	unsigned char sha1[20];
 	struct object *it;
@@ -792,6 +792,8 @@ static int add_parents_only(struct rev_info *revs, const char *arg, int flags)
 	if (it->type != OBJ_COMMIT)
 		return 0;
 	commit = (struct commit *)it;
+	if (flags_to_clear)
+	    clear_commit_marks(commit, flags_to_clear); 
 	for (parents = commit->parents; parents; parents = parents->next) {
 		it = &parents->item->object;
 		it->flags |= flags;
@@ -887,9 +889,18 @@ int handle_revision_arg(const char *arg, struct rev_info *revs,
 			int flags,
 			int cant_be_filename)
 {
+    return handle_revision_arg_clearing_flags (arg, revs, flags, 0, cant_be_filename); 
+}
+
+
+
+int handle_revision_arg_clearing_flags (const char *arg, struct rev_info *revs,
+					int flags,  int flags_to_clear, 
+					int cant_be_filename)
+{
 	unsigned mode;
 	char *dotdot;
-	struct object *object;
+	struct object *object, *object_deref;
 	unsigned char sha1[20];
 	int local_flags;
 
@@ -915,6 +926,10 @@ int handle_revision_arg(const char *arg, struct rev_info *revs,
 
 			a = lookup_commit_reference(from_sha1);
 			b = lookup_commit_reference(sha1);
+			if (a && flags_to_clear) 
+			    clear_commit_marks(a, flags_to_clear); 
+			if (b && flags_to_clear)
+			    clear_commit_marks(b, flags_to_clear); 
 			if (!a || !b) {
 				die(symmetric ?
 				    "Invalid symmetric difference expression %s...%s" :
@@ -945,14 +960,14 @@ int handle_revision_arg(const char *arg, struct rev_info *revs,
 	dotdot = strstr(arg, "^@");
 	if (dotdot && !dotdot[2]) {
 		*dotdot = 0;
-		if (add_parents_only(revs, arg, flags))
+		if (add_parents_only(revs, arg, flags, flags_to_clear))
 			return 0;
 		*dotdot = '^';
 	}
 	dotdot = strstr(arg, "^!");
 	if (dotdot && !dotdot[2]) {
 		*dotdot = 0;
-		if (!add_parents_only(revs, arg, flags ^ UNINTERESTING))
+		if (!add_parents_only(revs, arg, flags ^ UNINTERESTING, flags_to_clear))
 			*dotdot = '^';
 	}
 
@@ -965,7 +980,17 @@ int handle_revision_arg(const char *arg, struct rev_info *revs,
 		return -1;
 	if (!cant_be_filename)
 		verify_non_filename(revs->prefix, arg);
-	object = get_reference(revs, arg, sha1, flags ^ local_flags);
+
+	object = parse_object(sha1); 
+	if (!object)
+	    die("bad object %s", arg);
+	
+	object_deref = deref_tag(object, NULL, 0); 
+	if (object_deref && object_deref->type == OBJ_COMMIT)
+	    if (flags_to_clear)
+		clear_commit_marks((struct commit *) object_deref, flags_to_clear); 
+
+	object->flags |= flags ^ local_flags; 
 	add_pending_object_with_mode(revs, object, arg, mode);
 	return 0;
 }
diff --git a/revision.h b/revision.h
index a14deef..36f78bb 100644
--- a/revision.h
+++ b/revision.h
@@ -143,6 +143,7 @@ extern void parse_revision_opt(struct rev_info *revs, struct parse_opt_ctx_t *ct
 				 const struct option *options,
 				 const char * const usagestr[]);
 extern int handle_revision_arg(const char *arg, struct rev_info *revs,int flags,int cant_be_filename);
+extern int handle_revision_arg_clearing_flags(const char *arg, struct rev_info *revs,int flags, int flags_to_clear, int cant_be_filename);
 
 extern int prepare_revision_walk(struct rev_info *revs);
 extern struct commit *get_revision(struct rev_info *revs);
diff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh
index 0f04b2e..3472aaa 100755
--- a/t/t5516-fetch-push.sh
+++ b/t/t5516-fetch-push.sh
@@ -91,6 +91,71 @@ test_expect_success 'fetch without wildcard' '
 	)
 '
 
+
+test_expect_success 'fetch --summary branch update' '
+	mk_empty &&
+	(
+		cd testrepo &&
+
+		git fetch  .. refs/heads/master:refs/remotes/origin/master &&
+
+		git update-ref refs/remotes/origin/master refs/remotes/origin/master^ &&
+
+		git fetch --summary .. refs/heads/master:refs/remotes/origin/master 2>stderr &&
+
+		r=$(git show-ref -s --verify refs/remotes/origin/master) &&
+		test "z$r" = "z$the_commit" &&
+
+
+		test 1 = $(git for-each-ref refs/remotes/origin | wc -l) &&
+
+		  egrep  "^    > [a-fA-F0-9]+ second$" stderr &&
+		! egrep  "^    > [a-fA-F0-9]+ repo$" stderr
+	)
+	'
+
+test_expect_success 'fetch --summary new branch' '
+	mk_empty &&p
+	(
+		cd testrepo &&
+		git fetch --summary .. refs/heads/master:refs/remotes/origin/master 2>stderr &&
+
+		r=$(git show-ref -s --verify refs/remotes/origin/master) &&
+		test "z$r" = "z$the_commit" &&
+
+		test 1 = $(git for-each-ref refs/remotes/origin | wc -l) &&
+
+		egrep  "^    > [a-fA-F0-9]+ second$" stderr &&
+		egrep  "^    > [a-fA-F0-9]+ repo$" stderr
+	)
+        '
+
+test_expect_success 'fetch --summary forced update' '
+	mk_empty &&
+	(
+		cd testrepo &&
+		git fetch .. refs/heads/master:refs/remotes/origin/master &&
+
+		git checkout refs/remotes/origin/master^ &&
+		: >path3 &&
+		git add path3 &&
+		test_tick &&
+		git commit -a -m third &&
+		git update-ref refs/remotes/origin/master HEAD &&
+
+		git fetch .. -f --summary refs/heads/master:refs/remotes/origin/master 2>stderr &&
+
+		r=$(git show-ref -s --verify refs/remotes/origin/master) &&
+		test "z$r" = "z$the_commit" &&
+
+		test 1 = $(git for-each-ref refs/remotes/origin | wc -l) &&
+
+		egrep  "^    < [a-fA-F0-9]+ third$" stderr &&
+		egrep  "^    > [a-fA-F0-9]+ second$" stderr
+	)
+
+'
+
 test_expect_success 'fetch with wildcard' '
 	mk_empty &&
 	(
@@ -153,6 +218,62 @@ test_expect_success 'push without wildcard' '
 	)
 '
 
+test_expect_success 'push --summary new branch' '
+	mk_empty &&
+
+	git push --summary testrepo refs/heads/master:refs/remotes/origin/master 2>stderr &&
+	(
+		cd testrepo &&
+		r=$(git show-ref -s --verify refs/remotes/origin/master) &&
+		test "z$r" = "z$the_commit" &&
+
+		test 1 = $(git for-each-ref refs/remotes/origin | wc -l)
+	) &&
+
+	egrep  "^    > [a-fA-F0-9]+ second$" stderr &&
+	egrep  "^    > [a-fA-F0-9]+ repo$" stderr
+'
+
+test_expect_success 'push --summary branch update' '
+	mk_empty &&
+
+	git push testrepo refs/heads/master:refs/remotes/origin/master &&
+
+	git --git-dir testrepo/.git update-ref refs/remotes/origin/master refs/remotes/origin/master^ &&
+
+	git push --summary testrepo refs/heads/master:refs/remotes/origin/master 2>stderr &&
+	(
+		cd testrepo &&
+		r=$(git show-ref -s --verify refs/remotes/origin/master) &&
+		test "z$r" = "z$the_commit" &&
+
+		test 1 = $(git for-each-ref refs/remotes/origin | wc -l)
+	) &&
+
+	  egrep  "^    > [a-fA-F0-9]+ second$" stderr &&
+	! egrep  "^    > [a-fA-F0-9]+ repo$" stderr
+'
+
+
+test_expect_success 'push --summary forced update' '
+	mk_empty &&
+
+	git push testrepo refs/heads/master:refs/remotes/origin/master &&
+
+	git checkout master^ &&
+	: >path3 &&
+	git add path3 &&
+	test_tick &&
+	git commit -a -m third &&
+
+	git push --summary -f testrepo HEAD:refs/remotes/origin/master 2>stderr &&
+
+	egrep  "^    < [a-fA-F0-9]+ second$" stderr &&
+	egrep  "^    > [a-fA-F0-9]+ third$" stderr &&
+
+	git checkout master
+'
+
 test_expect_success 'push with wildcard' '
 	mk_empty &&
 
diff --git a/transport.c b/transport.c
index 3846aac..1e3fa7a 100644
--- a/transport.c
+++ b/transport.c
@@ -8,6 +8,7 @@
 #include "bundle.h"
 #include "dir.h"
 #include "refs.h"
+#include "builtin.h"
 #include "branch.h"
 
 /* rsync support */
@@ -642,17 +643,20 @@ static const char *status_abbrev(unsigned char sha1[20])
 	return find_unique_abbrev(sha1, DEFAULT_ABBREV);
 }
 
-static void print_ok_ref_status(struct ref *ref, int porcelain)
+static void print_ok_ref_status(struct ref *ref, int porcelain, int summary)
 {
+	char quickref[84];
+	int summary_impossible = 0;
+
 	if (ref->deletion)
 		print_ref_status('-', "[deleted]", ref, NULL, NULL, porcelain);
-	else if (is_null_sha1(ref->old_sha1))
+	else if (is_null_sha1(ref->old_sha1)) {
 		print_ref_status('*',
 			(!prefixcmp(ref->name, "refs/tags/") ? "[new tag]" :
 			"[new branch]"),
 			ref, ref->peer_ref, NULL, porcelain);
-	else {
-		char quickref[84];
+		strcpy(quickref, status_abbrev(ref->new_sha1));
+	} else {
 		char type;
 		const char *msg;
 
@@ -661,6 +665,8 @@ static void print_ok_ref_status(struct ref *ref, int porcelain)
 			strcat(quickref, "...");
 			type = '+';
 			msg = "forced update";
+			if (!lookup_commit_reference_gently(ref->old_sha1, 1))
+				summary_impossible = 1;
 		} else {
 			strcat(quickref, "..");
 			type = ' ';
@@ -670,9 +676,17 @@ static void print_ok_ref_status(struct ref *ref, int porcelain)
 
 		print_ref_status(type, quickref, ref, ref->peer_ref, msg, porcelain);
 	}
+
+	if (summary) {
+		if (summary_impossible) {
+			fprintf(stderr, "    %s is unavailable\n", status_abbrev(ref->old_sha1));
+		} else {
+			print_summary_for_push_or_fetch(quickref, summary);
+		}
+	}
 }
 
-static int print_one_push_status(struct ref *ref, const char *dest, int count, int porcelain)
+static int print_one_push_status(struct ref *ref, const char *dest, int count, int porcelain, int summary)
 {
 	if (!count)
 		fprintf(stderr, "To %s\n", dest);
@@ -704,7 +718,7 @@ static int print_one_push_status(struct ref *ref, const char *dest, int count, i
 						 "remote failed to report status", porcelain);
 		break;
 	case REF_STATUS_OK:
-		print_ok_ref_status(ref, porcelain);
+		print_ok_ref_status(ref, porcelain, summary);
 		break;
 	}
 
@@ -712,7 +726,7 @@ static int print_one_push_status(struct ref *ref, const char *dest, int count, i
 }
 
 static void print_push_status(const char *dest, struct ref *refs,
-			      int verbose, int porcelain, int * nonfastforward)
+							  int verbose, int porcelain, int summary, int *nonfastforward)
 {
 	struct ref *ref;
 	int n = 0;
@@ -720,19 +734,19 @@ static void print_push_status(const char *dest, struct ref *refs,
 	if (verbose) {
 		for (ref = refs; ref; ref = ref->next)
 			if (ref->status == REF_STATUS_UPTODATE)
-				n += print_one_push_status(ref, dest, n, porcelain);
+				n += print_one_push_status(ref, dest, n, porcelain, summary);
 	}
 
 	for (ref = refs; ref; ref = ref->next)
 		if (ref->status == REF_STATUS_OK)
-			n += print_one_push_status(ref, dest, n, porcelain);
+			n += print_one_push_status(ref, dest, n, porcelain, summary);
 
 	*nonfastforward = 0;
 	for (ref = refs; ref; ref = ref->next) {
 		if (ref->status != REF_STATUS_NONE &&
 		    ref->status != REF_STATUS_UPTODATE &&
 		    ref->status != REF_STATUS_OK)
-			n += print_one_push_status(ref, dest, n, porcelain);
+			n += print_one_push_status(ref, dest, n, porcelain, summary);
 		if (ref->status == REF_STATUS_REJECT_NONFASTFORWARD)
 			*nonfastforward = 1;
 	}
@@ -1014,8 +1028,8 @@ int transport_set_option(struct transport *transport,
 }
 
 int transport_push(struct transport *transport,
-		   int refspec_nr, const char **refspec, int flags,
-		   int *nonfastforward)
+				   int refspec_nr, const char **refspec,
+				   int flags, int summary, int *nonfastforward)
 {
 	*nonfastforward = 0;
 	verify_remote_names(refspec_nr, refspec);
@@ -1058,8 +1072,8 @@ int transport_push(struct transport *transport,
 
 		if (!quiet || err)
 			print_push_status(transport->url, remote_refs,
-					verbose | porcelain, porcelain,
-					nonfastforward);
+							  verbose | porcelain, porcelain,
+							  summary, nonfastforward);
 
 		if (flags & TRANSPORT_PUSH_SET_UPSTREAM)
 			set_upstreams(transport, remote_refs, pretend);
diff --git a/transport.h b/transport.h
index 7cea5cc..f87b2e9 100644
--- a/transport.h
+++ b/transport.h
@@ -124,9 +124,10 @@ int transport_set_option(struct transport *transport, const char *name,
 			 const char *value);
 
 int transport_push(struct transport *connection,
-		   int refspec_nr, const char **refspec, int flags,
+		   int refspec_nr, const char **refspec, int flags, int summary,
 		   int * nonfastforward);
 
+
 const struct ref *transport_get_remote_refs(struct transport *transport);
 
 int transport_fetch_refs(struct transport *transport, struct ref *refs);
-- 
1.6.3.3.415.gbe1e

Re: [PATCH v4] add --summary option to git-push and git-fetch

[Junio C Hamano]

Larry D'Anna <larry@elder-gods.org> writes:

Please don't use M-F-T to deflect a direct response meant to you away from
you.  I also do not want people wasting _my_ time by following your M-F-T
and sending their comments meant to _you_ coming to me with my address on
To: line, as I prioritize incoming messages based on where my address is
in the header, and do not want you waste _other's_ time by making them
correct their "To:" like to avoid wasting my time.

>> > @@ -373,12 +379,15 @@ static int store_updated_refs(const char *raw_url, const char *remote_name,
>> >  				fputc(url[i], fp);
>> >  		fputc('\n', fp);
>> >  
>> > -		if (ref)
>> > -			rc |= update_local_ref(ref, what, note);
>> > -		else
>> > +		if (ref) {
>> > +			*quickref = 0;
>> > +			rc |= update_local_ref(ref, what, note, quickref);
>> 
>> Makes me wonder why update_local_ref() does not put that NUL upon entry.
>
> I'm not sure what you mean.  Could you elaborate?

Why is it necessary for the caller to do "*quickref = '\0'" before calling
update_local_ref()?  Shouldn't your updated u-l-r be doing that clearing,
so that the callers don't have to worry about it?

>> > +	init_revisions(&rev, NULL);
>> > +	rev.prune = 0;
>> > +	assert(!handle_revision_arg(quickref, &rev, 0, 1));
>> > +	assert(!prepare_revision_walk(&rev));
>> > +
>> > +	while ((commit = get_revision(&rev)) != NULL) {
>> > +		struct strbuf buf = STRBUF_INIT;
>> > +		if (limit == 0) {
>> > +			fprintf(stderr, "    ...\n");
>> 
>> How would you know, when you asked 20 and you showed 20 here, that there
>> is no more to come?
>
> If there's more it will print the "...", if there isn't then it won't.

If your limit is 20 and if you unconditionally say "..." after pulling 20
from the pool, the consumer of your output would think "Ah, I see 20 but
that is only I asked for 20, and the ... means there are more".  But that
is incorrect because your 21st call to get_revision() might have yielded
NULL in which case you had only 20 after all.

You cannot do "..." correctly without pulling one more than the limit from
the pool.

>> > +test_expect_success 'fetch --summary forced update' '
>> > +	mk_empty &&
>> > +	(
>> > ...
>> > +	)
>> > +
>> > +'
>> 
>> There are at least two missing combinations. (1) "fetch --summary" to
>> fetch a new branch, and (2) "fetch --summary" does not try segfaulting by
>> accessing unavailable information after a failed fetch.
>> 
>> The same comment applies to the push side of the tests.
>
> What would be a good way to induce a failed fetch for this test?

Not having a valid ref or repo, perhaps.  I dunno---it's been quite a
while since I saw the patch.

Re: [PATCH v4] add --summary option to git-push and git-fetch

[Junio C Hamano]

Larry D'Anna <larry@elder-gods.org> writes:

Please don't use M-F-T to deflect a direct response meant to you away from
you.  I also do not want people wasting _my_ time by following your M-F-T
and sending their comments meant to _you_ coming to me with my address on
To: line, as I prioritize incoming messages based on where my address is
in the header, and do not want you waste _other's_ time by making them
correct their "To:" like to avoid wasting my time.

>> > @@ -373,12 +379,15 @@ static int store_updated_refs(const char *raw_url, const char *remote_name,
>> >  				fputc(url[i], fp);
>> >  		fputc('\n', fp);
>> >  
>> > -		if (ref)
>> > -			rc |= update_local_ref(ref, what, note);
>> > -		else
>> > +		if (ref) {
>> > +			*quickref = 0;
>> > +			rc |= update_local_ref(ref, what, note, quickref);
>> 
>> Makes me wonder why update_local_ref() does not put that NUL upon entry.
>
> I'm not sure what you mean.  Could you elaborate?

Why is it necessary for the caller to do "*quickref = '\0'" before calling
update_local_ref()?  Shouldn't your updated u-l-r be doing that clearing,
so that the callers don't have to worry about it?

>> > +	init_revisions(&rev, NULL);
>> > +	rev.prune = 0;
>> > +	assert(!handle_revision_arg(quickref, &rev, 0, 1));
>> > +	assert(!prepare_revision_walk(&rev));
>> > +
>> > +	while ((commit = get_revision(&rev)) != NULL) {
>> > +		struct strbuf buf = STRBUF_INIT;
>> > +		if (limit == 0) {
>> > +			fprintf(stderr, "    ...\n");
>> 
>> How would you know, when you asked 20 and you showed 20 here, that there
>> is no more to come?
>
> If there's more it will print the "...", if there isn't then it won't.

If your limit is 20 and if you unconditionally say "..." after pulling 20
from the pool, the consumer of your output would think "Ah, I see 20 but
that is only I asked for 20, and the ... means there are more".  But that
is incorrect because your 21st call to get_revision() might have yielded
NULL in which case you had only 20 after all.

You cannot do "..." correctly without pulling one more than the limit from
the pool.

>> > +test_expect_success 'fetch --summary forced update' '
>> > +	mk_empty &&
>> > +	(
>> > ...
>> > +	)
>> > +
>> > +'
>> 
>> There are at least two missing combinations. (1) "fetch --summary" to
>> fetch a new branch, and (2) "fetch --summary" does not try segfaulting by
>> accessing unavailable information after a failed fetch.
>> 
>> The same comment applies to the push side of the tests.
>
> What would be a good way to induce a failed fetch for this test?

Not having a valid ref or repo, perhaps.  I dunno---it's been quite a
while since I saw the patch.

Re: [PATCH v4] add --summary option to git-push and git-fetch

[Junio C Hamano]

Junio C Hamano <gitster@pobox.com> writes:

>>> How would you know, when you asked 20 and you showed 20 here, that there
>>> is no more to come?
>>
>> If there's more it will print the "...", if there isn't then it won't.
>
> If your limit is 20 and if you unconditionally say "..." after pulling 20
> from the pool, the consumer of your output would think "Ah, I see 20 but
> that is only I asked for 20, and the ... means there are more".  But that
> is incorrect because your 21st call to get_revision() might have yielded
> NULL in which case you had only 20 after all.
>
> You cannot do "..." correctly without pulling one more than the limit from
> the pool.

Ah, I either misremembered the loop or didn't read it correctly.

Either way, the loop in your v5 looks correct ("read, check count to say
... and exit if limit goes down to zero, show one, decrement and go to
top").

[PATCH v6] add --summary option to git-push and git-fetch

[Larry D'Anna]

--summary will cause git-push to output a one-line of each commit pushed.
--summary=n will display at most n commits for each ref pushed.

$ git push --dry-run --summary origin :
To /home/larry/gitsandbox/a
   80f0e50..5593a38  master -> master
    > 5593a38 foo
    > 81c03f8 bar

Fetch works the same way.

Signed-off-by: Larry D'Anna <larry@elder-gods.org>
---
 Changes since last version:
 
 * moved *quickref = 0 into update_local_ref
 * added tests to confirm failed pushes and fetches don't segfault

 Documentation/fetch-options.txt |    6 ++
 Documentation/git-push.txt      |    6 ++
 builtin-fetch.c                 |   22 +++++--
 builtin-log.c                   |   35 ++++++++++
 builtin-push.c                  |   17 +++--
 builtin.h                       |    2 +
 revision.c                      |   35 +++++++++--
 revision.h                      |    1 +
 t/t5516-fetch-push.sh           |  136 +++++++++++++++++++++++++++++++++++++++
 transport.c                     |   42 ++++++++----
 transport.h                     |    3 +-
 11 files changed, 274 insertions(+), 31 deletions(-)

diff --git a/Documentation/fetch-options.txt b/Documentation/fetch-options.txt
index fe716b2..53bf049 100644
--- a/Documentation/fetch-options.txt
+++ b/Documentation/fetch-options.txt
@@ -25,6 +25,12 @@ endif::git-pull[]
 	fetches is a descendant of `<lbranch>`.  This option
 	overrides that check.
 
+--summary::
+	Print a one-line summary of each commit fetched.
+
+--summary=<n>::
+	Like --summary, but with a limit of <n> commits per ref.
+
 -k::
 --keep::
 	Keep downloaded pack.
diff --git a/Documentation/git-push.txt b/Documentation/git-push.txt
index 73a921c..25e0dec 100644
--- a/Documentation/git-push.txt
+++ b/Documentation/git-push.txt
@@ -86,6 +86,12 @@ nor in any Push line of the corresponding remotes file---see below).
 --dry-run::
 	Do everything except actually send the updates.
 
+--summary::
+	Print a one-line summary of each commit pushed.
+
+--summary=<n>::
+	Like --summary, but with a limit of <n> commits per ref.
+
 --porcelain::
 	Produce machine-readable output.  The output status line for each ref
 	will be tab-separated and sent to stdout instead of stderr.  The full
diff --git a/builtin-fetch.c b/builtin-fetch.c
index 8654fa7..5e1e91b 100644
--- a/builtin-fetch.c
+++ b/builtin-fetch.c
@@ -32,6 +32,7 @@ static const char *depth;
 static const char *upload_pack;
 static struct strbuf default_rla = STRBUF_INIT;
 static struct transport *transport;
+static int summary;
 
 static struct option builtin_fetch_options[] = {
 	OPT__VERBOSITY(&verbosity),
@@ -58,6 +59,9 @@ static struct option builtin_fetch_options[] = {
 		    "allow updating of HEAD ref"),
 	OPT_STRING(0, "depth", &depth, "DEPTH",
 		   "deepen history of shallow clone"),
+	{ OPTION_INTEGER, 0, "summary", &summary, "n", "print a summary of [at most n] fetched commits",
+	  PARSE_OPT_OPTARG, NULL, 20
+	},
 	OPT_END()
 };
 
@@ -210,13 +214,15 @@ static int s_update_ref(const char *action,
 
 static int update_local_ref(struct ref *ref,
 			    const char *remote,
-			    char *display)
+			    char *display,
+			    char *quickref)
 {
 	struct commit *current = NULL, *updated;
 	enum object_type type;
 	struct branch *current_branch = branch_get(NULL);
 	const char *pretty_ref = prettify_refname(ref->name);
 
+	*quickref = 0;
 	*display = 0;
 	type = sha1_object_info(ref->new_sha1, NULL);
 	if (type < 0)
@@ -273,11 +279,12 @@ static int update_local_ref(struct ref *ref,
 		sprintf(display, "%c %-*s %-*s -> %s%s", r ? '!' : '*',
 			SUMMARY_WIDTH, what, REFCOL_WIDTH, remote, pretty_ref,
 			r ? "  (unable to update local ref)" : "");
+		if (!r)
+			strcpy(quickref, find_unique_abbrev(ref->new_sha1, DEFAULT_ABBREV));
 		return r;
 	}
 
 	if (in_merge_bases(current, &updated, 1)) {
-		char quickref[83];
 		int r;
 		strcpy(quickref, find_unique_abbrev(current->object.sha1, DEFAULT_ABBREV));
 		strcat(quickref, "..");
@@ -288,7 +295,6 @@ static int update_local_ref(struct ref *ref,
 			pretty_ref, r ? "  (unable to update local ref)" : "");
 		return r;
 	} else if (force || ref->force) {
-		char quickref[84];
 		int r;
 		strcpy(quickref, find_unique_abbrev(current->object.sha1, DEFAULT_ABBREV));
 		strcat(quickref, "...");
@@ -314,6 +320,7 @@ static int store_updated_refs(const char *raw_url, const char *remote_name,
 	struct commit *commit;
 	int url_len, i, note_len, shown_url = 0, rc = 0;
 	char note[1024];
+	char quickref[84];
 	const char *what, *kind;
 	struct ref *rm;
 	char *url, *filename = dry_run ? "/dev/null" : git_path("FETCH_HEAD");
@@ -390,11 +397,13 @@ static int store_updated_refs(const char *raw_url, const char *remote_name,
 		fputc('\n', fp);
 
 		if (ref)
-			rc |= update_local_ref(ref, what, note);
-		else
+			rc |= update_local_ref(ref, what, note, quickref);
+		else {
+			strcpy(quickref, find_unique_abbrev(rm->old_sha1, DEFAULT_ABBREV));
 			sprintf(note, "* %-*s %-*s -> FETCH_HEAD",
 				SUMMARY_WIDTH, *kind ? kind : "branch",
 				 REFCOL_WIDTH, *what ? what : "HEAD");
+		}
 		if (*note) {
 			if (verbosity >= 0 && !shown_url) {
 				fprintf(stderr, "From %.*s\n",
@@ -404,6 +413,9 @@ static int store_updated_refs(const char *raw_url, const char *remote_name,
 			if (verbosity >= 0)
 				fprintf(stderr, " %s\n", note);
 		}
+		if (summary && quickref[0])
+			print_summary_for_push_or_fetch(quickref, summary);
+
 	}
 	free(url);
 	fclose(fp);
diff --git a/builtin-log.c b/builtin-log.c
index 8d16832..55ac4e4 100644
--- a/builtin-log.c
+++ b/builtin-log.c
@@ -1350,3 +1350,38 @@ int cmd_cherry(int argc, const char **argv, const char *prefix)
 	free_patch_ids(&ids);
 	return 0;
 }
+
+
+void print_summary_for_push_or_fetch(const char *quickref, int limit)
+{
+	struct rev_info rev;
+	int i, max;
+	struct object *obj;
+	struct commit *commit;
+
+	init_revisions(&rev, NULL);
+	rev.prune = 0;
+	assert(!handle_revision_arg_clearing_flags(quickref, &rev, 0, ALL_REV_FLAGS, 1));
+	assert(!prepare_revision_walk(&rev));
+
+	while ((commit = get_revision(&rev)) != NULL) {
+		struct strbuf buf = STRBUF_INIT;
+		if (limit == 0) {
+			fprintf(stderr, "    ...\n");
+			break;
+		}
+		if (!commit->buffer) {
+			enum object_type type;
+			unsigned long size;
+			commit->buffer =
+				read_sha1_file(commit->object.sha1, &type, &size);
+			if (!commit->buffer)
+				die("Cannot read commit %s", sha1_to_hex(commit->object.sha1));
+		}
+		format_commit_message(commit, "    %m %h %s\n", &buf, 0);
+		fputs(buf.buf, stderr);
+		strbuf_release(&buf);
+		limit--;
+	}
+	fputs("\n", stderr);
+}
diff --git a/builtin-push.c b/builtin-push.c
index 5df6608..30f5a61 100644
--- a/builtin-push.c
+++ b/builtin-push.c
@@ -101,7 +101,7 @@ static void setup_default_push_refspecs(void)
 	}
 }
 
-static int push_with_options(struct transport *transport, int flags)
+static int push_with_options(struct transport *transport, int flags, int summary)
 {
 	int err;
 	int nonfastforward;
@@ -114,7 +114,7 @@ static int push_with_options(struct transport *transport, int flags)
 	if (flags & TRANSPORT_PUSH_VERBOSE)
 		fprintf(stderr, "Pushing to %s\n", transport->url);
 	err = transport_push(transport, refspec_nr, refspec, flags,
-			     &nonfastforward);
+						 summary, &nonfastforward);
 	if (err != 0)
 		error("failed to push some refs to '%s'", transport->url);
 
@@ -132,7 +132,7 @@ static int push_with_options(struct transport *transport, int flags)
 	return 1;
 }
 
-static int do_push(const char *repo, int flags)
+static int do_push(const char *repo, int flags, int summary)
 {
 	int i, errs;
 	struct remote *remote = remote_get(repo);
@@ -184,14 +184,14 @@ static int do_push(const char *repo, int flags)
 		for (i = 0; i < url_nr; i++) {
 			struct transport *transport =
 				transport_get(remote, url[i]);
-			if (push_with_options(transport, flags))
+			if (push_with_options(transport, flags, summary))
 				errs++;
 		}
 	} else {
 		struct transport *transport =
 			transport_get(remote, NULL);
 
-		if (push_with_options(transport, flags))
+ 		if (push_with_options(transport, flags, summary))
 			errs++;
 	}
 	return !!errs;
@@ -203,6 +203,8 @@ int cmd_push(int argc, const char **argv, const char *prefix)
 	int tags = 0;
 	int rc;
 	const char *repo = NULL;	/* default repository */
+	int summary = 0;
+
 	struct option options[] = {
 		OPT_BIT('q', "quiet", &flags, "be quiet", TRANSPORT_PUSH_QUIET),
 		OPT_BIT('v', "verbose", &flags, "be verbose", TRANSPORT_PUSH_VERBOSE),
@@ -218,6 +220,9 @@ int cmd_push(int argc, const char **argv, const char *prefix)
 		OPT_BOOLEAN( 0 , "thin", &thin, "use thin pack"),
 		OPT_STRING( 0 , "receive-pack", &receivepack, "receive-pack", "receive pack program"),
 		OPT_STRING( 0 , "exec", &receivepack, "receive-pack", "receive pack program"),
+		{ OPTION_INTEGER, 0, "summary", &summary, "n", "print a summary of [at most n] pushed commits",
+		  PARSE_OPT_OPTARG, NULL, 20
+		},
 		OPT_BIT('u', "set-upstream", &flags, "set upstream for git pull/status",
 			TRANSPORT_PUSH_SET_UPSTREAM),
 		OPT_END()
@@ -239,7 +244,7 @@ int cmd_push(int argc, const char **argv, const char *prefix)
 		set_refspecs(argv + 1, argc - 1);
 	}
 
-	rc = do_push(repo, flags);
+	rc = do_push(repo, flags, summary);
 	if (rc == -1)
 		usage_with_options(push_usage, options);
 	else
diff --git a/builtin.h b/builtin.h
index e8202f3..ec411be 100644
--- a/builtin.h
+++ b/builtin.h
@@ -19,6 +19,7 @@ extern int commit_tree(const char *msg, unsigned char *tree,
 		struct commit_list *parents, unsigned char *ret,
 		const char *author);
 extern int check_pager_config(const char *cmd);
+extern void print_summary_for_push_or_fetch(const char *quickref, int limit);
 
 extern int cmd_add(int argc, const char **argv, const char *prefix);
 extern int cmd_annotate(int argc, const char **argv, const char *prefix);
@@ -122,4 +123,5 @@ extern int cmd_show_ref(int argc, const char **argv, const char *prefix);
 extern int cmd_pack_refs(int argc, const char **argv, const char *prefix);
 extern int cmd_replace(int argc, const char **argv, const char *prefix);
 
+
 #endif
diff --git a/revision.c b/revision.c
index 3ba6d99..cf3103a 100644
--- a/revision.c
+++ b/revision.c
@@ -768,7 +768,7 @@ static void handle_reflog(struct rev_info *revs, unsigned flags)
 	for_each_reflog(handle_one_reflog, &cb);
 }
 
-static int add_parents_only(struct rev_info *revs, const char *arg, int flags)
+static int add_parents_only(struct rev_info *revs, const char *arg, int flags, int flags_to_clear)
 {
 	unsigned char sha1[20];
 	struct object *it;
@@ -792,6 +792,8 @@ static int add_parents_only(struct rev_info *revs, const char *arg, int flags)
 	if (it->type != OBJ_COMMIT)
 		return 0;
 	commit = (struct commit *)it;
+	if (flags_to_clear)
+	    clear_commit_marks(commit, flags_to_clear); 
 	for (parents = commit->parents; parents; parents = parents->next) {
 		it = &parents->item->object;
 		it->flags |= flags;
@@ -887,9 +889,18 @@ int handle_revision_arg(const char *arg, struct rev_info *revs,
 			int flags,
 			int cant_be_filename)
 {
+    return handle_revision_arg_clearing_flags (arg, revs, flags, 0, cant_be_filename); 
+}
+
+
+
+int handle_revision_arg_clearing_flags (const char *arg, struct rev_info *revs,
+					int flags,  int flags_to_clear, 
+					int cant_be_filename)
+{
 	unsigned mode;
 	char *dotdot;
-	struct object *object;
+	struct object *object, *object_deref;
 	unsigned char sha1[20];
 	int local_flags;
 
@@ -915,6 +926,10 @@ int handle_revision_arg(const char *arg, struct rev_info *revs,
 
 			a = lookup_commit_reference(from_sha1);
 			b = lookup_commit_reference(sha1);
+			if (a && flags_to_clear) 
+			    clear_commit_marks(a, flags_to_clear); 
+			if (b && flags_to_clear)
+			    clear_commit_marks(b, flags_to_clear); 
 			if (!a || !b) {
 				die(symmetric ?
 				    "Invalid symmetric difference expression %s...%s" :
@@ -945,14 +960,14 @@ int handle_revision_arg(const char *arg, struct rev_info *revs,
 	dotdot = strstr(arg, "^@");
 	if (dotdot && !dotdot[2]) {
 		*dotdot = 0;
-		if (add_parents_only(revs, arg, flags))
+		if (add_parents_only(revs, arg, flags, flags_to_clear))
 			return 0;
 		*dotdot = '^';
 	}
 	dotdot = strstr(arg, "^!");
 	if (dotdot && !dotdot[2]) {
 		*dotdot = 0;
-		if (!add_parents_only(revs, arg, flags ^ UNINTERESTING))
+		if (!add_parents_only(revs, arg, flags ^ UNINTERESTING, flags_to_clear))
 			*dotdot = '^';
 	}
 
@@ -965,7 +980,17 @@ int handle_revision_arg(const char *arg, struct rev_info *revs,
 		return -1;
 	if (!cant_be_filename)
 		verify_non_filename(revs->prefix, arg);
-	object = get_reference(revs, arg, sha1, flags ^ local_flags);
+
+	object = parse_object(sha1); 
+	if (!object)
+	    die("bad object %s", arg);
+	
+	object_deref = deref_tag(object, NULL, 0); 
+	if (object_deref && object_deref->type == OBJ_COMMIT)
+	    if (flags_to_clear)
+		clear_commit_marks((struct commit *) object_deref, flags_to_clear); 
+
+	object->flags |= flags ^ local_flags; 
 	add_pending_object_with_mode(revs, object, arg, mode);
 	return 0;
 }
diff --git a/revision.h b/revision.h
index a14deef..36f78bb 100644
--- a/revision.h
+++ b/revision.h
@@ -143,6 +143,7 @@ extern void parse_revision_opt(struct rev_info *revs, struct parse_opt_ctx_t *ct
 				 const struct option *options,
 				 const char * const usagestr[]);
 extern int handle_revision_arg(const char *arg, struct rev_info *revs,int flags,int cant_be_filename);
+extern int handle_revision_arg_clearing_flags(const char *arg, struct rev_info *revs,int flags, int flags_to_clear, int cant_be_filename);
 
 extern int prepare_revision_walk(struct rev_info *revs);
 extern struct commit *get_revision(struct rev_info *revs);
diff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh
index 0f04b2e..9e49714 100755
--- a/t/t5516-fetch-push.sh
+++ b/t/t5516-fetch-push.sh
@@ -91,6 +91,80 @@ test_expect_success 'fetch without wildcard' '
 	)
 '
 
+
+test_expect_success 'fetch --summary branch update' '
+	mk_empty &&
+	(
+		cd testrepo &&
+
+		git fetch  .. refs/heads/master:refs/remotes/origin/master &&
+
+		git update-ref refs/remotes/origin/master refs/remotes/origin/master^ &&
+
+		git fetch --summary .. refs/heads/master:refs/remotes/origin/master 2>stderr &&
+
+		r=$(git show-ref -s --verify refs/remotes/origin/master) &&
+		test "z$r" = "z$the_commit" &&
+
+
+		test 1 = $(git for-each-ref refs/remotes/origin | wc -l) &&
+
+		  egrep  "^    > [a-fA-F0-9]+ second$" stderr &&
+		! egrep  "^    > [a-fA-F0-9]+ repo$" stderr
+	)
+	'
+
+test_expect_success 'fetch --summary bad ref' '
+	mk_empty &&
+	(
+		cd testrepo &&
+		! git fetch --summary .. foo:bar 2>stderr  &&
+		! grep "seg" stderr
+	)
+	'
+
+test_expect_success 'fetch --summary new branch' '
+	mk_empty &&
+	(
+		cd testrepo &&
+		git fetch --summary .. refs/heads/master:refs/remotes/origin/master 2>stderr &&
+
+		r=$(git show-ref -s --verify refs/remotes/origin/master) &&
+		test "z$r" = "z$the_commit" &&
+
+		test 1 = $(git for-each-ref refs/remotes/origin | wc -l) &&
+
+		egrep  "^    > [a-fA-F0-9]+ second$" stderr &&
+		egrep  "^    > [a-fA-F0-9]+ repo$" stderr
+	)
+        '
+
+test_expect_success 'fetch --summary forced update' '
+	mk_empty &&
+	(
+		cd testrepo &&
+		git fetch .. refs/heads/master:refs/remotes/origin/master &&
+
+		git checkout refs/remotes/origin/master^ &&
+		: >path3 &&
+		git add path3 &&
+		test_tick &&
+		git commit -a -m third &&
+		git update-ref refs/remotes/origin/master HEAD &&
+
+		git fetch .. -f --summary refs/heads/master:refs/remotes/origin/master 2>stderr &&
+
+		r=$(git show-ref -s --verify refs/remotes/origin/master) &&
+		test "z$r" = "z$the_commit" &&
+
+		test 1 = $(git for-each-ref refs/remotes/origin | wc -l) &&
+
+		egrep  "^    < [a-fA-F0-9]+ third$" stderr &&
+		egrep  "^    > [a-fA-F0-9]+ second$" stderr
+	)
+
+'
+
 test_expect_success 'fetch with wildcard' '
 	mk_empty &&
 	(
@@ -153,6 +227,68 @@ test_expect_success 'push without wildcard' '
 	)
 '
 
+test_expect_success 'push --summary bad ref' '
+	mk_empty &&
+	! git push --summary testrepo foo:bar 2>stderr  &&
+	! grep "seg" stderr
+'
+
+test_expect_success 'push --summary new branch' '
+	mk_empty &&
+
+	git push --summary testrepo refs/heads/master:refs/remotes/origin/master 2>stderr &&
+	(
+		cd testrepo &&
+		r=$(git show-ref -s --verify refs/remotes/origin/master) &&
+		test "z$r" = "z$the_commit" &&
+
+		test 1 = $(git for-each-ref refs/remotes/origin | wc -l)
+	) &&
+
+	egrep  "^    > [a-fA-F0-9]+ second$" stderr &&
+	egrep  "^    > [a-fA-F0-9]+ repo$" stderr
+'
+
+test_expect_success 'push --summary branch update' '
+	mk_empty &&
+
+	git push testrepo refs/heads/master:refs/remotes/origin/master &&
+
+	git --git-dir testrepo/.git update-ref refs/remotes/origin/master refs/remotes/origin/master^ &&
+
+	git push --summary testrepo refs/heads/master:refs/remotes/origin/master 2>stderr &&
+	(
+		cd testrepo &&
+		r=$(git show-ref -s --verify refs/remotes/origin/master) &&
+		test "z$r" = "z$the_commit" &&
+
+		test 1 = $(git for-each-ref refs/remotes/origin | wc -l)
+	) &&
+
+	  egrep  "^    > [a-fA-F0-9]+ second$" stderr &&
+	! egrep  "^    > [a-fA-F0-9]+ repo$" stderr
+'
+
+
+test_expect_success 'push --summary forced update' '
+	mk_empty &&
+
+	git push testrepo refs/heads/master:refs/remotes/origin/master &&
+
+	git checkout master^ &&
+	: >path3 &&
+	git add path3 &&
+	test_tick &&
+	git commit -a -m third &&
+
+	git push --summary -f testrepo HEAD:refs/remotes/origin/master 2>stderr &&
+
+	egrep  "^    < [a-fA-F0-9]+ second$" stderr &&
+	egrep  "^    > [a-fA-F0-9]+ third$" stderr &&
+
+	git checkout master
+'
+
 test_expect_success 'push with wildcard' '
 	mk_empty &&
 
diff --git a/transport.c b/transport.c
index 3846aac..1e3fa7a 100644
--- a/transport.c
+++ b/transport.c
@@ -8,6 +8,7 @@
 #include "bundle.h"
 #include "dir.h"
 #include "refs.h"
+#include "builtin.h"
 #include "branch.h"
 
 /* rsync support */
@@ -642,17 +643,20 @@ static const char *status_abbrev(unsigned char sha1[20])
 	return find_unique_abbrev(sha1, DEFAULT_ABBREV);
 }
 
-static void print_ok_ref_status(struct ref *ref, int porcelain)
+static void print_ok_ref_status(struct ref *ref, int porcelain, int summary)
 {
+	char quickref[84];
+	int summary_impossible = 0;
+
 	if (ref->deletion)
 		print_ref_status('-', "[deleted]", ref, NULL, NULL, porcelain);
-	else if (is_null_sha1(ref->old_sha1))
+	else if (is_null_sha1(ref->old_sha1)) {
 		print_ref_status('*',
 			(!prefixcmp(ref->name, "refs/tags/") ? "[new tag]" :
 			"[new branch]"),
 			ref, ref->peer_ref, NULL, porcelain);
-	else {
-		char quickref[84];
+		strcpy(quickref, status_abbrev(ref->new_sha1));
+	} else {
 		char type;
 		const char *msg;
 
@@ -661,6 +665,8 @@ static void print_ok_ref_status(struct ref *ref, int porcelain)
 			strcat(quickref, "...");
 			type = '+';
 			msg = "forced update";
+			if (!lookup_commit_reference_gently(ref->old_sha1, 1))
+				summary_impossible = 1;
 		} else {
 			strcat(quickref, "..");
 			type = ' ';
@@ -670,9 +676,17 @@ static void print_ok_ref_status(struct ref *ref, int porcelain)
 
 		print_ref_status(type, quickref, ref, ref->peer_ref, msg, porcelain);
 	}
+
+	if (summary) {
+		if (summary_impossible) {
+			fprintf(stderr, "    %s is unavailable\n", status_abbrev(ref->old_sha1));
+		} else {
+			print_summary_for_push_or_fetch(quickref, summary);
+		}
+	}
 }
 
-static int print_one_push_status(struct ref *ref, const char *dest, int count, int porcelain)
+static int print_one_push_status(struct ref *ref, const char *dest, int count, int porcelain, int summary)
 {
 	if (!count)
 		fprintf(stderr, "To %s\n", dest);
@@ -704,7 +718,7 @@ static int print_one_push_status(struct ref *ref, const char *dest, int count, i
 						 "remote failed to report status", porcelain);
 		break;
 	case REF_STATUS_OK:
-		print_ok_ref_status(ref, porcelain);
+		print_ok_ref_status(ref, porcelain, summary);
 		break;
 	}
 
@@ -712,7 +726,7 @@ static int print_one_push_status(struct ref *ref, const char *dest, int count, i
 }
 
 static void print_push_status(const char *dest, struct ref *refs,
-			      int verbose, int porcelain, int * nonfastforward)
+							  int verbose, int porcelain, int summary, int *nonfastforward)
 {
 	struct ref *ref;
 	int n = 0;
@@ -720,19 +734,19 @@ static void print_push_status(const char *dest, struct ref *refs,
 	if (verbose) {
 		for (ref = refs; ref; ref = ref->next)
 			if (ref->status == REF_STATUS_UPTODATE)
-				n += print_one_push_status(ref, dest, n, porcelain);
+				n += print_one_push_status(ref, dest, n, porcelain, summary);
 	}
 
 	for (ref = refs; ref; ref = ref->next)
 		if (ref->status == REF_STATUS_OK)
-			n += print_one_push_status(ref, dest, n, porcelain);
+			n += print_one_push_status(ref, dest, n, porcelain, summary);
 
 	*nonfastforward = 0;
 	for (ref = refs; ref; ref = ref->next) {
 		if (ref->status != REF_STATUS_NONE &&
 		    ref->status != REF_STATUS_UPTODATE &&
 		    ref->status != REF_STATUS_OK)
-			n += print_one_push_status(ref, dest, n, porcelain);
+			n += print_one_push_status(ref, dest, n, porcelain, summary);
 		if (ref->status == REF_STATUS_REJECT_NONFASTFORWARD)
 			*nonfastforward = 1;
 	}
@@ -1014,8 +1028,8 @@ int transport_set_option(struct transport *transport,
 }
 
 int transport_push(struct transport *transport,
-		   int refspec_nr, const char **refspec, int flags,
-		   int *nonfastforward)
+				   int refspec_nr, const char **refspec,
+				   int flags, int summary, int *nonfastforward)
 {
 	*nonfastforward = 0;
 	verify_remote_names(refspec_nr, refspec);
@@ -1058,8 +1072,8 @@ int transport_push(struct transport *transport,
 
 		if (!quiet || err)
 			print_push_status(transport->url, remote_refs,
-					verbose | porcelain, porcelain,
-					nonfastforward);
+							  verbose | porcelain, porcelain,
+							  summary, nonfastforward);
 
 		if (flags & TRANSPORT_PUSH_SET_UPSTREAM)
 			set_upstreams(transport, remote_refs, pretend);
diff --git a/transport.h b/transport.h
index 7cea5cc..f87b2e9 100644
--- a/transport.h
+++ b/transport.h
@@ -124,9 +124,10 @@ int transport_set_option(struct transport *transport, const char *name,
 			 const char *value);
 
 int transport_push(struct transport *connection,
-		   int refspec_nr, const char **refspec, int flags,
+		   int refspec_nr, const char **refspec, int flags, int summary,
 		   int * nonfastforward);
 
+
 const struct ref *transport_get_remote_refs(struct transport *transport);
 
 int transport_fetch_refs(struct transport *transport, struct ref *refs);
-- 
1.6.3.3.415.gbe1e

Re: [PATCH v6] add --summary option to git-push and git-fetch

[Ilari Liusvaara]

On Fri, Jan 29, 2010 at 11:16:31PM -0800, Junio C Hamano wrote:
 
> As I said in my review during the earlier rounds, I do not know if it is
> safe to use the flags and do the traversal inside this same process.  You
> may be clearing the flags to protect your traversal (one per branch) from
> stepping on each other, but how would this affect the use of object flags
> in existing parts of the "push" machinery?  Is the reasoning that even if
> push calls into traversal code and after it walked the commit ancestry for
> its own purpose, your addition will clear the flags and existing code will
> never look at object flags again, so this new code is free to use them and
> all is Ok?  As long as you made sure that nobody looks at object flags you
> modified, then I am fine with that---I just don't know if that is what is
> happening here, and that is why I am asking.
> 
> I'd need help from the usual "transport" suspects for this patch.

Well, I can say smart transports implemented by remote helpers are similar
to ssh://&co (no surprise, they connect differently, but use the same underlying
client code). Furthermore, actual remote helper stub code doesn't seem to play
with revisions.

And the actual remote helper parts seem to use clean memory image anyway
(they exec).

So that leaves the following:
- git:// "layer 7" (git://, ssh://, file:// & co.[*])
- rsync:// (third-class anyway)

Also, what about multiple-URL case? Don't know if there are problems, but it
seems to be quite rarely tested...

[*] OTOH, this is extremely heavily used code, so breakages here will usually
be pretty visible.

-Ilari

Re: [PATCH v6] add --summary option to git-push and git-fetch

[Junio C Hamano]

Ilari Liusvaara <ilari.liusvaara@elisanet.fi> writes:

> Also, what about multiple-URL case? Don't know if there are problems, but it
> seems to be quite rarely tested...

Pushing to more than one deliberately chooses to fork for each remote IIRC
to avoid any funnies.

> [*] OTOH, this is extremely heavily used code, so breakages here will usually
> be pretty visible.

I'd actually like to avoid anybody being hit.

Thanks for a quick response.

Re: [PATCH v6] add --summary option to git-push and git-fetch

[Ilari Liusvaara]

On Sat, Jan 30, 2010 at 12:04:25AM -0800, Junio C Hamano wrote:
> Ilari Liusvaara <ilari.liusvaara@elisanet.fi> writes:
> 
> > Also, what about multiple-URL case? Don't know if there are problems, but it
> > seems to be quite rarely tested...
> 
> Pushing to more than one deliberately chooses to fork for each remote IIRC
> to avoid any funnies.

I don't see any forking in the code. It redoes transport_get() (transports
can't really be reused...) and in the end does transport_disconnect().

And besides, what it did with internal protocol following remote helper
didn't look very much like what would happen if it forked...

-Ilari

Re: [PATCH v6] add --summary option to git-push and git-fetch

[Tay Ray Chuan]

Hi,

On Sat, Jan 30, 2010 at 10:05 AM, Larry D'Anna <larry@elder-gods.org> wrote:
> --summary will cause git-push to output a one-line of each commit pushed.
> --summary=n will display at most n commits for each ref pushed.
>
> $ git push --dry-run --summary origin :
> To /home/larry/gitsandbox/a
>   80f0e50..5593a38  master -> master
>    > 5593a38 foo
>    > 81c03f8 bar
>
> Fetch works the same way.

I'm sorry for being late to this discussion; I see the time and work
you've put in this and all the previous revisions. But I do have an
objection to implementing this behaviour using the option --summary,
on the grounds of UI-consistency.

I believe git users are already familiar with --summary in git-diff
and git-log, and it might confuse them when the output of --summary
for git-push and git-fetch looks different.

Also, I wonder what is the motivation behind displaying this
information. Perhaps you are including this to produce output for an
IDE? If this is included upstream, then I believe this is the first
instance of such verbose information being displayed. Even a
fast-forward merge (the closest I can think of that matches this)
doesn't show this - I still have to do git log rev1^..rev2 to see.the
intervening revisions (inclusive).

-- 
Cheers,
Ray Chuan

Re: [PATCH v6] add --summary option to git-push and git-fetch

[Daniel Barkalow]

On Fri, 29 Jan 2010, Junio C Hamano wrote:

> As I said in my review during the earlier rounds, I do not know if it is
> safe to use the flags and do the traversal inside this same process.  You
> may be clearing the flags to protect your traversal (one per branch) from
> stepping on each other, but how would this affect the use of object flags
> in existing parts of the "push" machinery?  Is the reasoning that even if
> push calls into traversal code and after it walked the commit ancestry for
> its own purpose, your addition will clear the flags and existing code will
> never look at object flags again, so this new code is free to use them and
> all is Ok?  As long as you made sure that nobody looks at object flags you
> modified, then I am fine with that---I just don't know if that is what is
> happening here, and that is why I am asking.
> 
> I'd need help from the usual "transport" suspects for this patch.

I'm pretty sure that the built-in transport implementations all clear the 
flags themselves before using them. The fetch side has to be able to fetch 
twice in order to handle tags, and the push side has to be able to push to 
multiple destinations. So both parts should be defending themselves 
against flags that are specificly confusing to that part.

	-Daniel
*This .sig left intentionally blank*

Re: [PATCH v6] add --summary option to git-push and git-fetch

[Larry D'Anna]

* Junio C Hamano (gitster@pobox.com) [100130 02:16]:
> Larry D'Anna <larry@elder-gods.org> writes:
> > +
> > +	object = parse_object(sha1); 
> > +	if (!object)
> > +	    die("bad object %s", arg);
> > +	
> > +	object_deref = deref_tag(object, NULL, 0); 
> > +	if (object_deref && object_deref->type == OBJ_COMMIT)
> > +	    if (flags_to_clear)
> > +		clear_commit_marks((struct commit *) object_deref, flags_to_clear); 
> > +
> > +	object->flags |= flags ^ local_flags; 
> 
> This smells somewhat fishy---what is the reason this "peel and mark" needs
> to be done only in this codepath, and none of the other callers of
> get_reference() need a similar logic, for example?
> 
> In general, why do you need to sprinkle clear-commit-marks all over the
> place?  

My idea was to call call clear_commit_marks on the "roots" of the revision arg,
and since handle_revision_arg looks up those roots in several different places,
i had to put clear_commit_marks in each of those places.  the reason the patch
is particularly ugly in this spot is that the other places where i put
clear_commit_marks, I already had a struct commit *, but here i just had a
object that might be a tag.

> This is not a rhetorical question (I haven't reviewed all the
> codepath involved for quite some time), but naïvely it appears it would be
> a lot simpler if you can let the existing code to do all the revision
> parsing and preparation to add to the pending object array as usual, and
> clear the flags from them before you let prepare_revision_walk() to start
> traversing the commit, but you probably had some reason why that simpler
> approach would not work and did it this way.  What am I missing?

The "existing code" being the caller of print_summary_for_push_or_fetch?  I
suppose I just wanted to keep the patches interference with update_local_ref to
a minimum, so I had it just grab the existing variable "quickref" out of that
function, because that was all the info I really needed to print the summary.

So i guess you're saying that it would be better for update_local_ref and
print_summary_for_push_or_fetch to clear the flags, and just pass a rev_info for
print_summary_for_push_or_fetch instead of quickref?

   --larry

Re: [PATCH v6] add --summary option to git-push and git-fetch

[Larry D'Anna]

* Larry D'Anna (larry@elder-gods.org) [100131 19:57]:
> So i guess you're saying that it would be better for update_local_ref and
> print_summary_for_push_or_fetch to clear the flags, and just pass a rev_info for
> print_summary_for_push_or_fetch instead of quickref?

So, should I submit a version of the patch that does it this way?  Should it use
a subprocess?  Should the option be called something other than --summary?
Should I just forget about it?

  --larry

Re: [PATCH v6] add --summary option to git-push and git-fetch

[Junio C Hamano]

Larry D'Anna <larry@elder-gods.org> writes:

> * Larry D'Anna (larry@elder-gods.org) [100131 19:57]:
>> So i guess you're saying that it would be better for update_local_ref and
>> print_summary_for_push_or_fetch to clear the flags, and just pass a rev_info for
>> print_summary_for_push_or_fetch instead of quickref?
>
> So, should I submit a version of the patch that does it this way?  Should it use
> a subprocess?  Should the option be called something other than --summary?

I dunno.  If it delegated to a subprocess it would certainly be easier to
review and get convinced that the change won't affect object flags for
other parts of the system in bad ways, but there obviously is a
performance downside.

I vaguely recall there also were comments on the output format not being
consistent with output of similar nature from other parts of the system,
but I am not the one who is particularly interested in this feature, so
I'll let you and the list decide.

Re: [PATCH v6] add --summary option to git-push and git-fetch

[Junio C Hamano]

Junio C Hamano <gitster@pobox.com> writes:

> I dunno.  If it delegated to a subprocess it would certainly be easier to
> review and get convinced that the change won't affect object flags for
> other parts of the system in bad ways, but there obviously is a
> performance downside.

You fundamentally cannot use the same "summary" logic for push and fetch,
and it is especially true if you are doing it inside the same process, I
think.

When you force a fetch, you will have the complete histories for both old
and new, as you started from old (and I am assuming that you are fsck
clean) and you successfully fetched new.  When you force a push, however,
you may already have the old in your object store, but there is no
guarantee that you have the complete history leading to it (i.e. you may
have got the tip commit left by an earlier fetch done with a commit walker
that you interrupted in the middle).

So at the very least, your "summary_impossible" logic should work a lot
harder than a single lookup-commit-reference-gently; it needs to walk the
ancestry until you hit some ref to prove that you have a complete history
for that commit, without dying.  Otherwise get_revision() loop inside
print_summary_for_push_or_fetch() would say "oops -- I don't have the
parent commit" when it tries to call add_parents_to_list() and die.

Doing the summary traversal inside a subprocess would simplify the
handling of the error for such a case, I guess.

My gut feeling from the beginning has been that a patch that touches
revision.c for this topic would add unacceptable cruft to the already
complex logic in that library for no real gain.  Doing the traversal
inside a subprocess would allay that worry as well ;-)