builtin/apply: make apply_all_patches() return 128 or 1 on error

To finish libifying the apply functionality, apply_all_patches() should not die() or exit() in case of error, but return either 128 or 1, so that it gives the same exit code as when die() or exit(1) is called. This way scripts relying on the exit code don't need to be changed. While doing that we must take care that file descriptors are properly closed and, if needed, reset to a sensible value. Also, according to the lockfile API, when finished with a lockfile, one should either commit it or roll it back. This is even more important now that the same lockfile can be passed to init_apply_state() many times to be reused by series of calls to the apply lib functions. Helped-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> Helped-by: Johannes Schindelin <Johannes.Schindelin@gmx.de> Helped-by: Eric Sunshine <sunshine@sunshineco.com> Signed-off-by: Christian Couder <chriscool@tuxfamily.org> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Christian Couder committed Aug 8, 2016 at 23:03 UTC fef7ba5353095e87b3bcd712fa15eb71e1f53b30
1 file changed +26 -11
builtin/apply.c
+26 -11
@@ -4578,15 +4578,18 @@ static int apply_all_patches(struct apply_state *state,
4578 arg);
4579
4580 fd = open(arg, O_RDONLY);
4581 - if (fd < 0)
4582 - die_errno(_("can't open patch '%s'"), arg);
4581 + if (fd < 0) {
4582 + error(_("can't open patch '%s': %s"), arg, strerror(errno));
4583 + res = -128;
4584 + goto end;
4585 + }
4586 read_stdin = 0;
4587 set_default_whitespace_mode(state);
4588 res = apply_patch(state, fd, arg, options);
4589 + close(fd);
4590 if (res < 0)
4591 goto end;
4592 errs |= res;
4589 - close(fd);
4593 }
4594 set_default_whitespace_mode(state);
4595 if (read_stdin) {
@@ -4606,11 +4609,14 @@ static int apply_all_patches(struct apply_state *state,
4609 squelched),
4610 squelched);
4611 }
4609 - if (state->ws_error_action == die_on_ws_error)
4610 - die(Q_("%d line adds whitespace errors.",
4611 - "%d lines add whitespace errors.",
4612 - state->whitespace_error),
4613 - state->whitespace_error);
4612 + if (state->ws_error_action == die_on_ws_error) {
4613 + error(Q_("%d line adds whitespace errors.",
4614 + "%d lines add whitespace errors.",
4615 + state->whitespace_error),
4616 + state->whitespace_error);
4617 + res = -128;
4618 + goto end;
4619 + }
4620 if (state->applied_after_fixing_ws && state->apply)
4621 warning("%d line%s applied after"
4622 " fixing whitespace errors.",
@@ -4624,15 +4630,24 @@ static int apply_all_patches(struct apply_state *state,
4630 }
4631
4632 if (state->update_index) {
4627 - if (write_locked_index(&the_index, state->lock_file, COMMIT_LOCK))
4628 - die(_("Unable to write new index file"));
4633 + res = write_locked_index(&the_index, state->lock_file, COMMIT_LOCK);
4634 + if (res) {
4635 + error(_("Unable to write new index file"));
4636 + res = -128;
4637 + goto end;
4638 + }
4639 state->newfd = -1;
4640 }
4641
4642 return !!errs;
4643
4644 end:
4635 - exit(res == -1 ? 1 : 128);
4645 + if (state->newfd >= 0) {
4646 + rollback_lock_file(state->lock_file);
4647 + state->newfd = -1;
4648 + }
4649 +
4650 + return (res == -1 ? 1 : 128);
4651 }
4652
4653 int cmd_apply(int argc, const char **argv, const char *prefix)