Junio C Hamano [off-list ref] writes:
Sorry, but I am confused by all of the above.
We write the thing out with write_entry(), possibly applying smudge
filters and eol conversion to the "git" representation to create the
"working tree" representation in this codepath, right? The resulting
file matches what the user's configuration told us to produce.
Until the working tree file is changed by somebody after the above
happens, we shouldn't have to check the contents of the file to see
if there is a difference. By definition, that has to match the
contents expected to be there by Git.
The only case I can think of that the above does not hold is when
the smuge/clean and the eol conversion are not a correct round-trip
operation pairs, but that would be a misconfiguration. Otherwise,
we'd be _always_ comparing the contents without relying on the
cached stat info for any paths whose in-core and working tree
representations are different, not just those that are configured
with misbehaving smudge/clean pair, no?
To put it differently, if a blob stored at path has CRLF line
endings and .gitattributes is changed after the fact to say that it
must have LF line endings, we should treat it as a broken transitory
state. There may have to be a way to "fix" an already "wrong" blob
in the index that is milder than "rm --cached && add .", but I do
not think write_entry(), which is shared by all the normal codepaths
that writes out to the working tree, is the best place to do so, if
doing so forces the contents of the paths to be always re-checked,
just in case the user is in such a broken transitory state.
On 01/24, Junio C Hamano wrote:
Junio C Hamano [off-list ref] writes:
quoted
Sorry, but I am confused by all of the above.
We write the thing out with write_entry(), possibly applying smudge
filters and eol conversion to the "git" representation to create the
"working tree" representation in this codepath, right? The resulting
file matches what the user's configuration told us to produce.
Until the working tree file is changed by somebody after the above
happens, we shouldn't have to check the contents of the file to see
if there is a difference. By definition, that has to match the
contents expected to be there by Git.
The only case I can think of that the above does not hold is when
the smuge/clean and the eol conversion are not a correct round-trip
operation pairs, but that would be a misconfiguration. Otherwise,
we'd be _always_ comparing the contents without relying on the
cached stat info for any paths whose in-core and working tree
representations are different, not just those that are configured
with misbehaving smudge/clean pair, no?
To put it differently, if a blob stored at path has CRLF line
endings and .gitattributes is changed after the fact to say that it
must have LF line endings, we should treat it as a broken transitory
state.
Right, I wasn't considering this as a broken state, because t0025 uses
just this to transition between the states.
There may have to be a way to "fix" an already "wrong" blob
in the index that is milder than "rm --cached && add .", but I do
not think write_entry(), which is shared by all the normal codepaths
that writes out to the working tree, is the best place to do so, if
doing so forces the contents of the paths to be always re-checked,
just in case the user is in such a broken transitory state.
Maybe I'm misunderstanding something, but the contents of the paths
are only re-checked if we are in such a broken transition state, and
the file stored in git has crlf line endings, and thus would be
normalized. In this case we currently re-check the contents of that
file anyway, at least when the file and the index have the same mtime,
and we actually show the correct output.
I'm not too familiar with the eol conversion code, so I might be
missing something.
--
Thomas