From: Junio C Hamano <gitster@pobox.com>
To: "Torsten Bögershausen" <tboegi@web.de>
Cc: Mike Crowe <mac@mcrowe.com>,
git@vger.kernel.org, Jeff King <peff@peff.net>
Subject: Re: [PATCH v1 1/1] git diff --quiet exits with 1 on clean tree with CRLF conversions
Date: Sat, 04 Mar 2017 11:59:10 -0800 [thread overview]
Message-ID: <xmqq4lz87r01.fsf@gitster.mtv.corp.google.com> (raw)
In-Reply-To: <9c9eeb35-e1c1-ec46-1d85-ef6a05886880@web.de> ("Torsten =?utf-8?Q?B=C3=B6gershausen=22's?= message of "Sat, 4 Mar 2017 07:25:52 +0100")
Torsten Bögershausen <tboegi@web.de> writes:
>>> enum safe_crlf crlf_warn = (safe_crlf == SAFE_CRLF_FAIL
>>> ? SAFE_CRLF_WARN
>>> : safe_crlf);
>>> + if (size_only)
>>> + crlf_warn = SAFE_CRLF_FALSE;
>>
>> If you were to go this route, it may be sufficient to change its
>> initialization from WARN to FALSE _unconditionally_, because this
>> function uses the convert_to_git() only to _show_ the differences by
>> computing canonical form out of working tree contents, and the
>> conversion is not done to _write_ into object database to create a
>> new object.
>
> Hm, since when (is it not used) ?
Since forever, but my statement above said "this function", which may
have confused you, where it could have said diff_populate_filespec().
Surely it is possible for somebody to diff_populate_filespec(s, 0)
and then call hash_sha1_file(s->data, s->size, "blob", ...) to write
the data into the object database to create a new object. But that
sounds really crazy, no?
> The SAFE_CRLF_FAIL was converted into WARN here:
> commit 5430bb283b478991a979437a79e10dcbb6f20e28
> Author: Junio C Hamano <gitster@pobox.com>
> Date: Mon Jun 24 14:35:04 2013 -0700
>
> diff: demote core.safecrlf=true to core.safecrlf=warn
Yes.
> Does this all means that, looking back, 5430bb283b478991 could have been more
> aggressive and could have used SAFE_CRLF_FALSE ?
That is pretty much the statement, to which you said "since when",
suspects.
> And we can do this change now?
I am not sure. The conversion the safe-crlf code does is unsafe and
it is a disservice to users not to warn whenever we notice they are
risking information loss. Maybe time they run "git diff" is not a
good time to warn, as they may not be actually adding the file
as-is, but if warning against information loss at "git diff" time is
important enough, the I think that should not be squelched by the
"--quiet" option, which is about "do not show the patch text
output". It should not be taken as "do not diagnose any errors".
next prev parent reply other threads:[~2017-03-04 19:59 UTC|newest]
Thread overview: 29+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-02-17 21:26 git diff --quiet exits with 1 on clean tree with CRLF conversions Mike Crowe
2017-02-17 22:05 ` Junio C Hamano
2017-02-17 22:19 ` Mike Crowe
2017-02-20 15:33 ` Mike Crowe
2017-02-20 21:25 ` Junio C Hamano
2017-02-25 15:32 ` Mike Crowe
2017-02-27 20:17 ` Junio C Hamano
2017-02-28 18:06 ` Torsten Bögershausen
2017-02-28 21:50 ` Junio C Hamano
2017-03-01 17:04 ` [PATCH v1 1/1] " tboegi
2017-03-01 21:14 ` Junio C Hamano
2017-03-01 21:54 ` Junio C Hamano
2017-03-02 8:53 ` Jeff King
2017-03-02 17:52 ` Junio C Hamano
2017-03-02 19:12 ` Jeff King
2017-03-02 18:51 ` [PATCH v2] diff: do not short-cut CHECK_SIZE_ONLY check in diff_populate_filespec() Junio C Hamano
2017-03-02 14:20 ` [PATCH v1 1/1] git diff --quiet exits with 1 on clean tree with CRLF conversions Mike Crowe
2017-03-02 18:20 ` Torsten Bögershausen
2017-03-02 18:33 ` Junio C Hamano
2017-03-02 20:03 ` Mike Crowe
2017-03-03 17:02 ` Torsten Bögershausen
2017-03-03 17:47 ` Junio C Hamano
2017-03-04 6:25 ` Torsten Bögershausen
2017-03-04 19:59 ` Junio C Hamano [this message]
2017-03-01 21:25 ` Mike Crowe
2017-03-01 23:29 ` Junio C Hamano
2017-03-02 18:17 ` Torsten Bögershausen
2017-03-03 17:01 ` Mike Crowe
2017-03-02 15:38 ` git status reports file modified when only line-endings have changed (was git diff --quiet exits with 1 on clean tree with CRLF conversions) Mike Crowe
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=xmqq4lz87r01.fsf@gitster.mtv.corp.google.com \
--to=gitster@pobox.com \
--cc=git@vger.kernel.org \
--cc=mac@mcrowe.com \
--cc=peff@peff.net \
--cc=tboegi@web.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.