[PATCH 2/3] rev-list: Introduce --no-output to avoid /dev/null redirects

[PATCH 2/3] rev-list: Introduce --no-output to avoid /dev/null redirects

[Shawn O. Pearce]

Some uses of git-rev-list are to run it with --objects to see if
a range of objects between two or more commits is fully connected
or not.  In such a case the caller doesn't care about the actual
object names or hash hints so formatting this data only for it to
be dumped to /dev/null by a redirect is a waste of CPU time.  If
all the caller needs is the exit status then --no-output can be
used to bypass the commit and object formatting.

Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---
 Documentation/git-rev-list.txt |    9 +++++++++
 builtin-rev-list.c             |   26 ++++++++++++++++++++++----
 2 files changed, 31 insertions(+), 4 deletions(-)

diff --git a/Documentation/git-rev-list.txt b/Documentation/git-rev-list.txt
index 4852804..66ddbd3 100644
--- a/Documentation/git-rev-list.txt
+++ b/Documentation/git-rev-list.txt
@@ -20,6 +20,7 @@ SYNOPSIS
 	     [ \--not ]
 	     [ \--all ]
 	     [ \--stdin ]
+	     [ \--no-output ]
 	     [ \--topo-order ]
 	     [ \--parents ]
 	     [ \--timestamp ]
@@ -270,6 +271,14 @@ limiting may be applied.
 	In addition to the '<commit>' listed on the command
 	line, read them from the standard input.
 
+--no-output::
+
+	Don't print anything to standard output.  This form of
+	git-rev-list is primarly meant to allow the caller to
+	test the exit status to see if a range of objects is fully
+	connected (or not).  It is faster than redirecting stdout
+	to /dev/null as the output does not have to be formatted.
+
 --cherry-pick::
 
 	Omit any commit that introduces the same change as
diff --git a/builtin-rev-list.c b/builtin-rev-list.c
index 2dec887..634b1f4 100644
--- a/builtin-rev-list.c
+++ b/builtin-rev-list.c
@@ -26,6 +26,7 @@ static const char rev_list_usage[] =
 "    --remove-empty\n"
 "    --all\n"
 "    --stdin\n"
+"    --no-output\n"
 "  ordering output:\n"
 "    --topo-order\n"
 "    --date-order\n"
@@ -50,6 +51,7 @@ static int show_timestamp;
 static int hdr_termination;
 static const char *header_prefix;
 
+static void noshow_commit(struct commit *commit);
 static void show_commit(struct commit *commit)
 {
 	if (show_timestamp)
@@ -93,6 +95,11 @@ static void show_commit(struct commit *commit)
 		strbuf_release(&buf);
 	}
 	maybe_flush_or_die(stdout, "stdout");
+	noshow_commit(commit);
+}
+
+static void noshow_commit(struct commit *commit)
+{
 	if (commit->parents) {
 		free_commit_list(commit->parents);
 		commit->parents = NULL;
@@ -101,6 +108,12 @@ static void show_commit(struct commit *commit)
 	commit->buffer = NULL;
 }
 
+static void noshow_object(struct object_array_entry *p)
+{
+	if (p->item->type == OBJ_BLOB && !has_sha1_file(p->item->sha1))
+		die("missing blob object '%s'", sha1_to_hex(p->item->sha1));
+}
+
 static void show_object(struct object_array_entry *p)
 {
 	/* An object with name "foo\n0000000..." can be used to
@@ -108,9 +121,7 @@ static void show_object(struct object_array_entry *p)
 	 */
 	const char *ep = strchr(p->name, '\n');
 
-	if (p->item->type == OBJ_BLOB && !has_sha1_file(p->item->sha1))
-		die("missing blob object '%s'", sha1_to_hex(p->item->sha1));
-
+	noshow_object(p);
 	if (ep) {
 		printf("%s %.*s\n", sha1_to_hex(p->item->sha1),
 		       (int) (ep - p->name),
@@ -527,6 +538,7 @@ int cmd_rev_list(int argc, const char **argv, const char *prefix)
 	int read_from_stdin = 0;
 	int bisect_show_vars = 0;
 	int bisect_find_all = 0;
+	int nooutput = 0;
 
 	git_config(git_default_config);
 	init_revisions(&revs, prefix);
@@ -565,6 +577,10 @@ int cmd_rev_list(int argc, const char **argv, const char *prefix)
 			read_revisions_from_stdin(&revs);
 			continue;
 		}
+		if (!strcmp(arg, "--no-output")) {
+			nooutput = 1;
+			continue;
+		}
 		usage(rev_list_usage);
 
 	}
@@ -640,7 +656,9 @@ int cmd_rev_list(int argc, const char **argv, const char *prefix)
 		}
 	}
 
