On Sun, 28 Aug 2005, Robert Fitzsimons wrote:
> A patch which added one line to an empty file or removed the only line
> from a one line file would be incorrectly detected as an invalid new
> or delete patch.  Added a new test case.

This patch looks wrong or at least needs some more thought. The test was
done the way it was done for a reason. For example:

        if (patch->is_new != !oldlines)
                return error("new file depends on old contents");

actually has two error cases (admittedly it re-uses the same error message 
for both, which is a bit confusing):

 - is_new, and oldlines != 0

        This is the obvious error, and the one the message talk about.

 - !is_new, and oldlines == 0

        In kernel usage, empty files are not legal, and empty files get 
        deleted. So this actually was intentional, even if the error 
        message ends up being less than perfect.

The "is_delete" thing is the same. At least for the kernel, I want a patch 
that removes all content to also always _delete_ the file, because 
otherwise old-style patches are ambiguous. Is it a delete, or is it a 
"leave an empty file"?

Now, git patches aren't ambigious, so while git patches can clearly say 
"this is a patch to an existing empty file" vs "this patch creates this 
file", I wanted to have the behaviour where we maintain the principle that 
empty files are wrong.

Now, that's a kernel-specific thing, so it may be that enabling this
behaviour this should be a command line flag, but the point is that I 
don't want to entirely lose the test.

To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to