On Wed, Nov 23, 2011 at 10:26 AM, Artem Bityutskiy <dedekind1@xxxxxxxxx> wrote:
> From: Artem Bityutskiy <artem.bityutskiy@xxxxxxxxxxxxxxx>
> $ git apply --check /bin/bash
> $ echo $?
> 0
>
> Not exactly what I expected :-) The same happnes if you use an empty file.
>
> Signed-off-by: Artem Bityutskiy <artem.bityutskiy@xxxxxxxxxxxxxxx>
> ---
>
> Note, I did not extensively test it!
>
> Makefile | 2 +-
> builtin/apply.c | 8 +++++---
> 2 files changed, 6 insertions(+), 4 deletions(-)
>
> diff --git a/builtin/apply.c b/builtin/apply.c
> index 84a8a0b..2d6862a 100644
> --- a/builtin/apply.c
> +++ b/builtin/apply.c
> @@ -3596,9 +3596,6 @@ static int write_out_results(struct patch *list, int skipped_patch)
> int errs = 0;
> struct patch *l;
>
> - if (!list && !skipped_patch)
> - return error("No changes");
> -
I think write_out_results() can lose the skipped_patch parameter now.
> for (phase = 0; phase < 2; phase++) {
> l = list;
> while (l) {
> @@ -3741,6 +3738,11 @@ static int apply_patch(int fd, const char *filename, int options)
> !apply_with_reject)
> exit(1);
>
> + if (!list && !skipped_patch) {
> + error("No changes");
> + exit(1);
You can use die() instead of error followed by exit.
Also, I don't see any reason why this check shouldn't be moved up so
that it is performed immediately after the while loop, avoiding any
unnecessary work.
btw. die'ing like this affects diffstat, numstat, summary etc. too
since now they will report an error instead of just displaying an
informational message describing zero changes when passed a bogus
patch. But that seems correct to me.
I would have also suggested changing the error message to something
more descriptive than "No changes", but apparently apply is a plumbing
command. It seems kind of an "in-between plumbing and porcelain"
command.
It still seems like the correct behavior change with respect to
diffstat, numstat et al that I mentioned above after rethinking from a
plumbing perspective. git apply should error out if it cannot
recognize its input.
-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