-	traverse_commit_list(&revs, show_commit, show_object);
+	traverse_commit_list(&revs,
+		nooutput ? noshow_commit : show_commit,
+		nooutput ? noshow_object : show_object);
 
 	return 0;
 }
-- 
1.5.3.5.1590.gfadfad

Re: [PATCH 2/3] rev-list: Introduce --no-output to avoid /dev/null redirects

[Alex Riesen]

Shawn O. Pearce, Thu, Nov 08, 2007 09:00:52 +0100:
> Some uses of git-rev-list are to run it with --objects to see if
> a range of objects between two or more commits is fully connected
> or not.  In such a case the caller doesn't care about the actual
> object names or hash hints so formatting this data only for it to
> be dumped to /dev/null by a redirect is a waste of CPU time.  If
> all the caller needs is the exit status then --no-output can be
> used to bypass the commit and object formatting.

We already have --quiet and even --exit-code.

Re: [PATCH 2/3] rev-list: Introduce --no-output to avoid /dev/null redirects

[Junio C Hamano]

"Shawn O. Pearce" <spearce@spearce.org> writes:

> @@ -640,7 +656,9 @@ int cmd_rev_list(int argc, const char **argv, const char *prefix)
>  		}
>  	}
>  
> -	traverse_commit_list(&revs, show_commit, show_object);
> +	traverse_commit_list(&revs,
> +		nooutput ? noshow_commit : show_commit,
> +		nooutput ? noshow_object : show_object);
>  
>  	return 0;
>  }

The function names noshow_xxx() looked a bit funny, but I do not
offhand have better alternatives to offer.

This allows "--bisect-vars --no-output" and "--bisect-all
--bisect-vars --no-output" but makes them behave differently.  I
do not think --no-output is useful with bisection anyway but
maybe it makes sense to forbid the combination?

Re: [PATCH 2/3] rev-list: Introduce --no-output to avoid /dev/null redirects

[Alex Riesen]

Junio C Hamano, Fri, Nov 09, 2007 08:32:01 +0100:
> "Shawn O. Pearce" <spearce@spearce.org> writes:
> 
> > @@ -640,7 +656,9 @@ int cmd_rev_list(int argc, const char **argv, const char *prefix)
> >  		}
> >  	}
> >  
> > -	traverse_commit_list(&revs, show_commit, show_object);
> > +	traverse_commit_list(&revs,
> > +		nooutput ? noshow_commit : show_commit,
> > +		nooutput ? noshow_object : show_object);
> >  
> >  	return 0;
> >  }
> 
> The function names noshow_xxx() looked a bit funny, but I do not
> offhand have better alternatives to offer.

"hide", "skip", "ignore"?

Re: [PATCH 2/3] rev-list: Introduce --no-output to avoid /dev/null redirects

[Junio C Hamano]

Alex Riesen <raa.lkml@gmail.com> writes:

> Junio C Hamano, Fri, Nov 09, 2007 08:32:01 +0100:
>> "Shawn O. Pearce" <spearce@spearce.org> writes:
>> 
>> > @@ -640,7 +656,9 @@ int cmd_rev_list(int argc, const char **argv, const char *prefix)
>> >  		}
>> >  	}
>> >  
>> > -	traverse_commit_list(&revs, show_commit, show_object);
>> > +	traverse_commit_list(&revs,
>> > +		nooutput ? noshow_commit : show_commit,
>> > +		nooutput ? noshow_object : show_object);
>> >  
>> >  	return 0;
>> >  }
>> 
>> The function names noshow_xxx() looked a bit funny, but I do not
>> offhand have better alternatives to offer.
>
> "hide", "skip", "ignore"?

But look at what the functions do.  The original show_xxx() was
to print and then process.  Shawn splitted them into show_xxx()
and noshow_xxx(), leaft the printing part in the former, made
the former call the latter at the end, and moved the processing
to the latter.  So it is not any of the three words.

Re: [PATCH 2/3] rev-list: Introduce --no-output to avoid /dev/null redirects

[Shawn O. Pearce]

Junio C Hamano <gitster@pobox.com> wrote:
> Alex Riesen <raa.lkml@gmail.com> writes:
> > Junio C Hamano, Fri, Nov 09, 2007 08:32:01 +0100:
> >> 
> >> The function names noshow_xxx() looked a bit funny, but I do not
> >> offhand have better alternatives to offer.
> >
> > "hide", "skip", "ignore"?
> 
> But look at what the functions do.  The original show_xxx() was
> to print and then process.  Shawn splitted them into show_xxx()
> and noshow_xxx(), leaft the printing part in the former, made
> the former call the latter at the end, and moved the processing
> to the latter.  So it is not any of the three words.

In my latest patch series I've gone with "finish_xxx()" as it is
finishing our usage of that object.  I'll be resubmitting a new
series soon.

-- 
Shawn.