[PATCH] Enable git rev-list to parse --quiet

Enable git rev-list to parse --quiet

git rev-list never sees the --quiet option because --quiet is
also an option for diff-files.

Example:

$ ./git rev-list --quiet ^HEAD~2 HEAD
1e102bf7c83281944ffd9202a7d35c514e4a5644
3bf0dd1f4e75ee1591169b687ce04dff00ae2e3e
$ echo $?
0

The fix scans the argument list to detect --quiet before passing it
to setup_revisions(). It also arranges to count the number of commits
or objects (whether sent to STDOUT or not) so --quiet can return an
appropriate exit code (1 if there were commits/objects, 0 otherwise).

After fix:

$ ./git rev-list --quiet ^HEAD~2 HEAD
$ echo $?
1
---

 builtin-rev-list.c |   28 ++++++++++++++++++++++++----
 1 files changed, 24 insertions(+), 4 deletions(-)

diff --git a/builtin-rev-list.c b/builtin-rev-list.c
index 8e1720c..e2e5e13 100644
--- a/builtin-rev-list.c
+++ b/builtin-rev-list.c

@@ -52,6 +52,11 @@ static const char rev_list_usage[] =
 
 static struct rev_info revs;
 
+/* Count of number of commits or objects noticed (even if not output).
+ * Used by --quiet option to set an appropriate exit status.
+ */
+static int seen_count;
+
 static int bisect_list;
 static int show_timestamp;
 static int hdr_termination;

@@ -167,12 +172,14 @@ static void finish_commit(struct commit *commit)
 	}
 	free(commit->buffer);
 	commit->buffer = NULL;
+	seen_count++;
 }
 
 static void finish_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));
+	seen_count++;
 }
 
 static void show_object(struct object_array_entry *p)

@@ -588,6 +595,17 @@ int cmd_rev_list(int argc, const char **argv, const char *prefix)
 	init_revisions(&revs, prefix);
 	revs.abbrev = 0;
 	revs.commit_format = CMIT_FMT_UNSPECIFIED;
+
+	/* Parse options which are also recognised by git-diff-files */
+	for (i = 1 ; i < argc; i++) {
+		const char *arg = argv[i];
+
+		if (!strcmp(arg, "--quiet")) {
+			quiet = 1;
+			continue;
+		}
+	}
+
 	argc = setup_revisions(argc, argv, &revs, NULL);
 
 	for (i = 1 ; i < argc; i++) {

@@ -621,10 +639,6 @@ int cmd_rev_list(int argc, const char **argv, const char *prefix)
 			read_revisions_from_stdin(&revs);
 			continue;
 		}
-		if (!strcmp(arg, "--quiet")) {
-			quiet = 1;
-			continue;
-		}
 		usage(rev_list_usage);
 
 	}

@@ -700,9 +714,15 @@ int cmd_rev_list(int argc, const char **argv, const char *prefix)
 		}
 	}
 
+	seen_count = 0;
+
 	traverse_commit_list(&revs,
 		quiet ? finish_commit : show_commit,
 		quiet ? finish_object : show_object);
 
+	if (quiet) {
+		return seen_count ? 1 : 0;
+	}
+
 	return 0;
 }

Nick Andrew [off-list ref] writes:

Enable git rev-list to parse --quiet

git rev-list never sees the --quiet option because --quiet is
also an option for diff-files.

Example:

$ ./git rev-list --quiet ^HEAD~2 HEAD
1e102bf7c83281944ffd9202a7d35c514e4a5644
3bf0dd1f4e75ee1591169b687ce04dff00ae2e3e
$ echo $?
0

The fix scans the argument list to detect --quiet before passing it
to setup_revisions(). It also arranges to count the number of commits
or objects (whether sent to STDOUT or not) so --quiet can return an
appropriate exit code (1 if there were commits/objects, 0 otherwise).

After fix:

Thanks for noticing, but this replaces one breakage with another.

Your new behaviour is a new "tell me if it is an empty set" option, and it
means quite different thing from what --quiet does.

The --quiet option is designed primarily for sanity checking after a
failed fetch by commit walkers.  Here is how it works (well, at least how
it is supposed to work).

Imagine you have this history:

	---o---o---X

and the other side has this history:

	---o---o---X---A---B---C

