From: Junio C Hamano <gitster@pobox.com>
To: "Torsten Bögershausen" <tboegi@web.de>
Cc: git@vger.kernel.org
Subject: Re: [PATCH v8 10/10] ce_compare_data() did not respect conversion
Date: Mon, 02 May 2016 12:33:02 -0700 [thread overview]
Message-ID: <xmqqpot4s1ap.fsf@gitster.mtv.corp.google.com> (raw)
In-Reply-To: <xmqqtwigtjfg.fsf@gitster.mtv.corp.google.com> (Junio C. Hamano's message of "Mon, 02 May 2016 11:16:03 -0700")
Junio C Hamano <gitster@pobox.com> writes:
> Torsten Bögershausen <tboegi@web.de> writes:
>
>> On 29.04.16 23:09, Junio C Hamano wrote:
>>
>>> Well, didn't I do exactly the above much earlier and discarded it
>>> because that breaks the definition of "diff"? Or is this doing
>>> something differently?
>>
>> Yes, and I try to sneak it in anyway ;-)
>>
>> I spend some time debugging how to get t6038 passed, and need
>> some more time.
>>
>> If 10/10 is a no-go (and it probably should be),
>> does it make sense to keep 1/10..4/10 and discard 5..10 for the moment ?
>
> Earlier patches in the series certainly felt alright. I do not
> remember noticing where it went in a strange direction to be honest.
Let's step back a bit and make sure we are on the same page. I
think this "series" conflates a bit too many things into a single
topic.
* The comparison between the index and the working tree, i.e. "git
diff", should compare result of convert_to_git() with what is in
the index, and the world around it should be made consistent with
that. Your separate "git blame" fix to add missing knowledge
that convert_to_git() would not do s/CRLF/LF/ for a path whose
index entry already is contaminated with CR falls into this
category and was a very good thing to do.
* A convert_to_git() and convert_to_working_tree() pair that do not
roundtrip would (by definition) leave contents in the working
tree, that, when passed through convert_to_git(), will be
different from the index, upon completion of "reset --hard". We
_should_ fix it so that "git diff" _reports_ differences.
Currently, lstat(2) based optimization hides this in a racy way
(when racy Git kicks in to reinspect the index and the working
tree file actually matches, it finds out that they do not match),
it is a bug that needs to be fixed, not 10/10 where it tries to
hide the differences consistently and spreads the bug. I haven't
studied 8/10 carefully yet, but it seems to attempt the same.
* I think the "text=auto eol=THIS" that did not mean "I do not care
to specify which ones are text files. Please detect the file
type, and for those automatically detected, please make sure that
the contents follwo THIS eol convention." was a bug, and what
07/10 tries to do is a good thing.
By the way, lack of the cover letter of this series made it more
painful to write a reply like this than necessary. A cover letter
for a trivial 3-patch series might be overkill, but for anything
with substance that spans more than 4-5 patches, a cover letter to
describe the overall direction would really help.
Thanks.
next prev parent reply other threads:[~2016-05-02 19:33 UTC|newest]
Thread overview: 126+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <Message-Id=xmqqio26nqk8.fsf@gitster.mtv.corp.google.com>
2016-02-11 16:16 ` [PATCH 1/3] git reset --hard gives clean working tree tboegi
2016-02-11 18:49 ` Junio C Hamano
2016-03-05 7:23 ` Torsten Bögershausen
2016-03-05 8:05 ` Junio C Hamano
2016-03-05 8:27 ` Torsten Bögershausen
2016-03-05 21:18 ` Junio C Hamano
2016-03-07 8:14 ` Junio C Hamano
2016-03-07 8:51 ` Junio C Hamano
2016-03-07 8:58 ` Torsten Bögershausen
2016-03-07 22:34 ` Junio C Hamano
2016-03-29 13:25 ` [PATCH v1 1/7] Make it possible to get sha1 for a path from the index tboegi
2016-03-29 13:28 ` Duy Nguyen
2016-03-29 13:31 ` Duy Nguyen
2016-03-29 15:05 ` Torsten Bögershausen
2016-03-29 19:32 ` Eric Sunshine
2016-03-29 13:25 ` [PATCH v1 2/7] convert.c: stream and early out tboegi
2016-03-29 13:25 ` [PATCH v1 3/7] Allow core.autocrlf=input and core.eol=crlf tboegi
2016-03-29 13:25 ` [PATCH v1 4/7] t0027: TC for combined attributes tboegi
2016-03-29 13:25 ` [PATCH v1 5/7] CRLF: unify the "auto" handling tboegi
2016-03-29 19:42 ` Eric Sunshine
2016-03-29 13:25 ` [PATCH v1 6/7] correct blame for files commited with CRLF tboegi
2016-03-29 17:21 ` Junio C Hamano
2016-03-29 19:51 ` Torsten Bögershausen
2016-03-29 19:58 ` Junio C Hamano
2016-03-29 20:25 ` Junio C Hamano
2016-03-29 20:32 ` Junio C Hamano
2016-03-29 20:50 ` Junio C Hamano
2016-03-30 17:48 ` Torsten Bögershausen
2016-03-29 13:25 ` [PATCH v1 7/7] convert.c: more safer crlf handling with text attribute tboegi
2016-03-29 18:37 ` Junio C Hamano
2016-04-01 16:08 ` [PATCH v2 1/7] Make it possible to get sha1 for a path from the index tboegi
2016-04-01 16:08 ` [PATCH v2 2/7] convert.c: stream and early out tboegi
2016-04-01 16:08 ` [PATCH v2 3/7] Allow core.autocrlf=input and core.eol=crlf tboegi
2016-04-01 22:20 ` Junio C Hamano
2016-04-01 16:08 ` [PATCH v2 4/7] t0027: TC for combined attributes tboegi
2016-04-01 22:22 ` Junio C Hamano
2016-04-01 16:08 ` [PATCH v2 5/7] CRLF: unify the "auto" handling tboegi
2016-04-01 22:25 ` Junio C Hamano
2016-04-01 16:08 ` [PATCH v2 6/7] correct blame for files commited with CRLF tboegi
2016-04-01 22:29 ` Junio C Hamano
2016-04-03 9:29 ` Torsten Bögershausen
2016-04-01 16:08 ` [PATCH v2 7/7] convert.c: more safer crlf handling with text attribute tboegi
2016-04-05 19:23 ` [PATCH v1] correct blame for files commited with CRLF tboegi
2016-04-05 20:57 ` Junio C Hamano
2016-04-05 21:12 ` Junio C Hamano
2016-04-06 4:17 ` Torsten Bögershausen
2016-04-19 13:24 ` [PATCH v5 1/4] t0027: Make more reliable tboegi
2016-04-19 13:26 ` [PATCH v5 2/4] convert: allow core.autocrlf=input and core.eol=crlf tboegi
2016-04-19 13:26 ` [PATCH v5 3/4] t0027: test cases for combined attributes tboegi
2016-04-19 21:32 ` Junio C Hamano
2016-04-20 15:52 ` Torsten Bögershausen
2016-04-19 13:26 ` [PATCH v5 4/4] convert.c: ident + core.autocrlf didn't work tboegi
2016-04-20 22:27 ` Junio C Hamano
2016-04-22 14:38 ` [PATCH v6 01/10] t0027: Make more reliable tboegi
2016-04-22 22:03 ` Junio C Hamano
2016-04-24 3:45 ` Torsten Bögershausen
2016-04-22 14:53 ` [PATCH v6 02/10] convert: allow core.autocrlf=input and core.eol=crlf tboegi
2016-04-22 14:53 ` [PATCH v6 03/10] t0027: test cases for combined attributes tboegi
2016-04-22 14:53 ` [PATCH v6 04/10] convert.c: ident + core.autocrlf didn't work tboegi
2016-04-22 14:53 ` [PATCH v6 05/10] read-cache: factor out get_sha1_from_index() helper tboegi
2016-04-22 14:53 ` [PATCH v6 06/10] convert.c: stream and early out tboegi
2016-04-22 14:53 ` [PATCH v6 07/10] convert: unify the "auto" handling of CRLF tboegi
2016-04-22 14:53 ` [PATCH v6 08/10] convert.c: more safer crlf handling with text attribute tboegi
2016-04-22 14:53 ` [PATCH v6 09/10] t6038; use crlf on all platforms tboegi
2016-04-22 14:53 ` [PATCH v6 10/10] ce_compare_data() did not respect conversion tboegi
2016-04-24 15:10 ` [PATCH v6b 01/10] t0027: Make commit_chk_wrnNNO() reliable tboegi
2016-04-24 15:11 ` [PATCH v6b 02/10] convert: allow core.autocrlf=input and core.eol=crlf tboegi
2016-04-24 15:11 ` [PATCH v6b 03/10] t0027: test cases for combined attributes tboegi
2016-04-24 15:11 ` [PATCH v6b 04/10] convert.c: ident + core.autocrlf didn't work tboegi
2016-04-24 15:11 ` [PATCH v6b 05/10] read-cache: factor out get_sha1_from_index() helper tboegi
2016-04-24 15:11 ` [PATCH v6b 06/10] convert.c: stream and early out tboegi
2016-04-24 15:11 ` [PATCH v6b 07/10] convert: unify the "auto" handling of CRLF tboegi
2016-04-24 15:11 ` [PATCH v6b 08/10] convert.c: more safer crlf handling with text attribute tboegi
2016-04-24 15:11 ` [PATCH v6b 09/10] t6038; use crlf on all platforms tboegi
2016-04-24 15:11 ` [PATCH v6b 10/10] ce_compare_data() did not respect conversion tboegi
2016-04-25 16:56 ` [PATCH v7 01/10] t0027: Make commit_chk_wrnNNO() reliable tboegi
2016-04-25 19:15 ` Junio C Hamano
2016-04-25 16:56 ` [PATCH v7 02/10] convert: allow core.autocrlf=input and core.eol=crlf tboegi
2016-04-25 16:56 ` [PATCH v7 03/10] t0027: test cases for combined attributes tboegi
2016-04-25 16:56 ` [PATCH v7 04/10] convert.c: ident + core.autocrlf didn't work tboegi
2016-04-25 16:56 ` [PATCH v7 05/10] read-cache: factor out get_sha1_from_index() helper tboegi
2016-04-25 16:56 ` [PATCH v7 06/10] convert.c: stream and early out tboegi
2016-04-25 16:56 ` [PATCH v7 07/10] convert: unify the "auto" handling of CRLF tboegi
2016-04-25 19:37 ` Junio C Hamano
2016-04-26 16:33 ` Torsten Bögershausen
2016-04-26 17:42 ` Junio C Hamano
2016-04-25 16:56 ` [PATCH v7 08/10] convert.c: more safer crlf handling with text attribute tboegi
2016-04-25 16:56 ` [PATCH v7 09/10] t6038; use crlf on all platforms tboegi
2016-04-25 16:56 ` [PATCH v7 10/10] ce_compare_data() did not respect conversion tboegi
2016-04-29 15:01 ` [PATCH v8 01/10] t0027: make commit_chk_wrnNNO() reliable tboegi
2016-04-29 15:01 ` [PATCH v8 02/10] convert: allow core.autocrlf=input and core.eol=crlf tboegi
2016-04-29 15:01 ` [PATCH v8 03/10] t0027: test cases for combined attributes tboegi
2016-04-29 15:01 ` [PATCH v8 04/10] convert.c: ident + core.autocrlf didn't work tboegi
2016-04-29 15:02 ` [PATCH v8 05/10] read-cache: factor out get_sha1_from_index() helper tboegi
2016-04-29 15:02 ` [PATCH v8 06/10] convert.c: stream and early out tboegi
2016-04-29 15:02 ` [PATCH v8 07/10] convert: unify the "auto" handling of CRLF tboegi
2016-11-25 15:48 ` Torsten Bögershausen
2016-11-27 16:22 ` [PATCH/RFC v1 1/1] New way to normalize the line endings tboegi
2016-11-29 19:15 ` Junio C Hamano
2017-04-12 11:48 ` [PATCH v2 1/1] Document how " tboegi
2016-04-29 15:02 ` [PATCH v8 08/10] convert.c: more safer crlf handling with text attribute tboegi
2016-04-29 15:02 ` [PATCH v8 09/10] t6038; use crlf on all platforms tboegi
2016-04-29 15:02 ` [PATCH v8 10/10] ce_compare_data() did not respect conversion tboegi
2016-04-29 18:20 ` Junio C Hamano
2016-04-29 21:09 ` Junio C Hamano
2016-05-01 16:27 ` Torsten Bögershausen
2016-05-02 18:16 ` Junio C Hamano
2016-05-02 19:33 ` Junio C Hamano [this message]
2016-05-03 16:02 ` Torsten Bögershausen
2016-05-03 18:31 ` Junio C Hamano
2016-05-04 4:07 ` Torsten Bögershausen
2016-05-04 7:23 ` Junio C Hamano
2016-05-06 8:54 ` Torsten Bögershausen
2016-05-06 17:11 ` Junio C Hamano
2016-05-07 6:10 ` [PATCH v9 0/6] convert-eol-autocrlf, old 5..10 now 1..6 tboegi
2016-05-07 6:10 ` [PATCH v9 1/6] read-cache: factor out get_sha1_from_index() helper tboegi
2016-05-09 19:54 ` Junio C Hamano
2016-05-07 6:11 ` [PATCH v9 2/6] convert.c: stream and early out tboegi
2016-05-09 20:29 ` Junio C Hamano
2016-05-11 4:30 ` Torsten Bögershausen
2016-05-07 6:11 ` [PATCH v9 3/6] convert: unify the "auto" handling of CRLF tboegi
2016-05-07 6:11 ` [PATCH v9 4/6] convert.c: more safer crlf handling with text attribute tboegi
2016-05-07 6:11 ` [PATCH v9 5/6] t6038; use crlf on all platforms tboegi
2016-05-07 6:11 ` [PATCH v9 6/6] convert: ce_compare_data() checks for a sha1 of a path tboegi
2016-02-11 16:16 ` [PATCH 2/3] Factor out convert_cmp_checkout() into convert.c tboegi
2016-02-11 16:16 ` [PATCH 3/3] convert.c: Optimize convert_cmp_checkout() for changed file len tboegi
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
List information: http://vger.kernel.org/majordomo-info.html
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=xmqqpot4s1ap.fsf@gitster.mtv.corp.google.com \
--to=gitster@pobox.com \
--cc=git@vger.kernel.org \
--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.
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).