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
+
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
