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);
}
Johannes Schindelin
