Re: [PATCH v2] sha1_file: pass empty buffer to index empty file

From: Junio C Hamano <gitster@pobox.com>
Date: May 15, 2015, 18:01 UTC
Message-ID: <xmqqlhgphg8x.fsf@gitster.dls.corp.google.com>
In-Reply-To: <1431645434-11790-1-git-send-email-gjthill@gmail.com>

Jim Hill <gjthill@gmail.com> writes:

>> check that 'err' does not contain the copy-fd error
>
> Implemented this out of necessity, because the add works and returns
> success despite the complaints to stderr.

That would mean that you found _another_ bug, wouldn't it? If copy-fd failed to read input to feed the external filter with, it must have returned an error to its caller, and somebody in the callchain is not paying attention to that error and pretending as if everything went well. That's a separate issue, though.

In any case, I think the following patch may make the test better (apply on top of yours).

 * A failure to run the filter with the right contents can be caught
   by examining the outcome.  I tweaked the filter to prepend an
   extra header line to the contents; if copy-fd failed to drive the
   filter, we wouldn't see the cleaned output to match that extra
   header line (and nothing else---as the contents we are feeding is
   an empty blob).

 * There is no need to create an extra commit; an uncommitted
   .gitattributes from the working tree would work just fine.

 * The "grep" is gone, with use of -i (questionable why it is
   needed), -q (generally, we do not squelch error output in
   individual tests, which is unnecessary and running tests with -v
   option less useful) and -s (same, withquestionable portability).

Thanks.

diff --git a/t/t0021-conversion.sh b/t/t0021-conversion.sh
index 5986bb0..a72d265 100755
--- a/t/t0021-conversion.sh
+++ b/t/t0021-conversion.sh
@@ -216,15 +216,17 @@ test_expect_success EXPENSIVE 'filter large file' '
 	! test -s err
 '
 
-test_expect_success "filtering empty file should not produce complaints" '
-	echo "emptyfile filter=cat" >>.gitattributes &&
-	git config filter.cat.clean cat &&
-	git config filter.cat.smudge cat &&
-	git add . &&
-	git commit -m "cat filter for emptyfile" &&
-	> emptyfile &&
-	git add emptyfile 2>err &&
-	! grep -Fiqs "bad file descriptor" err
+test_expect_success "filtering empty file should work correctly" '
+	write_script filter-clean.sh <<-EOF &&
+	echo "Extra Head" && cat
+	EOF
+	echo "emptyfile filter=check" >>.gitattributes &&
+	git config filter.check.clean "sh ./filter-clean.sh" &&
+	>emptyfile &&
+	git add emptyfile &&
+	echo "Extra Head" >expect &&
+	git cat-file blob :emptyfile >actual &&
+	test_cmp expect actual
 '
 
 test_done