patchEnable 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(-)

Nick Andrew <nick@nick-andrew.net> writes:

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 <gitster@pobox.com>
---
 builtin-rev-list.c |    6 +-----
 1 files changed, 1 insertions(+), 5 deletions(-)

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

And here is how I would do it if I were interested in such a feature.

-- >8 -- rev-list --check-empty

This new option squelches the output entirely and signals if the specified set is empty by its exit status. E.g.

    $ git rev-list --check-empty HEAD..HEAD

will exit with a non-zero status, and

    $ git rev-list --check-empty HEAD^..HEAD

will exit with zero status.

---
 builtin-rev-list.c |   15 ++++++++++++++-
 1 files changed, 14 insertions(+), 1 deletions(-)

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

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:

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

Nick Andrew <nick@nick-andrew.net> writes:

Did you read the whole thread before asking the above question?

IOW, does this answer the above question?

    http://mid.gmane.org/7vy73zd8ok.fsf@gitster.siamese.dyndns.org

On Sun, Jul 20, 2008 at 12:56:28AM -0700, Junio C Hamano wrote:

I took your answer to mean that I shouldn't be using git-rev-list for this purpose, so I asked whether there's a better way to do it. Johannes Schindelin gave a good answer to that.

I'm not happy with that patch due to this:

 static void finish_commit(struct commit *commit)
 {
+       if (check_empty)
+               exit(0);

Exiting a process from within a callback function seems to me to violate the principle of least surprise. If the return code should be zero then the cmd_rev_list function should return zero, and run_command will return zero and handle_internal_command will exit zero. There must be a better way to avoid redundant processing for the empty set case.

Nick.

Nick Andrew <nick@nick-andrew.net> writes:

Huh?  Who is surprised?

I do not know who taught you that "do not exit in a callback" dogma, but I suspect it was misrepresented when it was taught to you.

A library that calls your function back could be structured this way:

	lib() {
        	perform some set-up that affects external world;
                call your callback function;
                clean-up the effect of previous set-up action;
	}

and exiting from your callback function is not a good idea as it prevents the library from doing the necessary clean-up in such a case.

But that is true just in a(n extremely) general case. Your generalization is not particularly useful, methinks, and use of exit(0) in the patch is very well justified (rather, I do not think they even need justifying).

 - The callback you are looking at is not a general purpose callback for
   other program's use, but written for a specific use of rev-list;

 - The purpose of that exit(0) is to signal "there is something" as
   quickly as possible, which was what you wanted out of rev-list;

 - Revision traversal is a read-only operation and we know that there is
   no externally visible set-up done in the function you are calling to
   get your callback called, that needs cleaning up later --- this is not
   expected to change, as there are longstanding existing callback
   functions supplied to traverse_commit_list() that die() upon seeing
   errors already.