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

[Jeff King <peff@peff.net>]

On Sun, Mar 22, 2009 at 12:18:33AM -0700, Junio C Hamano wrote:

> 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.

I certainly haven't done exhaustive tests. Obviously the one I did was a
bit contrived. I just think it makes sense to have numbers rather than
saying "this probably doesn't do anything anymore".

> 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.

I think you and I have about the same feeling on this, then. As somebody
who does not actually use smudge/clean filters at all, I am willing to
defer to Dscho's opinion, which is based on practical experience.

> > 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?

Argh, sorry, of course I should have realized something was wrong with
my baseline when I saw 0505. I was building on a half-finished WIP
related to textconv, which obviously is broken. Sorry for the noise.

-Peff