From: Junio C Hamano <gitster@pobox.com>
To: Jim Hill <gjthill@gmail.com>
Cc: git@vger.kernel.org
Subject: Re: [PATCH v2] sha1_file: pass empty buffer to index empty file
Date: Fri, 15 May 2015 11:01:34 -0700 [thread overview]
Message-ID: <xmqqlhgphg8x.fsf@gitster.dls.corp.google.com> (raw)
In-Reply-To: <1431645434-11790-1-git-send-email-gjthill@gmail.com> (Jim Hill's message of "Thu, 14 May 2015 16:17:14 -0700")
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
next prev parent reply other threads:[~2015-05-15 18:01 UTC|newest]
Thread overview: 23+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-05-14 17:23 [PATCH] sha1_file: pass empty buffer to index empty file Jim Hill
2015-05-14 18:43 ` Junio C Hamano
2015-05-14 23:17 ` [PATCH v2] " Jim Hill
2015-05-15 18:01 ` Junio C Hamano [this message]
2015-05-15 23:31 ` Jim Hill
2015-05-16 18:48 ` Junio C Hamano
2015-05-16 20:06 ` [PATCH v3] " Jim Hill
2015-05-16 23:22 ` Junio C Hamano
2015-05-17 17:37 ` Junio C Hamano
2015-05-17 19:10 ` Junio C Hamano
2015-05-18 0:41 ` [PATCH v4] " Jim Hill
2015-05-19 6:37 ` [PATCH v3] " Jeff King
2015-05-19 18:11 ` Junio C Hamano
2015-05-19 18:17 ` Junio C Hamano
2015-05-19 18:35 ` Junio C Hamano
2015-05-19 19:48 ` Junio C Hamano
2015-05-19 22:14 ` Jeff King
2015-05-20 17:03 ` Junio C Hamano
2015-05-19 19:40 ` Eric Sunshine
2015-05-19 22:09 ` Jeff King
2015-05-20 17:25 ` Junio C Hamano
2015-05-20 17:38 ` Jeff King
2015-05-14 23:26 ` [PATCH] " Jim Hill
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=xmqqlhgphg8x.fsf@gitster.dls.corp.google.com \
--to=gitster@pobox.com \
--cc=git@vger.kernel.org \
--cc=gjthill@gmail.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.