And you run fetch over a dumb protocol; the commit walker fetches C,
discovers you do not have its parent B and tries to fetch it, and you
somehow kill that process.  Your repository will have:

	---o---o---X           C

Now, we do not mark C with our refs, so we do not say "Ok we have
everything leading up to C" when you re-run the same commit walker.
Instead, we'll let the walker walk again starting from C.  So we will
never in corrupt state.

But you might want to see if your repository has this kind of failure.
For that, you can run rev-list starting from C and X --- it will fail
after it finds out that C's parent is B and tries to read it.  And you
will learn the failure with the exit code from the command.  --quiet was
about squelching the output of "I've seen C", "I've seen X", as the only
thing you care about in that mode of usage is if the history is well
connected which is reported by the exit code.

-- >8 --
Subject: [PATCH] rev-list: honor --quiet option

Nick Andrew noticed that rev-list lets the --quiet option to be parsed by
underlying diff_options parser but did not pick up the result.  This
resulted in --quiet option to become effectively a no-op.

Signed-off-by: Junio C Hamano <redacted>
---
 builtin-rev-list.c |    6 +-----
 1 files changed, 1 insertions(+), 5 deletions(-)

diff --git a/builtin-rev-list.c b/builtin-rev-list.c
index 8e1720c..507201e 100644
--- a/builtin-rev-list.c
+++ b/builtin-rev-list.c

@@ -589,7 +589,7 @@ int cmd_rev_list(int argc, const char **argv, const char *prefix)
 	revs.abbrev = 0;
 	revs.commit_format = CMIT_FMT_UNSPECIFIED;
 	argc = setup_revisions(argc, argv, &revs, NULL);
-
+	quiet = DIFF_OPT_TST(&revs.diffopt, QUIET);
 	for (i = 1 ; i < argc; i++) {
 		const char *arg = argv[i];
 

@@ -621,10 +621,6 @@ int cmd_rev_list(int argc, const char **argv, const char *prefix)
 			read_revisions_from_stdin(&revs);
 			continue;
 		}
-		if (!strcmp(arg, "--quiet")) {
-			quiet = 1;
-			continue;
-		}
 		usage(rev_list_usage);
 
 	}

-- 
1.5.6.3.573.gd2d2

On Thu, Jul 17, 2008 at 10:42:21PM -0700, Junio C Hamano wrote:

Thanks for noticing, but this replaces one breakage with another.

Your new behaviour is a new "tell me if it is an empty set" option, and it
means quite different thing from what --quiet does.

Fair enough. Yes, I want to find out if it is an empty set. The
manpage does say "fully connected" which I interpreted to mean
that one set of commits is a subset of the other..

I want to automatically (e.g. in crontab) update a git repo to the latest
HEAD from a remote branch ... but with the possibility that the local
repo has local changes, and I want no chance of merge failure. In other
words, "git fetch remote; git merge origin/master" and only do the
merge if it's a fast-forward. If there are any local commits, or local
uncommitted changes, then leave the local working tree alone.

So my idea was to use "git rev-list --quiet master ^origin/master"
and check the exit code; if zero do "git merge origin/master". Without
a working "--quiet" nor exit code I can pipe the output to "wc -l"
but is there a more efficient/reliable way to implement the requirement?

Nick.

Hi,

On Fri, 18 Jul 2008, Nick Andrew wrote:

I want to automatically (e.g. in crontab) update a git repo to the 
latest HEAD from a remote branch ... but with the possibility that the 
local repo has local changes, and I want no chance of merge failure. In 
other words, "git fetch remote; git merge origin/master" and only do the 
merge if it's a fast-forward. If there are any local commits, or local 
uncommitted changes, then leave the local working tree alone.

So my idea was to use "git rev-list --quiet master ^origin/master" and 
check the exit code; if zero do "git merge origin/master". Without a 
working "--quiet" nor exit code I can pipe the output to "wc -l" but is 
there a more efficient/reliable way to implement the requirement?

Yes. Check if "$(git rev-parse master)" is different from "$(git rev-parse 
origin/master)" (to avoid unnecessary merging), and then that "$(git 
merge-base master origin/master)" is equal to "$(git rev-parse master)".

Note: this is plumbing, meant for scripting (which is exactly your 
scenario).  Do not teach this to new Git users.

Ciao,
Dscho