git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Linus Torvalds <torvalds@linux-foundation.org>
To: Marius Storm-Olsen <marius@trolltech.com>,
	Junio C Hamano <junkio@cox.net>
Cc: Git Mailing List <git@vger.kernel.org>
Subject: Re: [TESTCASE] Failing 'git am' when core.autocrlf=true
Date: Thu, 23 Aug 2007 12:23:39 -0700 (PDT)	[thread overview]
Message-ID: <alpine.LFD.0.999.0708231153150.30176@woody.linux-foundation.org> (raw)
In-Reply-To: <alpine.LFD.0.999.0708230956190.30176@woody.linux-foundation.org>



On Thu, 23 Aug 2007, Linus Torvalds wrote:
> 
> Junio - it fixes the test for me, but quite frankly, I don't see why 
> write-tree would *ever* change any non-tree index entries. But it does. I 
> think there's another bug somewhere, or I'm missing something.

Looking more carefully at the index file before and after, the _only_ 
difference really is the ce_size field for "foo".

In the fixed one, ce_size remains at 5 (which is correct: the file 
contents are "bar\n\r"), in the broken case it gets zeroed by 
git-write-tree for some reason.

And the reason is really interesting: it only happens if the index file 
has the same date as the entry in the index, in which case we end up doign 
the "careful" check in ce_smudge_racily_clean_entry() and there the 
"ce_modified_check_fs()" will end up re-reading the file, and if we don't 
have the right CRLF behaviour, we will now return DATA_CHANGED.

So the call-chain for this is:

	cmd_write_tree ->
	  write_tree ->
	    write_index ->
	      ce_smudge_racily_clean_entry -> 
		ce_modified_check_fs ->
		  ce_compare_data ->
		    index_fd ->
		      convert_to_git ->
			** wrong answer unless auto_crlf is set **

		and now "ce_smudge_racily_clean_entry()" will do
			ce->ce_size = htonl(0);

and the one-liner fix I sent out is actually the right fix.

This was harder to find than it should have been, because it actually 
depends on the datestamp of the index file matching the datestamp of the 
file in question!

		Linus

  parent reply	other threads:[~2007-08-23 19:24 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2007-08-23 14:07 [TESTCASE] Failing 'git am' when core.autocrlf=true Marius Storm-Olsen
2007-08-23 17:18 ` Linus Torvalds
2007-08-23 17:45   ` Marius Storm-Olsen
2007-08-23 18:41   ` Junio C Hamano
2007-08-23 19:25     ` Linus Torvalds
2007-08-23 19:23   ` Linus Torvalds [this message]
2007-08-23 19:44     ` Junio C Hamano
2007-08-23 20:11     ` Marius Storm-Olsen

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=alpine.LFD.0.999.0708231153150.30176@woody.linux-foundation.org \
    --to=torvalds@linux-foundation.org \
    --cc=git@vger.kernel.org \
    --cc=junkio@cox.net \
    --cc=marius@trolltech.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).