This patch series is one of the half dozen patch series left to move the bulk of rebase -i into a builtin. The purpose of this patch series is to switch the functions in sequencer.c from die()ing to returning errors instead, as proper library functions should do, to give callers a chance to clean up after an error. Changes since v2: - the commit message of the read_populate_opts() patch now clarifies why we do not take care of git_config_from_file() possibly die()ing. - the save_head() and the save_opts() conditionals are now separate, for improved readability. - the fast_forward_to() libification now happens in its own commit. - checkout_fast_forward_to() is now libified, too. - added a code comment to clarify why we don't care about git_parse_source() being able to die(). - added rollbacks in case of failure in read_and_refresh_cache() and save_head(). Johannes Schindelin (17): sequencer: lib'ify sequencer_pick_revisions() sequencer: do not die() in do_pick_commit() sequencer: lib'ify write_message() sequencer: lib'ify do_recursive_merge() sequencer: lib'ify do_pick_commit() sequencer: lib'ify walk_revs_populate_todo() sequencer: lib'ify prepare_revs() sequencer: lib'ify read_and_refresh_cache() sequencer: lib'ify read_populate_todo() sequencer: lib'ify read_populate_opts() sequencer: lib'ify create_seq_dir() sequencer: lib'ify save_head() sequencer: lib'ify save_todo() sequencer: lib'ify save_opts() sequencer: lib'ify fast_forward_to() lib'ify checkout_fast_forward_to() sequencer: ensure to release the lock when we could not read the index merge.c | 9 ++- sequencer.c | 197 ++++++++++++++++++++++++++++++++++++++---------------------- 2 files changed, 131 insertions(+), 75 deletions(-) Published-As: https://github.com/dscho/git/releases/tag/libify-sequencer-v3 Fetch-It-Via: git fetch https://github.com/dscho/git libify-sequencer-v3 Interdiff vs v2: diff --git a/merge.c b/merge.c index 5db7d56..23866c9 100644 --- a/merge.c +++ b/merge.c @@ -57,7 +57,8 @@ int checkout_fast_forward(const unsigned char *head, refresh_cache(REFRESH_QUIET); - hold_locked_index(lock_file, 1); + if (hold_locked_index(lock_file, 0) < 0) + return -1; memset(&trees, 0, sizeof(trees)); memset(&opts, 0, sizeof(opts)); @@ -90,7 +91,9 @@ int checkout_fast_forward(const unsigned char *head, } if (unpack_trees(nr_trees, t, &opts)) return -1; - if (write_locked_index(&the_index, lock_file, COMMIT_LOCK)) - die(_("unable to write new index file")); + if (write_locked_index(&the_index, lock_file, COMMIT_LOCK)) { + rollback_lock_file(lock_file); + return error(_("unable to write new index file")); + } return 0; } diff --git a/sequencer.c b/sequencer.c index b6481bb..eec8a60 100644 --- a/sequencer.c +++ b/sequencer.c @@ -644,14 +644,18 @@ static int read_and_refresh_cache(struct replay_opts *opts) { static struct lock_file index_lock; int index_fd = hold_locked_index(&index_lock, 0); - if (read_index_preload(&the_index, NULL) < 0) + if (read_index_preload(&the_index, NULL) < 0) { + rollback_lock_file(&index_lock); return error(_("git %s: failed to read the index"), action_name(opts)); + } refresh_index(&the_index, REFRESH_QUIET|REFRESH_UNMERGED, NULL, NULL, NULL); if (the_index.cache_changed && index_fd >= 0) { - if (write_locked_index(&the_index, &index_lock, COMMIT_LOCK)) + if (write_locked_index(&the_index, &index_lock, COMMIT_LOCK)) { + rollback_lock_file(&index_lock); return error(_("git %s: failed to refresh the index"), action_name(opts)); + } } rollback_lock_file(&index_lock); return 0; @@ -812,6 +816,12 @@ static int read_populate_opts(struct replay_opts **opts) { if (!file_exists(git_path_opts_file())) return 0; + /* + * The function git_parse_source(), called from git_config_from_file(), + * may die() in case of a syntactically incorrect file. We do not care + * about this case, though, because we wrote that file ourselves, so we + * are pretty certain that it is syntactically correct. + */ if (git_config_from_file(populate_opts_cb, git_path_opts_file(), *opts) < 0) return error(_("Malformed options sheet: %s"), git_path_opts_file()); @@ -853,14 +863,20 @@ static int save_head(const char *head) int fd; fd = hold_lock_file_for_update(&head_lock, git_path_head_file(), 0); - if (fd < 0) + if (fd < 0) { + rollback_lock_file(&head_lock); return error_errno(_("Could not lock HEAD")); + } strbuf_addf(&buf, "%s\n", head); - if (write_in_full(fd, buf.buf, buf.len) < 0) + if (write_in_full(fd, buf.buf, buf.len) < 0) { + rollback_lock_file(&head_lock); return error_errno(_("Could not write to %s"), git_path_head_file()); - if (commit_lock_file(&head_lock) < 0) + } + if (commit_lock_file(&head_lock) < 0) { + rollback_lock_file(&head_lock); return error(_("Error wrapping up %s."), git_path_head_file()); + } return 0; } @@ -1135,8 +1151,9 @@ int sequencer_pick_revisions(struct replay_opts *opts) return -1; if (get_sha1("HEAD", sha1) && (opts->action == REPLAY_REVERT)) return error(_("Can't revert as initial commit")); - if (save_head(sha1_to_hex(sha1)) || - save_opts(opts)) + if (save_head(sha1_to_hex(sha1))) + return -1; + if (save_opts(opts)) return -1; return pick_commits(todo_list, opts); } -- 2.10.0.windows.1.10.g803177d base-commit: 6ebdac1bab966b720d776aa43ca188fe378b1f4b