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 majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html