[PATCH] Teach git log --check to return an appropriate error code

From: Peter Valdemar Mørch <4ux6as402@sneakemail.com>
Date: Aug 9, 2008, 06:57 UTC
Message-ID: <1218265054-19220-1-git-send-email-4ux6as402@sneakemail.com>
In-Reply-To: <alpine.DEB.1.00.0808081315060.9611@pacific.mpi-cbg.de.mpi-cbg.de>

From: Peter Valdemar Mørch <peter@morch.com>

Signed-off-by: Peter Valdemar Mørch <peter@morch.com>
---

	Ok. I take on the callenge. Thanks for a very helpful writeup!
	The patch in the end is very short. And since it doesn't
	follow your writeup, let me explain my rationale:
	
	Whether or not a check fails is stored in the
	DIFF_OPT_CHECK_FAILED field of flags in struct diff_options.
	This flag-field is only set (diff.c:1644), never cleared.
	Since the same diff_options is used throughout, it is enough
	to check that field at the end - it already does the
	accumulation because it never gets cleared.
	
	diff_result_code: The second argument to it is never used
	since (opt->output_format & DIFF_FORMAT_CHECKDIFF), so the
	value doesn't matter (0 would have been fine as you suggest).
	The return value is a bitfield, with |= 1 if HAS_CHANGES
	(clearly log has changes "always" - except e.g. "git log
	HEAD..HEAD") and |= 2 if CHECK_FAILED.
	
	Therefore I was left with either:
	
	* Return the value of diff_result_code ("always" |=1,
	sometimes |=2 if a check failed. This would put the burden
	on the caller to check different values of $?.
	
	* Return value of (diff_result_code & 02). Then I would
	suggest adding the constant 02 to a header file.
	
	* Pick out the logic from diff_result_code with respect to
	CHECK_FAILED. I chose this path. (I return 02 here too, and
	perhaps that *should* go in a header file. I decided not
	to.)

 builtin-log.c |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/builtin-log.c b/builtin-log.c
index f4975cf..45ce8ea 100644
--- a/builtin-log.c
+++ b/builtin-log.c
@@ -227,6 +227,10 @@ static int cmd_log_walk(struct rev_info *rev)
 		free_commit_list(commit->parents);
 		commit->parents = NULL;
 	}
+	if (rev->diffopt.output_format & DIFF_FORMAT_CHECKDIFF &&
+	    DIFF_OPT_TST(&rev->diffopt, CHECK_FAILED)) {
+		return 02;
+	}
 	return 0;
 }

-- 
1.6.0.rc2.6.gcd432.dirty