show/diff --check clarification needed
From: Yann Dirson <hidden>
Date: 2016-06-15 22:48:06
I am experiencing an unexpected behaviour, related to whitespace
(non-)checking.
In a my local repository I have a pre-commit hook checking for "git
diff-index --check --cached" (borrowed from the sample template), and
the server I push to does a "git diff-tree --check" on each pushed
commit to make sure someone who forgot the pre-commit hook does not
push a whitespace-unclean commit by error.
Now I had to import some 3rd-party stuff (an eclipse deltapack if you
wonder) which includes CRLF line-endings, but must not get modified
because of checksums. So I added the following .gitattributes to the
tree where I import this stuff:
$ cat deltapack/.gitattributes
* binary
.gitattributes -binary
That is just fine for the pre-commit hook, and "git show --check" on
it in my local repository reports no problems.
Now when I attempt to push, the update hook refuses my commit; and if
I run the same "git show --check" on the server (in the bare repo
where the ref has not been updated by the push, but where the pushed
commit has not been pruned yet), I get the complaints which I have
intended the .gitattributes to silent.
What happens is apparently that --check does not know which repository
revision to take .gitattributes from, and takes them from working
tree. Indeed, removing deltapack/.gitattributes from my worktree and
rerunning "show --check" on that unmodified commit does trigger the errors.
While having "diff-tree --check" defaulting to looking at working tree
by default may make sense, it would seem more natural for "show" to
use by default those of the commit we're looking at - and one could
argue that "diff-tree" could use the positive treeish for that matter
as well. But defaulting is one matter, it would be really useful to
be able to specify from where the attributes should be taken for any
given operation. Did not look at the code - anyone knows how easy
that would be ?
Unless I missed something, I suggest the following plan:
- document in maint that --check only takes worktree into account when
looking for .gitattributes, and more globally add a note to the
gitattributes manpage to explicitely say that too.
- add a global flag to allow something like
"git --attributes-tree=<treeish> <command>"
- adjust defaults to agreed-upon values
- add any config entries that would be meaningful
It may just be me having badly written the update hook - here is the
relevant part for reference. Under the "refs/heads/*,commit)" case of
the sample update hook, I have something like:
# checks to be done in all commits
# exclude all commits previously-reachable by any ref
exclude_oldrevs=$( git for-each-ref --format='^%(refname)' )
for rev in $(git rev-list $exclude_oldrevs $newrev); do
[...]
# special case for root commits
if git rev-parse HEAD^ >/dev/null 2>&1; then
ref=""
else
# use an empty tree to compare against
ref=4b825dc642cb6eb9a060e54bf8d69288fbee4904
fi
# checks for whitespace
if ! git diff-tree --check $ref $rev >/dev/null; then
echo "*** Whitespace problems in $rev" >&2
fails_whitespace=1
fi
[...]
done