Re: [PATCH] safecrlf: Add flag to convert_to_git() to disable safecrlf check

From: Dmitry Potapov <dpotapov@gmail.com>
Date: Jan 15, 2008, 10:26 UTC
Message-ID: <20080115102626.GE2963@dpotapov.dyndns.org>
In-Reply-To: <12003528401309-git-send-email-prohaska@zib.de>

On Tue, Jan 15, 2008 at 12:20:40AM +0100, Steffen Prohaska wrote:

Show 5 quoted lines

> 
> I looked briefly at the various places where convert_to_git() is
> called.  I think that only the one code path through index_fd()
> actually writes data to the repsitory.  Maybe someone else with
> a better understanding of git's internals should confirm this.

Your patch is certainly better than my quick hack for git-add, and perhaps you are right that the check should be done only when data are written, but it also means that there is no longer any warning when you are running git diff with the work tree, which would be useful, because it is what most users do before adding anything.

However, my real concern is that it seems we have two different heuristics for binary -- one that is used inside of convert.c and the other one is buffer_is_binary() in xdiff-interface.c.

So, I am running 'git diff' for some test file, and it says:

diff --git a/foo b/foo
index e965047..c102bdc 100644
Binary files a/foo and b/foo differ

okay, now I want to add this *binary* file, so I run 'git add':

warning: Stripped CRLF from foo.

I imagine a user saying: "What the hell! Why did this stupid Git
strip CRLF from my _binary_ file?"

And the current version of Git, which does not print CRLF warning,
seems to be dangerous, because when 'git diff' told me that it is
a _binary_ file, I expect that Git will put it as *binary*. So,
from the user's point of view, it looks like a bug.

So, I suppose that at least we should make is_binary heuristic in
convert.c more strict than those that is used by diff. Namely, if
there is at least one NUL byte in the buffer, it should be treated
as binary.


Dmitry