Re: [PATCH v3 3/3] correct ce_compare_data() in a middle of a merge

Junio C Hamano [off-list ref] writes:

I am however not convinced passing the full SHA-1 is a good
solution.  The make_cache_entry() function may be creating a cache
entry to stuff in a non-default index (i.e. not "the_index"), but
its caller does not have a way to tell it which index the resulting
cache entry will go, and calls refresh_cache_entry(), which always
assumes that the caller is interested in "the_index", so what
add_cacheinfo() does by calling make_cache_entry() feels wrong in
the first place.  Also, the call from update_file_flags() knows that
the working tree is in sync with the resulting cache entry.

Perhaps update_file_flags() should not even call add_cacheinfo()
there in the first place?  I wonder if it should just call
add_file_to_index()---after all, even though the current code
already knows that 99b633 _is_ the result it wants in the index, it
still goes to the filesystem and rehashes.

I dunno.  I really do not like that extra sha1 argument added all
over the callchain by this patch.

While the above still stands (i.e. I really want to see us find a
solution without the extra argument all over), I do not think
add_file_to_index() would work well here, as the stage #2 of the
index is contaminated with CRLF in this case, and getting rid of it
is the whole point of renormalization, I would think.  Of course,
you will get a normalized result if you merged in the other
direction, as your stage #2 would have normalized 99b633 when you
cleanly merge the CRLF version from the other side with
renormalization, and "git add" of the cleanly merged result would
keep the normalized version.

Perhaps that is an acceptable downside.  If your side is not
normalized, and if the merge result is the same as what you already
have, perhaps you deserve to keep that unnormalized result, and
you'd be better off normalizing your tree before doing a merge, if
you do not like the fact that your indexed blobs have CRLF.

Or did you mean other calls to add_cacheinfo()?