From: Jim Hill <gjthill@gmail•com>
To: Junio C Hamano <gitster@pobox•com>
Cc: git@vger•kernel.org
Subject: Re: [PATCH v2] sha1_file: pass empty buffer to index empty file
Date: Fri, 15 May 2015 16:31:53 -0700 [thread overview]
Message-ID: <20150515233153.GA4157@gadabout.domain.actdsltmp> (raw)
In-Reply-To: <xmqqlhgphg8x.fsf@gitster.dls.corp.google.com>
On Fri, May 15, 2015 at 11:01:34AM -0700, Junio C Hamano wrote:
> 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.
as you say, separate ... I think I stumbled over more than one:
setup:
~/sandbox/40$ git grl
core.autocrlf false
core.whitespace cr-at-eof
core.repositoryformatversion 0
core.filemode true
core.bare false
core.logallrefupdates true
filter.cat.smudge cat
filter.cat.clean echo Kilroy was here && cat
filter.cat.required true
~/sandbox/40$ git rm --cached -f --ignore-unmatch emptyfile
rm 'emptyfile'
with required filter:
~/sandbox/40$ cat emptyfile
~/sandbox/40$ git add emptyfile
~/sandbox/40$ git show :emptyfile
Kilroy was here
~/sandbox/40$ git config --unset filter.cat.required
then with not-required filter:
~/sandbox/40$ git rm --cached -f --ignore-unmatch emptyfile
error: copy-fd: read returned Bad file descriptor
error: cannot feed the input to external filter echo Kilroy was here && cat
error: external filter echo Kilroy was here && cat failed
rm 'emptyfile'
~/sandbox/40$ git show :emptyfile
fatal: Path 'emptyfile' exists on disk, but not in the index.
~/sandbox/40$ git add emptyfile
error: copy-fd: read returned Bad file descriptor
error: cannot feed the input to external filter echo Kilroy was here && cat
error: external filter echo Kilroy was here && cat failed
~/sandbox/40$ git show :emptyfile
~/sandbox/40$ git rm --cached emptyfile
rm 'emptyfile'
~/sandbox/40$ git add emptyfile
error: copy-fd: read returned Bad file descriptor
error: cannot feed the input to external filter echo Kilroy was here && cat
error: external filter echo Kilroy was here && cat failed
~/sandbox/40$ git rm --cached -f --ignore-unmatch emptyfile
rm 'emptyfile'
~/sandbox/40$
===
I don't understand rm's choices of when to run the filter, and the
apparently entirely separate code path for required filters is just
bothersome.
> * A failure to run the filter with the right contents can be caught
> by examining the outcome.
agreed. That's better anyway -- my few git greps didn't find any
empty-file filter tests anyway.
> * There is no need to create an extra commit; an uncommitted
> .gitattributes from the working tree would work just fine.
Done.
> * The "grep" is gone, with use of -i (questionable why it is
> needed),
Yah, I was bad-thinking strerror results might be a bit unpredictable, I
should have checked for a string under git's control instead. I'd just
assumed the 0 return was because non-required filters are allowed to
fail, got the above transcript while checking the assumption.
===
So, so long as we're testing empty-file filters, I figured I'd add real
empty-file filter tests, I think that covers it.
So is this better instead?
diff --git a/t/t0021-conversion.sh b/t/t0021-conversion.sh
index 5986bb0..fc2c644 100755
--- a/t/t0021-conversion.sh
+++ b/t/t0021-conversion.sh
@@ -216,15 +216,33 @@ 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 "filter: clean empty file" '
+ header=---in-repo-header--- &&
+ git config filter.in-repo-header.clean "echo $header && cat" &&
+ git config filter.in-repo-header.smudge "sed 1d" &&
+
+ echo "empty-in-worktree filter=in-repo-header" >>.gitattributes &&
+ > empty-in-worktree &&
+
+ echo $header > expected &&
+ git add empty-in-worktree &&
+ git show :empty-in-worktree > actual &&
+ test_cmp expected actual
+'
+
+test_expect_success "filter: smudge empty file" '
+ git config filter.empty-in-repo.smudge "echo smudge added line && cat" &&
+ git config filter.empty-in-repo.clean true &&
+
+ echo "empty-in-repo filter=empty-in-repo" >>.gitattributes &&
+
+ echo dead data walking > empty-in-repo &&
+ git add empty-in-repo &&
+
+ : > expected &&
+ git show :empty-in-repo > actual &&
+ test_cmp expected actual
'
test_done
+
next prev parent reply other threads:[~2015-05-15 23:32 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
2015-05-15 23:31 ` Jim Hill [this message]
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=20150515233153.GA4157@gadabout.domain.actdsltmp \
--to=gjthill@gmail$(echo .)com \
--cc=git@vger$(echo .)kernel.org \
--cc=gitster@pobox$(echo .)com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
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