Jeff King <peff@xxxxxxxx> 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? -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
