Re: An interaction with ce_match_stat_basic() and autocrlf

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

Linus Torvalds <torvalds@linux-foundation.org> writes:

> This patch should fix it, but I suspect we should think hard about that 
> change to ie_modified(), and see what the performance issues are (ie that 
> code has tried to avoid doing the more expensive ce_modified_check_fs() 
> for a reason).
>
> The change to diff.c is similarly interesting. It is logically wrong to 
> use the worktree_file there (since we have to read the object anyway), but 
> since "reuse_worktree_file" is also tied into the whole refresh logic, I 
> think the diff.c change is correct.
>
> I dunno. This is not meant to be applied, it is meant to be thought about.

There are a few cases around the changing value of autocrlf (and
filter attributes --- anything that affects convert_to_git() and
convert_to_working_tree()).

 * The cached stat information matches the work tree, but user
   changed convert_to_working_tree().  "git diff" reports
   nothing.  The user needs to remove the work tree file and
   check it out again.

 * The cached stat information matches the work tree, but user
   changed convert_to_git().  Again, diff reports nothing.  The
   user needs to "git add" to cause rehashing.

 * The cached stat information does not match.  What the working
   tree file stores hasn't changed, but convert_to_git() was
   changed.

   The fact that the working tree "file" contents did not change
   does not have much significance in this case.  What defines
   the "contents" as far as git is concerned is the combination
   of the working tree file contents _and_ what convert_to_git()
   does to it.

   Depending on the nature of the change to convert_to_git(),
   "git diff-files" may or may not report real changes in this
   case.

 * The working tree file has changed, and convert_to_git() also
   has changed.

   Depending on the nature of the change to convert_to_git(),
   "git diff" may or may not report change in this case.  The
   most extreme case is when unix2dos is run on the working tree
   file and convert_to_git() is made to strip CR.  The object
   registered in the index won't change in this case.

   But in practice, the most problematic case also falls into
   this category.  The user has _real_ changes to the work tree
   file, but at the same time flipped convert_to_git() to
   operate differently from before.  Users should not be making
   such a change, not because of git, but because a commit like
   that will be impossible to review (and understand three
   months later while archaeologying).

The ie_modified() change you suggested will not be hurt by the
first two cases (which I see are one-shot events and re-checkout
and re-add are good enough solution to them, and I do not want
them to hurt the performance for normal use cases).

I originally thought it was a _bug_, but I suspect the false
positive changes reported by "git diff" is even a good thing.