On Mon, Dec 5, 2011 at 1:27 PM, Junio C Hamano <gitster@xxxxxxxxx> wrote: > Brandon Casey <drafnel@xxxxxxxxx> 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@xxxxxxxxxxxxxxx> >> Signed-off-by: Brandon Casey <drafnel@xxxxxxxxx> >> --- >> >> 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 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html