git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Jeff King <peff@peff.net>
Cc: Yasushi SHOJI <yashi@atmark-techno.com>, git@vger.kernel.org
Subject: Re* diff: --quiet does not imply --exit-code if --diff-filter is present
Date: Tue, 31 May 2011 10:06:44 -0700	[thread overview]
Message-ID: <7vd3iykdej.fsf_-_@alter.siamese.dyndns.org> (raw)
In-Reply-To: <20110531162546.GA11321@sigill.intra.peff.net> (Jeff King's message of "Tue, 31 May 2011 12:25:46 -0400")

Jeff King <peff@peff.net> writes:

>> Thanks; a natural question is if we need the same for diff-index, then.
>
> No, it calls straight into unpack_trees. So I think the question is
> "should unpack_trees respect the QUICK optimization". I suspect it
> didn't happen simply because unpack_trees is so complex, and there are
> probably corner cases with merging.

Yeah, a somewhat hacky version would look like this.

-- >8 --
diff-index --quiet: learn the "stop feeding the backend early" logic

A negative return from the unpack callback function usually means unpack
failed for the entry and signals the unpack_trees() machinery to fail the
entire merge operation, immediately and there is no other way for the
callback to tell the machinery to exit early without reporting an error.

This is what we usually want to make a merge all-or-nothing operation, but
the machinery is also used for diff-index codepath by using a custom
unpack callback function. And we do sometimes want to exit early without
failing, namely when we are under --quiet and can short-cut the diff upon
finding the first difference.

Add "exiting_early" field to unpack_trees_options structure, to signal the
unpack_trees() machinery that the negative return value is not signaling
an error but an early return from the unpack_trees() machinery. As this by
definition hasn't unpacked everything, discard the resulting index just
like the failure codepath.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 diff-lib.c     |    7 ++++++-
 unpack-trees.c |    4 +++-
 unpack-trees.h |    1 +
 3 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/diff-lib.c b/diff-lib.c
index 9c29293..2e09500 100644
--- a/diff-lib.c
+++ b/diff-lib.c
@@ -433,8 +433,13 @@ static int oneway_diff(struct cache_entry **src, struct unpack_trees_options *o)
 	if (tree == o->df_conflict_entry)
 		tree = NULL;
 
-	if (ce_path_match(idx ? idx : tree, &revs->prune_data))
+	if (ce_path_match(idx ? idx : tree, &revs->prune_data)) {
 		do_oneway_diff(o, idx, tree);
+		if (diff_can_quit_early(&revs->diffopt)) {
+			o->exiting_early = 1;
+			return -1;
+		}
+	}
 
 	return 0;
 }
diff --git a/unpack-trees.c b/unpack-trees.c
index 07f8364..3a61d82 100644
--- a/unpack-trees.c
+++ b/unpack-trees.c
@@ -593,7 +593,7 @@ static int unpack_nondirectories(int n, unsigned long mask,
 static int unpack_failed(struct unpack_trees_options *o, const char *message)
 {
 	discard_index(&o->result);
-	if (!o->gently) {
+	if (!o->gently && !o->exiting_early) {
 		if (message)
 			return error("%s", message);
 		return -1;
@@ -1133,6 +1133,8 @@ return_failed:
 		display_error_msgs(o);
 	mark_all_ce_unused(o->src_index);
 	ret = unpack_failed(o, NULL);
+	if (o->exiting_early)
+		ret = 0;
 	goto done;
 }
 
diff --git a/unpack-trees.h b/unpack-trees.h
index 64f02cb..4b07683 100644
--- a/unpack-trees.h
+++ b/unpack-trees.h
@@ -47,6 +47,7 @@ struct unpack_trees_options {
 		     skip_sparse_checkout,
 		     gently,
 		     show_all_errors,
+		     exiting_early,
 		     dry_run;
 	const char *prefix;
 	int cache_bottom;

  reply	other threads:[~2011-05-31 17:07 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-05-31 10:34 diff: --quiet does not imply --exit-code if --diff-filter is present Yasushi SHOJI
2011-05-31 15:33 ` Jeff King
2011-05-31 15:46   ` Junio C Hamano
2011-05-31 16:25     ` Jeff King
2011-05-31 17:06       ` Junio C Hamano [this message]
2011-05-31 17:14         ` Re* " Jeff King
2011-05-31 17:36           ` Junio C Hamano
2011-06-01  9:41   ` Yasushi SHOJI

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=7vd3iykdej.fsf_-_@alter.siamese.dyndns.org \
    --to=gitster@pobox.com \
    --cc=git@vger.kernel.org \
    --cc=peff@peff.net \
    --cc=yashi@atmark-techno.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).