Brandon Casey [off-list ref] writes:
When git apply is passed something that is not a patch, it does not produce
an error message or exit with a non-zero status if it was not actually
"applying" the patch i.e. --check or --numstat etc were supplied on the
command line.
Fix this by producing an error when apply fails to find any hunks whatsoever
while parsing the patch.
This will cause some of the output formats (--numstat, --diffstat, etc) to
produce an error when they formerly would have reported zero changes and
exited successfully. That seems like the correct behavior though. Failure
to recognize the input as a patch should be an error.
Plus, add a test.
Reported-by: Artem Bityutskiy <artem.bityutskiy@linux.intel.com>
Signed-off-by: Brandon Casey <redacted>
---
Initially, I was reluctant to change the error message, thinking that
error messages for plumbing commands were not supposed to change. But I
think I was wrong in that thought, so I changed the error message so it
was a more descriptive "unrecognized input".
I am still reluctant to see
$ git apply </dev/null
error: unrecognized input
instead of "error: No changes", though.
"git apply --check" is about asking "do you see anything offending in the
diff?" and it is not "git apply --dry-run" that asks "do you promise if I
feed this for real to you you will apply it without complaint?".
I am slightly in favor of answering "well you do not have a diff to begin
with, which in itself is suspicious" to "do you see anything offending?"
question, but I have to admit that it is an equally valid answer to say
"no, there is nothing offending in the diff.", which is what we do with
the current code.
So, I dunno.
On Mon, Dec 5, 2011 at 1:27 PM, Junio C Hamano [off-list ref] wrote:
Brandon Casey [off-list ref] writes:
quoted
When git apply is passed something that is not a patch, it does not produce
an error message or exit with a non-zero status if it was not actually
"applying" the patch i.e. --check or --numstat etc were supplied on the
command line.
Fix this by producing an error when apply fails to find any hunks whatsoever
while parsing the patch.
This will cause some of the output formats (--numstat, --diffstat, etc) to
produce an error when they formerly would have reported zero changes and
exited successfully. That seems like the correct behavior though. Failure
to recognize the input as a patch should be an error.
Plus, add a test.
Reported-by: Artem Bityutskiy <artem.bityutskiy@linux.intel.com>
Signed-off-by: Brandon Casey <redacted>
---
Initially, I was reluctant to change the error message, thinking that
error messages for plumbing commands were not supposed to change. But I
think I was wrong in that thought, so I changed the error message so it
was a more descriptive "unrecognized input".
I am still reluctant to see
$ git apply </dev/null
error: unrecognized input
instead of "error: No changes", though.
I'm not partial to "unrecognized input", but I thought it was more
descriptive of what happened than "No changes". This error message is
only printed out when absolutely no hunks were found while parsing the
input.
"git apply --check" is about asking "do you see anything offending in the
diff?" and it is not "git apply --dry-run" that asks "do you promise if I
feed this for real to you you will apply it without complaint?".
I am slightly in favor of answering "well you do not have a diff to begin
with, which in itself is suspicious" to "do you see anything offending?"
question, but I have to admit that it is an equally valid answer to say
"no, there is nothing offending in the diff.", which is what we do with
the current code.
So, I dunno.
I think the current code is a little inconsistent with respect to
empty or bogus non-diff input.
It seems more consistent that if it is an error to tell git apply to
apply zero hunks, then it is also an error to --check zero hunks, or
--stat etc. In all cases the cause is the same: failure to find any
hunks in the input because the input was not a diff.
Also, the man page description of --check says that it checks "if the
patch is applicable to the current working tree and/or the index".
The new behavior would answer that with "no, this patch is not
applicable ... since no hunks were found", rather than "yes, because
no hunks were found". But I'm really arguing on the side of
"unrecognized input should be an error", since the new behavior would
also be an error for --stat, --numstat, etc.
-Brandon