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.
Linus
-
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
