Re: [PATCH] Respect core.autocrlf when preparing temporary files for external diff

[Junio C Hamano <gitster@pobox.com>]

Jeff King <peff@peff.net> writes:

> ...how expensive is the check for convert_to_working_tree? It should
> just be a gitattributes lookup, shouldn't it? Which:
>
>   a. we are doing anyway and caching
>
>   b. which takes a fraction of a second (try "time git ls-files | git
>      check-attr --stdin diff >/dev/null", which should give a
>      worst-case).

Ok.  Although I already queued the removal to 'pu' for tonight's pushout
and it is way too late to revert that, I think I didn't have to remove the
function.  The codepath that lets you cheat by borrowing from the checkout
runs convert_to_git() when it borrows, and if you are seeing a meaningful
optimization even with that overhead, perhaps it would be worth keeping.

There is another check that should be there but is missing from the
current implementation of reuse_worktree_file(), other than the "is
convert_to_working_tree() a no-op for this path?" check.  The last check
in the function would say "Yeah, we can reuse it" if the ce is marked
"assume unchanged"; we do not want to blindly reuse the file from the work
tree in that case.

> Anyway, I was planning to make a patch to always feed textconv the
> _clean_ version of each file. My thinking was:
>
>   1. Then tools get a consistent view of the data across platforms.
>      I.e., my textconv munger or external diff script will work no
>      matter what you think the working tree should look like.
>
>   2. The tool may want the clean version, or it may want the smudged
>      version. Or it may be able to operate on either. If we give it a
>      format it doesn't like, it will have to undo whatever we did.
>
>      For most cases, we start with the clean file (i.e., from a tree or
>      from the index).  If we hand out the clean file and the script
>      doesn't like it, it pays the cost to smudge once. If we hand it the
>      smudged file and the script doesn't like it, we pay the cost to
>      smudge _and_ the script pays the cost to clean.

While the purist in me says #1 above is the right argument to make for
feeding "clean" version, I suspect that the textconv or extdiff tools more
often are not made from scratch and ported across platforms than are
cobbled up together out of tools the script writer finds on his platform.
I suspect that Dscho's "a tempfile should look like a checkout" would be
much friendlier to them in practice for this reason.

> For some reason, with your patch the tempfiles are created with mode
> 0005 for me (whereas they are usually 0505), which makes open() in the
> called script unhappy.  Looking over the patch text, though, I have no
> idea what change could be causing that.

Neither 0005 nor 0505 sounds correct to me; shouldn't they be 0600 or
something like that?