Re: [PATCH] builtin-apply: check for empty files when detecting creation patch

From: Linus Torvalds <torvalds@linux-foundation.org>
Date: May 13, 2008, 22:58 UTC
Message-ID: <alpine.LFD.1.10.0805131552410.3019@woody.linux-foundation.org>
In-Reply-To: <7vlk2dsujm.fsf@gitster.siamese.dyndns.org>

On Tue, 13 May 2008, Junio C Hamano wrote:

Show 7 quoted lines

> Linus Torvalds <torvalds@linux-foundation.org> writes:
> >
> > So non-/dev/null'ness means absolutely nothing. It means "don't know", and 
> > we should leave is_new and is_delete as -1.
> 
> Ok, then what's the judgement for the original issue?  Is it a user error
> to have a tracked absolutely empty file in the index?

I think this is the fundamental problem:

	..
	if (patch->is_new < 0 && !oldlines) {
		patch->is_new = 1;
	..

because that logic simply isn't right. (is_new < 0 && !oldlines) does *not* mean that it must be new.

We can say it the other way around, of course:

	if (patch->is_new < 0 && oldlines)
		patch->is_new = 0;

and that's a valid rule, but I think we already would never set "is_new" to -1 if we had old lines, so that would probably be a pointless thing to do.

So: remove the check for (is_new < 0 && !oldlines) because it doesn't 
actually add any information, and leave "is_new" as unknown until later 
when we actually *see* that file or not. Hmm?

		Linus